guix: Quality of life improvements by dongcarl · Pull Request #20619 · bitcoin/bitcoin

dongcarl · 2020-12-11T00:48:58Z

After live-demo-ing a Guix build (which completed successfully!) on achow101's stream, I realized there were a few quality of life improvements which can be made to improve the user experience of our Guix build process. Here are a few of them.

Notable changes:

When MAX_JOBS is specified, both guix time-machine and guix environment will now build up to MAX_JOBS packages at a time when creating the build environment
The instructions for using substitutes were incorrect, and has now been replaced with a SUBSTITUTE_URLS environment variable, which works well with shell's IFS splitting rules
New ADDITIONAL_GUIX_{COMMON,TIMEMACHINE}_FLAGS options, for more granular customization of the build process.
README cleanup

practicalswift · 2020-12-11T06:53:08Z

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

Thanks for improving things! :)

laanwj · 2020-12-11T09:14:56Z

Thanks, going to test this.

hebasto · 2020-12-11T09:35:13Z

Concept ACK.

DrahtBot · 2020-12-11T11:58:16Z

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.

laanwj · 2020-12-12T03:40:27Z

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)

laanwj · 2020-12-12T13:54:06Z

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.

maflcko · 2020-12-12T16:38:32Z

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.

laanwj · 2020-12-12T16:57:47Z

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.

dongcarl · 2020-12-12T21:02:10Z

So there are two caches for depends:

SOURCES_PATH (depends/sources), which is already honored by guix-build.sh (doc).
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

dongcarl · 2020-12-12T21:05:32Z

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!

DrahtBot · 2020-12-13T07:56:07Z

Guix builds

File	commit `b189780` (master)	commit 3cdcf06ed844e031fce0c3554c769064d59859c1 (master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`7f03c6b17e599f56...`	`8ac2b51082a040ad...`
*-aarch64-linux-gnu.tar.gz	`8c9c7f1f8a656d50...`	`b91be712e5e67c6e...`
*-arm-linux-gnueabihf-debug.tar.gz	`9eacf7861d164d52...`	`46148e81f82566e6...`
*-arm-linux-gnueabihf.tar.gz	`098218c7ac3cb595...`	`27dc514ba47eb86e...`
*-riscv64-linux-gnu-debug.tar.gz	`bf61ef7be67bcbde...`	`26b9a96f75f17fc1...`
*-riscv64-linux-gnu.tar.gz	`73a459a722e2f4fb...`	`7a4976e66d64d74e...`
*-win-unsigned.tar.gz	`f118bf91966f5056...`	`02ed80c3a2612465...`
*-win64-debug.zip	`8eaf66ff95aed950...`	`9c3afa92775867f9...`
*-win64-setup-unsigned.exe	`d76f733d6aa466a8...`	`1135cb86cf7f1f3a...`
*-win64.zip	`38dab9495992031e...`	`edef8ce730348c9a...`
*-x86_64-linux-gnu-debug.tar.gz	`6eff7c40d564ba2c...`	`cc8462e8b93bc1c9...`
*-x86_64-linux-gnu.tar.gz	`563234ddf0046332...`	`3547efe6d8c8f12c...`
*.tar.gz	`6efeed3f6a0504dc...`	`c1fc05763ffb1c74...`
guix_build.log	`124e1266f75d3058...`	`70e76265a9e2e203...`
guix_build.log.diff		`12db592c5fbb8e3e...`

laanwj · 2020-12-13T19:19:19Z

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.

dongcarl · 2020-12-14T00:47:00Z

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!

laanwj · 2020-12-14T09:44:27Z

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.

laanwj · 2020-12-14T11:17:29Z

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?

DrahtBot · 2020-12-14T19:12:47Z

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

dongcarl · 2020-12-22T20:03:38Z

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

dongcarl · 2021-01-08T16:41:02Z

Pushed 235cb967130fd33cc5413fe19fb8917def6a6388 -> 570e43f

Rebased on top of master

fanquake

ACK 570e43f - lets move this forward.

a5bf0c224b94fae527df497738eb0595e3392e3eddfe7f720807037847d258d7  output/bitcoin-570e43fe72e1-aarch64-linux-gnu-debug.tar.gz
a8f10c45c27e54333a0d6669d3b7d7b793356067d7f328e2acc757a06ec1050e  output/bitcoin-570e43fe72e1-aarch64-linux-gnu.tar.gz
ba718e45f70d5204808d3a70df9732f9680aee5552dca5a67c29e2e5833562b4  output/bitcoin-570e43fe72e1-arm-linux-gnueabihf-debug.tar.gz
de152cdbe0d9fde9f427ae49fd34adc57b7c4578e15579d206138a175a2d7c37  output/bitcoin-570e43fe72e1-arm-linux-gnueabihf.tar.gz
e8aa890632dca2486bb61a0a50136d18b3d60c70c69799dfe4f449c17f3f9ac1  output/bitcoin-570e43fe72e1-riscv64-linux-gnu-debug.tar.gz
77aaf75aae626b960192aa4de20dbfb625f893dd1caf94fc44f9978e7d7186e7  output/bitcoin-570e43fe72e1-riscv64-linux-gnu.tar.gz
de9c212405f227f80a3aa882ee994202bbc834137f94a73779d99e9410b86246  output/bitcoin-570e43fe72e1-win-unsigned.tar.gz
98c1e14e46aaa83201befdd54f3aac4229f0a5f246ed464b4fcd6e31550f7eea  output/bitcoin-570e43fe72e1-win64-debug.zip
cf45a21423bad9357526b92305530e7199de8a574d4c02641423cca4404273c0  output/bitcoin-570e43fe72e1-win64-setup-unsigned.exe
08c7a10b51ef24db6d8dd5e10bdfad7ad2dad2cc1b8d8a70f89ed80b8faeb18b  output/bitcoin-570e43fe72e1-win64.zip
5f1fd6c1b9bcb1d7522411e859a7865337bfbfe19388444f36f4d871c76079a6  output/bitcoin-570e43fe72e1-x86_64-linux-gnu-debug.tar.gz
0e3968c071aeeac5bbd0b193a9aac52e536bd756ba86705da773d2c9654efa66  output/bitcoin-570e43fe72e1-x86_64-linux-gnu.tar.gz
e7cd77024e3d3e37b7618bf3e32d3386144e1a3aefa9886ead9379be3790175f  output/src/bitcoin-570e43fe72e1.tar.gz

dongcarl added Build system DrahtBot Guix build requested labels Dec 11, 2020

dongcarl mentioned this pull request Dec 11, 2020

guix: Build support for macOS #17920

Merged

hebasto reviewed Dec 11, 2020

View reviewed changes

Comment thread doc/guix.md Outdated

bitcoin deleted a comment from curtcurt87 Dec 12, 2020

DrahtBot removed the DrahtBot Guix build requested label Dec 13, 2020

dongcarl force-pushed the 2020-12-guix-fixups branch 3 times, most recently from 3b0e157 to 235cb96 Compare December 22, 2020 00:26

Limpisey168 approved these changes Dec 23, 2020

View reviewed changes

dongcarl added 3 commits January 8, 2021 11:40

guix: Make guix honor MAX_JOBS setting

444fcfc

guix: Add SUBSTITUTE_URLS option

0f31e24

guix: Add ADDITIONAL_GUIX_{COMMON,TIMEMACHINE}_FLAGS options

93b6a85

dongcarl added 10 commits January 8, 2021 11:40

guix: Remove README development environment section

cfa7ceb

guix: Update HOSTS README entry for new architectures

402e3a5

guix: Small updates to README wording

34f0fda

docs: Point to contrib/guix/README.md in doc/guix.md

036dc74

genbuild: Specify rev-parse length

38b7b2e

guix: Add section headings to guix-build.sh

57f9533

guix: Add more sanity checks to guix-build.sh

d27ff8b

guix: Move OUTDIR determination+creation to guix-build.sh

0b7cd07

guix: Move DISTSRC determination to guix-build.sh

2f9d1fd

guix: Print build params inside/outside of container

570e43f

dongcarl force-pushed the 2020-12-guix-fixups branch from 235cb96 to 570e43f Compare January 8, 2021 16:40

fanquake approved these changes Jan 12, 2021

View reviewed changes

fanquake merged commit 1801715 into bitcoin:master Jan 12, 2021

sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Jan 12, 2021

Merge bitcoin#20619: guix: Quality of life improvements

2d13168

dongcarl mentioned this pull request Jan 12, 2021

guix: Potential NSIS non-reproducibility #20919

Closed

bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022

Conversation

dongcarl commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

practicalswift commented Dec 11, 2020

Uh oh!

laanwj commented Dec 11, 2020

Uh oh!

hebasto commented Dec 11, 2020

Uh oh!

Uh oh!

DrahtBot commented Dec 11, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Conflicts

Uh oh!

laanwj commented Dec 12, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj commented Dec 12, 2020

Uh oh!

maflcko commented Dec 12, 2020

Uh oh!

laanwj commented Dec 12, 2020

Uh oh!

dongcarl commented Dec 12, 2020

Uh oh!

dongcarl commented Dec 12, 2020

Uh oh!

DrahtBot commented Dec 13, 2020

Guix builds

Uh oh!

laanwj commented Dec 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dongcarl commented Dec 14, 2020

Uh oh!

laanwj commented Dec 14, 2020

Uh oh!

laanwj commented Dec 14, 2020

Uh oh!

DrahtBot commented Dec 14, 2020

Uh oh!

dongcarl commented Dec 22, 2020

Uh oh!

dongcarl commented Jan 8, 2021

Uh oh!

fanquake left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

dongcarl commented Dec 11, 2020 •

edited

Loading

DrahtBot commented Dec 11, 2020 •

edited

Loading

laanwj commented Dec 12, 2020 •

edited

Loading

laanwj commented Dec 13, 2020 •

edited

Loading