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

4 files changed, 132 insertions, 3 deletions

diff --git a/changelog.d/3308.deprecation.rst b/changelog.d/3308.deprecation.rst
new file mode 100644
index 00000000..550da6b5
--- /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`.
diff --git a/docs/userguide/quickstart.rst b/docs/userguide/quickstart.rst
index 14e607d4..b157c35a 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).
diff --git a/setuptools/command/build_py.py b/setuptools/command/build_py.py
index ac7cff95..91f47416 100644
--- a/setuptools/command/build_py.py
+++ b/setuptools/command/build_py.py
@@ -9,6 +9,9 @@ import io
 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
 
 
@@ -130,6 +133,7 @@ class build_py(orig.build_py):
             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))
@@ -140,8 +144,13 @@ class build_py(orig.build_py):
                 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):
@@ -241,3 +250,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)
diff --git a/setuptools/tests/test_build_py.py b/setuptools/tests/test_build_py.py
index f6f0d944..13fa64de 100644
--- a/setuptools/tests/test_build_py.py
+++ b/setuptools/tests/test_build_py.py
@@ -3,9 +3,14 @@ import stat
 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):
     """
@@ -102,3 +107,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")