diff --git a/newsfragments/5296.bugfix.rst b/newsfragments/5296.bugfix.rst new file mode 100644 index 0000000000..b717a3acf2 --- /dev/null +++ b/newsfragments/5296.bugfix.rst @@ -0,0 +1 @@ +Fixed a regression where the executable bit (and other mode bits) could be lost for ``package_data`` files that are also collected as Python modules (e.g. ``.py`` files inside PEP 420 implicit namespace subpackages), because the mode-restoring copy performed by ``build_py.build_package_data`` could be silently skipped by an ``mtime``-based "already up-to-date" check -- by :user:`agu2347` diff --git a/setuptools/command/build_py.py b/setuptools/command/build_py.py index 3c7c2d1bd6..48b4af2f90 100644 --- a/setuptools/command/build_py.py +++ b/setuptools/command/build_py.py @@ -172,6 +172,17 @@ def build_package_data(self) -> None: for target, srcfile in self._get_package_data_output_mapping(): self.mkpath(os.path.dirname(target)) _outf, _copied = self.copy_file(srcfile, target) + # `copy_file` may skip the actual copy (and with it, the + # mode-preserving chmod) if `target` already looks + # up-to-date based on an mtime comparison. This can happen + # when the same file was already staged by build_module() + # with preserve_mode=False, e.g. for package_data files + # that are also collected as Python modules (as happens + # with PEP 420 implicit namespace subpackages). Explicitly + # sync the mode bits here so package_data files always end + # up with the permissions of their source, regardless of + # whether the "up-to-date" skip kicked in. See #5296. + os.chmod(target, stat.S_IMODE(os.stat(srcfile).st_mode)) make_writable(target) def analyze_manifest(self) -> None: diff --git a/setuptools/tests/test_build_py.py b/setuptools/tests/test_build_py.py index 62ed408f3c..5ab32bc6df 100644 --- a/setuptools/tests/test_build_py.py +++ b/setuptools/tests/test_build_py.py @@ -122,6 +122,47 @@ def test_executable_data(tmpdir_cwd): ) +@pytest.mark.xfail( + 'platform.system() == "Windows"', + reason="On Windows, files do not have executable bits", + raises=AssertionError, + strict=True, +) +def test_executable_data_in_implicit_namespace_subpackage(tmpdir_cwd): + """ + A package_data file that lives inside an implicit namespace + subpackage (no __init__.py) is copied twice while building: + once by build_module() (which discovers it as a Python module + and copies it with preserve_mode=False), and once by + build_package_data() (which should copy it with preserve_mode=True + to restore the correct mode). If both copies end up with + identical mtimes, the "up-to-date" staleness check in copy_file + can cause the second, mode-restoring copy to be skipped, leaving + the file non-executable. + + Regression test for #5296. + """ + dist = Distribution( + dict( + script_name='setup.py', + script_args=['build_py'], + packages=['pkg', 'pkg.scripts'], + package_data={'pkg': ['scripts/*']}, + ) + ) + os.makedirs('pkg/scripts') + open('pkg/__init__.py', 'wb').close() + open('pkg/scripts/run-me.py', 'wb').close() + os.chmod('pkg/scripts/run-me.py', 0o700) + + dist.parse_command_line() + dist.run_commands() + + assert os.stat('build/lib/pkg/scripts/run-me.py').st_mode & stat.S_IEXEC, ( + "Script is not executable" + ) + + EXAMPLE_WITH_MANIFEST = { "setup.cfg": DALS( """