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 Build Date from changelog again #933

Closed

Conversation

@bmwiedemann
Copy link
Contributor

bmwiedemann commented Nov 10, 2019

Set Build Date from changelog again

This is needed because fa303d5 had moved the code that used getenv("SOURCE_DATE_EPOCH") to run before we do setenv here.

See https://reproducible-builds.org/ for why this is good.

Fixes #932

@bmwiedemann bmwiedemann force-pushed the bmwiedemann:builddate branch from 98b8744 to 29d32b3 Nov 10, 2019
@Conan-Kudo

This comment has been minimized.

Copy link
Member

Conan-Kudo commented Nov 10, 2019

@pmatilai I think this qualifies as a regression to fix in 4.15.1.

@pmatilai

This comment has been minimized.

Copy link
Contributor

pmatilai commented Nov 11, 2019

Why do we need this code in two places now?

@bmwiedemann

This comment has been minimized.

Copy link
Contributor Author

bmwiedemann commented Nov 11, 2019

Why do we need this code in two places now?

The old one is still needed in case SOUCE_DATE_EPOCH is set by the caller of rpmbuild.
The new one is now needed for the case where we use the last changelog date via %source_date_epoch_from_changelog Y

The duplication is rather small, though.

@pmatilai

This comment has been minimized.

Copy link
Contributor

pmatilai commented Nov 11, 2019

It's not a big duplication but such things look fishy nevertheless.
At the very least that explanation needs to go the the commit message.

fa303d5 had moved the code that used SOURCE_DATE_EPOCH
to run before we do setenv here.

The old code is still needed in case SOUCE_DATE_EPOCH is set by
the caller of rpmbuild.
The new code is now needed for the case where we use the last
changelog date via %source_date_epoch_from_changelog Y

See https://reproducible-builds.org/ for why this is good.

Fixes #932
@bmwiedemann bmwiedemann force-pushed the bmwiedemann:builddate branch from 29d32b3 to 7182540 Nov 11, 2019
@bmwiedemann

This comment has been minimized.

Copy link
Contributor Author

bmwiedemann commented Nov 11, 2019

Added it.

@pmatilai

This comment has been minimized.

Copy link
Contributor

pmatilai commented Nov 11, 2019

Thanks.

In the meanwhile, I ended up looking at the bigger picture, commit fa303d5 moved buildhost and buildtime calculation to a wrong place conceptually. So when it's wrong both conceptually and functionally... can you test if #935 works for you?

@pmatilai

This comment has been minimized.

Copy link
Contributor

pmatilai commented Nov 11, 2019

Resolved via #935, thanks for bringing this to our attention and the patch though!

@pmatilai pmatilai closed this Nov 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.