Simplify python local dist handling code. (pantsbuild#5480)

Previously, BuildLocalPythonDistributions generated local dists and put them
in products. Downstream tasks then synthesized PythonRequirementLibrary
targets to point to the local dists, if they needed to resolve them. This was clunky,
and required special knowledge of local dist issues to reside in multiple downstream
tasks.

This change has BuildLocalPythonDistributions itself inject the synthetic
PythonRequirementLibrary targets - one per dist - and stitches them into the
build graph. Downstream tasks no longer need to know or care about local dists
(apart from declaring them as required products), they just see PythonRequirementLibrary
targets, whether synthetic or organic, and handle them uniformly.

Note that previously PythonBinaryCreate had to do some footwork to ensure that only the
dists depended on by a specific PythonBinary were used when creating that binary.
This now happens naturally, since we generate one requirement library per dist,
and the dependencies are wired up in the right way.

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:

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)]

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.

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()

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.
 

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])