cmake: (release) version handling is broken #31898

Create a test tag: v99.99 Checkout v99.99 and perform a guix build:./contrib/guix/guix-build. Take the dist-archive tarball: /bitcoin/guix-build-99.99/output/dist-archive/bitcoin-99.99.tar.gz and perform a build:

tar xf bitcoin-99.99.tar.gz
cd bitcoin-99.99
cmake -B build
cmake --build build
# ./build/src/bitcoind --version 
Bitcoin Core daemon version v28.99.0-g43e287b3ff5f0d223b0911b6ef90054ce5eb69d2

Shouldn't this behaviour be tested for releases:

--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -21,7 +21,7 @@ set(CLIENT_VERSION_MAJOR 28)
 set(CLIENT_VERSION_MINOR 99)
 set(CLIENT_VERSION_BUILD 0)
 set(CLIENT_VERSION_RC 0)
-set(CLIENT_VERSION_IS_RELEASE "false")
+set(CLIENT_VERSION_IS_RELEASE "true")
 set(COPYRIGHT_YEAR "2025")
 
 # During the enabling of the CXX and CXXOBJ languages, we modify

?

CLIENT_VERSION_IS_RELEASE only influences the version suffix information. https://github.com/bitcoin/bitcoin/blob/43e287b3ff5f0d223b0911b6ef90054ce5eb69d2/src/clientversion.cpp#L40-L44

So even if that was set we'd stil be producing a tarball from a tag v99.99, which contained the version information 28.99.

What was the autotools behavior, to compare?

What was the autotools behavior, to compare?

The tarball for 28.1 contains the version information 28.1.

i.e

wget https://bitcoincore.org/bin/bitcoin-core-28.1/bitcoin-28.1.tar.gz
tar xf bitcoin-28.1.tar.gz
cd bitcoin-28.1
./autogen.sh
./configure
make
# ./src/bitcoind --version
Bitcoin Core version v28.1.0
Copyright (C) 2009-2024 The Bitcoin Core developers

Shouldn't this behaviour be tested for releases: CLIENT_VERSION_IS_RELEASE only influences the version suffix information.

Looking at the code further, if BUILD_GIT_TAG is defined, which should be the case when the top commit is tagged according to https://github.com/bitcoin/bitcoin/blob/43e287b3ff5f0d223b0911b6ef90054ce5eb69d2/src/clientversion.cpp#L28-L40

then CLIENT_VERSION_IS_RELEASE shouldn't be used in creating the version string. So it shouldn't matter if that is set or not.

Shouldn't this behaviour be tested for releases: CLIENT_VERSION_IS_RELEASE only influences the version suffix information.

Looking at the code further, if BUILD_GIT_TAG is defined, which should be the case when the top commit is tagged according to

I don't think BUILD_GIT_TAG is defined:

$ wget https://bitcoincore.org/bin/bitcoin-core-28.1/bitcoin-28.1.tar.gz
$ tar xf bitcoin-28.1.tar.gz
$ cd bitcoin-28.1
$ ./autogen.sh
$ ./configure
$ make
$ cat src/obj/build.h
// No build information available

What was the autotools behavior, to compare?

I've reproduced all steps from #31898#issue-2860887337 for the pre-CMake commit:

$ tar xf bitcoin-99.99.tar.gz
$ cd bitcoin-99.99/
$ ./autogen.sh
$ ./configure 
$ make -j $(nproc)
$ ./src/bitcoind --version
Bitcoin Core version v28.99.0-g80f00cafdeef0600fa1a5e906686720786c2336c
Copyright (C) 2009-2024 The Bitcoin Core developers

Please contribute if you find Bitcoin Core useful. Visit
<https://bitcoincore.org/> for further information about the software.
The source code is available from <https://github.com/bitcoin/bitcoin>.

This is experimental software.
Distributed under the MIT software license, see the accompanying file COPYING
or <https://opensource.org/licenses/MIT>

Thus, there is no change in behaviour during the migration from Autotools to CMake.

So then the bug was just ported/these code paths untested during the transition?

So then the bug was just ported/these code paths untested during the transition?

The mirroring of behaviour was tested.

I guess I'm just confused why something that is obviously a bug would be ported as-is. There's no scenario where it makes sense to have a binary output version information that doesn't match the tag it was built from.

In any case, we'll have to fix the nonsensical behaviour in master.

@fanquake

Could you demonstrate how a release source tarball becomes broken when all steps in the release process have been followed?

For reference, I reviewed and tested this specifically for the cmake switch last year and I concluded that everything has been ported accurately. So I don't think there is a regression or a release-blocker.

In any case, it would be good to include full steps to reproduce here, starting from a fresh clone, because IIRC in some edge cases, the parent directory structure may influence the result if it is a git dir. The edge-cases aren't well documented, sometimes only mentioned in the release notes:

https://github.com/bitcoin/bitcoin/blame/43e287b3ff5f0d223b0911b6ef90054ce5eb69d2/doc/release-notes/release-notes-0.20.0.md#L56

It would be nice to flatten the complexity here to something simple, but I don't see how this can be achieved without possibly breaking someones build-setup.

Since guix builds from this tarball, how is it that the guix build results in binaries the output the correct version, prior to cmake?

Edit:

I don't see these issues when performing all of the release steps. When setting the version numbers and tagging as laid out in release-process.md, everything works as expected.

Looking at the code further, if BUILD_GIT_TAG is defined, which should be the case when the top commit is tagged according to

It looks like it's only defined when building inside of a git tree. This is the same behavior as with autotools. My understanding is that the version numbers are also put in the build system because builds will not always occur inside of a git tree.

Since guix builds from this tarball, how is it that the guix build results in binaries the output the correct version, prior to cmake?

The correct version comes from CLIENT_VERSION_STRING, which is set in:https://github.com/bitcoin/bitcoin/blob/785649f3977517a4ba45c5d2fedfbda778fb52cb/CMakeLists.txt#L50

It looks like it's only defined when building inside of a git tree. This is the same behavior as with autotools. My understanding is that the version numbers are also put in the build system because builds will not always occur inside of a git tree.

BUILD_GIT_TAG or BUILD_GIT_COMMIT are defined in the bitcoin-build-info.h header, which is generated by the build system depending on the build environment.

Additionally, the source archive has a commit hash hardcoded in:https://github.com/bitcoin/bitcoin/blob/785649f3977517a4ba45c5d2fedfbda778fb52cb/src/clientversion.cpp#L33

A tag can probably also be hardcoded.

but I don't see how this can be achieved without possibly breaking someones build-setup.

Given we are already breaking all build setups in the CMake switch, it seems like the right time to make any further changes?

It's pretty clear the code here is convoluted/overcomplicated. Just the fact that the release process derives the same version number from two different places is odd (and is what hides the bug here).

Given that the commit id is hardcoded into the git archive, it could make sense to hardcode everything else as well and then just prefer that over any external environment influences. This should allow to drop BITCOIN_GENBUILD_NO_GIT, because whatever result it will calculate will be ignored.

In theory, it could be implemented with a small helper script to inject the hardcoded values into the source code (along with the current commit id), then create a new "dummy" commit, then call git archive on it. The dummy commit id wouldn't end up in the archive and everything should still be deterministic.

Should this issue still be tagged for v29.0? If so, what immediate actions are expected?

I've remove the milestone, but I think there's plenty we could do here. I've also removed (release) as that seemed to be distracting from the actual bugs. Building from a tag should work regarded of being a "release", and there are codepaths for this in the current code.

It seems that the first thing to decide is the source of the version information:

• the CLIENT_VERSION_* variables set by the build system, or
• a GitHub tag

It seems that the first thing to decide is the source of the version information:

• the CLIENT_VERSION_* variables set by the build system, or

• a GitHub tag

A related discussion happens in https://github.com/bitcoin-core/bitcoin-maintainer-tools/issues/178.

It seems that the first thing to decide is the source of the version information:

• the CLIENT_VERSION_* variables set by the build system, or
• a GitHub tag

A related discussion happens in bitcoin-core/bitcoin-maintainer-tools#178.

Paraphrasing the discussion from https://github.com/bitcoin-core/bitcoin-maintainer-tools/issues/178: tags used for releases must follow versioning defined by the build system. This implies that the build system is the primary source of version information.

This older comment from @theuni supports the same idea:

Looking at this closer, I think the way to work this out is to focus the version stuff in one place or another. The logic is currently triplicated between the configure.ac, the generated build.h, and version.cpp. I believe there's no reason for that other than legacy cruft, so it makes sense to unify them so that it's sanely handled in one place.

The only real distinction that needs to be made is "building from git" or "building from tarball". In the latter case, the version should already been hard-coded.

Building on a clean machine from bitcoin-29.0rc3.tar.gz results in:

# ./build/bin/bitcoind -version
Bitcoin Core daemon version v29.0.0rc3
Copyright (C) 2009-2025 The Bitcoin Core developers

Please contribute if you find Bitcoin Core useful. Visit
<https://bitcoincore.org/> for further information about the software.
The source code is available from <https://github.com/bitcoin/bitcoin>.

This is experimental software.
Distributed under the MIT software license, see the accompanying file COPYING
or <https://opensource.org/licenses/MIT>

Therefore, I don’t see anything behaving incorrectly.

Therefore, I don’t see anything behaving incorrectly.

I'd say the issue here is about an "unclean" machine (a machine where the build happens to happen in a git dir). Though, it looks like the issue will also be fixed by https://github.com/bitcoin/bitcoin/pull/32220/files, which implements #31898 (comment) in a more clean and straightforward way.

Closing for now, in favour of #33911, as we might reafactor this in some way. Will follow up there.