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

build: add --enable-determinism configure flag #18135

Merged
merged 1 commit into from
Feb 28, 2020

Conversation

fanquake
Copy link
Member

@fanquake fanquake commented Feb 13, 2020

This adds a --enable-determinsm configure flag, which if used, will enable additional compile / link time flags to make subsequent builds of bitcoind deterministic.

The first flag enabled is --no-insert-timestamp. This prevents the linker from embedding timestamps, and makes consecutive builds of bitcoind.exe deterministic. This will likely also be used for Guix Windows builds.

--no-insert-timestamp:

The option --no-insert-timestamp can be used to insert a zero value for the timestamp, this ensuring that binaries produced from identical sources will compare identically.

Diff of consecutive builds of master:

--- bitcoind.exe.1
+++ bitcoind.exe.2
@@ -2,20 +2,20 @@
 00000060: 7420 6265 2072 756e 2069 6e20 444f 5320  t be run in DOS 
 00000070: 6d6f 6465 2e0d 0d0a 2400 0000 0000 0000  mode....$.......
-00000080: 5045 0000 6486 1400 57e8 445e 00da 6900  PE..d...W.D^..i.
+00000080: 5045 0000 6486 1400 e3e9 445e 00da 6900  PE..d.....D^..i.
 00000090: e015 0100 f000 2600 0b02 021f 00de 4900  ......&.......I.
 000000a0: 00b0 5b00 008a 0000 e014 0000 0010 0000  ..[.............
 000000b0: 0000 4000 0000 0000 0010 0000 0002 0000  ..@.............
 000000c0: 0400 0000 0000 0000 0500 0200 0000 0000  ................
-000000d0: 00f0 6a00 0006 0000 bd31 af00 0300 6001  ..j......1....`.
+000000d0: 00f0 6a00 0006 0000 d434 af00 0300 6001  ..j......4....`.
 000000e0: 0000 2000 0000 0000 0010 0000 0000 0000  .. .............
@@ -373594,15 +373594,15 @@
 005b35f0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
-005b3600: 0000 0000 57e8 445e 0000 0000 7ce1 5b00  ....W.D^....|.[.
+005b3600: 0000 0000 e2e9 445e 0000 0000 7ce1 5b00  ......D^....|.[.
 005b3610: 0100 0000 2200 0000 2200 0000 28e0 5b00  ...."..."...(.[.

@practicalswift
Copy link
Contributor

Concept ACK: deterministic is better than non-deterministic

Thanks for doing this!

@practicalswift
Copy link
Contributor

ACK 73b33c1 -- patch looks correct

@fanquake
Copy link
Member Author

Might roll this into an --enable-determinism type build option instead.

@DrahtBot
Copy link
Contributor

Gitian builds

File commit 0c20809
(master)
commit 7657ee6
(master and this pull)
bitcoin-0.19.99-aarch64-linux-gnu-debug.tar.gz ba02e2ad0f12fdda... f81ad882b99a2290...
bitcoin-0.19.99-aarch64-linux-gnu.tar.gz a632796ad6b48bbe... b9a7a832858e81cc...
bitcoin-0.19.99-arm-linux-gnueabihf-debug.tar.gz c00a51909a01a49e... 36bf3af01812ea6d...
bitcoin-0.19.99-arm-linux-gnueabihf.tar.gz 0176bfb927f79d55... ddcc8e5373cb5484...
bitcoin-0.19.99-osx-unsigned.dmg 49a0d8cdd5d04ab5... 0272d60588119a88...
bitcoin-0.19.99-osx64.tar.gz d0d87df494449017... fde68b669577ff9f...
bitcoin-0.19.99-riscv64-linux-gnu-debug.tar.gz 19f571edf6bc3393... 91301dbec97ff3d9...
bitcoin-0.19.99-riscv64-linux-gnu.tar.gz 732d73778a5d58a7... e11593d94d84d855...
bitcoin-0.19.99-win64-debug.zip bca1edb8df891e2b... 3de3ba9488007ada...
bitcoin-0.19.99-win64-setup-unsigned.exe 9556eee3b4eec400... 5d3b932c69bc2560...
bitcoin-0.19.99-win64.zip 05d88d65048ae275... 7b9346f2fccb94d8...
bitcoin-0.19.99-x86_64-linux-gnu-debug.tar.gz 2955b63489a3dbff... 69e7cce1edffd916...
bitcoin-0.19.99-x86_64-linux-gnu.tar.gz d2d7f7c6ac6ab0cd... 2a4c2b24f93583c8...
bitcoin-0.19.99.tar.gz d5d44d1204506fa6... 52e69fdd6dd9c8e0...
bitcoin-core-linux-0.20-res.yml 9b2a3218a8e38d83... d064ff329211c46c...
bitcoin-core-osx-0.20-res.yml 4ef8e194ef35b518... f2b0cdb9f43b9eb7...
bitcoin-core-win-0.20-res.yml 86e22c0457a236ca... 0ee661f3fd690e26...
linux-build.log 93d975c20c7af9a9... a6df9fade59471d3...
osx-build.log 196af88ce6d3ba83... ae527c4d2157cf38...
win-build.log 903e3b3f2dcb7361... 26d679c40b7f97d0...
bitcoin-core-linux-0.20-res.yml.diff c4b736da08afab29...
bitcoin-core-osx-0.20-res.yml.diff f4fcd7bdf63f5fdb...
bitcoin-core-win-0.20-res.yml.diff 5aeaa9e1e48dde95...
linux-build.log.diff 584a960110476668...
osx-build.log.diff 6852c48171fa0d88...
win-build.log.diff 521e12e99c87fc90...

@practicalswift
Copy link
Contributor

@fanquake Don't we want determinism by default? :)

@luke-jr
Copy link
Member

luke-jr commented Feb 15, 2020

Concept NACK, build preferences should be set by the user.

(--enable-determinism seems fair though)

If used, this will enable additional compile / link time flags
that will make subsequent builds of bitcoind determinisitic.
@fanquake fanquake changed the title build: add --no-insert-timestamp to Windows linker flags build: add --enable-determinism configure flag Feb 27, 2020
@fanquake
Copy link
Member Author

I've updated this to be an --enable-determinism configure flag instead.

@practicalswift
Copy link
Contributor

ACK 3d9b41e -- patch looks correct

@DrahtBot
Copy link
Contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@maflcko
Copy link
Member

maflcko commented Feb 27, 2020

How is this different from libfaketime, which we use in the gitian builds?

@fanquake
Copy link
Member Author

fanquake commented Feb 28, 2020

How is this different from libfaketime,

This bypasses the need for libfaketime, and I'd much prefer be able to pass an additional configure flag, than having to install, setup and use it. There are a couple other flags I plan on adding here too, such as passing -D to libtool on macOS, rather than having to set ZERO_AR_DATE.

@laanwj
Copy link
Member

laanwj commented Feb 28, 2020

Yes, the idea is to not need faketime anymore at some point. It was always a hack because so many of the GNU and Qt tools embedded timestamps in all kind of unexpected places. We've slowly been working to get rid of those.

I like putting this under an option instead of making it configure default.

ACK 3d9b41e

@laanwj laanwj merged commit 5ad80be into bitcoin:master Feb 28, 2020
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Feb 28, 2020
3d9b41e build: add --enable-determinism configure flag (fanquake)

Pull request description:

  This adds a `--enable-determinsm` configure flag, which if used, will enable additional compile / link time flags to make subsequent builds of bitcoind deterministic.

  The first flag enabled is `--no-insert-timestamp`. This prevents the linker from embedding timestamps, and makes consecutive builds of `bitcoind.exe` deterministic. This will likely also be used for [Guix Windows builds](bitcoin#17595 (comment)).

  [`--no-insert-timestamp`](https://manpages.debian.org/buster/binutils-mingw-w64-x86-64/x86_64-w64-mingw32-ld.bfd.1.en.html):
  > The option --no-insert-timestamp can be used to insert a zero value for the timestamp, this ensuring that binaries produced from identical sources will compare identically.

  Diff of consecutive builds of [master](bitcoin@2bdc476):
  ```diff
  --- bitcoind.exe.1
  +++ bitcoind.exe.2
  @@ -2,20 +2,20 @@
   00000060: 7420 6265 2072 756e 2069 6e20 444f 5320  t be run in DOS
   00000070: 6d6f 6465 2e0d 0d0a 2400 0000 0000 0000  mode....$.......
  -00000080: 5045 0000 6486 1400 57e8 445e 00da 6900  PE..d...W.D^..i.
  +00000080: 5045 0000 6486 1400 e3e9 445e 00da 6900  PE..d.....D^..i.
   0000009: e015 0100 f000 2600 0b02 021f 00de 4900  ......&.......I.
   000000a0: 00b0 5b00 008a 0000 e014 0000 0010 0000  ..[.............
   000000b0: 0000 4000 0000 0000 0010 0000 0002 0000  ..@.............
   000000c0: 0400 0000 0000 0000 0500 0200 0000 0000  ................
  -000000d0: 00f0 6a00 0006 0000 bd31 af00 0300 6001  ..j......1....`.
  +000000d0: 00f0 6a00 0006 0000 d434 af00 0300 6001  ..j......4....`.
   000000e0: 0000 2000 0000 0000 0010 0000 0000 0000  .. .............
  @@ -373594,15 +373594,15 @@
   005b35f0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  -005b3600: 0000 0000 57e8 445e 0000 0000 7ce1 5b00  ....W.D^....|.[.
  +005b3600: 0000 0000 e2e9 445e 0000 0000 7ce1 5b00  ......D^....|.[.
   005b3610: 0100 0000 2200 0000 2200 0000 28e0 5b00  ...."..."...(.[.
  ```

ACKs for top commit:
  practicalswift:
    ACK 3d9b41e -- patch looks correct
  laanwj:
    ACK 3d9b41e

Tree-SHA512: 1ff9dab7fa818b1fc6b0eb3b7a1e0468aac9e2578f4451aa300a648f883fa83f83722067f1adf27ebad54157790857d1501d573adbd7ebdf6962858cc669960d
@maflcko
Copy link
Member

maflcko commented Feb 28, 2020

So we can remove libfaketime from gitian as well and replace it with this configure flag and/or appropriate SOURCE_DATE_EPOCH exports?

@fanquake fanquake deleted the no_insert_timestamp_ld branch February 28, 2020 22:22
@hebasto
Copy link
Member

hebasto commented Jul 30, 2020

@MarcoFalke

So we can remove libfaketime from gitian as well and replace it with this configure flag and/or appropriate SOURCE_DATE_EPOCH exports?

Mind looking into #19627?

sidhujag pushed a commit to syscoin-core/syscoin that referenced this pull request Nov 10, 2020
3d9b41e build: add --enable-determinism configure flag (fanquake)

Pull request description:

  This adds a `--enable-determinsm` configure flag, which if used, will enable additional compile / link time flags to make subsequent builds of bitcoind deterministic.

  The first flag enabled is `--no-insert-timestamp`. This prevents the linker from embedding timestamps, and makes consecutive builds of `bitcoind.exe` deterministic. This will likely also be used for [Guix Windows builds](bitcoin#17595 (comment)).

  [`--no-insert-timestamp`](https://manpages.debian.org/buster/binutils-mingw-w64-x86-64/x86_64-w64-mingw32-ld.bfd.1.en.html):
  > The option --no-insert-timestamp can be used to insert a zero value for the timestamp, this ensuring that binaries produced from identical sources will compare identically.

  Diff of consecutive builds of [master](bitcoin@2bdc476):
  ```diff
  --- bitcoind.exe.1
  +++ bitcoind.exe.2
  @@ -2,20 +2,20 @@
   00000060: 7420 6265 2072 756e 2069 6e20 444f 5320  t be run in DOS
   00000070: 6d6f 6465 2e0d 0d0a 2400 0000 0000 0000  mode....$.......
  -00000080: 5045 0000 6486 1400 57e8 445e 00da 6900  PE..d...W.D^..i.
  +00000080: 5045 0000 6486 1400 e3e9 445e 00da 6900  PE..d.....D^..i.
   0000009: e015 0100 f000 2600 0b02 021f 00de 4900  ......&.......I.
   000000a0: 00b0 5b00 008a 0000 e014 0000 0010 0000  ..[.............
   000000b0: 0000 4000 0000 0000 0010 0000 0002 0000  ..@.............
   000000c0: 0400 0000 0000 0000 0500 0200 0000 0000  ................
  -000000d0: 00f0 6a00 0006 0000 bd31 af00 0300 6001  ..j......1....`.
  +000000d0: 00f0 6a00 0006 0000 d434 af00 0300 6001  ..j......4....`.
   000000e0: 0000 2000 0000 0000 0010 0000 0000 0000  .. .............
  @@ -373594,15 +373594,15 @@
   005b35f0: 0000 0000 0000 0000 0000 0000 0000 0000  ................
  -005b3600: 0000 0000 57e8 445e 0000 0000 7ce1 5b00  ....W.D^....|.[.
  +005b3600: 0000 0000 e2e9 445e 0000 0000 7ce1 5b00  ......D^....|.[.
   005b3610: 0100 0000 2200 0000 2200 0000 28e0 5b00  ...."..."...(.[.
  ```

ACKs for top commit:
  practicalswift:
    ACK 3d9b41e -- patch looks correct
  laanwj:
    ACK 3d9b41e

Tree-SHA512: 1ff9dab7fa818b1fc6b0eb3b7a1e0468aac9e2578f4451aa300a648f883fa83f83722067f1adf27ebad54157790857d1501d573adbd7ebdf6962858cc669960d
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants