Support python2 in the setup.py rule. (#8956)

* Support python2 in the setup.py rule. - Detect the requested python version. - If python 2, only use __init__.py to determine packages (if python 3, any .py file denotes a package, per PEP 420). - Detect pkg_resources-style namespace package stanzas, and use them to populate the namespace_packages setup kwarg. Note that this change modifies some previous behavior that, after much diving into the innards of setuptools, was determined to be incorrect. Specifically: - Implicit namespace packages a la PEP 420 (and, for that matter, pkgutil-style namespace packages) are not supposed to be listed in the namespace_packages kwarg. - The empty string as a key in package_data means "include these resource patterns in all packages", rather than "here are resources at the root package", as we previously assumed There is in fact no principled way to load resources relative to the root package, so we omit those from package_data, and only enumerate resources that are under some named package. - resources() targets no longer define packages: the resource loading mechanism in pkg_resources will only load relative to an actual python module, so we now assign resources to their closest parent package, when computing package_data.
pantsbuild · Feb 1, 2020 · c6f9fcf · c6f9fcf
1 parent c2950cb
commit c6f9fcf
Show file tree

Hide file tree

Showing 5 changed files with 222 additions and 79 deletions.
diff --git a/src/python/pants/backend/python/rules/BUILD b/src/python/pants/backend/python/rules/BUILD
@@ -4,6 +4,7 @@
 python_library(
   dependencies=[
     '3rdparty/python:dataclasses',
+    '3rdparty/python:setuptools',
     'src/python/pants/backend/python/subsystems',
     'src/python/pants/build_graph',
     'src/python/pants/engine/legacy:graph',
@@ -33,6 +34,7 @@ python_tests(
     'src/python/pants/engine/legacy:graph',
     'src/python/pants/engine/legacy:structs',
     'src/python/pants/rules/core',
+    'src/python/pants/testutil:interpreter_selection_utils',
     'src/python/pants/testutil:test_base',
     'src/python/pants/testutil/engine:util',
     'src/python/pants/testutil/option',

diff --git a/src/python/pants/backend/python/rules/run_setup_py.py b/src/python/pants/backend/python/rules/run_setup_py.py
@@ -17,6 +17,7 @@
   PackageDatum,
   distutils_repr,
   find_packages,
+  is_python2,
   source_root_or_raise,
 )
 from pants.backend.python.rules.setuptools import Setuptools
@@ -34,9 +35,11 @@
   DirectoryWithPrefixToAdd,
   DirectoryWithPrefixToStrip,
   FileContent,
+  FilesContent,
   InputFilesContent,
   PathGlobs,
   Snapshot,
+  SnapshotSubset,
   Workspace,
 )
 from pants.engine.goal import Goal, GoalSubsystem
@@ -135,6 +138,7 @@ class AncestorInitPyFiles:
 @dataclass(frozen=True)
 class SetupPySourcesRequest:
   hydrated_targets: HydratedTargets
+  py2: bool  # Whether to use py2 or py3 package semantics.
 
 
 @dataclass(frozen=True)
@@ -154,7 +158,7 @@ class SetupPySources:
 class SetupPyChrootRequest:
   """A request to create a chroot containing a setup.py and the sources it operates on."""
   exported_target: ExportedTarget
-  py2: bool = False  # Whether to use py2 or py3 namespace package semantics.
+  py2: bool  # Whether to use py2 or py3 package semantics.
 
 
 @dataclass(frozen=True)
@@ -230,7 +234,7 @@ def validate_args(args: Tuple[str, ...]):
 
 @goal_rule
 async def run_setup_pys(targets: HydratedTargets, options: SetupPyOptions, console: Console,
-                        provenance_map: AddressProvenanceMap,
+                        provenance_map: AddressProvenanceMap, python_setup: PythonSetup,
                         distdir: DistDir, workspace: Workspace) -> SetupPy:
   """Run setup.py commands on all exported targets addressed."""
   args = tuple(options.values.args)
@@ -261,9 +265,12 @@ async def run_setup_pys(targets: HydratedTargets, options: SetupPyOptions, conso
     )
     exported_targets = list(set(owners))
 
-  chroots = await MultiGet(Get[SetupPyChroot](SetupPyChrootRequest(target))
-                           for target in exported_targets)
+  py2 = is_python2((getattr(target.adaptor, 'compatibility', None) for target in targets),
+                   python_setup)
+  chroots = await MultiGet(Get[SetupPyChroot](
+    SetupPyChrootRequest(target, py2)) for target in exported_targets)
 
+  # If args were provided, run setup.py with them; Otherwise just dump chroots.
   if args:
     setup_py_results = await MultiGet(
       Get[RunSetupPyResult](RunSetupPyRequest(exported_target, chroot, tuple(args)))
@@ -272,7 +279,7 @@ async def run_setup_pys(targets: HydratedTargets, options: SetupPyOptions, conso
 
     for exported_target, setup_py_result in zip(exported_targets, setup_py_results):
       addr = exported_target.hydrated_target.address.reference()
-      console.print_stderr(f'Writing contents of dist dir for {addr} to {distdir.relpath}')
+      console.print_stderr(f'Writing dist for {addr} under {distdir.relpath}/.')
       workspace.materialize_directory(
         DirectoryToMaterialize(setup_py_result.output, path_prefix=str(distdir.relpath))
       )
@@ -319,8 +326,6 @@ async def run_setup_py(
   )
   # The setuptools dist dir, created by it under the chroot (not to be confused with
   # pants's own dist dir, at the buildroot).
-  # TODO: The user can change this with the --dist-dir flag to the sdist and bdist_wheel commands.
-  #  See https://github.com/pantsbuild/pants/issues/8912.
   dist_dir = 'dist/'
   request = setuptools_setup.requirements_pex.create_execute_request(
     python_setup=python_setup,
@@ -341,15 +346,9 @@ async def run_setup_py(
 
 @rule
 async def generate_chroot(request: SetupPyChrootRequest) -> SetupPyChroot:
-  if request.py2:
-    # TODO: Implement Python 2 support.  This will involve, among other things: merging ancestor
-    # __init__.py files into the chroot, detecting packages based on the presence of __init__.py,
-    # and inspecting all __init__.py files for the namespace package incantation.
-    raise UnsupportedPythonVersion('Running setup.py commands not supported for Python 2.')
-
   owned_deps = await Get[OwnedDependencies](DependencyOwner(request.exported_target))
   targets = HydratedTargets(od.hydrated_target for od in owned_deps)
-  sources = await Get[SetupPySources](SetupPySourcesRequest(targets))
+  sources = await Get[SetupPySources](SetupPySourcesRequest(targets, py2=request.py2))
   requirements = await Get[ExportedTargetRequirements](DependencyOwner(request.exported_target))
 
   # Nest the sources under the src/ prefix.
@@ -412,9 +411,15 @@ async def get_sources(request: SetupPySourcesRequest,
   ancestor_init_pys = await Get[AncestorInitPyFiles](HydratedTargets, targets)
   sources_digest = await Get[Digest](
     DirectoriesToMerge(directories=tuple([*stripped_srcs_digests, *ancestor_init_pys.digests])))
-  sources_snapshot = await Get[Snapshot](Digest, sources_digest)
+  init_pys_snapshot = await Get[Snapshot](
+    SnapshotSubset(sources_digest, PathGlobs(['**/__init__.py'])))
+  init_py_contents = await Get[FilesContent](Digest, init_pys_snapshot.directory_digest)
+
   packages, namespace_packages, package_data = find_packages(
-    source_root_config.get_source_roots(), zip(targets, stripped_srcs_list), sources_snapshot)
+    source_roots=source_root_config.get_source_roots(),
+    tgts_and_stripped_srcs=list(zip(targets, stripped_srcs_list)),
+    init_py_contents=init_py_contents,
+    py2=request.py2)
   return SetupPySources(digest=sources_digest, packages=packages,
                         namespace_packages=namespace_packages, package_data=package_data)
 

diff --git a/src/python/pants/backend/python/rules/run_setup_py_test.py b/src/python/pants/backend/python/rules/run_setup_py_test.py
@@ -45,6 +45,9 @@
 from pants.testutil.test_base import TestBase
 
 
+_namespace_decl = "__import__('pkg_resources').declare_namespace(__name__)"
+
+
 class TestSetupPyBase(TestBase):
 
   @classmethod
@@ -76,7 +79,7 @@ def rules(cls):
   def assert_chroot(self, expected_files, expected_setup_kwargs, addr):
     chroot = self.request_single_product(
       SetupPyChroot,
-      Params(SetupPyChrootRequest(ExportedTarget(self.tgt(addr))),
+      Params(SetupPyChrootRequest(ExportedTarget(self.tgt(addr)), py2=False),
              SourceRootConfig.global_instance()))
     snapshot = self.request_single_product(Snapshot, Params(chroot.digest))
     assert sorted(expected_files) == sorted(snapshot.files)
@@ -87,7 +90,7 @@ def assert_error(self, addr: str, exc_cls: Type[Exception]):
     with pytest.raises(ExecutionError) as excinfo:
       self.request_single_product(
         SetupPyChroot,
-        Params(SetupPyChrootRequest(ExportedTarget(self.tgt(addr))),
+        Params(SetupPyChrootRequest(ExportedTarget(self.tgt(addr)), py2=False),
                SourceRootConfig.global_instance()))
     ex = excinfo.value
     assert len(ex.wrapped_exceptions) == 1
@@ -112,17 +115,18 @@ def test_generate_chroot(self) -> None:
                      provides=setup_py(name='foo', version='1.2.3').with_binaries(
                        foo_main='src/python/foo/qux:bin'))
     """))
+    self.create_file('src/python/foo/__init__.py', _namespace_decl)
     self.create_file('src/python/foo/foo.py', '')
     self.assert_chroot(
       ['src/foo/qux/__init__.py', 'src/foo/qux/qux.py', 'src/foo/resources/js/code.js',
-       'src/foo/foo.py', 'setup.py', 'MANIFEST.in'],
+       'src/foo/__init__.py', 'src/foo/foo.py', 'setup.py', 'MANIFEST.in'],
       {
         'name': 'foo',
         'version': '1.2.3',
         'package_dir': {'': 'src'},
-        'packages': ['foo', 'foo.qux', 'foo.resources'],
-        'namespace_packages': ['foo', 'foo.resources'],
-        'package_data': {'foo.resources': ['js/code.js']},
+        'packages': ['foo', 'foo.qux'],
+        'namespace_packages': ['foo'],
+        'package_data': {'foo': ['resources/js/code.js']},
         'install_requires': ['baz==1.1.1'],
         'entry_points': {'console_scripts': ['foo_main=foo.qux.bin']}
       },
@@ -164,7 +168,7 @@ def assert_sources(self, expected_files, expected_packages, expected_namespace_p
                      expected_package_data, addrs):
     srcs = self.request_single_product(
       SetupPySources,
-      Params(SetupPySourcesRequest(HydratedTargets([self.tgt(addr) for addr in addrs])),
+      Params(SetupPySourcesRequest(HydratedTargets([self.tgt(addr) for addr in addrs]), py2=False),
              SourceRootConfig.global_instance()))
     chroot_snapshot = self.request_single_product(Snapshot, Params(srcs.digest))
 
@@ -181,56 +185,53 @@ def test_get_sources(self) -> None:
     """))
     self.create_file('src/python/foo/bar/baz/baz1.py', '')
     self.create_file('src/python/foo/bar/baz/baz2.py', '')
-    self.create_file('src/python/foo/bar/__init__.py', '')
+    self.create_file('src/python/foo/bar/__init__.py', _namespace_decl)
     self.create_file('src/python/foo/qux/BUILD', 'python_library()')
     self.create_file('src/python/foo/qux/__init__.py', '')
     self.create_file('src/python/foo/qux/qux.py', '')
     self.create_file('src/python/foo/resources/BUILD', 'resources(sources=["js/code.js"])')
     self.create_file('src/python/foo/resources/js/code.js', '')
-    self.create_file('resources/BUILD', 'resources(sources=["css/style.css"])')
-    self.create_file('resources/css/style.css', '')
     self.create_file('src/python/foo/__init__.py', '')
 
     self.assert_sources(
       expected_files=['foo/bar/baz/baz1.py', 'foo/bar/__init__.py', 'foo/__init__.py'],
-      expected_packages=['foo.bar.baz'],
-      expected_namespace_packages=['foo.bar.baz'],
+      expected_packages=['foo', 'foo.bar', 'foo.bar.baz'],
+      expected_namespace_packages=['foo.bar'],
       expected_package_data={},
       addrs=['src/python/foo/bar/baz:baz1'])
 
     self.assert_sources(
       expected_files=['foo/bar/baz/baz2.py', 'foo/bar/__init__.py', 'foo/__init__.py'],
-      expected_packages=['foo.bar.baz'],
-      expected_namespace_packages=['foo.bar.baz'],
+      expected_packages=['foo', 'foo.bar', 'foo.bar.baz'],
+      expected_namespace_packages=['foo.bar'],
       expected_package_data={},
       addrs=['src/python/foo/bar/baz:baz2'])
 
     self.assert_sources(
       expected_files=['foo/qux/qux.py', 'foo/qux/__init__.py', 'foo/__init__.py'],
-      expected_packages=['foo.qux'],
+      expected_packages=['foo', 'foo.qux'],
       expected_namespace_packages=[],
       expected_package_data={},
       addrs=['src/python/foo/qux'])
 
     self.assert_sources(
       expected_files= ['foo/bar/baz/baz1.py', 'foo/bar/__init__.py',
                        'foo/qux/qux.py', 'foo/qux/__init__.py', 'foo/__init__.py',
-                       'foo/resources/js/code.js', 'css/style.css'],
-      expected_packages=['foo.bar.baz', 'foo.qux', 'foo.resources'],
-      expected_namespace_packages=['foo.bar.baz', 'foo.resources'],
-      expected_package_data={'foo.resources': ('js/code.js',), '': ('css/style.css',)},
-      addrs=['src/python/foo/bar/baz:baz1', 'src/python/foo/qux', 'src/python/foo/resources',
-             'resources'])
+                       'foo/resources/js/code.js'],
+      expected_packages=['foo', 'foo.bar', 'foo.bar.baz', 'foo.qux'],
+      expected_namespace_packages=['foo.bar'],
+      expected_package_data={'foo': ('resources/js/code.js',)},
+      addrs=['src/python/foo/bar/baz:baz1', 'src/python/foo/qux', 'src/python/foo/resources'])
 
     self.assert_sources(
       expected_files=['foo/bar/baz/baz1.py', 'foo/bar/baz/baz2.py',
                       'foo/bar/__init__.py', 'foo/qux/qux.py', 'foo/qux/__init__.py',
-                      'foo/__init__.py', 'foo/resources/js/code.js', 'css/style.css'],
-      expected_packages=['foo.bar.baz', 'foo.qux', 'foo.resources'],
-      expected_namespace_packages=['foo.bar.baz', 'foo.resources'],
-      expected_package_data={'foo.resources': ('js/code.js',), '': ('css/style.css',)},
+                      'foo/__init__.py', 'foo/resources/js/code.js'],
+      expected_packages=['foo', 'foo.bar', 'foo.bar.baz', 'foo.qux'],
+      expected_namespace_packages=['foo.bar'],
+      expected_package_data={'foo': ('resources/js/code.js',)},
       addrs=['src/python/foo/bar/baz:baz1', 'src/python/foo/bar/baz:baz2', 'src/python/foo/qux',
-             'src/python/foo/resources', 'resources'])
+             'src/python/foo/resources'])
 
 
 class TestGetRequirements(TestSetupPyBase):