search

drivendataorg / repro-zipfile

A tiny, zero-dependency replacement for Python's zipfile.ZipFile for creating reproducible/deterministic ZIP archives.
Other
12 stars 4 forks source link

Ensure that the timezone doesn't leak into zips. #8

Closed thatch closed 8 months ago

thatch commented 8 months ago

Hello from a fellow connoisseur of zip files. I saw a new release of this project and wondered how you did it, and found one source of non-deterministic builds in that the system timezone affects how SOURCE_DATE_EPOCH is used.

I'm not able to run the tests on CI in my fork, so I don't know if Windows is passing with this test. I'm a little concerned that "Availability: Unix" in the docs for time.tzset means the function doesn't even exist, but I'm hoping it does exist and is a no-op. CI will tell.

codecov[bot] commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (be33cff) 90.7% compared to head (660a6d4) 90.6%. Report is 3 commits behind head on main.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #8 +/- ## ======================================= - Coverage 90.7% 90.6% -0.1% ======================================= Files 2 2 Lines 162 161 -1 ======================================= - Hits 147 146 -1 Misses 15 15 ``` | [Files](https://app.codecov.io/gh/drivendataorg/repro-zipfile/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg) | Coverage Δ | | |---|---|---| | [repro\_zipfile.py](https://app.codecov.io/gh/drivendataorg/repro-zipfile/pull/8?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg#diff-cmVwcm9femlwZmlsZS5weQ==) | `86.9% <100.0%> (ø)` | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/drivendataorg/repro-zipfile/pull/8/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=drivendataorg)
jayqi commented 8 months ago

Ahhh, thanks for the catch! I definitely did not pay enough attention when I looked up how to convert from epoch seconds to a struct_time object.

I approved this PR to run the CI, and unfortunately, it looks like time.tzset indeed is not available for Windows. Based on this StackOverflow post, I don't think there's an alternative way to change the timezone either. I'm fine with just making the timezone change a no-op on Windows and skip testing it for Windows.