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

Use Changelog date instead of build date #180

Closed
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
4 participants
@bmwiedemann
Contributor

bmwiedemann commented Sep 2, 2017

and always use UTC
to make builds reproducible.
See https://reproducible-builds.org/ for why this is good.

Note that we leave cmake's TIMESTAMP call
because it already supports overriding the build time.

I tested only the automake portion with gnucash-2.6.17 on openSUSE Tumbleweed
which had the same change in src/core-utils/Makefile.am

@gjanssens

This comment has been minimized.

Show comment
Hide comment
@gjanssens

gjanssens Sep 2, 2017

Member

I'm all for reproducible builds, so thanks for providing a patch. My only concern is that the ChangeLog file is not very consistently updated in gnucash. It is for each release, so for release builds it will work. However intermediate development builds may all share the same, outdated build date. Would that be an issue ?

Member

gjanssens commented Sep 2, 2017

I'm all for reproducible builds, so thanks for providing a patch. My only concern is that the ChangeLog file is not very consistently updated in gnucash. It is for each release, so for release builds it will work. However intermediate development builds may all share the same, outdated build date. Would that be an issue ?

@bmwiedemann

This comment has been minimized.

Show comment
Hide comment
@bmwiedemann

bmwiedemann Sep 2, 2017

Contributor

From my testing (see http://rb.zq1.de/compare.factory-20170825/gnucash-compare.out), build date was used in two places: man page - there it should not matter
and via GNUCASH_BUILD_DATE embedded in /usr/bin/gnucash and /usr/lib64/gnucash/libgncmod-gnome-utils.so
which seems to be used in gnucash-bin.c, gnc-splash.c and gnc-main-window.c so possibly displayed somewhere in the UI.

Contributor

bmwiedemann commented Sep 2, 2017

From my testing (see http://rb.zq1.de/compare.factory-20170825/gnucash-compare.out), build date was used in two places: man page - there it should not matter
and via GNUCASH_BUILD_DATE embedded in /usr/bin/gnucash and /usr/lib64/gnucash/libgncmod-gnome-utils.so
which seems to be used in gnucash-bin.c, gnc-splash.c and gnc-main-window.c so possibly displayed somewhere in the UI.

@bmwiedemann

This comment has been minimized.

Show comment
Hide comment
@bmwiedemann

bmwiedemann Sep 2, 2017

Contributor

Actually, the places that show GNUCASH_BUILD_DATE also show GNUCASH_SCM_REV which should give a much better hint at what version is actually running there for intermediate developer builds.
And people who want the old behaviour back (e.g. for nightly builds) could just do touch Changelog before the build.

Contributor

bmwiedemann commented Sep 2, 2017

Actually, the places that show GNUCASH_BUILD_DATE also show GNUCASH_SCM_REV which should give a much better hint at what version is actually running there for intermediate developer builds.
And people who want the old behaviour back (e.g. for nightly builds) could just do touch Changelog before the build.

@jralls

This comment has been minimized.

Show comment
Hide comment
@jralls

jralls Sep 3, 2017

Member

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

Sorry, that's a steaming pile of you-know-what. There's more than build dates that affect the hashes of the binaries, one of which is mentioned on the reproducible build page: Compiler and version. If we accept the requirements of that page we should also pick a single compiler version and flags and fail Cmake/configure if that exact combination isn't in use. Not mentioned on that page is that the ABI version of every dependency will also affect the binary's hashes.

BTW, ChangeLog only gets committed during the release process, but it's a build product: You get a new one every time you clean and rebuild.

Member

jralls commented Sep 3, 2017

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

Sorry, that's a steaming pile of you-know-what. There's more than build dates that affect the hashes of the binaries, one of which is mentioned on the reproducible build page: Compiler and version. If we accept the requirements of that page we should also pick a single compiler version and flags and fail Cmake/configure if that exact combination isn't in use. Not mentioned on that page is that the ABI version of every dependency will also affect the binary's hashes.

BTW, ChangeLog only gets committed during the release process, but it's a build product: You get a new one every time you clean and rebuild.

@gjanssens

This comment has been minimized.

Show comment
Hide comment
@gjanssens

gjanssens Sep 3, 2017

Member

Well, reproducible builds is not something that can be done in the scope of a single application. It can only be achieved in a fully controlled environment such as a distribution. What we can do on the gnucash level is removing the warts the would prevent a distribution for achieving that goal. Using arbitrary dates during build is such a wart.
Still I think the date of the ChangeLog file is not the best solution. If you want a deterministic date, use the commit date for the commit you're building from.
While that would add little to the information that's already there (the commit hash), I'd include the date anyway because it gives the user a rough indication of how old the build is without having to trace the commit hash (I think most even wouldn't know how to do that).
The build date on the other hand is useful during development. I sometimes have to rebuild an older version of gnucash for testing purposes (perhaps against newer versions of dependencies, with different build options,...) Having the build date encapsulated in the application is a helpful hint to see which build I'm running.
So what I propose is this: use a different date source for development and release builds. For development builds, continue to use the build date. For release builds use the commit date of the commit.

Member

gjanssens commented Sep 3, 2017

Well, reproducible builds is not something that can be done in the scope of a single application. It can only be achieved in a fully controlled environment such as a distribution. What we can do on the gnucash level is removing the warts the would prevent a distribution for achieving that goal. Using arbitrary dates during build is such a wart.
Still I think the date of the ChangeLog file is not the best solution. If you want a deterministic date, use the commit date for the commit you're building from.
While that would add little to the information that's already there (the commit hash), I'd include the date anyway because it gives the user a rough indication of how old the build is without having to trace the commit hash (I think most even wouldn't know how to do that).
The build date on the other hand is useful during development. I sometimes have to rebuild an older version of gnucash for testing purposes (perhaps against newer versions of dependencies, with different build options,...) Having the build date encapsulated in the application is a helpful hint to see which build I'm running.
So what I propose is this: use a different date source for development and release builds. For development builds, continue to use the build date. For release builds use the commit date of the commit.

@jralls

This comment has been minimized.

Show comment
Hide comment
@jralls

jralls Sep 3, 2017

Member

For release builds use the commit date of the commit.

OK, to be perfectly clear "the commit date of the tagged release commit".

Member

jralls commented Sep 3, 2017

For release builds use the commit date of the commit.

OK, to be perfectly clear "the commit date of the tagged release commit".

@jralls

This comment has been minimized.

Show comment
Hide comment
@jralls

jralls Sep 3, 2017

Member

Having the build date encapsulated in the application is a helpful hint to see which build I'm running.

Hmm, I'd think not: In that scenario all of the build dates will be close to the same depending on how long it takes to figure out whatever the problem is. Maybe we should add a CMake variable/configure flag "build tag" that can be added to the id string so (to borrow from another PR) one could have e.g. "2.6.17-abcd1234-with_ofx" and "2.6.17-abcd1234-without_ofx" to help keep straight what one is testing.

Member

jralls commented Sep 3, 2017

Having the build date encapsulated in the application is a helpful hint to see which build I'm running.

Hmm, I'd think not: In that scenario all of the build dates will be close to the same depending on how long it takes to figure out whatever the problem is. Maybe we should add a CMake variable/configure flag "build tag" that can be added to the id string so (to borrow from another PR) one could have e.g. "2.6.17-abcd1234-with_ofx" and "2.6.17-abcd1234-without_ofx" to help keep straight what one is testing.

@gjanssens

This comment has been minimized.

Show comment
Hide comment
@gjanssens

gjanssens Sep 3, 2017

Member

Yes, good point. So let's not make a release/development distinction on which date to include. Let's just keep that the commit date of the revision being built. For release builds that should be the same as "the commit date of the tagged release commit" because that's the commit to do the release from.

As for the build tag that would be nice to have though there are only so many characters one can sensibly stuff in a version string. Another approach might be to extend the about dialog with a tab or sub window with build information. That could then be more comprehensive and provide details on which commands have been invoked to create this build, which dependencies were used and so on. This is however straying away from the actual goal of this PR - stop using arbitrary data in the build so that distros can achieve a reproducible build.

So I'd be fine with just a PR for using the commit date instead of the build date.

Member

gjanssens commented Sep 3, 2017

Yes, good point. So let's not make a release/development distinction on which date to include. Let's just keep that the commit date of the revision being built. For release builds that should be the same as "the commit date of the tagged release commit" because that's the commit to do the release from.

As for the build tag that would be nice to have though there are only so many characters one can sensibly stuff in a version string. Another approach might be to extend the about dialog with a tab or sub window with build information. That could then be more comprehensive and provide details on which commands have been invoked to create this build, which dependencies were used and so on. This is however straying away from the actual goal of this PR - stop using arbitrary data in the build so that distros can achieve a reproducible build.

So I'd be fine with just a PR for using the commit date instead of the build date.

@jralls

This comment has been minimized.

Show comment
Hide comment
@jralls

jralls Sep 4, 2017

Member

I think fiddling the "about" dialog might be getting carried away a bit. A few extra characters when one's trying to figure out a bug or test a new feature should be sufficient--in many cases even "A" and "B" would suffice... though one would have to remember to change the build tag when building each iteration of each version.

I agree about the PR. Fix it to use commit date of the revision and merge.

Member

jralls commented Sep 4, 2017

I think fiddling the "about" dialog might be getting carried away a bit. A few extra characters when one's trying to figure out a bug or test a new feature should be sufficient--in many cases even "A" and "B" would suffice... though one would have to remember to change the build tag when building each iteration of each version.

I agree about the PR. Fix it to use commit date of the revision and merge.

bmwiedemann added some commits Sep 2, 2017

Use Changelog date instead of build date
and always use UTC
to make builds reproducible.
See https://reproducible-builds.org/ for why this is good.
Allow to override build date
in order to make builds reproducible.
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 that we leave cmake's TIMESTAMP call
because it already supports overriding the build time this way.

The Makefile.am change is designed to work with both GNU date and BSD date.
@bmwiedemann

This comment has been minimized.

Show comment
Hide comment
@bmwiedemann

bmwiedemann Sep 4, 2017

Contributor

I split the previous change into the man-page part, that seems to be uncontroversial, because it only records month+year anyway
and a 2nd commit for the UI-visible build date which leaves GNUCASH_BUILD_DATE as an actual build date by default to help developers, but allows distributions to override it by setting $SOURCE_DATE_EPOCH (usually to the distribution's changelog date, so even adding patches in a distro updates it)

Now you can either merge it as is and optionally later add the commit date logic on top
or you cherry-pick the first commit and do just the commit date for GNUCASH_BUILD_DATE (not exactly sure how)

Contributor

bmwiedemann commented Sep 4, 2017

I split the previous change into the man-page part, that seems to be uncontroversial, because it only records month+year anyway
and a 2nd commit for the UI-visible build date which leaves GNUCASH_BUILD_DATE as an actual build date by default to help developers, but allows distributions to override it by setting $SOURCE_DATE_EPOCH (usually to the distribution's changelog date, so even adding patches in a distro updates it)

Now you can either merge it as is and optionally later add the commit date logic on top
or you cherry-pick the first commit and do just the commit date for GNUCASH_BUILD_DATE (not exactly sure how)

@limitedAtonement

This comment has been minimized.

Show comment
Hide comment
@limitedAtonement

limitedAtonement Sep 4, 2017

Contributor

Does using the commit timestamp for the release commit mean it won't be possible to build from a tarball (no repository present)?

Contributor

limitedAtonement commented Sep 4, 2017

Does using the commit timestamp for the release commit mean it won't be possible to build from a tarball (no repository present)?

@gjanssens

This comment has been minimized.

Show comment
Hide comment
@gjanssens

gjanssens Sep 4, 2017

Member

The commit date can obviously only be extracted when building from the repository. We are already extracting the commit hash at that time to be used later on in the release tarballs. It's stored in a file called gnc-vcs-info.h which is added to the dist tarballs. It's only a matter of also extracting the commit date and storing it in the same file.

Member

gjanssens commented Sep 4, 2017

The commit date can obviously only be extracted when building from the repository. We are already extracting the commit hash at that time to be used later on in the release tarballs. It's stored in a file called gnc-vcs-info.h which is added to the dist tarballs. It's only a matter of also extracting the commit date and storing it in the same file.

@gjanssens

This comment has been minimized.

Show comment
Hide comment
@gjanssens

gjanssens Oct 5, 2017

Member

I'll close this one as the issue was solved based on PR#189.

Member

gjanssens commented Oct 5, 2017

I'll close this one as the issue was solved based on PR#189.

@gjanssens gjanssens closed this Oct 5, 2017

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