Without losing the new and improved git-archive method of ensuring we get the entire source included, this fixes two one remaining regressions to standard source code distribution expectations:
./configure and not need maintainer tools.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
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.
| File | commit 978c5a212240fd03af13d6f72ba3c27da6298f61(master) | commit b3ccb885028531a89aef0e010036ea3fe79d1036(master and this pull) |
|---|---|---|
| bitcoin-*-aarch64-linux-gnu-debug.tar.gz | a7900f03fdbecc03... |
c028ccae2d68b45b... |
| bitcoin-*-aarch64-linux-gnu.tar.gz | 36b4ba0eff91c058... |
ebd0ec03feb1445f... |
| bitcoin-*-arm-linux-gnueabihf-debug.tar.gz | a21b10d10a75c2d2... |
9c0008f526a0a713... |
| bitcoin-*-arm-linux-gnueabihf.tar.gz | 63ae46d262582c69... |
4c81f707cc4b75d6... |
| bitcoin-*-osx-unsigned.dmg | 0422bc13b9dd88c5... |
2aa016fc104b05de... |
| bitcoin-*-osx64.tar.gz | 4b58e5ea071d1465... |
b7ba33216121ca9b... |
| bitcoin-*-riscv64-linux-gnu-debug.tar.gz | 26930d1065f00e44... |
c913d2fbf930a643... |
| bitcoin-*-riscv64-linux-gnu.tar.gz | 7ed0548483ac2399... |
52862432c89e4e5d... |
| bitcoin-*-win64-debug.zip | 8dfb2cfac8097b8f... |
8994f550bd426890... |
| bitcoin-*-win64-setup-unsigned.exe | 2add96057ae3fbda... |
be865006f9cd1f2e... |
| bitcoin-*-win64.zip | fc566aacd3d3f90a... |
2626ecaaf26a9b14... |
| bitcoin-*-x86_64-linux-gnu-debug.tar.gz | 40e22958ff9865ff... |
3481e8ff857eb030... |
| bitcoin-*-x86_64-linux-gnu.tar.gz | 3b312c2c0456391d... |
30a025b342414ebf... |
| bitcoin-*.tar.gz | d02114e741cbf83f... |
b66c90d783fbb822... |
| bitcoin-core-linux-0.21-res.yml | 9c0c036a266a3022... |
337505633195587a... |
| bitcoin-core-osx-0.21-res.yml | 8aaeef8dc2d63c5b... |
644bcb3bc4331bc5... |
| bitcoin-core-win-0.21-res.yml | 012d9094c6b92dd8... |
1fa1e445bbdb1a3c... |
| linux-build.log | 94a388ad77efc68f... |
c136bf16f81c0804... |
| osx-build.log | cde6e82e5f552bec... |
2e0a0044b22a6558... |
| win-build.log | 8bc04e6d3fefaef9... |
601fc37b1ad3ea9e... |
| bitcoin-core-linux-0.21-res.yml.diff | fda54493f5f1bb18... |
|
| bitcoin-core-osx-0.21-res.yml.diff | b80e6fdf7baaca2e... |
|
| bitcoin-core-win-0.21-res.yml.diff | c6ea6a7d33fa1a81... |
|
| linux-build.log.diff | 9ca9f7f27091faac... |
|
| osx-build.log.diff | ccd37c6a8c7ef6d4... |
|
| win-build.log.diff | e83cb7c3e107c1b9... |
| File | commit e5b9308920a151946b83694fe1701d90316a2a9e(master) | commit de73e4f81866bc4f333fd3b6025c93ad56a07256(master and this pull) |
|---|---|---|
| bitcoin-0.20.99-aarch64-linux-gnu-debug.tar.gz | bfa9f239a7fe270a... |
f929954bd4f694d1... |
| bitcoin-0.20.99-aarch64-linux-gnu.tar.gz | d013fc584dcdcad0... |
82511f93d64c2625... |
| bitcoin-0.20.99-arm-linux-gnueabihf-debug.tar.gz | 04e65cd9842e06e3... |
2501877f77030144... |
| bitcoin-0.20.99-arm-linux-gnueabihf.tar.gz | ac387cc990d16cbb... |
64eb7acf282e398d... |
| bitcoin-0.20.99-riscv64-linux-gnu-debug.tar.gz | 652bcc7554c6e41c... |
15aa64b7bf5ba3ae... |
| bitcoin-0.20.99-riscv64-linux-gnu.tar.gz | cb40c72590519edf... |
7a6ca68d8bbd9ed3... |
| bitcoin-0.20.99-win-unsigned.tar.gz | f0500f89bc7b736a... |
44fe15916ccb6aee... |
| bitcoin-0.20.99-win64-debug.zip | be8f2dd8bec85cae... |
d7832489713ca3c8... |
| bitcoin-0.20.99-win64-setup-unsigned.exe | d4bc6bba8904dc65... |
d4bc6bba8904dc65... |
| bitcoin-0.20.99-win64.zip | 48a9958e63b940a3... |
d4003d8f43d602df... |
| bitcoin-0.20.99-x86_64-linux-gnu-debug.tar.gz | 92ef64b319a4b3b3... |
9b3293b49b70a150... |
| bitcoin-0.20.99-x86_64-linux-gnu.tar.gz | 592213321383a66c... |
fb847c977044c496... |
| bitcoin-0.20.99.tar.gz | 63274a6760c28139... |
13d90c6c5b49024f... |
| guix_build.log | af29b815b77e0759... |
158b2f0e5d39286e... |
| guix_build.log.diff | 9eb6eefa94bf2817... |
git ls-files etc in #18478. I see no decision-making relating to intentionally excluding configure (which is a standard/expected part of source code distributions), only a few people being okay with losing it as a side effect.
Why not follow @MarcoFalke’s suggestion on IRC:
<MarcoFalke> luke-jr: Maybe split out the first commit into a pull, so that gitian builds can be performed on it more easily?
?
19+
20+cd ..
21+tar \
22+ --format=ustar \
23+ --exclude autom4te.cache \
24+ --exclude .deps \
Are these the only things we need to exclude?
Would be nice to have a comparison between “make dist minus all the git-tracked files” vs “bootstrapped files created this way minus the git-tracked files”
0tar -tpf build/out/src/bitcoin-3897f3a2ec07.tar.gz | cut -b 22- | sort
1git ls-files | perl -nle '$a=$_; while (s[/[^/]+/?$][/]) { if (not exists $d{$_}) { $d{$_} = undef; print } } print $a' | sort
Result:
0Makefile.in
1aclocal.m4
2build-aux/compile
3build-aux/config.guess
4build-aux/config.sub
5build-aux/depcomp
6build-aux/install-sh
7build-aux/ltmain.sh
8build-aux/m4/libtool.m4
9build-aux/m4/ltoptions.m4
10build-aux/m4/ltsugar.m4
11build-aux/m4/ltversion.m4
12build-aux/m4/lt~obsolete.m4
13build-aux/missing
14build-aux/test-driver
15configure
16doc/man/Makefile.in
17src/Makefile.in
18src/config/bitcoin-config.h.in
19src/secp256k1/Makefile.in
20src/secp256k1/aclocal.m4
21src/secp256k1/build-aux/compile
22src/secp256k1/build-aux/config.guess
23src/secp256k1/build-aux/config.sub
24src/secp256k1/build-aux/depcomp
25src/secp256k1/build-aux/install-sh
26src/secp256k1/build-aux/ltmain.sh
27src/secp256k1/build-aux/m4/libtool.m4
28src/secp256k1/build-aux/m4/ltoptions.m4
29src/secp256k1/build-aux/m4/ltsugar.m4
30src/secp256k1/build-aux/m4/ltversion.m4
31src/secp256k1/build-aux/m4/lt~obsolete.m4
32src/secp256k1/build-aux/missing
33src/secp256k1/build-aux/test-driver
34src/secp256k1/configure
35src/secp256k1/src/libsecp256k1-config.h.in
36src/univalue/Makefile.in
37src/univalue/aclocal.m4
38src/univalue/build-aux/compile
39src/univalue/build-aux/config.guess
40src/univalue/build-aux/config.sub
41src/univalue/build-aux/depcomp
42src/univalue/build-aux/install-sh
43src/univalue/build-aux/ltmain.sh
44src/univalue/build-aux/m4/libtool.m4
45src/univalue/build-aux/m4/ltoptions.m4
46src/univalue/build-aux/m4/ltsugar.m4
47src/univalue/build-aux/m4/ltversion.m4
48src/univalue/build-aux/m4/lt~obsolete.m4
49src/univalue/build-aux/missing
50src/univalue/build-aux/test-driver
51src/univalue/configure
52src/univalue/univalue-config.h.in
For the record, my thinking in #18478 originally was this:
make dist seems to be designed to know exactly which bootstrapped files to include (or not include) in its archivegit archive or git ls-files seems to be good at knowing all the other files we want to include in its archiveWhich means that if we combine these two, we’ll get the best of both worlds, without having to list anything. This would also be immune to future changes in how bootstrapped files are named, and additional “bootstrapped but shouldn’t be included” files like .deps or autom4te.cache.
make dist?No it doesn’t, because we only care that make dist produces an archive that includes the bootstrapped files, it can miss all the files in share, contrib, etc. and that would be fine, as we’ll just either:
git ls-files in EXTRA_DIST like in #18478, orgit archive archiveBoth of which will include all the tracked files
@dongcarl Your (1) would miss the substitutions git-archive makes in src/clientversion.cpp. I don’t see a problem with (2), although it’s slightly more complex and requires care I don’t know if we want to spend time reviewing (in particular, tar is picky about specific tar file formats when concatenating, and even if we just use --update, there’s a risk that the git archive timestamp is after the gitian reference timestamp, which might make problems)
The solution I implemented here aims to keep it simple and obviously correct.
- The tarball should be pre-autogen’d, so the user can begin with
./configureand not need maintainer tools.
I have no experience as a package maintainer so I kindly ask you to share the real-world cases when a user cannot begin with ./autogen.sh. It seems having maintainer tools is not a much burden for a user, no?
I have no experience as a package maintainer so I kindly ask you to share the real-world cases when a user cannot begin with
./autogen.sh
It used to be common to want to distribute source packages for unix systems that might not have gnu tools installed (solaris, irix, hpux, bsds) but would have a bare-bones make, c compiler, and posix shell.
To easily support building packages on these systems, the packages would not just include human readable and writable source code, but would also come with big generated configure, libtool, etc, scripts generated by the autotools pipeline and m4.
Today since we already require gnu make, a c++11 compiler, boost, libevent, berkeley db, bash, and python for many practical use cases of the source tarball, it does not seem like it is asking to much to require autotools as well. I think the main benefits of dropping the autotools generated files are having fewer differences between git checkout builds and tarball builds, not having unfathomably long megabyte shell scripts mixed in with real source code, and having simpler diffs between releases. This opinion might be a personal heresy, though. I wouldn’t want to deprive anyone of 30,000 line barely readable shell scripts if it’s what people are clamoring for.
I don’t know of any case where the user can’t install autotools, but this isn’t about “can’t”…
Including the configure script, and starting with just ./configure is standard, and what people expect both in general and even specifically for Bitcoin Core. There are no real benefits to suddenly not including it, and doing so this way does not add much complexity.
Furthermore, I don’t see any way to actually fix #18349 deterministically without some preprocessing as in #18902, so it’s not like we could simply use git-archive alone in any case.
Including the configure script, and starting with just
./configureis standard, and what people expect both in general and even specifically for Bitcoin Core. There are no real benefits to suddenly not including it, and doing so this way does not add much complexity.
From UNIX BUILD NOTES – To Build:
./autogen.sh ./configure make make install # optional
Docs seem to prevail over standard :)
just
./configureis standard
Not an actual standard, just a common practice that’s become less common because of new build systems and languages and git, and because it doesn’t have logical or practical benefits
Nothing here stops people from using autogen, as documented.
Again, there is no benefit to removing configure and breaking the de facto standard practice.
Again, there is no benefit to removing configure
It’s fine to disagree and I’m not interested in delaying this PR if it contains an essential fix and other desired changes. I just don’t want to to be misrepresented. So from #18818 (comment), “I think the main benefits of dropping the autotools generated files are having fewer differences between git checkout builds and tarball builds, not having unfathomably long megabyte shell scripts mixed in with real source code, and having simpler diffs between releases.”
tACK 3897f3a2ec077f2be8f0d08f28e79165a6cae6d4
Concept ACK on maintaining the behaviour from earlier releases, if it’s not too much trouble. This is even an improvement, because it adds depends to the source tarball.
I never used the source tarball, and can only imagine needing it if Github disappeared and there’s no reliable place to git clone from.
On the one hand I share @ryanofsky’s preference for keeping the tarball small and having the user run ./autogen.sh themselves. On the other hand it’s nice to have as few dependencies on top of the vanilla OS as possible, since everything in the tar file was created in a reproducible way.
Suggestions for followup:
Current process
I created a fresh Ubuntu 20.04 (virtual) machine and downloaded the v0.19.1 release source code, which extracts itself into bitcoin-0.19.1.
There’s no documentation that points this out, but I can install fewer packages than docs/build-unix suggest, i.e. skip libtool autotools-dev automake. I can also skip the ./autogen step:
0apt install build-essential pkg-config libdb-dev libdb++-dev libboost-system-dev libboost-filesystem-dev libboost-chrono-dev libboost-test-dev libboost-thread-dev libssl-dev libevent-dev
1./configure --with-incompatible-bdb
(this is without the GUI, also libssl-dev is not needed anymore for v0.20)
This PR
First I made a Gitian Linux build for this PR, using my regular Gitian machine:
0cd bitcoin
1git remote add luke-jr https://github.com/luke-jr/bitcoin.git
2cd ..
3./gitian-build-0.20.py -c -u https://github.com/luke-jr/bitcoin -o l -b -j 10 -m 20000 -d -D -n sjors fix_gitian_src_202004
(gitian-build-0.20.py is a copy of bitcoin/contrib/gitian-build.py)
05a1d50cb6ae06ac2f38f7a2882886a4286b388b9e2c1a09dda1f41921170476f src/bitcoin-3897f3a2ec07.tar.gz
1b5626bc1b1b59379e657974ef1a6994aed49f1814f5c0d74cc022b762d18f699 bitcoin-core-linux-0.21-res.yml
I then downloaded the source package to my VM. This extracts into bitcoin-3897f3a2ec07.
I removed all apt packages except build-essential pkg-config, installed automake libtool autotools-dev, and built with depends.
Including the configure script, and starting with just
./configureis standard, and what people expect both in general and even specifically for Bitcoin Core. There are no real benefits to suddenly not including it, and doing so this way does not add much complexity.From UNIX BUILD NOTES – To Build:
./autogen.sh ./configure make make install # optional
Docs seem to prevail over standard :)
This is standard procedure for the project’s developers. Please don’t conflate that with end-user procedure. We bootstrap because distro’s and end-user-code-builders (read: not us) expect it. I don’t see any reason to discontinue.
Honestly I really can’t follow this anymore. I agree with @luke-jr’s goals.
-0.
Removed from 0.21 milestone for now
(The autogen.sh part was controversial and is not a regression-bugfix)
🐙 This pull request conflicts with the target branch and needs rebase.
Want to unsubscribe from rebase notifications on this pull request? Just convert this pull request to a “draft”.
