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

Merged
merged 3 commits into from Sep 28, 2017

Conversation

Projects
None yet
4 participants
@bmwiedemann
Contributor

bmwiedemann commented Sep 18, 2017

in order to make font builds reproducible
such as the kde-oxygen-fonts openSUSE package.

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.

@jtanx

This comment has been minimized.

Show comment
Hide comment
@jtanx

jtanx Sep 18, 2017

Contributor

Is this already covered by #3134?

Contributor

jtanx commented Sep 18, 2017

Is this already covered by #3134?

@bmwiedemann

This comment has been minimized.

Show comment
Hide comment
@bmwiedemann

bmwiedemann Sep 18, 2017

Contributor

yes. I'll have a look if I can help get it merged.

Contributor

bmwiedemann commented Sep 18, 2017

yes. I'll have a look if I can help get it merged.

gioele and others added some commits Sep 2, 2017

Improve GetTime function
to only call getenv once
Use GetTime in more places
in order to make more kinds of font operations reproducible
@bmwiedemann

This comment has been minimized.

Show comment
Hide comment
@bmwiedemann

bmwiedemann Sep 19, 2017

Contributor

@frank-trampe I cherry-picked the nice infrastructure commit from @gioele
GetTime is now used in the 4 places that I found to be relevant for openSUSE font packages to build reproducibly.
I did some testing and therefore am confident that it works.

Contributor

bmwiedemann commented Sep 19, 2017

@frank-trampe I cherry-picked the nice infrastructure commit from @gioele
GetTime is now used in the 4 places that I found to be relevant for openSUSE font packages to build reproducibly.
I did some testing and therefore am confident that it works.

@gioele

This comment has been minimized.

Show comment
Hide comment
@gioele

gioele Sep 24, 2017

Contributor

Thanks @bmwiedemann, it looks good to me.

I sugesst we merge this for the moment, then we can go after all the other failure cases pointed out by @volth in #3134.

Contributor

gioele commented Sep 24, 2017

Thanks @bmwiedemann, it looks good to me.

I sugesst we merge this for the moment, then we can go after all the other failure cases pointed out by @volth in #3134.

@volth

This comment has been minimized.

Show comment
Hide comment
@volth

volth Sep 24, 2017

For reproducible builds it also needed to overwrite GetAuthor, there is often Linux user name which is not stable but something like "nixbld4". Anyway, it is good to make the first little step towards reproducible builds.

volth commented Sep 24, 2017

For reproducible builds it also needed to overwrite GetAuthor, there is often Linux user name which is not stable but something like "nixbld4". Anyway, it is good to make the first little step towards reproducible builds.

@gioele

This comment has been minimized.

Show comment
Hide comment
@gioele

gioele Sep 24, 2017

Contributor

@volth: indeed, that is the idea. Now this basic cases, later all cases.

@jtanx, @frank-trampe: if there are no objections I could merge this code (overriding time() to make it reproducible) in the next days.

Contributor

gioele commented Sep 24, 2017

@volth: indeed, that is the idea. Now this basic cases, later all cases.

@jtanx, @frank-trampe: if there are no objections I could merge this code (overriding time() to make it reproducible) in the next days.

gioele added a commit to gioele/fontforge that referenced this pull request Sep 27, 2017

@gioele gioele merged commit 078a173 into fontforge:master Sep 28, 2017

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 18.605%
Details
@gioele

This comment has been minimized.

Show comment
Hide comment
@gioele

gioele Sep 28, 2017

Contributor

Merged, thanks @bmwiedemann.

Contributor

gioele commented Sep 28, 2017

Merged, thanks @bmwiedemann.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment