Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warn about deprecation of behaviour that considers modules/packages as "data" when include_package_data=True #3308

Merged
merged 5 commits into from May 16, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions 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`.
2 changes: 1 addition & 1 deletion docs/userguide/quickstart.rst
Expand Up @@ -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).
Expand Down
54 changes: 52 additions & 2 deletions setuptools/command/build_py.py
Expand Up @@ -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


Expand Down Expand Up @@ -129,6 +132,7 @@ def analyze_manifest(self):
src_dirs[assert_relative(self.get_package_dir(package))] = package

self.run_command('egg_info')
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))
Expand All @@ -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 check.is_module(f):
continue # it's a module, not data
else:
importable = check.importable_subpackage(src_dirs[d], f)
if importable:
check.warn(importable)
mf.setdefault(src_dirs[d], []).append(path)

def get_data_files(self):
Expand Down Expand Up @@ -240,3 +249,44 @@ 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 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 included as a package by using
setuptools' `packages` configuration field or the proper discovery methods.

You can read more about "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_subpackage(self, parent, file):
pkg = Path(file).parent
parts = list(itertools.takewhile(str.isidentifier, pkg.parts))
if parts:
return ".".join([parent, *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)
71 changes: 71 additions & 0 deletions setuptools/tests/test_build_py.py
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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")