refactor python_dist() testing -- and add some more testing

src/python/pants/backend/python/subsystems/python_native_code.py

@@ -4,7 +4,8 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
-from builtins import str
+import logging
+from textwrap import dedent
 
 from pants.backend.native.subsystems.native_toolchain import NativeToolchain
 from pants.backend.native.targets.native_library import NativeLibrary
@@ -19,6 +20,9 @@
 from pants.util.objects import SubclassesOf
 
 
+logger = logging.getLogger(__name__)
+
+
 class PythonNativeCode(Subsystem):
   """A subsystem which exposes components of the native backend to the python backend."""
 
@@ -74,22 +78,6 @@ def _any_targets_have_native_sources(self, targets):
           return True
     return False
 
-  def _get_targets_by_declared_platform_with_placeholders(self, targets_by_platform):
-    """
-    Aggregates a dict that maps a platform string to a list of targets that specify the platform.
-    If no targets have platforms arguments, return a dict containing platforms inherited from
-    the PythonSetup object.
-
-    :param tgts: a list of :class:`Target` objects.
-    :returns: a dict mapping a platform string to a list of targets that specify the platform.
-    """
-
-    if not targets_by_platform:
-      for platform in self._python_setup.platforms:
-        targets_by_platform[platform] = ['(No target) Platform inherited from either the '
-                                          '--platforms option or a pants.ini file.']
-    return targets_by_platform
-
   def check_build_for_current_platform_only(self, targets):
     """
     Performs a check of whether the current target closure has native sources and if so, ensures
@@ -99,26 +87,26 @@ def check_build_for_current_platform_only(self, targets):
     :return: a boolean value indicating whether the current target closure has native sources.
     :raises: :class:`pants.base.exceptions.IncompatiblePlatformsError`
     """
+    # TODO(#5949): convert this to checking if the closure of python requirements has any
+    # platform-specific packages (maybe find the platforms there too?).
     if not self._any_targets_have_native_sources(targets):
       return False
 
-    targets_by_platform = pex_build_util.targets_by_platform(targets, self._python_setup)
-    platforms_with_sources = self._get_targets_by_declared_platform_with_placeholders(targets_by_platform)
+    platforms_with_sources = pex_build_util.targets_by_platform(targets, self._python_setup)
     platform_names = list(platforms_with_sources.keys())
 
-    if len(platform_names) < 1:
-      raise self.PythonNativeCodeError(
-        "Error: there should be at least one platform in the target closure, because "
-        "we checked that there are native sources.")
-
-    if platform_names == ['current']:
+    if not platform_names:
+      return True
+    elif platform_names == ['current']:
       return True
 
-    raise IncompatiblePlatformsError(
-      'The target set contains one or more targets that depend on '
-      'native code. Please ensure that the platform arguments in all relevant targets and build '
-      'options are compatible with the current platform. Found targets for platforms: {}'
-      .format(str(platforms_with_sources)))
+    raise IncompatiblePlatformsError(dedent("""\
+      The target set contains one or more targets that depend on native code. Please ensure that the
+      platform arguments in python_binary() targets, as well as the value of
+      --{}-platforms, are compatible with the current platform.
+      Platform assignments for python targets: {}
+      """.format(PythonSetup.get_options_scope_equivalent_flag_component(),
+                 platforms_with_sources)))
 
 
 class BuildSetupRequiresPex(ExecutablePexTool):

src/python/pants/backend/python/tasks/build_local_python_distributions.py

@@ -215,7 +215,7 @@ def _prepare_and_create_dist(self, interpreter, shared_libs_product, versioned_t
     setup_requires_pex = self._build_setup_requires_pex_settings.bootstrap(
       interpreter, setup_reqs_pex_path, extra_reqs=setup_reqs_to_resolve)
     self.context.log.debug('Using pex file as setup.py interpreter: {}'
-                           .format(setup_requires_pex))
+                           .format(setup_requires_pex.path()))
 
     self._create_dist(
       dist_target,
@@ -281,6 +281,7 @@ def _create_dist(self,
                     interpreter=setup_requires_pex.path(),
                     command=setup_py_snapshot_version_argv))
 
+  # TODO: convert this into a SimpleCodegenTask, which does the exact same thing as this method!
   def _inject_synthetic_dist_requirements(self, dist, req_lib_addr):
     """Inject a synthetic requirements library that references a local wheel.
 
@@ -289,9 +290,9 @@ def _inject_synthetic_dist_requirements(self, dist, req_lib_addr):
     :return: a :class: `PythonRequirementLibrary` referencing the locally-built wheel.
     """
     whl_dir, base = split_basename_and_dirname(dist)
-    whl_metadata = base.split('-')
-    req_name = '=='.join([whl_metadata[0], whl_metadata[1]])
-    req = PythonRequirement(req_name, repository=whl_dir)
+    name, version = base.split('-')[0:2]
+    full_req_string = '{}=={}'.format(name, version)
+    req = PythonRequirement(full_req_string, repository=whl_dir)
     self.context.build_graph.inject_synthetic_target(req_lib_addr, PythonRequirementLibrary,
                                                      requirements=[req])
 

tests/python/pants_test/backend/python/tasks/native/test_ctypes.py

@@ -4,7 +4,7 @@
 
 from __future__ import absolute_import, division, print_function, unicode_literals
 
-from builtins import str
+from textwrap import dedent
 
 from twitter.common.collections import OrderedDict
 
@@ -13,9 +13,10 @@
 from pants.backend.native.tasks.c_compile import CCompile
 from pants.backend.native.tasks.cpp_compile import CppCompile
 from pants.backend.native.tasks.link_shared_libraries import LinkSharedLibraries
+from pants.backend.python.targets.python_binary import PythonBinary
 from pants.backend.python.targets.python_distribution import PythonDistribution
+from pants.base.exceptions import IncompatiblePlatformsError
 from pants.util.meta import classproperty
-from pants_test.backend.python.tasks.python_task_test_base import check_wheel_platform_matches_host
 from pants_test.backend.python.tasks.util.build_local_dists_test_base import \
   BuildLocalPythonDistributionsTestBase
 
@@ -38,13 +39,13 @@ def run_before_task_types(cls):
       'ctypes_native_library': NativeArtifact(lib_name='c-math-lib'),
       'sources': ['c_math_lib.c', 'c_math_lib.h'],
       'filemap': {
-        'c_math_lib.c': """\
-#include "c_math_lib.h"
-int add_two(int x) { return x + 2; }
-        """,
-        'c_math_lib.h': """\
-int add_two(int);
-        """,
+        'c_math_lib.c': dedent("""\
+        #include "c_math_lib.h"
+        int add_two(int x) { return x + 2; }
+        """),
+        'c_math_lib.h': dedent("""\
+        int add_two(int);
+        """),
       },
     }),
 
@@ -55,15 +56,15 @@ def run_before_task_types(cls):
       'dependencies': ['src/python/plat_specific_c_dist:ctypes_c_library'],
       'filemap': {
         '__init__.py': '',
-        'setup.py': """\
-from setuptools import setup, find_packages
-setup(
-  name='platform_specific_ctypes_c_dist',
-  version='0.0.0',
-  packages=find_packages(),
-  data_files=[('', ['libc-math-lib.so'])],
-)
-        """,
+        'setup.py': dedent("""\
+        from setuptools import setup, find_packages
+        setup(
+          name='platform_specific_ctypes_c_dist',
+          version='0.0.0',
+          packages=find_packages(),
+          data_files=[('', ['libc-math-lib.so'])],
+        )
+        """),
       },
     }),
 
@@ -73,13 +74,13 @@ def run_before_task_types(cls):
       'ctypes_native_library': NativeArtifact(lib_name='cpp-math-lib'),
       'sources': ['cpp_math_lib.cpp', 'cpp_math_lib.hpp'],
       'filemap': {
-        'cpp_math_lib.cpp': """\
-#include "cpp_math_lib.hpp"
-int add_two(int x) { return (x++) + 1; }
-        """,
-        'cpp_math_lib.hpp': """\
-int add_two(int);
-        """,
+        'cpp_math_lib.cpp': dedent("""\
+        #include "cpp_math_lib.hpp"
+        int add_two(int x) { return (x++) + 1; }
+        """),
+        'cpp_math_lib.hpp': dedent("""\
+        int add_two(int);
+        """),
       },
     }),
 
@@ -90,39 +91,80 @@ def run_before_task_types(cls):
       'dependencies': ['src/python/plat_specific_cpp_dist:ctypes_cpp_library'],
       'filemap': {
         '__init__.py': '',
-        'setup.py': """\
-from setuptools import setup, find_packages
-setup(
-  name='platform_specific_ctypes_cpp_dist',
-  version='0.0.0',
-  packages=find_packages(),
-  data_files=[('', ['libcpp-math-lib.so'])],
-)
-        """,
+        'setup.py': dedent("""\
+        from setuptools import setup, find_packages
+        setup(
+          name='platform_specific_ctypes_cpp_dist',
+          version='0.0.0',
+          packages=find_packages(),
+          data_files=[('', ['libcpp-math-lib.so'])],
+        )
+        """),
       },
     }),
 
   ])
 
   def test_ctypes_c_dist(self):
     platform_specific_dist = self.target_dict['platform_specific_ctypes_c_dist']
-    context, synthetic_target, snapshot_version = self._create_distribution_synthetic_target(
-      platform_specific_dist, extra_targets=[self.target_dict['ctypes_c_library']])
-    self.assertEqual(['platform_specific_ctypes_c_dist==0.0.0+{}'.format(snapshot_version)],
-                      [str(x.requirement) for x in synthetic_target.requirements.value])
-    local_wheel_products = context.products.get('local_wheels')
-    local_wheel = self.retrieve_single_product_at_target_base(
-      local_wheel_products, platform_specific_dist)
-    self.assertTrue(check_wheel_platform_matches_host(local_wheel))
+    self._assert_dist_and_wheel_identity(
+      'platform_specific_ctypes_c_dist', '0.0.0', self.ExpectedPlatformType.current,
+      platform_specific_dist, extra_targets=[self.target_dict['ctypes_c_library']],
+    )
 
   def test_ctypes_cpp_dist(self):
     platform_specific_dist = self.target_dict['platform_specific_ctypes_cpp_dist']
-    context, synthetic_target, snapshot_version = self._create_distribution_synthetic_target(
-      platform_specific_dist, extra_targets=[self.target_dict['ctypes_cpp_library']])
-    self.assertEqual(['platform_specific_ctypes_cpp_dist==0.0.0+{}'.format(snapshot_version)],
-                      [str(x.requirement) for x in synthetic_target.requirements.value])
-
-    local_wheel_products = context.products.get('local_wheels')
-    local_wheel = self.retrieve_single_product_at_target_base(
-      local_wheel_products, platform_specific_dist)
-    self.assertTrue(check_wheel_platform_matches_host(local_wheel))
+    self._assert_dist_and_wheel_identity(
+      'platform_specific_ctypes_cpp_dist', '0.0.0', self.ExpectedPlatformType.current,
+      platform_specific_dist, extra_targets=[self.target_dict['ctypes_cpp_library']],
+    )
+
+  def test_multiplatform_python_setup_resolve_bypasses_python_setup(self):
+    self.set_options_for_scope('python-setup',
+                               platforms=['current', 'linux-x86_64', 'macosx_10_14_x86_64'])
+    platform_specific_dist = self.target_dict['platform_specific_ctypes_cpp_dist']
+    self._assert_dist_and_wheel_identity(
+      'platform_specific_ctypes_cpp_dist', '0.0.0', self.ExpectedPlatformType.current,
+      platform_specific_dist,
+      extra_targets=[self.target_dict['ctypes_cpp_library']],
+    )
+
+  def test_resolve_for_native_sources_allows_current_platform_only(self):
+    platform_specific_dist = self.target_dict['platform_specific_ctypes_cpp_dist']
+    compatible_python_binary_target = self.make_target(
+      spec='src/python/plat_specific:bin',
+      target_type=PythonBinary,
+      dependencies=[platform_specific_dist],
+      entry_point='this-will-not-run',
+      platforms=['current'],
+    )
+    self._assert_dist_and_wheel_identity(
+      'platform_specific_ctypes_cpp_dist', '0.0.0', self.ExpectedPlatformType.current,
+      platform_specific_dist,
+      extra_targets=[
+        self.target_dict['ctypes_cpp_library'],
+        compatible_python_binary_target,
+      ])
+
+  def test_multiplatform_resolve_with_binary(self):
+    platform_specific_dist = self.target_dict['platform_specific_ctypes_cpp_dist']
+    incompatible_python_binary_target = self.make_target(
+      spec='src/python/plat_specific:bin',
+      target_type=PythonBinary,
+      dependencies=[platform_specific_dist],
+      entry_point='this-will-not-run',
+      platforms=['current', 'linux-x86_64', 'macosx_10_14_x86_64'],
+    )
+    with self.assertRaisesWithMessage(IncompatiblePlatformsError, dedent("""\
+      The target set contains one or more targets that depend on native code. Please ensure that the
+      platform arguments in python_binary() targets, as well as the value of
+      --python-setup-platforms, are compatible with the current platform.
+      Platform assignments for python targets: {'current': OrderedSet([PythonBinary(src/python/plat_specific:bin)]), 'linux-x86_64': OrderedSet([PythonBinary(src/python/plat_specific:bin)]), 'macosx_10_14_x86_64': OrderedSet([PythonBinary(src/python/plat_specific:bin)])}
+      """)):
+      self._assert_dist_and_wheel_identity(
+        'platform_specific_ctypes_cpp_dist', '0.0.0', self.ExpectedPlatformType.current,
+        platform_specific_dist,
+        extra_targets=[
+          self.target_dict['ctypes_cpp_library'],
+          incompatible_python_binary_target,
+        ])

tests/python/pants_test/backend/python/tasks/python_task_test_base.py

@@ -35,9 +35,8 @@ def name_and_platform(whl):
   return dist, version, normalize_platform_tag(platform_tag)
 
 
-def check_wheel_platform_matches_host(wheel_dist):
-  _, _, wheel_platform = name_and_platform(wheel_dist)
-  return wheel_platform == normalize_platform_tag(pep425tags.get_platform())
+def normalized_current_platform():
+  return normalize_platform_tag(pep425tags.get_platform())
 
 
 class PythonTaskTestBase(InterpreterCacheTestMixin, TaskTestBase):

tests/python/pants_test/backend/python/tasks/test_build_local_python_distributions.py

@@ -100,18 +100,19 @@ class TestBuildLocalPythonDistributions(BuildLocalPythonDistributionsTestBase):
 
   def test_create_distribution(self):
     universal_dist = self.target_dict['universal']
-    self._assert_dist_and_wheel_identity('universal_dist', '0.0.0', 'any', universal_dist)
+    self._assert_dist_and_wheel_identity(
+      'universal_dist', '0.0.0', self.ExpectedPlatformType.universal, universal_dist)
 
   def test_python_dist_setup_requires(self):
     setup_requires_dist = self.target_dict['setup_requires']
     self._assert_dist_and_wheel_identity(
-      'setup_requires_dist_united_states', '0.0.0', 'any',
+      'setup_requires_dist_united_states', '0.0.0', self.ExpectedPlatformType.universal,
       setup_requires_dist, extra_targets=[self.target_dict['pycountry']])
 
   def test_install_requires(self):
     install_requires_dist = self.target_dict['install_requires']
     self._assert_dist_and_wheel_identity(
-      'install_requires_dist', '0.0.0', 'any',
+      'install_requires_dist', '0.0.0', self.ExpectedPlatformType.universal,
       install_requires_dist)
 
   def test_install_requires_conflict(self):

tests/python/pants_test/backend/python/tasks/util/build_local_dists_test_base.py

@@ -18,8 +18,10 @@
 from pants.backend.python.tasks.select_interpreter import SelectInterpreter
 from pants.util.collections import assert_single_element
 from pants.util.meta import classproperty
+from pants.util.objects import enum
 from pants_test.backend.python.tasks.python_task_test_base import (PythonTaskTestBase,
-                                                                   name_and_platform)
+                                                                   name_and_platform,
+                                                                   normalized_current_platform)
 from pants_test.task_test_base import DeclarativeTaskTestMixin
 
 
@@ -73,7 +75,7 @@ def _get_dist_snapshot_version(self, task, python_dist_target):
     return re.sub(r'[^a-zA-Z0-9]', '.', versioned_target_fingerprint.lower())
 
   def _create_distribution_synthetic_target(self, python_dist_target, extra_targets=[]):
-    all_specified_targets = list(self.target_dict.values())
+    all_specified_targets = list(self.target_dict.values()) + list(extra_targets)
     result = self.invoke_tasks(
       # We set `target_closure` to check that all the targets in the build graph are exactly the
       # ones we've just created before building python_dist()s (which creates further targets).
@@ -98,6 +100,9 @@ def _create_distribution_synthetic_target(self, python_dist_target, extra_target
 
     return context, synthetic_target, snapshot_version
 
+  class ExpectedPlatformType(enum(['universal', 'current'])):
+    """Whether to check that the produced wheel has the 'any' platform, or the current one."""
+
   def _assert_dist_and_wheel_identity(self, expected_name, expected_version, expected_platform,
                                       dist_target, **kwargs):
     context, synthetic_target, fingerprint_suffix = self._create_distribution_synthetic_target(
@@ -113,4 +118,8 @@ def _assert_dist_and_wheel_identity(self, expected_name, expected_version, expec
     dist, version, platform = name_and_platform(local_wheel)
     self.assertEquals(dist, expected_name)
     self.assertEquals(version, expected_snapshot_version)
-    self.assertEquals(platform, expected_platform)
+
+    expected_platform.resolve_for_enum_variant({
+      'universal': lambda: self.assertEquals(platform, 'any'),
+      'current': lambda: self.assertEquals(platform, normalized_current_platform()),
+    })()