Skip to content

Commit

Permalink
satisfy python_dist setup_requires with a pex to resolve transitive d…
Browse files Browse the repository at this point in the history
…eps, and some other unrelated native toolchain changes (#6275)

### Problem

This PR solves two separate problems at once. It should have been split into two separate PRs earlier, but has undergone significant review and it would be a significant amount of unnecessary effort to do that at this point.

#### Problem 1
The `setup_requires` kwarg of `python_dist()` holds addresses for `python_requirement_library()` targets. Currently, these requirements are fetched into a directory which is added to `PYTHONPATH`. This currently produces an invalid set of packages (for reasons I'm not quite sure of), which can cause errors on `import` statements within the `setup.py`.

#### Problem 2
*See #6273 for more context on this second issue:* `distutils` does all sorts of things with our environment variables (e.g. adding `CFLAGS` to `LDFLAGS`), so until #6273 is merged, trying to modify anything but the `PATH` in the environment in which we invoke `setup.py` in for `python_dist()` targets causes lots of failures in many scenarios.

### Solution

#### Solution 1:
- Extract out the pex-building logic from the `Conan` subsystem into an `ExecutablePexTool` base class.
- Collect `setup_requires` into a pex, using `ExecutablePexTool`.

#### Solution 2
- Only modify the `PATH` in the environment dict produced from `SetupPyExecutionEnvironment`, no other env vars relevant to compilation.
  - Also, prepend our toolchain to the current PATH (commented inline, with a TODO pointing to #6273).
- Add `LC_ALL=C` to the environment as well so compilation error output doesn't have decode errors on smart quotes.

### Result

#### Result 1
`setup_requires` now fetches all necessary transitive deps into a pex, which means you can reliably import python requirements declared in `setup_requires` in the `python_dist()`'s `setup.py`.

#### Result 2
Some of the native toolchain code is slightly simplified until #6273 is merged.
  • Loading branch information
cosmicexplorer authored Sep 4, 2018
1 parent 81af00b commit 6b11c59
Show file tree
Hide file tree
Showing 17 changed files with 296 additions and 198 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

main_file = 'main.py'

python_dist(
sources=rglobs('*.py', exclude=[main_file]),
setup_requires=[
'testprojects/pants-plugins/3rdparty/python/pants',
],
)

# TODO: add another python_binary() with compatibility=['CPython>=3.5'] once pants is released using
# python 3.
python_binary(
name='bin',
sources=[main_file],
dependencies=[':pants_setup_requires'],
)
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

import pkg_resources


hello_module_version = pkg_resources.get_distribution('hello_again').version

if __name__ == '__main__':
print(hello_module_version)
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# coding=utf-8
# Copyright 2018 Pants project contributors (see CONTRIBUTORS.md).
# Licensed under the Apache License, Version 2.0 (see LICENSE).

from __future__ import absolute_import, division, print_function, unicode_literals

from setuptools import setup, find_packages

from pants.version import VERSION as pants_version

setup(
name='hello_again',
version=pants_version,
packages=find_packages(),
)
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os
from setuptools import setup, find_packages

# We require pycountry with setup_requires argument to this setup script's
# We require pycountry with setup_requires argument to this setup script's
# corresponding python_dist.
import pycountry

Expand Down
1 change: 1 addition & 0 deletions src/python/pants/backend/native/subsystems/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ python_library(
'src/python/pants/backend/native/config',
'src/python/pants/backend/native/subsystems/binaries',
'src/python/pants/backend/native/subsystems/utils',
'src/python/pants/binaries',
'src/python/pants/engine:rules',
'src/python/pants/engine:selectors',
'src/python/pants/subsystem',
Expand Down
52 changes: 14 additions & 38 deletions src/python/pants/backend/native/subsystems/conan.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,23 @@
from __future__ import absolute_import, division, print_function, unicode_literals

import logging
import os

from pex.interpreter import PythonInterpreter
from pex.pex import PEX
from pex.pex_builder import PEXBuilder
from pex.pex_info import PexInfo

from pants.backend.python.python_requirement import PythonRequirement
from pants.backend.python.subsystems.python_repos import PythonRepos
from pants.backend.python.subsystems.python_setup import PythonSetup
from pants.backend.python.tasks.pex_build_util import dump_requirements
from pants.base.build_environment import get_pants_cachedir
from pants.subsystem.subsystem import Subsystem
from pants.util.dirutil import safe_concurrent_creation
from pants.util.objects import datatype
from pants.binaries.executable_pex_tool import ExecutablePexTool
from pants.util.memo import memoized_property


logger = logging.getLogger(__name__)


class Conan(Subsystem):
class Conan(ExecutablePexTool):
"""Pex binary for the conan package manager."""
options_scope = 'conan'

entry_point = 'conans.conan'

# TODO: It would be great if these requirements could be drawn from a BUILD file (potentially with
# a special target specified in BUILD.tools)?
default_conan_requirements = (
'conan==1.4.4',
'PyJWT>=1.4.0, <2.0.0',
Expand All @@ -46,34 +40,16 @@ class Conan(Subsystem):
'deprecation>=2.0, <2.1'
)

@classmethod
def implementation_version(cls):
return super(Conan, cls).implementation_version() + [('Conan', 0)]

@classmethod
def subsystem_dependencies(cls):
return super(Conan, cls).subsystem_dependencies() + (PythonRepos, PythonSetup)

@classmethod
def register_options(cls, register):
super(Conan, cls).register_options(register)
register('--conan-requirements', type=list, default=cls.default_conan_requirements,
advanced=True, help='The requirements used to build the conan client pex.')

class ConanBinary(datatype(['pex'])):
"""A `conan` PEX binary."""
@classmethod
def implementation_version(cls):
return super(Conan, cls).implementation_version() + [('Conan', 0)]

def bootstrap_conan(self):
pex_info = PexInfo.default()
pex_info.entry_point = 'conans.conan'
conan_bootstrap_dir = os.path.join(get_pants_cachedir(), 'conan_support')
conan_pex_path = os.path.join(conan_bootstrap_dir, 'conan_binary')
interpreter = PythonInterpreter.get()
if not os.path.exists(conan_pex_path):
with safe_concurrent_creation(conan_pex_path) as safe_path:
builder = PEXBuilder(safe_path, interpreter, pex_info=pex_info)
reqs = [PythonRequirement(req) for req in self.get_options().conan_requirements]
dump_requirements(builder, interpreter, reqs, logger)
builder.freeze()
conan_binary = PEX(conan_pex_path, interpreter)
return self.ConanBinary(pex=conan_binary)
@memoized_property
def base_requirements(self):
return [PythonRequirement(req) for req in self.get_options().conan_requirements]
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
import re
from distutils.dir_util import copy_tree

from pex.interpreter import PythonInterpreter

from pants.backend.native.config.environment import Platform
from pants.backend.native.subsystems.conan import Conan
from pants.backend.native.targets.external_native_library import ExternalNativeLibrary
from pants.base.build_environment import get_pants_cachedir
from pants.base.exceptions import TaskError
from pants.invalidation.cache_manager import VersionedTargetSet
from pants.task.task import Task
Expand Down Expand Up @@ -73,6 +76,9 @@ class NativeExternalLibraryFetch(Task):
options_scope = 'native-external-library-fetch'
native_library_constraint = Exactly(ExternalNativeLibrary)

conan_pex_subdir = 'conan-support'
conan_pex_filename = 'conan.pex'

class NativeExternalLibraryFetchError(TaskError):
pass

Expand Down Expand Up @@ -101,9 +107,21 @@ def product_types(cls):
def cache_target_dirs(self):
return True

@memoized_property
def _conan_python_interpreter(self):
return PythonInterpreter.get()

@memoized_property
def _conan_pex_path(self):
return os.path.join(get_pants_cachedir(),
self.conan_pex_subdir,
self.conan_pex_filename)

@memoized_property
def _conan_binary(self):
return Conan.scoped_instance(self).bootstrap_conan()
return Conan.scoped_instance(self).bootstrap(
self._conan_python_interpreter,
self._conan_pex_path)

def execute(self):
native_lib_tgts = self.context.targets(self.native_library_constraint.satisfied_by)
Expand Down Expand Up @@ -155,16 +173,16 @@ def _get_conan_data_dir_path_for_package(self, pkg_dir_path, pkg_sha):
pkg_sha)

def _remove_conan_center_remote_cmdline(self, conan_binary):
return conan_binary.pex.cmdline(['remote',
'remove',
'conan-center'])
return conan_binary.cmdline(['remote',
'remove',
'conan-center'])

def _add_pants_conan_remote_cmdline(self, conan_binary, remote_index_num, remote_url):
return conan_binary.pex.cmdline(['remote',
'add',
'pants-conan-remote-' + str(remote_index_num),
remote_url,
'--insert'])
return conan_binary.cmdline(['remote',
'add',
'pants-conan-remote-' + str(remote_index_num),
remote_url,
'--insert'])

def ensure_conan_remote_configuration(self, conan_binary):
"""
Expand Down Expand Up @@ -243,7 +261,7 @@ def _fetch_packages(self, vt, vts_results_dir):
# Prepare conan command line and ensure remote is configured properly.
self.ensure_conan_remote_configuration(self._conan_binary)
args = conan_requirement.fetch_cmdline_args
cmdline = self._conan_binary.pex.cmdline(args)
cmdline = self._conan_binary.cmdline(args)

self.context.log.debug('Running conan.pex cmdline: {}'.format(cmdline))
self.context.log.debug('Conan remotes: {}'.format(self.get_options().conan_remotes))
Expand Down
Loading

0 comments on commit 6b11c59

Please sign in to comment.