SOURCE_DATE_EPOCH is astandardised 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 UTC

Seehttps://reproducible-builds.org/docs/source-date-epoch/
details in issue:#91279

Issue:ZipFile.writestr should respect SOURCE_DATE_EPOCH #91279

Copy link

ghost commentedSep 24, 2024•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.

bedevere-appbot added the awaiting review label

Sep 24, 2024

bedevere-appbot mentioned this pull request

Sep 24, 2024

ZipFile.writestr should respect SOURCE_DATE_EPOCH#91279

Closed

ZeroIntensity reviewed

Sep 24, 2024

View reviewed changes

Misc/NEWS.d/next/Library/2024-09-24-20-25-52.gh-issue-91279.9oMUwW.rst OutdatedShow resolvedHide resolved

Copy link

Member

ZeroIntensity commentedSep 24, 2024

Could you add a test for this?

Copy link

ContributorAuthor

Wulian233 commentedSep 24, 2024

Could you add a test for this?

Yes, this change should also be synchronized in the tests.

Note:zipfile.writestr is not a function, it is thezipfile.ZipFile.writestr method

https://github.com/python/cpython/blob/main/Lib/test/test_zipfile/_path/test_path.py#L668-L668

Copy link

Member

ZeroIntensity commentedSep 24, 2024

Oh! TIL

Wulian233 requested a review fromjaraco as acode owner

September 25, 2024 11:33

ZeroIntensity reviewed

Sep 25, 2024

View reviewed changes

Lib/test/test_zipfile/_path/test_path.py OutdatedShow resolvedHide resolved

Wulian233 marked this pull request as draft

September 25, 2024 13:40

bedevere-appbot removed the awaiting review label

Sep 25, 2024

jaraco reviewed

Sep 25, 2024

View reviewed changes

Lib/test/test_zipfile/_path/test_path.py OutdatedShow resolvedHide resolved

Wulian233 force-pushed theSOURCE_DATE_EPOCH branch from115adc6 to6a19ed1Compare

September 26, 2024 11:36

Wulian233 commented

Sep 27, 2024

View reviewed changes

Lib/test/test_zipfile/test_core.py OutdatedShow resolvedHide resolved

Wulian233 marked this pull request as ready for review

September 27, 2024 12:56

bedevere-appbot added the awaiting review label

Sep 27, 2024

ZeroIntensity approved these changes

Sep 27, 2024

View reviewed changes

Copy link

Member

ZeroIntensity left a comment

Choose a reason for hiding this comment

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

LGTM.

bedevere-appbot added awaiting core review and removed awaiting review labels

Sep 27, 2024

Wulian233 mentioned this pull request

Nov 2, 2024

gh-123424: addZipInfo._for_archive to set suitable default properties#123429

Merged

jaraco requested changes

Dec 29, 2024

View reviewed changes

Lib/test/test_zipfile/test_core.py OutdatedShow resolvedHide resolved

bedevere-appbot added awaiting changes and removed awaiting core review labels

Dec 29, 2024

Copy link

bedevere-appbot commentedDec 29, 2024

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 phraseI have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Wulian233 force-pushed theSOURCE_DATE_EPOCH branch fromb7ca376 to3126ac3Compare

December 30, 2024 11:44

Wulian233 closed this

Dec 30, 2024

Wulian233 force-pushed theSOURCE_DATE_EPOCH branch from3126ac3 to81376feCompare

December 30, 2024 11:47

finish

693e66f

Wulian233 reopened this

Dec 30, 2024

Copy link

ContributorAuthor

Wulian233 commentedDec 30, 2024•
edited
Loading

I have made the requested changes; please review again

bedevere-appbot added awaiting change review and removed awaiting changes labels

Dec 30, 2024

Copy link

bedevere-appbot commentedDec 30, 2024

Thanks for making the requested changes!

@jaraco: please review the changes made to this pull request.

bedevere-appbot requested a review fromjaraco

December 30, 2024 12:06

Update __init__.py

d07b4ad

jaraco requested changes

Jan 1, 2025

View reviewed changes

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]

Copy link

Member

jaracoJan 1, 2025

Choose a reason for hiding this comment

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

I'm a little uneasy thattime.localtime() is now replaced bytime.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 thatgmtime is the correct usage here?

Copy link

ContributorAuthor

Wulian233Jan 1, 2025•
edited
Loading

Choose a reason for hiding this comment

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

I checked the documentation. Shouldtime.gmtime return UTC time and not change with time zones?time.localtime says "Like gmtime() but converts to local time"

https://docs.python.org/3.14/library/time.html#time.gmtime

Copy link

Member

jaracoJan 1, 2025

Choose a reason for hiding this comment

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

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?

Copy link

ContributorAuthor

Wulian233Jan 1, 2025

Choose a reason for hiding this comment

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

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😥