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

Merged
merged 1 commit into from Feb 1, 2017

Conversation

ffesti
Copy link
Contributor

@ffesti ffesti commented Feb 1, 2017

Alternative implementation for #141

It sets the environment variable during the build stage and not while parsing the spec which should be free from side effects.

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.
@ffesti
Copy link
Contributor Author

ffesti commented Feb 1, 2017

@bmwiedemann would this work for you?
I left you as author of the patch but I can reset it if you wish.

@bmwiedemann
Copy link
Contributor

change looks reasonable. I am giving it a round of testing.
Under what condition would headerGet return false here? OOM? no changelog entry found?

@bmwiedemann
Copy link
Contributor

test looks good.

@ffesti ffesti merged commit 0e87aed into rpm-software-management:master Feb 1, 2017
@ffesti ffesti deleted the SOURCE_DATE_EPOCH branch February 1, 2017 17:41
@ffesti
Copy link
Contributor Author

ffesti commented Feb 2, 2017

Hmm, I am wondering if the
getenv("SOURCE_DATE_EPOCH") == NULL
condition is really correct here. The problem is that rpmbuild might be used to build multiple packages in one go. But even if that was not possible someone using the API could.
Do we really need to give precedence of an already set SOURCE_DATE_EPOCH environment variable over %source_date_epoch_from_changelog ?

@bmwiedemann
Copy link
Contributor

I put it there because the spec says:

Build systems MUST NOT overwrite this variable for child processes to consume if it is already present.

But then, it probably does not apply if we had set it ourselves (and not the user) for the previous rpm to build.

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