Merge pull request #2855 from abravalheri/fix-2849

Fix 2849: Problem with custom commands inheriting directly from `distutils`

Author: jaraco

changelog.d/2849.misc.rst

@@ -0,0 +1,3 @@
+Add fallback for custom ``build_py`` commands inheriting directly from
+:mod:`distutils`, while still handling ``include_package_data=True`` for
+``sdist``.

setuptools/command/egg_info.py

@@ -618,7 +618,15 @@ def _safe_data_files(self, build_py):
         Therefore, avoid triggering any attempt of
         analyzing/building the manifest again.
         """
-        return build_py.get_data_files_without_manifest()
+        if hasattr(build_py, 'get_data_files_without_manifest'):
+            return build_py.get_data_files_without_manifest()
+
+        log.warn(
+            "Custom 'build_py' does not implement "
+            "'get_data_files_without_manifest'.\nPlease extend command classes"
+            " from setuptools instead of distutils."
+        )
+        return build_py.get_data_files()
 
 
 def write_file(filename, contents):

setuptools/tests/test_sdist.py

@@ -6,6 +6,7 @@
 import unicodedata
 import contextlib
 import io
+from unittest import mock
 
 import pytest
 
@@ -106,6 +107,13 @@ def source_dir(self, tmpdir):
         with tmpdir.as_cwd():
             yield
 
+    def assert_package_data_in_manifest(self, cmd):
+        manifest = cmd.filelist.files
+        assert os.path.join('sdist_test', 'a.txt') in manifest
+        assert os.path.join('sdist_test', 'b.txt') in manifest
+        assert os.path.join('sdist_test', 'c.rst') not in manifest
+        assert os.path.join('d', 'e.dat') in manifest
+
     def test_package_data_in_sdist(self):
         """Regression test for pull request #4: ensures that files listed in
         package_data are included in the manifest even if they're not added to
@@ -120,11 +128,7 @@ def test_package_data_in_sdist(self):
         with quiet():
             cmd.run()
 
-        manifest = cmd.filelist.files
-        assert os.path.join('sdist_test', 'a.txt') in manifest
-        assert os.path.join('sdist_test', 'b.txt') in manifest
-        assert os.path.join('sdist_test', 'c.rst') not in manifest
-        assert os.path.join('d', 'e.dat') in manifest
+        self.assert_package_data_in_manifest(cmd)
 
     def test_package_data_and_include_package_data_in_sdist(self):
         """
@@ -142,11 +146,58 @@ def test_package_data_and_include_package_data_in_sdist(self):
         with quiet():
             cmd.run()
 
-        manifest = cmd.filelist.files
-        assert os.path.join('sdist_test', 'a.txt') in manifest
-        assert os.path.join('sdist_test', 'b.txt') in manifest
-        assert os.path.join('sdist_test', 'c.rst') not in manifest
-        assert os.path.join('d', 'e.dat') in manifest
+        self.assert_package_data_in_manifest(cmd)
+
+    @mock.patch('setuptools.command.egg_info.log')
+    def test_custom_build_py(self, log_stub):
+        """
+        Ensure projects defining custom build_py don't break
+        when creating sdists (issue #2849)
+        """
+        from distutils.command.build_py import build_py as OrigBuildPy
+
+        using_custom_command_guard = mock.Mock()
+
+        class CustomBuildPy(OrigBuildPy):
+            """
+            Some projects have custom commands inheriting from `distutils`
+            """
+
+            def get_data_files(self):
+                using_custom_command_guard()
+                return super().get_data_files()
+
+        setup_attrs = {**SETUP_ATTRS, 'include_package_data': True}
+        assert setup_attrs['package_data']
+
+        dist = Distribution(setup_attrs)
+        dist.script_name = 'setup.py'
+        cmd = sdist(dist)
+        cmd.ensure_finalized()
+
+        # Make sure we use the custom command
+        cmd.cmdclass = {'build_py': CustomBuildPy}
+        cmd.distribution.cmdclass = {'build_py': CustomBuildPy}
+        assert cmd.distribution.get_command_class('build_py') == CustomBuildPy
+
+        with quiet():
+            cmd.run()
+
+        using_custom_command_guard.assert_called()
+        self.assert_package_data_in_manifest(cmd)
+
+        warn_stub = log_stub.warn
+        warn_stub.assert_called()
+        for call in warn_stub.call_args_list:
+            args, _kw = call
+            if "setuptools instead of distutils" in args[0]:
+                return
+        else:
+            raise AssertionError(
+                "The user should have been warned to extend setuptools command"
+                " classes instead of distutils",
+                warn_stub.call_args_list
+            )
 
     def test_setup_py_exists(self):
         dist = Distribution(SETUP_ATTRS)