-
-
Notifications
You must be signed in to change notification settings - Fork 32.7k
gh-75707: tarfile: Add optional open() argument "mtime" #138117
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
base: main
Are you sure you want to change the base?
Conversation
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
5c79be4
to
40264fc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please update the documentation of tarfile.open() & co.
Do you think I should do something similar to |
I'm actually torn between:
|
I just realized my test is not failable, since |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it's better to just allow setting a timestamp (eg: timestamp
or mtime
)?
Reproducibility is a strong promise that should be confirmed with tests: is it across the same files? the same user? the same machine? are supported compressions fully deterministic always?
Lines 891 to 892 in 96b7a2e
uid = 'User ID of the user who originally stored this member.', gid = 'Group ID of the user who originally stored this member.', Lines 901 to 902 in 96b7a2e
uname = 'User name.', gname = 'Group name.',
Perhaps we could enable the user to create a reproducible file, and document on how.
40264fc
to
8087788
Compare
Good points. That was very naïve of me :) |
The GNU tar documentation has some pointers for making "more reproducible" archives (https://www.gnu.org/software/tar/manual/html_section/Reproducibility.html). |
8087788
to
112aa92
Compare
This PR is now just adding the |
Is there a test to ensure that not setting the mtime still functions correctly? |
112aa92
to
5c472a0
Compare
I added tests that use the workaround of temporarily setting |
That's a fine solution for single-threaded tests, but if test are run in parallel there could eventually be hard-to-reproduce bugs. |
5c472a0
to
3bdbfee
Compare
This makes it possible to set the gzip header mtime field without overriding time.time(), making it useful when creating reproducible archives.
3bdbfee
to
7a7b637
Compare
This makes it possible to create reproducible .tar.gz files without overriding time.time(), by setting the gzip header field mtime to 0.