gh-125260: gzip: let compress mtime default to 0#125261
gh-125260: gzip: let compress mtime default to 0#125261AA-Turner merged 4 commits intopython:mainfrom
Conversation
|
This needs a NEWS entry, as this is a user-facing error. Also, could you add a test for this? |
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
I have rebased it onto main now. I had probably based it off the old master branch. I would appreciate some help with the NEWS and test. I only saw Misc/NEWS.d/3.13.0a1.rst but nothing for 3.14 yet. |
|
@ZeroIntensity I'm not sure about backport labels here as the function works as intended (so this is a feature) -- what was your rationale? |
|
Hmm, is this a feature? The original issue was marked as a bug, so I thought it would be backportable. |
|
It was opened with the bug template, hence the bug label, but it's a judgement call as to 'bug' or 'feature'. Personally I would be hesitant to backport, but others may have different opinions. A |
|
I think that's right, I'll update the labels. My bad! |
3750071 to
c83107b
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @AA-Turner: please review the changes made to this pull request. |
this follows GNU gzip, which defaults to store 0 as mtime for compressing stdin, where no file mtime is involved This makes gzip.compress(str) output deterministic by default and greatly helps reproducible builds.
mcepl
left a comment
There was a problem hiding this comment.
Yes, that looks like a great idea!
The reason why Bernhard would like to consider it as a bug (and thus backport it) is that it breaks reproducible builds. |
|
As AA-Turner said, "the function works as intended (so this is a feature)" and thus not a bug. Only bugs can be backported. |
|
@bmwiedemann please don't force-push, as it makes PRs harder to review. See the devguide for similar guidance. We squash-merge all commits. |
AA-Turner
left a comment
There was a problem hiding this comment.
I've had to leave a couple of comments as diffs rather than suggestions due to limitations in the GH review interface -- sorry.
A
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
|
All commit authors signed the Contributor License Agreement. |
…eZ0Mb.rst Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
|
Thanks @bmwiedemann! A |
this follows GNU gzip, which defaults to
store 0 as mtime for compressing stdin, where no file mtime is involved
This makes gzip.compress(str) output deterministic by default and greatly helps reproducible builds.
📚 Documentation preview 📚: https://cpython-previews--125261.org.readthedocs.build/