[gitian] Default reference_datetime to commit author date

MarcoFalke commented at 10:34 am on January 4, 2016: member

MarcoFalke commented at 10:39 am on January 4, 2016: member

@jonasschnelli I don’t want to break your debian box, so would you mind testing this after updating your gitian-builder to the latest version?

laanwj added the label Build system on Jan 4, 2016

laanwj commented at 10:55 am on January 4, 2016: member

Ping @theuni

Concept ACK however:

There’s a potential determinism problem with making the datetime depend on the bitcoin repository: the dependencies are built with datetime set to a single commit date, then cached, then next time they’ll be reused. If the dependencies embed the datetime anywhere, different builders can get different result depending on when they built the dependencies.

MarcoFalke commented at 11:59 am on January 4, 2016: member

Appears to be working:

0$ curl https://bitcoin.jonasschnelli.ch/pulls/7256/bitcoin-0.12.99-linux64.tar.gz -s | tar ztv | head -3
1drwxr-xr-x root/root         0 2015-06-01 02:00 bitcoin-0.12.99/
2drwxr-xr-x root/root         0 2015-06-01 02:00 bitcoin-0.12.99/bin/
3-rwxr-xr-x root/root   2470520 2015-06-01 02:00 bitcoin-0.12.99/bin/bench_bitcoin
4$ curl https://bitcoin.jonasschnelli.ch/pulls/7283/bitcoin-0.12.99-linux64.tar.gz -s | tar ztv | head -3
5drwxr-xr-x root/root         0 2016-01-04 12:11 bitcoin-0.12.99/
6drwxr-xr-x root/root         0 2016-01-04 12:11 bitcoin-0.12.99/bin/
7-rwxr-xr-x root/root   2470520 2016-01-04 12:11 bitcoin-0.12.99/bin/bench_bitcoin

jonasschnelli commented at 12:00 pm on January 4, 2016: contributor

Agree with @laanwj: would it be possible to build and cache the dependencies still with a fixed time? Maybe first someone needs to check if the datetime results/gets included somehow in the dependencies.

MarcoFalke force-pushed on Jan 4, 2016

laanwj commented at 10:49 am on January 5, 2016: member

BTW: Ugh. I think it was a mistake to add the build date there. It’s worthless at most, and deceptive at the least. If we want the date of last commit in there, would make sense to add it after the “Client version”.

theuni commented at 8:15 pm on January 5, 2016: member

@laanwj I’m all for removing the build date and git commit from tagged releases. That would also mean that we could merge the signing steps, since the only reason they’re staggered is the changed commit hash in the binary. @jonasschnelli I believe we’ve already removed all time sources from the depends. @laanwj I believe that would already be the case?

laanwj commented at 8:22 am on January 7, 2016: member

@laanwj I believe that would already be the case?

Faketime prevents that at the moment by setting the date/time stamp to a fixed value

MarcoFalke commented at 11:28 am on January 7, 2016: member

Then, wouldn’t stuff like be656283f98896df0bc8634d446b2873f9fed573 break this as well?

Anyway, depends could use a different (static) date other than reference_datetime.

laanwj commented at 9:54 am on January 18, 2016: member

You’re right, but be65628 is a one-time change. So dependencies built before and after that may differ. The problem with this is that the time/date will change every time.

Not sure it is an issue, but as this is a non-critical detail I’d like to be sure it doesn’t cause problems down the line before merging it.

MarcoFalke force-pushed on Feb 1, 2016

laanwj commented at 1:35 pm on February 4, 2016: member

Needs rebase

MarcoFalke force-pushed on Feb 5, 2016

MarcoFalke force-pushed on Feb 26, 2016

MarcoFalke commented at 9:14 am on February 27, 2016: member

@laanwj I’ve been running gitian builder to see if the results match:

yml source	target commit	cache populated by	result
e7ea5db	e7ea5db	e7ea5db	041a0f82a0c4… bitcoin-0.12.99-linux32.tar.gz
this pull’s commit	this pull’s commit	e7ea5db	49fd8bbb3fcc… bitcoin-0.12.99-linux32.tar.gz
this pull’s commit	this pull’s commit	this pull’s commit	49fd8bbb3fcc… bitcoin-0.12.99-linux32.tar.gz
e7ea5db	e7ea5db	this pull’s commit	041a0f82a0c4… bitcoin-0.12.99-linux32.tar.gz

So this appears to indicate that the result is independent ~~from the cache~~ of which commit created the cache.

[gitian] Default reference_datetime to commit author date fa58c76b9f

MarcoFalke force-pushed on Mar 1, 2016

MarcoFalke commented at 11:49 am on March 14, 2016: member

@theuni Do you think this is ready to merge?

laanwj assigned theuni on Apr 1, 2016

theuni commented at 6:14 pm on April 4, 2016: member

Sorry for the delay, testing this.

theuni commented at 6:52 pm on April 4, 2016: member

Yes, after taking a closer look, unfortunately this will not play nice with depends. Mostly because of the timestamps inside of tarballs, but also .a’s and other problematic formats. Note that the end-results may or may not be the same, but the intermediates would differ.

We can revisit this later though, as options for tar/binutils/ld/etc have been added recently that reliably purge this data.

The only realistic thing I see that we can do here for now without a major overhaul would be to set the faketime once to a hard-coded date for depends, so that it remains the same regardless of the commit used, then reset for the bitcoin build itself.

MarcoFalke commented at 12:42 pm on April 5, 2016: member

set the faketime once to a hard-coded date for depends @theuni Something like the last commit I pushed (untested)?

theuni commented at 4:47 pm on April 5, 2016: member

@MarcoFalke Yep, I think that approach should be fine.

MarcoFalke force-pushed on Apr 8, 2016

[gitian] hardcode datetime for depends fa42a675c0

MarcoFalke force-pushed on Apr 10, 2016

MarcoFalke commented at 9:22 pm on April 10, 2016: member

Not sure why gitian fails on the windows build…

 0[gitian-builder]$ tail -8 var/build.log
 1checking whether make sets $(MAKE)... yes
 2checking whether make supports nested variables... yes
 3checking whether to enable maintainer-specific portions of Makefiles... yes
 4checking whether make supports nested variables... (cached) yes
 5checking whether the C++ compiler works... no
 6configure: error: in `/home/ubuntu/build/bitcoin':
 7configure: error: C++ compiler cannot create executables
 8See `config.log' for more details
 9
10[gitian-builder]$ ./libexec/on-target tail -5 ./build/bitcoin/config.log
11#define PACKAGE_STRING "Bitcoin Core 0.12.99"
12#define PACKAGE_BUGREPORT "https://github.com/bitcoin/bitcoin/issues"
13#define PACKAGE_URL "https://bitcoincore.org/"
14
15configure: exit 77

arowser commented at 8:45 am on May 25, 2016: contributor

Can one of the admins verify this patch?

laanwj commented at 10:41 am on May 30, 2016: member

I think it would be better to remove use of the build date in our binary completely, making the use of faketime unnecessary. We don’t have control over that for the dependencies but we do for our own code.

If you want the date/time of the last commit somewhere in the binary there are better ways to do that (e.g. through genbuild.sh) than a faketime.

MarcoFalke commented at 7:07 pm on May 30, 2016: member

The motivation for this pull was to avoid regular “bump date” pulls such as #7251#issue-123874815.

The build date was removed in #7732 and I don’t think we should add the commit date to the gui.

MarcoFalke commented at 7:10 pm on May 30, 2016: member

If this pull gets closes, it might be worth to update https://github.com/bitcoin/bitcoin/blob/950be19727a581970591d8f8138dfe4725750382/doc/release-process.md#bitcoin-maintainersrelease-engineers-update-version-in-sources and mention to bump the date in the same go.

MarcoFalke force-pushed on Jun 5, 2016

laanwj commented at 8:56 am on June 9, 2016: member

There’s another place where this matters: the times/dates in the .zip files and .tar.gz archives of the binary distribution. I guess that makes it worth doing this.

The build date was removed in #7732

Only from the GUI though. It’s still logged in two places:

0src/init.cpp:    LogPrintf("Bitcoin version %s (%s)\n", FormatFullVersion(), CLIENT_DATE);
1src/wallet/rpcdump.cpp:    file << strprintf("# Wallet dump created by Bitcoin %s (%s)\n", CLIENT_BUILD, CLIENT_DATE);

laanwj commented at 8:58 am on June 9, 2016: member

If this pull gets closes, it might be worth to update

Well my point was more ‘why would we care about this’. If there is no reason to care about this date at all, there’s no point in bumping it either. E.g. after making sure it doesn’t get shown in the GUI or log anymore. But I suppose the dates in archives are a reason to care that it is somewhat up to date. It looks stupid to have files from 1970.

laanwj merged this on Jun 9, 2016

laanwj closed this on Jun 9, 2016

laanwj referenced this in commit fd9881ae67 on Jun 9, 2016

MarcoFalke deleted the branch on Jun 9, 2016

zkbot referenced this in commit 4ee9d712b5 on Oct 17, 2016

codablock referenced this in commit 5a14a86e8c on Sep 16, 2017

codablock referenced this in commit 89f2f7b6d0 on Sep 19, 2017

codablock referenced this in commit 856e546785 on Dec 22, 2017

andvgal referenced this in commit f8b4acbd30 on Jan 6, 2019

MarcoFalke locked this on Sep 8, 2021

[gitian] Default reference_datetime to commit author date #7283