From 72970969893f7f25555f837869b0a3d9ff0605fc Mon Sep 17 00:00:00 2001 From: Chris Livingston Date: Fri, 1 Dec 2017 12:41:17 -0800 Subject: [PATCH] First pass at kwlzn change suggestions --- .../python/targets/python_distribution.py | 14 ++------------ .../pants/backend/python/tasks/pex_build_util.py | 9 ++++++--- .../backend/python/tasks/python_binary_create.py | 5 ++--- .../backend/python/tasks/resolve_requirements.py | 5 ++--- .../tasks/resolve_requirements_task_base.py | 16 +++++++++++----- 5 files changed, 23 insertions(+), 26 deletions(-) diff --git a/src/python/pants/backend/python/targets/python_distribution.py b/src/python/pants/backend/python/targets/python_distribution.py index c9cbad242df..20e5bea25a8 100644 --- a/src/python/pants/backend/python/targets/python_distribution.py +++ b/src/python/pants/backend/python/targets/python_distribution.py @@ -13,9 +13,8 @@ from pants.base.payload_field import PrimitiveField class PythonDistribution(PythonTarget): - """A Python distribution containing c/cpp extensions. + """A Python distribution. - :API: public """ @classmethod @@ -23,15 +22,6 @@ def alias(cls): return 'python_distribution' - def __init__(self, - platforms=(), - **kwargs): + def __init__(self, **kwargs): payload = Payload() - payload.add_fields({ - 'platforms': PrimitiveField(tuple(maybe_list(platforms or []))) - }) super(PythonDistribution, self).__init__(sources=[], payload=payload, **kwargs) - - @property - def platforms(self): - return self.payload.platforms 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 9f08a23d66c..e0f556f570a 100644 --- a/src/python/pants/backend/python/tasks/pex_build_util.py +++ b/src/python/pants/backend/python/tasks/pex_build_util.py @@ -40,7 +40,7 @@ def has_python_sources(tgt): return is_python_target(tgt) and tgt.has_sources() -def has_python_and_c_sources(tgt): +def is_local_python_dist(tgt): return isinstance(tgt, PythonDistribution) @@ -165,14 +165,15 @@ def build_python_distribution_from_target(target, workdir): whl_dist = chroot_deps_contents[0] # TODO: find better way to grab .whl from chroot whl_location = os.path.join(pydist_workdir, fingerprint, '.deps', whl_dist) return whl_location - return None + else: + raise TaskError('Failed to package python distribution for target: %s', target.name) def dump_python_distributions(builder, dist_targets, workdir, log): """Dump python distribution targets into a given builder :param builder: Dump the python_distributions into this builder. - :param dist_targets: PythonDistribution targets to dump + :param dist_targets: A list of `PythonDistribution` targets to build. :param workdir: Working directory for python targets (./pantsd/python) :param log: Use this logger. """ @@ -181,6 +182,8 @@ def dump_python_distributions(builder, dist_targets, workdir, log): locations = set() for tgt in dist_targets: whl_location = build_python_distribution_from_target(tgt, workdir) + if whl_location in locations: + raise TaskError('Two wheels of the same name have been created: %s.', whl_location) if whl_location: locations.add(whl_location) 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 6c0c9a2ca66..9fe36255b15 100644 --- a/src/python/pants/backend/python/tasks/python_binary_create.py +++ b/src/python/pants/backend/python/tasks/python_binary_create.py @@ -14,9 +14,8 @@ from pants.backend.python.targets.python_binary import PythonBinary from pants.backend.python.tasks2.pex_build_util import (dump_python_distributions, dump_requirements, dump_sources, - has_python_and_c_sources, has_python_requirements, has_python_sources, - has_resources) + has_resources, is_local_python_dist) from pants.base.build_environment import get_buildroot from pants.base.exceptions import TaskError from pants.build_graph.target_scopes import Scopes @@ -125,7 +124,7 @@ def _create_binary(self, binary_tgt, results_dir): builder.add_interpreter_constraint(constraint) elif has_python_requirements(tgt): req_tgts.append(tgt) - elif has_python_and_c_sources(tgt): + elif is_local_python_dist(tgt): python_dist_targets.append(tgt) # Dump everything into the builder's chroot. diff --git a/src/python/pants/backend/python/tasks/resolve_requirements.py b/src/python/pants/backend/python/tasks/resolve_requirements.py index e3b13080f8a..53df7133336 100644 --- a/src/python/pants/backend/python/tasks/resolve_requirements.py +++ b/src/python/pants/backend/python/tasks/resolve_requirements.py @@ -5,8 +5,7 @@ from __future__ import (absolute_import, division, generators, nested_scopes, print_function, unicode_literals, with_statement) -from pants.backend.python.tasks2.pex_build_util import (has_python_and_c_sources, - has_python_requirements) +from pants.backend.python.tasks2.pex_build_util import has_python_requirements, is_local_python_dist from pants.backend.python.tasks2.resolve_requirements_task_base import ResolveRequirementsTaskBase @@ -20,6 +19,6 @@ def product_types(cls): def execute(self): req_libs = self.context.targets(has_python_requirements) - python_dist_targets = self.context.targets(has_python_and_c_sources) + python_dist_targets = self.context.targets(is_local_python_dist) pex = self.resolve_requirements(req_libs, python_dist_targets=python_dist_targets) self.context.products.register_data(self.REQUIREMENTS_PEX, pex) 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 c0afe987fbd..b676e4d2192 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 @@ -29,7 +29,13 @@ class ResolveRequirementsTaskBase(Task): def prepare(cls, options, round_manager): round_manager.require_data(PythonInterpreter) - def resolve_requirements(self, req_libs, python_dist_targets=None): + def resolve_requirements(self, req_libs, local_python_dist_targets=None): + """Requirements resolution for PEX files. + + :param req_libs: A list of :class:`PythonRequirementLibrary` targets to resolve. + :param local_python_dist_targets: A list of :class:`PythonDistribution` targets to resolve. + :returns a PEX containing target requirements and any specified python dist targets. + """ with self.invalidated(req_libs) as invalidation_check: # If there are no relevant targets, we still go through the motions of resolving # an empty set of requirements, to prevent downstream tasks from having to check @@ -48,12 +54,12 @@ def resolve_requirements(self, req_libs, python_dist_targets=None): if not os.path.isdir(path): with safe_concurrent_creation(path) as safe_path: self._build_requirements_pex(interpreter, safe_path, req_libs, - python_dist_targets=python_dist_targets) + local_python_dist_targets=local_python_dist_targets) return PEX(path, interpreter=interpreter) - def _build_requirements_pex(self, interpreter, path, req_libs, python_dist_targets=None): + def _build_requirements_pex(self, interpreter, path, req_libs, local_python_dist_targets=None): builder = PEXBuilder(path=path, interpreter=interpreter, copy=True) dump_requirements(builder, interpreter, req_libs, self.context.log) - if python_dist_targets: - dump_python_distributions(builder, python_dist_targets, self.workdir, self.context.log) + if local_python_dist_targets: + dump_python_distributions(builder, local_python_dist_targets, self.workdir, self.context.log) builder.freeze()