Preserve package_data's mode when its copy is skipped as up to date - #5319
Open
afonsojanu wants to merge 2 commits into
Open
Preserve package_data's mode when its copy is skipped as up to date#5319afonsojanu wants to merge 2 commits into
afonsojanu wants to merge 2 commits into
Conversation
A package_data file with a .py extension, sitting in a directory that setuptools' own package discovery also picks up as a package, gets copied twice: once as a module through build_packages (which always copies with preserve_mode=False), and once as package data through build_package_data (which does preserve mode). The second copy is the one meant to win, but copy_file's own "only copy if the source is newer" check can decide the file is already up to date and skip it outright, since both copies land within the same build and read the same source mtime. Mode isn't part of that staleness comparison at all, so a skip there left the file stuck on whatever the module copy wrote, silently dropping things like an executable bit. build_package_data now copies the mode directly from the source file after calling copy_file, regardless of whether copy_file decided the content itself needed refreshing. This is cheap and idempotent, and keeps the two copies from disagreeing about a file's mode no matter which one happened to run last.
Contributor
|
Tick the box to add this pull request to the merge queue (same as
|
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes
A package_data file with a .py extension can end up living in a directory that setuptools' own package discovery also treats as a package (most commonly an implicit namespace package with no
__init__.py). When that happens the file gets built twice: once as a plain module, which copies without preserving mode, and once as package data, which does preserve mode. The second copy is supposed to win, butcopy_file's own staleness check can decide the destination is already up to date (both copies happen moments apart, reading the same source mtime) and skip the byte copy entirely, leaving the file stuck on whatever mode the module copy left it in. That's how an executable script shipped throughpackage_dataquietly loses its executable bit once it collides with module discovery this way.build_package_datanow copies the file's mode directly from its source right after callingcopy_file, independent of whethercopy_fileitself decided a fresh byte copy was needed. Added a regression test reproducing the exact collision (a package registered alongside a subpackage that only exists because of a package_data file inside it), and confirmed it fails without the fix and passes with it.Closes #5296
Pull Request Checklist
newsfragments/.(See documentation for details)