From 2e66eb7147ae1e8a799b2276c7340f48f63a7220 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Mon, 1 Nov 2021 19:47:33 +0000 Subject: Fix 1461: Better loop breaker for `manifest_maker` The inconsistency for the `package_data` configuration in sdists when `include_package_data=True` in #1461 have been causing some problems for the community for a while, as also shown in #2835. As pointed out by [@jaraco](https://github.com/pypa/setuptools/issues/1461#issuecomment-749092366), this was being caused by a mechanism to break the recursion between the `egg_info` and `sdist` commands. In summary the loop is caused by the following behaviour: - the `egg_info` command uses a subclass of `sdist` (`manifest_maker`) to calculate the MANIFEST, - the `sdist` class needs to know the MANIFEST to calculate the data files when `include_package_data=True` Previously, the mechanism to break this loop was to simply ignore the data files in `sdist` when `include_package_data=True`. The approach implemented in this change was to replace this mechanism, by allowing `manifest_maker` to override the `_safe_data_files` method from `sdist`. --- Please notice [an extensive experiment] (https://github.com/abravalheri/experiment-setuptools-package-data) was carried out to investigate the previous confusing behaviour. There is also [a simplified theoretical analysis] (https://github.com/pyscaffold/pyscaffold/pull/535#issuecomment-956296407) comparing the observed behavior in the experiment and the expected one. This analysis point out to the same offender indicated by [@jaraco](https://github.com/pypa/setuptools/issues/1461#issuecomment-749092366) (which is being replaced in this change). --- setuptools/command/egg_info.py | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'setuptools/command/egg_info.py') diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index 18b81340..ff009861 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -608,6 +608,17 @@ class manifest_maker(sdist): 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 we have to 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 -- cgit v1.2.1 From 2d38be3259311220d069996b3a906f171dfddafc Mon Sep 17 00:00:00 2001 From: "Jason R. Coombs" Date: Tue, 2 Nov 2021 20:54:51 -0400 Subject: Reformat docstring and rewrite in imperative voice. --- setuptools/command/egg_info.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'setuptools/command/egg_info.py') diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index ff009861..4165c35e 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -610,12 +610,13 @@ class manifest_maker(sdist): 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 + 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 we have to avoid triggering any attempt of analyzing/building - the manifest again. + Therefore, avoid triggering any attempt of + analyzing/building the manifest again. """ return build_py.get_data_files_without_manifest() -- cgit v1.2.1 From 08235e88d10179c89ed80a4dd9bf2d18c76b478d Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 4 Nov 2021 12:30:37 +0000 Subject: Handle custom build_py inheriting from distutils According to #2849, some projects, including important data-science packages rely on `distutils` when creating custom commands, instead of extending the ones provided by setuptools. This change should accomodate this use case, while also warning the users to migrate. --- setuptools/command/egg_info.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'setuptools/command/egg_info.py') diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index 4165c35e..8ae27d87 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -618,7 +618,15 @@ class manifest_maker(sdist): 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): -- cgit v1.2.1 From e2220331136c3a60b8d70a6b4eaad05816c9b637 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 5 Nov 2021 14:14:29 +0000 Subject: Use warning instead of log for distutils command As discussed in #2855, using an actual warning instead of the logger allow users to control what gets displayed via warning filters. --- setuptools/command/egg_info.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'setuptools/command/egg_info.py') diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py index 8ae27d87..f2210292 100644 --- a/setuptools/command/egg_info.py +++ b/setuptools/command/egg_info.py @@ -621,10 +621,11 @@ class manifest_maker(sdist): if hasattr(build_py, 'get_data_files_without_manifest'): return build_py.get_data_files_without_manifest() - log.warn( + warnings.warn( "Custom 'build_py' does not implement " "'get_data_files_without_manifest'.\nPlease extend command classes" - " from setuptools instead of distutils." + " from setuptools instead of distutils.", + SetuptoolsDeprecationWarning ) return build_py.get_data_files() -- cgit v1.2.1