Concept ACK: high quality life is better than low quality life

Thanks for improving things! :)

Thanks, going to test this.

Concept ACK.

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

• guix: Build support for macOS #17920 (guix: Build support for macOS by dongcarl)

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.

I was a bit confused by /bitcoin in the following error, I think it would be better if it showed the full path.

user@guix:~/bitcoin$ ./contrib/guix/guix-build.sh
make: Entering directory '/home/user/bitcoin/depends'
…
At most 8 jobs will run at once...
DISTSRC directory '/bitcoin/distsrc-x86_64-linux-gnu' exists, probably because of previous builds... Aborting...

Edit: oh it's literally bind-mounting the repository there! (inside a container, i guess)

Something that would be nice to mention (I don't think it's mentioned in the documentation, but maybe read over it) is which files can be cached. It mentions "start from a clean bitcoin core tree" but I'm sure e.g. depends/sources can be cached to save some time and bandwidth downloading, for example.

While depends/sources can be cached, depends needs to be cached under a key lookup that hashes at least the compiler version. See #17248 . @DrahtBot does this.

While depends/sources can be cached, depends needs to be cached under a key lookup that hashes at least the compiler version. See #17248 . @DrahtBot does this.

Hmf, it seems this is something that the guix build wrapper itself knows better than the user.

So there are two caches for depends:

1. SOURCES_PATH (depends/sources), which is already honored by guix-build.sh (doc).
2. BASE_CACHE (where the build results go), which is not yet honored by guix-build.sh.

Honoring BASE_CACHE is a bit more tricky, as we want to avoid cache false-hits. I just opened a PR that helps the situation quite a bit though: #20629

I think it would be better if it showed the full path.

You're right! For this particular case, I will see if I can do the check before execing into the container so that we have the full path.

In general though, I found that -ffile-prefix-map= hasn't reliably produced reproducible output in the past, this is why I do the rename when bind mounting. I might give -ffile-prefix-map= another try in the future though!

Guix builds

I deterministically get the following output when building this commit:

3952acb71553cd300b6055a8be8799be570e459e9492b353da52bf2029979655  ./bitcoin-29359b904f40-aarch64-linux-gnu-debug.tar.gz
b3cb1886585fa2fb0698e52c8432df9fef3f1c3dafd8da78a326dc6620e02f10  ./bitcoin-29359b904f40-aarch64-linux-gnu.tar.gz
0e74c62318182d8d2c8abb9fde2ce9b7fdc0cdb17ef1135ed94b87d491c606db  ./bitcoin-29359b904f40-arm-linux-gnueabihf-debug.tar.gz
d3ef6e67bfc516fccdcee7bee60380a7e3d4cf5ec6bf8746119bc74d264f218b  ./bitcoin-29359b904f40-arm-linux-gnueabihf.tar.gz
55be295a36d887f9c8b4d29b69dd316f06727289057446a50d2162311ed408a6  ./bitcoin-29359b904f40-riscv64-linux-gnu-debug.tar.gz
27decc48380544e8eef3b94f87d0089c3d9a12b5bcc47c8f0d054c6e69bb1300  ./bitcoin-29359b904f40-riscv64-linux-gnu.tar.gz
f40beab31f80cdaf958df198d1904b500794ced1dece84eb6bcbe53be8385056  ./bitcoin-29359b904f40-win-unsigned.tar.gz
cdccbb5927e5e2d2e954a11e1f20072ef353c6567fcf9ef2ce97c5d7bcc06efd  ./bitcoin-29359b904f40-win64-debug.zip
c95025da58e76590251ad15967ff55368d7eb0b7fd877b7e42e66288fc7720ee  ./bitcoin-29359b904f40-win64-setup-unsigned.exe
b9d1111e8781b1ef0efb51c3a760b138541cad24e0732104e6221a5d928371d0  ./bitcoin-29359b904f40-win64.zip
cf3183bfa0788b4ea5d4d552a46a44a040839a3388b032e07d776d4c69a1ac2f  ./bitcoin-29359b904f40-x86_64-linux-gnu-debug.tar.gz
ca08a13a2655149723c663366b0af4413ac02ed2f80e98d01f2dc4dd8da4fe88  ./bitcoin-29359b904f40-x86_64-linux-gnu.tar.gz
786330b2edcd1caf9d4bf1d191588683dc83780c8489545f89d888c780b8dc82  ./src/bitcoin-29359b904f40.tar.gz

In general though, I found that -ffile-prefix-map= hasn't reliably produced reproducible output in the past, this is why I do the rename when bind mounting. I might give -ffile-prefix-map= another try in the future though!

Thanks. Yes, I think the bind-mounting makes perfect sense. It's bound to be more reliable than any compiler option.

I was unable to reproduce these results, but I think I know exactly what's causing the non-reproducibility. I have recently fixed this in the macOS PR (dc42abf) and will cherry-pick it here tomorrow. Depending on what PR gets merged first, we'll just rebase the other one on top!

Depending on what PR gets merged first, we'll just rebase the other one on top!

FWIW my build is in a clean Ubuntu 20.04 VM, no git configuration file. So I'm not sure the hash length change matters. But we'll see. It can't hurt at least.
Oh right … the default "short" hash length also depends on disambiguation (it's longer if it's somehow ambiguous) depending on what other commits are known to the repository. This could be it, yes.

I think the section name "Using a version of Guix with guix time-machine capabilities" is slightly confusing, as (at least what I understand) it's about upgrading to a version of Guix. Maybe "Upgrading to a version of Guix with guix time-machine capabilities" would be a better title. Or maybe, as it looks like recent installs don't need this, delete/move it to a footnote?

🕵️ @harding has been requested to review this pull request as specified in the REVIEWERS file.

Pushed 29359b904f40805842447d4efa680bcc45bfdb57 -> 235cb967130fd33cc5413fe19fb8917def6a6388

• Textually point to contrib/guix/README.md from doc/guix.md instead of using a symlink
• Added rev-parse length specification for reproducibility
• Added a few more sanity checks to guix-build.sh
• Fail as soon as possible when DISTSRC directories already exist, and make the check outside the container so that the path makes sense to user

Pushed 235cb967130fd33cc5413fe19fb8917def6a6388 -> 570e43f

• Rebased on top of master