gh-91279: ZipFile.writestr now respect SOURCE_DATE_EPOCH #124435
Conversation
|
All commit authors signed the Contributor License Agreement. |
Misc/NEWS.d/next/Library/2024-09-24-20-25-52.gh-issue-91279.9oMUwW.rst
Outdated
Show resolved
Hide resolved
|
Could you add a test for this? |
Yes, this change should also be synchronized in the tests. Note: https://github.com/python/cpython/blob/main/Lib/test/test_zipfile/_path/test_path.py#L668-L668 |
|
Oh! TIL |
Wulian233
force-pushed
the
SOURCE_DATE_EPOCH
branch
from
115adc6 to
6a19ed1
Compare
|
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 |
Wulian233
force-pushed
the
SOURCE_DATE_EPOCH
branch
from
b7ca376 to
3126ac3
Compare
Wulian233
force-pushed
the
SOURCE_DATE_EPOCH
branch
from
3126ac3 to
81376fe
Compare
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @jaraco: please review the changes made to this pull request. |
Lib/zipfile/__init__.py
Outdated
| # gh-91279: Set the SOURCE_DATE_EPOCH to a specific timestamp | ||
| epoch = os.environ.get('SOURCE_DATE_EPOCH') | ||
| get_time = int(epoch) if epoch else time.time() | ||
| self.date_time = time.gmtime(get_time)[:6] |
There was a problem hiding this comment.
I'm a little uneasy that time.localtime() is now replaced by time.gmtime(). That means that the value will change depending on the local time zone of the machine running the code, which may not be adequately exercised in the code. Are there existing tests that validate that gmtime is the correct usage here?
There was a problem hiding this comment.
I checked the documentation. Should time.gmtime return UTC time and not change with time zones? time.localtime says "Like gmtime() but converts to local time"
There was a problem hiding this comment.
What I mean is that this change alters the behavior for all cases. The issue doesn't make mention of gmtime and only mentions localtime in relation to the current implementation. What's the motivation for changing it?
There was a problem hiding this comment.
Sorry, I see that I was mistaken. To implement this PR I originally thought it was necessary to change it to gmtime, but in fact, this modification should not have been made😥
|
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 made the requested changes; please review again Sorry again for the inconvenience |
|
Thanks for making the requested changes! @jaraco: please review the changes made to this pull request. |
No
SOURCE_DATE_EPOCHenvironment variable makes pip installing a package generate non-reproducible build artifacts.SOURCE_DATE_EPOCHis a standardised environment variable that distributions can set centrally and have build tools consume this in order to produce reproducible output. In practice, specifies the last modification of something, usually the source code, measured in the number seconds since the Unix epoch, ie. .SOURCE_DATE_EPOCHJanuary 1st 1970, 00:00:00 UTCSee https://reproducible-builds.org/docs/source-date-epoch/
details in issue: #91279