PACKAGE_* variables to CLIENT_*
#31042
The use of PACKAGE_NAME for the project’s variable name is problematic, as this name is commonly used in CMake’s interface variables. If third-party CMake code handles with scopes improperly, our PACKAGE_NAME variable could end up with an unexpected value.
This PR avoids such conflicts by renaming all PACKAGE_* variables to CLIENT_*.
The code in the master branch works correctly only incidentally. It is definitely broken in #30997.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For detailed information about the code coverage, see the test coverage report.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | TheCharlatan |
| Stale ACK | pablomartin4btc |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
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.
46@@ -47,14 +47,14 @@ project(BitcoinCore
47 LANGUAGES NONE
48 )
49
50-set(PACKAGE_VERSION ${PROJECT_VERSION})
51+set(BITCOIN_PACKAGE_VERSION ${PROJECT_VERSION})
52 if(CLIENT_VERSION_RC GREATER 0)
53- string(APPEND PACKAGE_VERSION "rc${CLIENT_VERSION_RC}")
54+ string(APPEND BITCOIN_PACKAGE_VERSION "rc${CLIENT_VERSION_RC}")
CLIENT_VERSION_ prefix here as well? (Maybe the two other variables others could use the CLIENT_ prefix as well?)
It seems CLIENT_NAME is already used in a different (non-build) context to denote the “user agent name”. Maybe a preparatory commit could be added to change that to UA_NAME, for clarity and to avoid a name clash?
0$ git grep --line-number CLIENT_NAME
1src/clientversion.cpp:23:const std::string CLIENT_NAME("Satoshi");
2src/clientversion.h:36:extern const std::string CLIENT_NAME;
3src/init.cpp:1470: strSubVersion = FormatSubVersion(CLIENT_NAME, CLIENT_VERSION, uacomments);
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/31403424642
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/32008030836
Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.
4@@ -5,8 +5,8 @@
5 function(generate_setup_nsi)
6 set(abs_top_srcdir ${PROJECT_SOURCE_DIR})
7 set(abs_top_builddir ${PROJECT_BINARY_DIR})
8- set(PACKAGE_URL ${PROJECT_HOMEPAGE_URL})
9- set(PACKAGE_TARNAME "bitcoin")
10+ set(CLIENT_URL ${PROJECT_HOMEPAGE_URL})
11+ set(CLIENT_TARNAME "bitcoin")
CLIENT_TARNAME re-naming is incomplete.
re-ACK 36a3de0af9a9120aa98a07ca4abbb77ee6e58ef9
Since my previous review: extended renaming to the rest of sources where PACKAGE_ namespace existed.
ACK 36a3de0af9a9120aa98a07ca4abbb77ee6e58ef9
The last two commits could be squashed together though? I don’t think that would complicate the scripted diff.
The last two commits could be squashed together though? I don’t think that would complicate the scripted diff.
Thanks! Squashed.
This change allows to the use of the `CLIENT_` namespace without
potential name clashes.
-BEGIN VERIFY SCRIPT-
sed -i "s/\<CLIENT_NAME\>/UA_NAME/g" $( git grep -l "CLIENT_NAME" ./src)
-END VERIFY SCRIPT-
The use of `PACKAGE_NAME` for the project's variable name is
problematic, as this name is commonly used in CMake's interface
variables. If third-party CMake code handles with scopes improperly,
our `PACKAGE_NAME` variable could end up with an unexpected value.
This change avoids such conflicts by renaming all `PACKAGE_*` variables
to `CLIENT_*`.
This change ensures consistent use of the `CLIENT_` namespace everywhere
in the repository.
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s/\<$1\>/$2/g" $( git grep -l "$1" ./cmake ./src :\(exclude\)./src/secp256k1 ./test ) ; }
ren PACKAGE_NAME CLIENT_NAME
ren PACKAGE_VERSION CLIENT_VERSION_STRING
ren PACKAGE_URL CLIENT_URL
ren PACKAGE_BUGREPORT CLIENT_BUGREPORT
-END VERIFY SCRIPT-Guix Build:
02df1f3823c8f689140e3a400f0fdc49d4fe0d2bc4d29402d9765bf9821147b28 guix-build-70713303b639/output/aarch64-linux-gnu/SHA256SUMS.part
133a4689e3200b0fbd56d8098b33cf9388baebe641d27dd6dc09813f56ea2abb1 guix-build-70713303b639/output/aarch64-linux-gnu/bitcoin-70713303b639-aarch64-linux-gnu-debug.tar.gz
235f43ddc81853d054544bbe70d7d8461867c026a7b4d3c9ae90ee248bf28c9de guix-build-70713303b639/output/aarch64-linux-gnu/bitcoin-70713303b639-aarch64-linux-gnu.tar.gz
3b87c9dd6990934021ef5a421e46d0159b822b1ddd4457a7bcbad3ab0763eb279 guix-build-70713303b639/output/arm-linux-gnueabihf/SHA256SUMS.part
4da4a4a1dcee0d6b0f210b25b8e0320dda1f2249efb90a7677cafcf2f00c1eb56 guix-build-70713303b639/output/arm-linux-gnueabihf/bitcoin-70713303b639-arm-linux-gnueabihf-debug.tar.gz
5925b1d6cf6fb2c59a34c74a05053278d88ed7ee84ddeb960f39599835fd31a8e guix-build-70713303b639/output/arm-linux-gnueabihf/bitcoin-70713303b639-arm-linux-gnueabihf.tar.gz
61a36013e0f1585b5c8ffe4e39fa24638481a2c38d8dafc9aaaece11698236b92 guix-build-70713303b639/output/arm64-apple-darwin/SHA256SUMS.part
79bfcbd2ddc619ccca320424a0520e79f8489c28207093e855bba48322a48f24a guix-build-70713303b639/output/arm64-apple-darwin/bitcoin-70713303b639-arm64-apple-darwin-unsigned.tar.gz
859c841f7b385bd9645cef3e4b1a5f93b2ef577a276e06418f919180ee23d7e4d guix-build-70713303b639/output/arm64-apple-darwin/bitcoin-70713303b639-arm64-apple-darwin-unsigned.zip
957019be46ef95ea27ca5a4fb3d29c78782988fbe15ee893aa912b43b3da7dfa0 guix-build-70713303b639/output/arm64-apple-darwin/bitcoin-70713303b639-arm64-apple-darwin.tar.gz
10ab6762e153f42dfb37d21d205c14a9ff3fd381b74141a45b9ea38ee129e856e5 guix-build-70713303b639/output/dist-archive/bitcoin-70713303b639.tar.gz
11fb9b9719b2769406a6d7d4ad05d737b954c51f374385cb44a336272c3c291736 guix-build-70713303b639/output/powerpc64-linux-gnu/SHA256SUMS.part
12d4f2cdf931b7bba6c4d9291ea930659c04fd6a7a3a9682c5268ee96145861c97 guix-build-70713303b639/output/powerpc64-linux-gnu/bitcoin-70713303b639-powerpc64-linux-gnu-debug.tar.gz
13f9db6ed69d00b9dfaa72fe7a0658e19b3c841ea0b4ada59a5cd4659dd330d392 guix-build-70713303b639/output/powerpc64-linux-gnu/bitcoin-70713303b639-powerpc64-linux-gnu.tar.gz
148167118fcef084f28831e6bece33eb2061404fa0f7e96d629a95d3f329ba5310 guix-build-70713303b639/output/riscv64-linux-gnu/SHA256SUMS.part
15819cd35f0c32a68a545347adb0f6bc6e53c0d243a4bacf37cebf99b400648b35 guix-build-70713303b639/output/riscv64-linux-gnu/bitcoin-70713303b639-riscv64-linux-gnu-debug.tar.gz
16521107405bccfd2032356dd06227391d696ebaf1edf87a24330b4d78bb55af49 guix-build-70713303b639/output/riscv64-linux-gnu/bitcoin-70713303b639-riscv64-linux-gnu.tar.gz
171eb6cbf97f5c9a0e66c204b141f6f4780374242b05ac2f55e47fdb571abbafda guix-build-70713303b639/output/x86_64-apple-darwin/SHA256SUMS.part
18509038ecfac467f801f06078868bf52de2918928faea4d2aae9a5784732f976c guix-build-70713303b639/output/x86_64-apple-darwin/bitcoin-70713303b639-x86_64-apple-darwin-unsigned.tar.gz
19213a190c1cfed5f40089b078cef860ffc1ba3a0bedc7e489e89658db40718bef guix-build-70713303b639/output/x86_64-apple-darwin/bitcoin-70713303b639-x86_64-apple-darwin-unsigned.zip
20691f5cd5e4f9659ef15161cc4f30a797c6abd803f60e4eeba36b9d391b23981d guix-build-70713303b639/output/x86_64-apple-darwin/bitcoin-70713303b639-x86_64-apple-darwin.tar.gz
213c2b462b300659fcf46537c67ceac879f3d5e850c988bb172bb6514dac7701c0 guix-build-70713303b639/output/x86_64-linux-gnu/SHA256SUMS.part
222d9cc4effcc041f87fb6054e56a206694ccf133c32ae62dc4bb6f2107a589beb guix-build-70713303b639/output/x86_64-linux-gnu/bitcoin-70713303b639-x86_64-linux-gnu-debug.tar.gz
23b3d11e2a55ae7cf03f601527816052214596e2270190f89a151eac4e8796c465 guix-build-70713303b639/output/x86_64-linux-gnu/bitcoin-70713303b639-x86_64-linux-gnu.tar.gz
24225ec52f051ec7410b29f30b9c36898f330fb5d94267f242b39a18711b4bbe37 guix-build-70713303b639/output/x86_64-w64-mingw32/SHA256SUMS.part
258f022cecbedf22324778983b9d8b7351d24a1d51297e7124f06bf82cc5490bbd guix-build-70713303b639/output/x86_64-w64-mingw32/bitcoin-70713303b639-win64-debug.zip
260243301dcff8850146901e77b0e58659594fb7108193004c11fa46d5ded439e9 guix-build-70713303b639/output/x86_64-w64-mingw32/bitcoin-70713303b639-win64-setup-unsigned.exe
2797938f0053cf1f361817d8bb750461a809f05983ea7d030d61222c53053e91e7 guix-build-70713303b639/output/x86_64-w64-mingw32/bitcoin-70713303b639-win64-unsigned.tar.gz
288e205188361743d291fe4317741a05b7d205c6a7df22c07338a1aaad49159507 guix-build-70713303b639/output/x86_64-w64-mingw32/bitcoin-70713303b639-win64.zip