Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid storing build time in gzip headers #13

Merged
merged 2 commits into from
Jul 25, 2024

Conversation

bmwiedemann
Copy link
Contributor

This allows to create bit-reproducible Python3.12/Python.devhelp.gz output.

See https://reproducible-builds.org/ for why this is good.

This patch was done while working on reproducible builds for openSUSE, sponsored by the NLnet NGI0 fund.

This allows to create bit-reproducible Python3.12/Python.devhelp.gz output.

See https://reproducible-builds.org/ for why this is good.
@AA-Turner
Copy link
Member

AA-Turner commented Jul 17, 2024

Thank you for the PR Bernhard. Please can you add a test?

I wonder also if we should use SOURCE DATE EPOCH instead, are there any drawbacks to lying to users about the GZip creation time?

cc @jayaddison who has done similar work in core Sphinx.

A

@bmwiedemann
Copy link
Contributor Author

bmwiedemann commented Jul 17, 2024

I think, we should avoid the extra logic of handling SOURCE_DATE_EPOCH
This change is equivalent to gzip -n which is the default behaviour since some years for compressing stdin, because there is no point in storing an mtime then. Also, bzip, xz and zstd formats don't have such a mtime header value and the gzip format only supports timestamps until 2106 anyway.

I'd appreciate, if you could add the test. I don't have much experience in python.

@jayaddison
Copy link
Contributor

When it's not possible to remove timestamps entirely from a build process, I think that SOURCE_DATE_EPOCH is generally the next-preferred alternative; partly because we can't know in advance whether the value is used by subsequent processes (and if it is, retaining the ability to influence the input stream via the environment variable, to detect downstream changes, is useful). What do you think?

@bmwiedemann
Copy link
Contributor Author

Do you have a pointer to some consumer of this file? I cannot imagine why they would use this timestamp.
Overall, I'd prefer to keep it simple, because that is less prone to break.

@jayaddison
Copy link
Contributor

Ok, thank you @bmwiedemann; no, I'm not aware of any consumers of that timestamp. I'm a Python contributor here but without write access to the repo, and am interested in reproducible builds, so: I propose that I'll open a pull request against your fork soon (within the next few days) to provide the test coverage.

@jayaddison
Copy link
Contributor

Ok, thank you @bmwiedemann; no, I'm not aware of any consumers of that timestamp. I'm a Python contributor here but without write access to the repo, and am interested in reproducible builds, so: I propose that I'll open a pull request against your fork soon (within the next few days) to provide the test coverage.

This has been opened as bmwiedemann#1 - please run/check the test coverage there (it should fail without the fix in place, and pass once the fix is introduced).

Note: that pull request is opened against the same branch that this pull request originates from -- so merging that one should result in corresponding updates here (including continuous integration checks).

@jayaddison
Copy link
Contributor

I'm not completely sure what is going on with the mypy linting failure; I wasn't initially able to replicate that locally, but can investigate again at some point in the next few days.

@jayaddison
Copy link
Contributor

I'm not completely sure what is going on with the mypy linting failure; I wasn't initially able to replicate that locally, but can investigate again at some point in the next few days.

I believe I've discovered the cause of these (unrelated) type-checking errors, and have opened #14 with a possible solution.

@AA-Turner AA-Turner merged commit 7278043 into sphinx-doc:master Jul 25, 2024
7 of 8 checks passed
@jayaddison
Copy link
Contributor

Thanks again!

AA-Turner pushed a commit that referenced this pull request Jul 27, 2024
Co-authored-by: James Addison <jay@jp-hosting.net>
(cherry picked from commit 7278043)
AA-Turner pushed a commit that referenced this pull request Jul 27, 2024
Co-authored-by: James Addison <jay@jp-hosting.net>
@bmwiedemann bmwiedemann deleted the gzip branch January 7, 2025 06:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants