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

Use pyproject.toml or SOURCE_DATE_EPOCH for generated file mtime #651

Closed
wants to merge 3 commits into from
Closed

Use pyproject.toml or SOURCE_DATE_EPOCH for generated file mtime #651

wants to merge 3 commits into from

Conversation

sneakers-the-rat
Copy link

Resolves (or rather, revisits): poetry-core#142

  • Added tests for changed code.
  • Updated documentation for changed code.

As far as I can find, there is no documentation for setting modified time in generated files.

  • Reviewed contributing docs, ran mypy, pre-commit checks, pytest.

Was wondering why there were a bunch of files with time == 0 on my disk, found this previous PR that set mtime to 0 for generated files for deterministic builds: #142 and also this comment at the end that suggested that pulling the time from pyproject.toml would be a good way to be deterministic while also having a sensible timestamp: #142 (comment) and went ahead and did that, also adding this suggestion to use SOURCE_DATE_EPOCH if set #142 (review)

@sonarcloud
Copy link

sonarcloud bot commented Oct 17, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@dimbleby
Copy link
Contributor

but this makes the build not (easily) reproducible: because the timestamp on pyproject.toml on your CI system is not the same as the timestamp on that file on my computer.

@sneakers-the-rat
Copy link
Author

but this makes the build not (easily) reproducible: because the timestamp on pyproject.toml on your CI system is not the same as the timestamp on that file on my computer.

wouldn't that be true of every file?

@dimbleby
Copy link
Contributor

yes, I guess so.

But in that case it seems that #142 didn't really come close to addressing python-poetry/poetry#1102: and it's hard to tell whether the change proposed here is a step in the right or the wrong direction.

@sneakers-the-rat
Copy link
Author

I think its at least no worse with improvements - any change to pyproject.toml (updating mtime) will also make a new repro build anyway, so by default its at least as good as hardcoding a zero while also not making files that break some of my mtime-based cleanup scripts (thats probably a me problem, but still time = 0 is clearly Not Right)

I Also added in using SOURCE_DATE_EPOCH which as far as I can tell the way towards reproducible builds?
https://reproducible-builds.org/docs/source-date-epoch/
It only applies it to generated files, but the method I wrote just consumes the variable when present and we could apply that to the rest of the files in the sdist relatively easily (if desired, maybe we add another repro build parameter, idk enough about whats desirable here to say, I just offered the PR because it made me set a floor to some cleanup scripts, looked easy to patch, and the prior PR indicated future work that would be good. If its no good then 🤷‍♀️)

@dimbleby
Copy link
Contributor

I don't feel particularly strongly about it. For me personally it's a little chunk of complexity with low value: but then - again for me personally - that's true of a lot of the poetry codebase!

We will leave it to someone who has the power to merge to decide whether they want to do so.

@radoering
Copy link
Member

To summarize:

  • Use deterministic time for generated sdist files #142 made sdist builds repoducible in the sense of "building twice in a row on the same machine". It did not make sdist builds reproducible if you checkout the source on different machines (at different times).
  • This PR does not change the status quo concerning repoducible builds. It just changes how the time of generated files is determined.

I personally don't care much if the time of generated files is 0 or the time of the pyproject.toml. I assume I'd be more eager to change this if there were other/more references where a time of zero is problematic.

Regarding SOURCE_DATE_EPOCH I suppose if you really care for repoducible builds you will want to apply SOURCE_DATE_EPOCH to all files. However, I'm not aware that someone requested more reproducible sdists in the last year. Thus, not sure if it's worth it.

@sneakers-the-rat sneakers-the-rat closed this by deleting the head repository Jul 6, 2024
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