Fix 1461: Better loop breaker for manifest_maker

bootstrap.egg-info/entry_points.txt

@@ -1,11 +1,15 @@
 [distutils.commands]
 egg_info = setuptools.command.egg_info:egg_info
+build_py = setuptools.command.build_py:build_py
+sdist = setuptools.command.sdist:sdist
 
 [distutils.setup_keywords]
 include_package_data = setuptools.dist:assert_bool
 install_requires = setuptools.dist:check_requirements
 extras_require = setuptools.dist:check_extras
 entry_points = setuptools.dist:check_entry_points
+exclude_package_data = setuptools.dist:check_package_data
+namespace_packages = setuptools.dist:check_nsp
 
 [egg_info.writers]
 PKG-INFO = setuptools.command.egg_info:write_pkg_info

changelog.d/1461.change.rst

@@ -0,0 +1,3 @@
+Fix inconsistency with ``include_package_data`` and ``packages_data`` in sdist
+by replacing the loop breaking mechanism between the ``sdist`` and
+``egg_info`` commands -- by :user:`abravalheri`

setuptools/command/build_py.py

@@ -67,6 +67,16 @@ def _get_data_files(self):
         self.analyze_manifest()
         return list(map(self._get_pkg_data_files, self.packages or ()))
 
+    def get_data_files_without_manifest(self):
+        """
+        Generate list of ``(package,src_dir,build_dir,filenames)`` tuples,
+        but without triggering any attempt to analyze or build the manifest.
+        """
+        # Prevent eventual errors from unset `manifest_files`
+        # (that would otherwise be set by `analyze_manifest`)
+        self.__dict__.setdefault('manifest_files', {})
+        return list(map(self._get_pkg_data_files, self.packages or ()))
+
     def _get_pkg_data_files(self, package):
         # Locate package source directory
         src_dir = self.get_package_dir(package)

setuptools/command/egg_info.py

@@ -608,6 +608,18 @@ def prune_file_list(self):
         self.filelist.exclude_pattern(r'(^|' + sep + r')(RCS|CVS|\.svn)' + sep,
                                       is_regex=1)
 
+    def _safe_data_files(self, build_py):
+        """
+        The parent class implementation of this method
+        (``sdist``) will try to include data files, which
+        might cause recursion problems when
+        ``include_package_data=True``.
+
+        Therefore, avoid triggering any attempt of
+        analyzing/building the manifest again.
+        """
+        return build_py.get_data_files_without_manifest()
+
 
 def write_file(filename, contents):
     """Create a file with the specified name and write 'contents' (a

setuptools/command/sdist.py

@@ -114,12 +114,15 @@ def _add_defaults_python(self):
 
     def _safe_data_files(self, build_py):
         """
-        Extracting data_files from build_py is known to cause
-        infinite recursion errors when `include_package_data`
-        is enabled, so suppress it in that case.
+        Since the ``sdist`` class is also used to compute the MANIFEST
+        (via :obj:`setuptools.command.egg_info.manifest_maker`),
+        there might be recursion problems when trying to obtain the list of
+        data_files and ``include_package_data=True`` (which in turn depends on
+        the files included in the MANIFEST).
+
+        To avoid that, ``manifest_maker`` should be able to overwrite this
+        method and avoid recursive attempts to build/analyze the MANIFEST.
         """
-        if self.distribution.include_package_data:
-            return ()
         return build_py.data_files
 
     def _add_data_files(self, data_files):

setuptools/tests/test_sdist.py

@@ -126,6 +126,28 @@ def test_package_data_in_sdist(self):
         assert os.path.join('sdist_test', 'c.rst') not in manifest
         assert os.path.join('d', 'e.dat') in manifest
 
+    def test_package_data_and_include_package_data_in_sdist(self):
+        """
+        Ensure package_data and include_package_data work
+        together.
+        """
+        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()
+
+        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
+
     def test_setup_py_exists(self):
         dist = Distribution(SETUP_ATTRS)
         dist.script_name = 'foo.py'