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

normalize sdist/wheel artifact file perms #1542

Merged
merged 4 commits into from
May 31, 2024
Merged

normalize sdist/wheel artifact file perms #1542

merged 4 commits into from
May 31, 2024

Conversation

mattp-
Copy link
Contributor

@mattp- mattp- commented May 28, 2024

fixes #1519.
currently hatchling just inherits a 700 on each from tempfile.mkstemp, which isn't particularly friendly to CI/build systems. We apply normalize_file_permissions to set the output artifacts to 644.

wrt tests, I just copypasta'd another one to add a new test explicitly for the perms test; if you'd like i can just remove and tack the logic into one of the existing tests instead (I couldn't find one that seemed specific enough).

currently hatchling just inherits a 700 on each from tempfile.mkstemp,
which isn't particularly friendly to CI/build systems. We apply
normalize_file_permissions to set the output artifacts to 644.
Copy link
Sponsor Collaborator

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

backend/src/hatchling/builders/sdist.py Outdated Show resolved Hide resolved
named normalize_artifact_permissions().
@mattp-
Copy link
Contributor Author

mattp- commented May 28, 2024

@ofek I believe this is ready for a second pass from you, thanks 👍

Copy link
Sponsor Collaborator

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty good! Can you move the modification to right after moving to the final location rather than modifying the temporary file?

@mattp-
Copy link
Contributor Author

mattp- commented May 31, 2024

@ofek sure, moved. should be good now i think 👍

Copy link
Sponsor Collaborator

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you very much!

@ofek ofek merged commit 8a6f3f4 into pypa:master May 31, 2024
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wheels are built with too strict file permissions
2 participants