Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Doc/whatsnew/3.14.rst
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,12 @@ zipinfo

(Contributed by Bénédikt Tran in :gh:`123424`.)

* :meth:`zipfile.ZipFile.writestr` now respect ``SOURCE_DATE_EPOCH`` that
distributions can set centrally and have build tools consume this in order
to produce reproducible output.

(Contributed by Jiahao Li in :gh:`91279`.)

.. Add improved modules above alphabetically, not here at the end.

Optimizations
Expand Down
31 changes: 30 additions & 1 deletion Lib/test/test_zipfile/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from random import randint, random, randbytes

from test import archiver_tests
from test.support import script_helper
from test.support import script_helper, os_helper
from test.support import (
findfile, requires_zlib, requires_bz2, requires_lzma,
captured_stdout, captured_stderr, requires_subprocess
Expand Down Expand Up @@ -1781,6 +1781,35 @@ def test_writestr_extended_local_header_issue1202(self):
zinfo.flag_bits |= zipfile._MASK_USE_DATA_DESCRIPTOR # Include an extended local header.
orig_zip.writestr(zinfo, data)

def test_write_with_source_date_epoch(self):
with os_helper.EnvironmentVarGuard() as env:
# Set the SOURCE_DATE_EPOCH environment variable to a specific timestamp
env['SOURCE_DATE_EPOCH'] = "1727440508"

with zipfile.ZipFile(TESTFN, "w") as zf:
zf.writestr("test_source_date_epoch.txt", "Testing SOURCE_DATE_EPOCH")

with zipfile.ZipFile(TESTFN, "r") as zf:
zip_info = zf.getinfo("test_source_date_epoch.txt")
get_time = time.gmtime(int(os.environ['SOURCE_DATE_EPOCH']))[:6]
# Compare each element of the date_time tuple
# Allow for a 1-second difference
for z_time, g_time in zip(zip_info.date_time, get_time):
self.assertAlmostEqual(z_time, g_time, delta=1)

def test_write_without_source_date_epoch(self):
if 'SOURCE_DATE_EPOCH' in os.environ:
del os.environ['SOURCE_DATE_EPOCH']

with zipfile.ZipFile(TESTFN, "w") as zf:
zf.writestr("test_no_source_date_epoch.txt", "Testing without SOURCE_DATE_EPOCH")

with zipfile.ZipFile(TESTFN, "r") as zf:
zip_info = zf.getinfo("test_no_source_date_epoch.txt")
current_time = time.gmtime()[:6]
for z_time, c_time in zip(zip_info.date_time, current_time):
self.assertAlmostEqual(z_time, c_time, delta=1)

def test_close(self):
"""Check that the zipfile is closed after the 'with' block."""
with zipfile.ZipFile(TESTFN2, "w") as zipfp:
Expand Down
6 changes: 5 additions & 1 deletion Lib/zipfile/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -614,7 +614,11 @@ def _for_archive(self, archive: ZipFile) -> Self:

Return self.
"""
self.date_time = time.localtime(time.time())[:6]
# 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

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 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?

Copy link
Contributor Author

@Wulian233 Wulian233 Jan 1, 2025

Choose a reason for hiding this comment

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

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"

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

Copy link
Member

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
Contributor Author

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😥


self.compress_type = archive.compression
self.compress_level = archive.compresslevel
if self.filename.endswith('/'): # pragma: no cover
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
:meth:`zipfile.ZipFile.writestr` now respect ``SOURCE_DATE_EPOCH`` that
distributions can set centrally and have build tools consume this in order
to produce reproducible output.
Loading