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

Allow to override build date with SOURCE_DATE_EPOCH #11227

Merged
merged 1 commit into from Dec 2, 2019

Conversation

@bmwiedemann
Copy link
Contributor

bmwiedemann commented Dec 2, 2019

Allow to override build date with SOURCE_DATE_EPOCH
in order to make builds reproducible.
See https://reproducible-builds.org/ for why this is good
and https://reproducible-builds.org/specs/source-date-epoch/
for the definition of this variable.

Kind: documentation / bug fix / feature / performance / infrastructure.

Fixes / closes #11037

  • Added / updated test-suite

This PR was done while working on reproducible builds for openSUSE.

@bmwiedemann bmwiedemann requested a review from ejgallego as a code owner Dec 2, 2019
configure.ml Outdated Show resolved Hide resolved
configure.ml Outdated Show resolved Hide resolved
Copy link
Member

ejgallego left a comment

Indeed, this does not work as noted by Gaëtan.

@bmwiedemann bmwiedemann force-pushed the bmwiedemann:date branch from 2b966e2 to f29e955 Dec 2, 2019
@bmwiedemann bmwiedemann requested review from SkySkimmer and ejgallego Dec 2, 2019
@bmwiedemann bmwiedemann force-pushed the bmwiedemann:date branch from f29e955 to a2f0ff0 Dec 2, 2019
@bmwiedemann bmwiedemann requested a review from coq/doc-maintainers as a code owner Dec 2, 2019
Copy link
Member

ejgallego left a comment

Thanks for the patch @bmwiedemann , it looks good to me!

IMHO we should document this variable in configure -help, so I propose you add a brief documentation and fix the indentation while you are at it.

configure.ml Outdated Show resolved Hide resolved
@bmwiedemann bmwiedemann force-pushed the bmwiedemann:date branch from a2f0ff0 to 11a262f Dec 2, 2019
@bmwiedemann

This comment has been minimized.

Copy link
Contributor Author

bmwiedemann commented Dec 2, 2019

I don't think it should go into configure -help output, because people doing reproducible builds already know very well about this variable, because it is used for gcc, rpm and so many more tools.
And other people will probably not need it.

But if you still want it, I will not be unhappy. I would just need some help finding the right place to patch.

@ejgallego

This comment has been minimized.

Copy link
Member

ejgallego commented Dec 2, 2019

I don't think it should go into configure -help output, because people doing reproducible builds already know very well about this variable, because it is used for gcc, rpm and so many more tools.

@bmwiedemann your call, you are the expert here. I am fine merging as is; if you want to add some documentation the easiest would be to tweak the "Available options" message parse_args () = ...

I certainly didn't know about the variable even if I am a bit familiar with RB.

This comment has been minimized.

Copy link
Member

Zimmi48 commented Dec 2, 2019 — with Octobox

Here or elsewhere (e.g. INSTALL file) but that Coq supports this absolutely needs to be documented beyond the changelog.

in order to make builds reproducible.
See https://reproducible-builds.org/ for why this is good
and https://reproducible-builds.org/specs/source-date-epoch/
for the definition of this variable.

Fixes #11037
@bmwiedemann bmwiedemann force-pushed the bmwiedemann:date branch from 11a262f to 80bd376 Dec 2, 2019
@bmwiedemann bmwiedemann requested a review from Zimmi48 as a code owner Dec 2, 2019
@bmwiedemann

This comment has been minimized.

Copy link
Contributor Author

bmwiedemann commented Dec 2, 2019

@Zimmi48 added some lines to INSTALL. Can you think of a better way to place/word it?

@Zimmi48
Zimmi48 approved these changes Dec 2, 2019
Copy link
Member

Zimmi48 left a comment

This wording does look pretty good to me.

@Zimmi48 Zimmi48 added this to the 8.11.0 milestone Dec 2, 2019
@ejgallego ejgallego merged commit 80bd376 into coq:master Dec 2, 2019
6 of 8 checks passed
6 of 8 checks passed
GitLab CI job library:ci-geocoq (pull request) script_failure on GitLab CI
Details
GitLab CI pipeline (pull request) Pipeline is running on GitLab CI
Details
coq.coq #20191202.30 succeeded
Details
coq.coq (Windows) Windows succeeded
Details
coq.coq (macOS) macOS succeeded
Details
doc:ml-api:odoc: ml-api artifact Link to ml-api build artifact.
Details
doc:refman: refman artifact Link to refman build artifact.
Details
doc:refman: stdlib artifact Link to stdlib build artifact.
Details
ejgallego added a commit that referenced this pull request Dec 2, 2019
Reviewed-by: Zimmi48
Reviewed-by: ejgallego
@coqbot coqbot added this to Request 8.11.0 inclusion in Coq 8.11 Dec 2, 2019
@bmwiedemann bmwiedemann deleted the bmwiedemann:date branch Dec 2, 2019
@bmwiedemann

This comment has been minimized.

Copy link
Contributor Author

bmwiedemann commented Dec 2, 2019

Thanks for your quick and helpful feedback.

For the remaining other issue I opened #11229

ppedrot added a commit to ppedrot/coq that referenced this pull request Dec 7, 2019
…EPOCH
@coqbot coqbot moved this from Request 8.11.0 inclusion to Shipped in 8.10.0 in Coq 8.11 Dec 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Coq 8.11
  
Shipped in 8.11.0
4 participants
You can’t perform that action at this time.