-
Notifications
You must be signed in to change notification settings - Fork 895
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
Respect $SOURCE_DATE_EPOCH in generate_bram_types_sim.py #4683
base: main
Are you sure you want to change the base?
Conversation
Downstream pull request: https://src.fedoraproject.org/rpms/yosys/pull-request/5 |
with open(filename, "w") as f: | ||
f.write("// **AUTOGENERATED FILE** **DO NOT EDIT**\n") | ||
f.write(f"// Generated by {sys.argv[0]} at {datetime.now(timezone.utc)}\n") | ||
f.write(f"// Generated by {sys.argv[0]} at {build_date}\n") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we even need the build date here? The file modification time indicates it already...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's particularly useful, but meh, I don't care either way, as long as the file doesn't change between rebuilds ;)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can go ahead and remove the generation time outright. It's an outlier among the other "Generated by" string formatting in the codebase
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated.
I'm working on build reproducibility of Fedora packages, and this patch fixes an issue observed in test rebuilds: the timestamp was set to the actual time of the build, making builds nonreproducible. Other "Generated by" strings do not include a timestamp, so drop it here too.
ca688a8
to
26a3478
Compare
What are the reasons/motivation for this change?
I'm working on build reproducibility of Fedora packages, and this patch fixes an issue observed in test rebuilds.
Explain how this is achieved.
The variable is set in build environments to allow the build to be reproducible (identical result in independent builds), see https://reproducible-builds.org/docs/source-date-epoch/. The code snippet is based on the sample from that page too.
If applicable, please suggest to reviewers how they can test the change.
Generate the output without $SOURCE_DATE_EPOCH set, which should result in a file with a current timestamp, and then again with SOURCE_DATE_EPOCH set, which should result in the specified timestamp.