From ddaee481a5dd142a189fdf97dfcff60b6dc2514e Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Fri, 16 Feb 2018 19:35:56 -0500 Subject: [PATCH 1/2] Simplify python local dist handling code. --- .../tasks/build_local_python_distributions.py | 57 +++++++++++++------ .../backend/python/tasks/pex_build_util.py | 44 -------------- .../python/tasks/python_binary_create.py | 17 +----- .../tasks/resolve_requirements_task_base.py | 14 +---- src/python/pants/build_graph/build_graph.py | 2 +- .../test_build_local_python_distributions.py | 12 ++-- 6 files changed, 56 insertions(+), 90 deletions(-) diff --git a/src/python/pants/backend/python/tasks/build_local_python_distributions.py b/src/python/pants/backend/python/tasks/build_local_python_distributions.py index cdbb7ecad2f..b1a93e12dfe 100644 --- a/src/python/pants/backend/python/tasks/build_local_python_distributions.py +++ b/src/python/pants/backend/python/tasks/build_local_python_distributions.py @@ -11,11 +11,14 @@ from pex.interpreter import PythonInterpreter +from pants.backend.python.python_requirement import PythonRequirement +from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary from pants.backend.python.tasks.pex_build_util import is_local_python_dist from pants.backend.python.tasks.setup_py import SetupPyRunner from pants.base.build_environment import get_buildroot from pants.base.exceptions import TargetDefinitionException, TaskError from pants.base.fingerprint_strategy import DefaultFingerprintStrategy +from pants.build_graph.address import Address from pants.task.task import Task from pants.util.dirutil import safe_mkdir @@ -24,11 +27,13 @@ class BuildLocalPythonDistributions(Task): """Create python distributions (.whl) from python_dist targets.""" options_scope = 'python-create-distributions' - PYTHON_DISTS = 'user_defined_python_dists' @classmethod def product_types(cls): - return [cls.PYTHON_DISTS] + # Note that we don't actually place the products in the product map. We stitch + # them into the build graph instead. This is just to force the round engine + # to run this task when dists need to be built. + return [PythonRequirementLibrary] @classmethod def prepare(cls, options, round_manager): @@ -40,24 +45,29 @@ def cache_target_dirs(self): def execute(self): dist_targets = self.context.targets(is_local_python_dist) - built_dists = set() + build_graph = self.context.build_graph if dist_targets: with self.invalidated(dist_targets, fingerprint_strategy=DefaultFingerprintStrategy(), invalidate_dependents=True) as invalidation_check: + for vt in invalidation_check.invalid_vts: + if vt.target.dependencies: + raise TargetDefinitionException( + vt.target, 'The `dependencies` field is disallowed on `python_dist` targets. ' + 'List any 3rd party requirements in the install_requirements argument ' + 'of your setup function.' + ) + self._create_dist(vt.target, vt.results_dir) + for vt in invalidation_check.all_vts: - if vt.valid: - built_dists.add(self._get_whl_from_dir(os.path.join(vt.results_dir, 'dist'))) - else: - if vt.target.dependencies: - raise TargetDefinitionException( - vt.target, 'The `dependencies` field is disallowed on `python_dist` targets. List any 3rd ' - 'party requirements in the install_requirements argument of your setup function.' - ) - built_dists.add(self._create_dist(vt.target, vt.results_dir)) - - self.context.products.register_data(self.PYTHON_DISTS, built_dists) + dist = self._get_whl_from_dir(os.path.join(vt.results_dir, 'dist')) + req_lib_addr = Address.parse('{}__req_lib'.format(vt.target.address.spec)) + self._inject_synthetic_dist_requirements(dist, req_lib_addr) + # Make any target that depends on the dist depend on the synthetic req_lib, + # for downstream consumption. + for dependent in build_graph.dependents_of(vt.target.address): + build_graph.inject_dependency(dependent, req_lib_addr) def _create_dist(self, dist_tgt, dist_target_dir): """Create a .whl file for the specified python_distribution target.""" @@ -76,9 +86,24 @@ def _create_dist(self, dist_tgt, dist_target_dir): # Build a whl using SetupPyRunner and return its absolute path. setup_runner = SetupPyRunner(dist_target_dir, 'bdist_wheel', interpreter=interpreter) setup_runner.run() - return self._get_whl_from_dir(os.path.join(dist_target_dir, 'dist')) - def _get_whl_from_dir(self, install_dir): + def _inject_synthetic_dist_requirements(self, dist, req_lib_addr): + """Inject a synthetic requirements library that references a local wheel. + + :param dist: Path of the locally built wheel to reference. + :param req_lib_addr: :class: `Address` to give to the synthetic target. + :return: a :class: `PythonRequirementLibrary` referencing the locally-built wheel. + """ + base = os.path.basename(dist) + whl_dir = os.path.dirname(dist) + whl_metadata = base.split('-') + req_name = '=='.join([whl_metadata[0], whl_metadata[1]]) + req = PythonRequirement(req_name, repository=whl_dir) + self.context.build_graph.inject_synthetic_target(req_lib_addr, PythonRequirementLibrary, + requirements=[req]) + + @staticmethod + def _get_whl_from_dir(install_dir): """Return the absolute path of the whl in a setup.py install directory.""" dists = glob.glob(os.path.join(install_dir, '*.whl')) if len(dists) == 0: diff --git a/src/python/pants/backend/python/tasks/pex_build_util.py b/src/python/pants/backend/python/tasks/pex_build_util.py index 8d64b169ad3..95b0486377a 100644 --- a/src/python/pants/backend/python/tasks/pex_build_util.py +++ b/src/python/pants/backend/python/tasks/pex_build_util.py @@ -11,7 +11,6 @@ from pex.resolver import resolve from twitter.common.collections import OrderedSet -from pants.backend.python.python_requirement import PythonRequirement from pants.backend.python.subsystems.python_setup import PythonSetup from pants.backend.python.targets.python_binary import PythonBinary from pants.backend.python.targets.python_distribution import PythonDistribution @@ -20,7 +19,6 @@ from pants.backend.python.targets.python_tests import PythonTests from pants.base.build_environment import get_buildroot from pants.base.exceptions import TaskError -from pants.build_graph.address import Address from pants.build_graph.files import Files from pants.python.python_repos import PythonRepos @@ -161,45 +159,3 @@ def _resolve_multi(interpreter, requirements, platforms, find_links): allow_prereleases=python_setup.resolver_allow_prereleases) return distributions - - -def inject_synthetic_dist_requirements(build_graph, local_built_dists, synthetic_address, - binary_tgt=None): - """Inject a synthetic requirements library from a local wheel. - - :param build_graph: The build graph needed for injecting synthetic targets. - :param local_built_dists: A list of paths to locally built wheels to package into - requirements libraries. - :param synthetic_address: A generative address for addressing synthetic targets. - :param binary_tgt: An optional parameter to be passed only when called by the - `python_binary_create` task. This is needed to ensure that only python_dist targets in a binary - target's closure are included in the binary for the case where a user specifies mulitple binary - targets in a single invocation of `./pants binary`. - :return: a :class: `PythonRequirementLibrary` containing a requirements that maps to a - locally-built wheel. - """ - def should_create_req(bin_tgt, loc): - if not bin_tgt: - return True - # Ensure that a target is in a binary target's closure. See docstring for more detail. - return any([tgt.id in loc for tgt in bin_tgt.closure()]) - - def python_requirement_from_wheel(path): - base = os.path.basename(path) - whl_dir = os.path.dirname(path) - whl_metadata = base.split('-') - req_name = '=='.join([whl_metadata[0], whl_metadata[1]]) - return PythonRequirement(req_name, repository=whl_dir) - - local_whl_reqs = [ - python_requirement_from_wheel(whl_location) - for whl_location in local_built_dists - if should_create_req(binary_tgt, whl_location) - ] - - if not local_whl_reqs: - return [] - - addr = Address.parse(synthetic_address) - build_graph.inject_synthetic_target(addr, PythonRequirementLibrary, requirements=local_whl_reqs) - return [build_graph.get_target(addr)] diff --git a/src/python/pants/backend/python/tasks/python_binary_create.py b/src/python/pants/backend/python/tasks/python_binary_create.py index 8769cc9883c..5572613402c 100644 --- a/src/python/pants/backend/python/tasks/python_binary_create.py +++ b/src/python/pants/backend/python/tasks/python_binary_create.py @@ -12,12 +12,10 @@ from pex.pex_info import PexInfo from pants.backend.python.targets.python_binary import PythonBinary -from pants.backend.python.tasks.build_local_python_distributions import \ - BuildLocalPythonDistributions +from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary from pants.backend.python.tasks.pex_build_util import (dump_requirement_libs, dump_sources, has_python_requirements, has_python_sources, - has_resources, - inject_synthetic_dist_requirements) + has_resources) from pants.base.build_environment import get_buildroot from pants.base.exceptions import TaskError from pants.build_graph.target_scopes import Scopes @@ -46,7 +44,7 @@ def cache_target_dirs(self): def prepare(cls, options, round_manager): round_manager.require_data(PythonInterpreter) round_manager.require_data('python') # For codegen. - round_manager.require_data(BuildLocalPythonDistributions.PYTHON_DISTS) + round_manager.optional_product(PythonRequirementLibrary) # For local dists. @staticmethod def is_binary(target): @@ -130,15 +128,6 @@ def _create_binary(self, binary_tgt, results_dir): # Dump everything into the builder's chroot. for tgt in source_tgts: dump_sources(builder, tgt, self.context.log) - - # Handle locally-built python distribution dependencies. - built_dists = self.context.products.get_data(BuildLocalPythonDistributions.PYTHON_DISTS) - if built_dists: - req_tgts = inject_synthetic_dist_requirements(self.context.build_graph, - built_dists, - ':'.join(2 * [binary_tgt.invalidation_hash()]), - binary_tgt) + req_tgts - dump_requirement_libs(builder, interpreter, req_tgts, self.context.log, binary_tgt.platforms) # Build the .pex file. diff --git a/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py b/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py index 98cfb5b0052..23467d298a9 100644 --- a/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py +++ b/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py @@ -12,10 +12,8 @@ from pex.pex_builder import PEXBuilder from pants.backend.python.python_requirement import PythonRequirement -from pants.backend.python.tasks.build_local_python_distributions import \ - BuildLocalPythonDistributions -from pants.backend.python.tasks.pex_build_util import (dump_requirement_libs, dump_requirements, - inject_synthetic_dist_requirements) +from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary +from pants.backend.python.tasks.pex_build_util import (dump_requirement_libs, dump_requirements) from pants.base.hash_utils import hash_all from pants.invalidation.cache_manager import VersionedTargetSet from pants.task.task import Task @@ -33,7 +31,7 @@ class ResolveRequirementsTaskBase(Task): @classmethod def prepare(cls, options, round_manager): round_manager.require_data(PythonInterpreter) - round_manager.require_data(BuildLocalPythonDistributions.PYTHON_DISTS) + round_manager.optional_product(PythonRequirementLibrary) # For local dists. def resolve_requirements(self, req_libs, local_dist_targets=None): """Requirements resolution for PEX files. @@ -61,12 +59,6 @@ def resolve_requirements(self, req_libs, local_dist_targets=None): # to cover the empty case. if not os.path.isdir(path): with safe_concurrent_creation(path) as safe_path: - # Handle locally-built python distribution dependencies. - built_dists = self.context.products.get_data(BuildLocalPythonDistributions.PYTHON_DISTS) - if built_dists: - req_libs = inject_synthetic_dist_requirements( - self.context.build_graph, built_dists, ':'.join(2 * [target_set_id]) - ) + req_libs builder = PEXBuilder(path=safe_path, interpreter=interpreter, copy=True) dump_requirement_libs(builder, interpreter, req_libs, self.context.log) builder.freeze() diff --git a/src/python/pants/build_graph/build_graph.py b/src/python/pants/build_graph/build_graph.py index aceca2ebef3..77214223dbc 100644 --- a/src/python/pants/build_graph/build_graph.py +++ b/src/python/pants/build_graph/build_graph.py @@ -183,7 +183,7 @@ def dependencies_of(self, address): return self._target_dependencies_by_address[address] def dependents_of(self, address): - """Returns the Targets which depend on the target at `address`. + """Returns the addresses of the targets that depend on the target at `address`. This method asserts that the address given is actually in the BuildGraph. diff --git a/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py b/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py index bc509eab684..f676bc3d755 100644 --- a/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py +++ b/tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py @@ -44,9 +44,13 @@ def setUp(self): sources=sources) def test_python_create_distributions(self): - context = self.context(target_roots=[self.python_dist_tgt], for_task_types=[BuildLocalPythonDistributions]) + context = self.context(target_roots=[self.python_dist_tgt], + for_task_types=[BuildLocalPythonDistributions]) + self.assertEquals([self.python_dist_tgt], context.build_graph.targets()) python_create_distributions_task = self.create_task(context) python_create_distributions_task.execute() - built_dists = context.products.get_data(BuildLocalPythonDistributions.PYTHON_DISTS) - self.assertGreater(len(built_dists), 0) - self.assertTrue(any(['my_dist-0.0.0-py2-none-any.whl' in dist for dist in list(built_dists)])) + synthetic_tgts = set(context.build_graph.targets()) - {self.python_dist_tgt} + self.assertEquals(1, len(synthetic_tgts)) + synthetic_target = next(iter(synthetic_tgts)) + self.assertEquals(['my_dist==0.0.0'], + [str(x.requirement) for x in synthetic_target.requirements.value]) From 0670733b914f159cda18f64da60f29f0329c7204 Mon Sep 17 00:00:00 2001 From: Benjy Weinberger Date: Sat, 17 Feb 2018 01:43:19 -0500 Subject: [PATCH 2/2] Fix lint error. --- .../backend/python/tasks/resolve_requirements_task_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py b/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py index 23467d298a9..1731ca88643 100644 --- a/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py +++ b/src/python/pants/backend/python/tasks/resolve_requirements_task_base.py @@ -13,7 +13,7 @@ from pants.backend.python.python_requirement import PythonRequirement from pants.backend.python.targets.python_requirement_library import PythonRequirementLibrary -from pants.backend.python.tasks.pex_build_util import (dump_requirement_libs, dump_requirements) +from pants.backend.python.tasks.pex_build_util import dump_requirement_libs, dump_requirements from pants.base.hash_utils import hash_all from pants.invalidation.cache_manager import VersionedTargetSet from pants.task.task import Task