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 #1841

Closed
wants to merge 1 commit into from

Conversation

bmwiedemann
Copy link

Allow to override build date with SOURCE_DATE_EPOCH
in order to make builds reproducible when building from tarball.
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.

This date call only works with GNU date, as before.

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

in order to make builds reproducible when building from tarball.
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.

This date call only works with GNU date, as before.

This PR was done while working on reproducible builds for openSUSE.
@pqarmitage
Copy link
Collaborator

@bmwiedemann How does $SOURCE_DATE_EPOCH get set?

I am also not clear why the patch using $SOURCE_DATE_EPOCH is needed. When building in a git tree I can see that this isn't needed, and also when building from a tarball it shouldn't be needed since the generated man pages with the date already set based on the latest commit date of the man page source are included in the tarball.

Having said that about building from the tarball, I can see that it doesn't work properly since the man pages have a build dependency on Makefile. Changing that dependency to be Makefile.am resolves the issue.

The only circumstance (after the change of the dependency from Makefile to Makefile.am) under which I can see $SOURCE_DATE_EPOCH being used is if the code is extracted from a tarball and then make distclean is run before building keepalived. This is not a correct way to build keepalived since datestamps and the git commit from which the tarball was generated are all lost.

At https://reproducible-builds.org/specs/source-date-epoch/ it refers to SOURCE_DATE_EPOCH being generated from the date of the latest commit in the version control system, and I think that is effectively what we do, but we don't use SOURCE_DATE_EPOCH. We could generate SOURCE_DATE_EPOCH when make dist is run, but I am not sure where it should be set, or how it would be used. Also, this wouldn't solve the problem when make distclean is run within an extracted tarball.

Using $SOURCE_DATE_EPOCH in the .TH section of the man pages seems to conflict with [man-pages(7)] (https://www.man7.org/linux/man-pages/man7/man-pages.7.html) which states that the date should be The date of the last nontrivial change that was made to the man page, so I am not clear that using $SOURCE_DATE_EPOCH complies with that (but the current way of doing it doesn't work either).

I think in order to be consistent, doc/man/man1/genhash.1.in also needs to be updated in line with the other man source pages.

I will push a commit that changes the make dependencies from Makefile to Makefile.am, and I look forward to your comments on the above.

@bmwiedemann
Copy link
Author

bmwiedemann commented Jan 29, 2021

In openSUSE, rpmbuild sets SOURCE_DATE_EPOCH from the last keepalived.changes entry time.

If the dependency change fixes it, I'm all for it, as it keeps the code simpler.

Independently of that, you might still want the simplification that comes with

git log -n 1 --format=%ct -- $@.in

@pqarmitage
Copy link
Collaborator

@bmwiedemann I had noticed your improvement re git log -n 1 --format=%ct -- $@.in and was already keen to incorporate that. I also like the way that you detect that the git log command failed, so my inclination is to go with:

`DATE=`date -u --date=@\"\`git log -n 1 --format=%ct -- $@.in 2>/dev/null | grep . || date --reference=$@.in +%s\`\" +\"%Y-%m-%d\"``

Fortunately this also works with the Busybox date command.

Regarding SOURCE_DATE_EPOCH being set by openSUSE rpmbuild, I presume that it is necessary to handle the situation where SOURCE_DATE_EPOCH is not set, in case is being built in an environment that has not got it set?

If you are happy with the above change to setting the date for the man pages, can we close this pull request?

@bmwiedemann
Copy link
Author

Without the pipes, you could also use the return code of git log directly.
So

git log -n 1 --format=%ct -- $@.in 2>/dev/null || date --reference=...

@pqarmitage
Copy link
Collaborator

@bmwiedemann Commit 54cd8e3 implements your suggested improvement above.

I have now added commit 27df5e6 so that the generation of tar files should also be repeatable by setting the file timestamps based on the git commit date of the files.

@bmwiedemann
Copy link
Author

Also worth mentioning that #1842 obsoleted this change.

Thanks for the nice patches.

@bmwiedemann bmwiedemann deleted the date branch January 30, 2021 19:22
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.

None yet

2 participants