From ed5d4bf58c70d5b4ad7b8ad42b27599e6a24be13 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 5 May 2022 19:33:20 +0100 Subject: [PATCH 1/5] Add test that capture transitional behaviour for build_py During the transition, `build_py` should warn when a module or package is included in the distribution as if it was "package data". --- setuptools/tests/test_build_py.py | 71 +++++++++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/setuptools/tests/test_build_py.py b/setuptools/tests/test_build_py.py index 19c8b780b8..ab591adc1e 100644 --- a/setuptools/tests/test_build_py.py +++ b/setuptools/tests/test_build_py.py @@ -3,9 +3,14 @@ import shutil import pytest +import jaraco.path +from path import Path +from setuptools import SetuptoolsDeprecationWarning from setuptools.dist import Distribution +from .textwrap import DALS + def test_directories_in_package_data_glob(tmpdir_cwd): """ @@ -79,3 +84,69 @@ def test_executable_data(tmpdir_cwd): assert os.stat('build/lib/pkg/run-me').st_mode & stat.S_IEXEC, \ "Script is not executable" + + +def test_excluded_subpackages(tmp_path): + files = { + "setup.cfg": DALS(""" + [metadata] + name = mypkg + version = 42 + + [options] + include_package_data = True + packages = find: + + [options.packages.find] + exclude = *.tests* + """), + "mypkg": { + "__init__.py": "", + "resource_file.txt": "", + "tests": { + "__init__.py": "", + "test_mypkg.py": "", + "test_file.txt": "", + } + }, + "MANIFEST.in": DALS(""" + global-include *.py *.txt + global-exclude *.py[cod] + prune dist + prune build + prune *.egg-info + """) + } + + with Path(tmp_path): + jaraco.path.build(files) + dist = Distribution({"script_name": "%PEP 517%"}) + dist.parse_config_files() + + build_py = dist.get_command_obj("build_py") + msg = r"Python recognizes 'mypkg\.tests' as an importable package" + with pytest.warns(SetuptoolsDeprecationWarning, match=msg): + # TODO: To fix #3260 we need some transition period to deprecate the + # existing behavior of `include_package_data`. After the transition, we + # should remove the warning and fix the behaviour. + build_py.finalize_options() + build_py.run() + + build_dir = Path(dist.get_command_obj("build_py").build_lib) + assert (build_dir / "mypkg/__init__.py").exists() + assert (build_dir / "mypkg/resource_file.txt").exists() + + # Setuptools is configured to ignore `mypkg.tests`, therefore the following + # files/dirs should not be included in the distribution. + for f in [ + "mypkg/tests/__init__.py", + "mypkg/tests/test_mypkg.py", + "mypkg/tests/test_file.txt", + "mypkg/tests", + ]: + with pytest.raises(AssertionError): + # TODO: Enforce the following assertion once #3260 is fixed + # (remove context manager and the following xfail). + assert not (build_dir / f).exists() + + pytest.xfail("#3260") From 56a8b90551411024f792efe853c48de5e5097e59 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 5 May 2022 19:34:46 +0100 Subject: [PATCH 2/5] Warn about packages/modules included as package data --- setuptools/command/build_py.py | 56 ++++++++++++++++++++++++++++++++-- 1 file changed, 54 insertions(+), 2 deletions(-) diff --git a/setuptools/command/build_py.py b/setuptools/command/build_py.py index c3fdc0927c..ba7b725993 100644 --- a/setuptools/command/build_py.py +++ b/setuptools/command/build_py.py @@ -8,6 +8,9 @@ import distutils.errors import itertools import stat +import warnings +from pathlib import Path +from setuptools._deprecation_warning import SetuptoolsDeprecationWarning from setuptools.extern.more_itertools import unique_everseen @@ -129,6 +132,7 @@ def analyze_manifest(self): src_dirs[assert_relative(self.get_package_dir(package))] = package self.run_command('egg_info') + checker = _IncludePackageDataAbuse() ei_cmd = self.get_finalized_command('egg_info') for path in ei_cmd.filelist.files: d, f = os.path.split(assert_relative(path)) @@ -139,8 +143,13 @@ def analyze_manifest(self): d, df = os.path.split(d) f = os.path.join(df, f) if d in src_dirs: - if path.endswith('.py') and f == oldf: - continue # it's a module, not data + if f == oldf: + if checker.is_module(f): + continue # it's a module, not data + else: + importable = checker.importable_item(src_dirs[d], f) + if importable: + checker.warn(importable) mf.setdefault(src_dirs[d], []).append(path) def get_data_files(self): @@ -240,3 +249,46 @@ def assert_relative(path): % path ) raise DistutilsSetupError(msg) + + +class _IncludePackageDataAbuse: + """Inform users that package or module is included as 'data file'""" + + MESSAGE = """\ + !!\n\n + ################################### + # Package/module would be ignored # + ################################### + Python recognizes {importable!r} as an importable package or module, however + it is included in the distribution as "data". + This behavior is likely to change in future versions of setuptools (and + therefore is considered deprecated). + + Please make sure that {importable!r} is recognized as a package/module by using + setuptools' `packages` configuration field or the proper package discovery methods. + + To find more information, look for "package discovery" and "data files" on + setuptools documentation page. + \n\n!! + """ + + def __init__(self): + self._already_warned = set() + + def is_module(self, file): + return file.endswith(".py") and file[:-len(".py")].isidentifier() + + def importable_item(self, pkg, file): + path = Path(file) + parents = path.parent.parts + module = [path.stem] if tuple(path.suffixes) == (".py",) else [] + parts = list(itertools.takewhile(str.isidentifier, [*parents, *module])) + if parts: + return ".".join([pkg, *parts]) + return None + + def warn(self, importable): + if importable not in self._already_warned: + msg = textwrap.dedent(self.MESSAGE).format(importable=importable) + warnings.warn(msg, SetuptoolsDeprecationWarning, stacklevel=2) + self._already_warned.add(importable) From f49868eb8a02bea3689ff4fdb54a5fc095d85d99 Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 5 May 2022 19:59:17 +0100 Subject: [PATCH 3/5] Add news fragment --- changelog.d/3308.deprecation.rst | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 changelog.d/3308.deprecation.rst diff --git a/changelog.d/3308.deprecation.rst b/changelog.d/3308.deprecation.rst new file mode 100644 index 0000000000..550da6b55f --- /dev/null +++ b/changelog.d/3308.deprecation.rst @@ -0,0 +1,8 @@ +Relying on ``include_package_data`` to ensure sub-packages are automatically +added to the build wheel distribution (as "data") is now considered a +deprecated practice. + +This behaviour was controversial and caused inconsistencies (#3260). + +Instead, projects are encouraged to properly configure ``packages`` or use +discovery tools. General information can be found in :doc:`userguide/package_discovery`. From 614c9f6210464c34d05a87cd5b790a24e6c12adc Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Thu, 5 May 2022 19:59:52 +0100 Subject: [PATCH 4/5] Quickfix missing comment mark in docs --- docs/userguide/quickstart.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/userguide/quickstart.rst b/docs/userguide/quickstart.rst index 2f77852178..7577b8b119 100644 --- a/docs/userguide/quickstart.rst +++ b/docs/userguide/quickstart.rst @@ -131,7 +131,7 @@ the packages in your project directory: [options] packages = find: # OR `find_namespaces:` if you want to use namespaces - [options.packages.find] (always `find` even if `find_namespaces:` was used before) + [options.packages.find] # (always `find` even if `find_namespaces:` was used before) # This section is optional # Each entry in this section is optional, and if not specified, the default values are: # `where=.`, `include=*` and `exclude=` (empty). From 2b218927334c58a655fc285a5c241828d394cffe Mon Sep 17 00:00:00 2001 From: Anderson Bravalheri Date: Fri, 6 May 2022 10:09:48 +0100 Subject: [PATCH 5/5] Simplify checks for abuse of include_package_data Previously, the checks would result in a warning per module additionally to the parent package. Now only one warning per parent package is issued. --- setuptools/command/build_py.py | 36 ++++++++++++++++------------------ 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/setuptools/command/build_py.py b/setuptools/command/build_py.py index ba7b725993..62f61e04f9 100644 --- a/setuptools/command/build_py.py +++ b/setuptools/command/build_py.py @@ -132,7 +132,7 @@ def analyze_manifest(self): src_dirs[assert_relative(self.get_package_dir(package))] = package self.run_command('egg_info') - checker = _IncludePackageDataAbuse() + check = _IncludePackageDataAbuse() ei_cmd = self.get_finalized_command('egg_info') for path in ei_cmd.filelist.files: d, f = os.path.split(assert_relative(path)) @@ -144,12 +144,12 @@ def analyze_manifest(self): f = os.path.join(df, f) if d in src_dirs: if f == oldf: - if checker.is_module(f): + if check.is_module(f): continue # it's a module, not data else: - importable = checker.importable_item(src_dirs[d], f) + importable = check.importable_subpackage(src_dirs[d], f) if importable: - checker.warn(importable) + check.warn(importable) mf.setdefault(src_dirs[d], []).append(path) def get_data_files(self): @@ -256,19 +256,19 @@ class _IncludePackageDataAbuse: MESSAGE = """\ !!\n\n - ################################### - # Package/module would be ignored # - ################################### - Python recognizes {importable!r} as an importable package or module, however - it is included in the distribution as "data". + ############################ + # Package would be ignored # + ############################ + Python recognizes {importable!r} as an importable package, however it is + included in the distribution as "data". This behavior is likely to change in future versions of setuptools (and therefore is considered deprecated). - Please make sure that {importable!r} is recognized as a package/module by using - setuptools' `packages` configuration field or the proper package discovery methods. + Please make sure that {importable!r} is included as a package by using + setuptools' `packages` configuration field or the proper discovery methods. - To find more information, look for "package discovery" and "data files" on - setuptools documentation page. + You can read more about "package discovery" and "data files" on setuptools + documentation page. \n\n!! """ @@ -278,13 +278,11 @@ def __init__(self): def is_module(self, file): return file.endswith(".py") and file[:-len(".py")].isidentifier() - def importable_item(self, pkg, file): - path = Path(file) - parents = path.parent.parts - module = [path.stem] if tuple(path.suffixes) == (".py",) else [] - parts = list(itertools.takewhile(str.isidentifier, [*parents, *module])) + def importable_subpackage(self, parent, file): + pkg = Path(file).parent + parts = list(itertools.takewhile(str.isidentifier, pkg.parts)) if parts: - return ".".join([pkg, *parts]) + return ".".join([parent, *parts]) return None def warn(self, importable):