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

set SOURCE_DATE_EPOCH from changelog #141

Conversation

bmwiedemann
Copy link
Contributor

set SOURCE_DATE_EPOCH from changelog timestamp if requested by macro
to allow for more reproducible builds of packages.

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.

note: if changelog has only dates without times, time component will always be 12:00:00
but that is usually fine

@bmwiedemann bmwiedemann force-pushed the source_date_epoch_from_changelog branch from dfcf6da to 3eca970 Compare January 30, 2017 11:54
@pmatilai
Copy link
Contributor

Um, changelog dates are a literal part of the spec just like, say, package version or a part of %build scriptlet is, and considered to be UTC so the values are not supposed to change arbitrarily between builds. The ability to override them arbitrarily makes things less reproducable.

If you're seeing different values entered into the changelog across different builds then maybe there's a bug or two, but in that case we should fix the bug(s) instead.

So in other words, I'm not convinced.

@bmwiedemann
Copy link
Contributor Author

This change is not about changing changelog dates, but using the topmost changelog date to set a variable that can be used inside the built software. Please have another look.

@pmatilai
Copy link
Contributor

pmatilai commented Feb 1, 2017

Oops, shouldn't comment on PR's when tired at the end of the day, clearly. Sorry about that.

Copy link
Contributor

@ffesti ffesti left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Builds without warnings.
As changelog needs to be ordered this simple approach is just fine.

The macro should be added to macros.in with some comments on what it does.

Enabling the feature breaks test 008 and 295 due to the rpmlog() message appearing in the output. This is probably unacceptable for rpmspec in general. The output must be limited to actual builds (but it is appreciated there).

if requested by macro
to allow for more reproducible builds of packages.

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.
@bmwiedemann bmwiedemann force-pushed the source_date_epoch_from_changelog branch from 3eca970 to 0f4d8b3 Compare February 1, 2017 09:57
@bmwiedemann
Copy link
Contributor Author

bmwiedemann commented Feb 1, 2017

@ffesti added to macros.in
and disabled the message for now, because I cannot find test 008 and 295 and I do not understand why it would get the message, which should only be output when %source_date_epoch_from_changelog macro is set. Is it set in your test environment? How to test locally?

@ffesti
Copy link
Contributor

ffesti commented Feb 1, 2017

The tests are run by "make check". The numbers are only assigned when actually running them (and change when tests get added in between).
Test 008 calls
rpmspec -q --qf "%{name}" /data/SPECS/hello.spec
And test 295 does basically the same using the Python API to parse the spec file and print some data.
The issue is that under those circumstances additional output is unexpected and unwanted.

@ffesti
Copy link
Contributor

ffesti commented Feb 1, 2017

Thinking a bit more about this this real issue is that the parsing of the spec file should not have a side effect. I'll try to come up with an alternative patch after lunch. Probably moving the code to build/build.c and getting the date from the srpm header.

@ffesti
Copy link
Contributor

ffesti commented Feb 1, 2017

Commited #143 instead. Closing.

@ffesti ffesti closed this Feb 1, 2017
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

3 participants