cmake: Use `AUTHOR_WARNING` for warnings #32865

fanquake · 2025-07-03T11:21:31Z

Rather than create our own warning logging/handling, which isn't ideal (#31476), use CMakes AUTHOR_WARNING (also pointed out by purpleKarrot here: #31665 (comment)).

Would result in failures like:

-- Performing Test LINKER_SUPPORTS__WL__Z_SEPARATE_CODE - Success
-- Could NOT find Python3 (missing: Python3_EXECUTABLE Interpreter) (Required is at least version "3.10")
CMake Error (dev) at CMakeLists.txt:592 (message):
  Minimum required Python not found.  Rpcauth tests are disabled.
This error is for project developers. Use -Wno-error=dev to suppress it.

-- Configuring incomplete, errors occurred!

Alternative to #31665 & #31669.
Would close #31476.

DrahtBot · 2025-07-03T11:21:38Z

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

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32865.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept NACK	luke-jr
Stale ACK	purpleKarrot, ryanofsky

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33144 (build: Set AUTHOR_WARNING on warnings by maflcko)

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.

ci/test/03_test_script.sh

fanquake · 2025-07-03T11:46:53Z

cc @purpleKarrot

hebasto · 2025-07-03T11:56:47Z

CMakeLists.txt

-if(configure_warnings)
-    message("  ******\n")
-    foreach(warning IN LISTS configure_warnings)
-      message(WARNING "${warning}")
-    endforeach()
-    message("  ******\n")
-endif()


All warnings were intentionally printed just after the summary to draw the user's attention to them.

Yeah, seems fine to keep this at the end for users and devs that have it not set? Maybe just use a trivial one-line patch?

diff --git a/CMakeLists.txt b/CMakeLists.txt index 4da12e0f46..c3dea019b4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -710,6 +710,7 @@ if(configure_warnings) message(WARNING "${warning}") endforeach() message(" ******\n") + message(AUTHOR_WARNING "Warnings have been encountered!") endif() # We want all build properties to be encapsulated properly.

Also, could enable Werror in "name": "dev-mode", preset?

Reintroducing this block + your patch will only do something if we also reintroduce the custom warning handling (configure_warnings doesn't come from CMake). The behavior of master is that the only warnings that could appear here are no python, tried to turn off ccache but couldn't, PIE doesn't work, (and no other CMake warnings/output).

Also, could enable Werror in "name": "dev-mode", preset?

Will add.

Reintroducing this block + your patch will only do something if we also reintroduce the custom warning handling (configure_warnings

Yes, that was my suggestion: Replace the commit with a trivial one-line patch.

Reintroducing this block + your patch will only do something if we also reintroduce the custom warning handling (configure_warnings

Yes, that was my suggestion: Replace the commit with a trivial one-line patch.

I support @maflcko's suggestion.

Done in #33144. It should be trivial to revert that one-line patch and rebase this pull on top of it, if there is need.

DrahtBot · 2025-07-03T12:45:23Z

🚧 At least one of the CI tasks failed.
_{Task macOS-cross, gui, no tests: https://github.com/bitcoin/bitcoin/runs/45287145804}
_{LLM reason (✨ experimental): The CI failure is caused by an linker error: "ld64.lld: error: unknown argument '--exclude-libs'" indicating that the linker does not support the '--exclude-libs' option used in the build process.}

Hints

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.

fanquake · 2025-07-03T14:58:23Z

Guix Build:

7c9bae09c11b66251bcab7abe66d3446da64ca2386fbc67066c1f8194df778b9  guix-build-04c4cc059e44/output/aarch64-linux-gnu/SHA256SUMS.part
ffb0670da219af40ce2c986e33b8446bbc6d60ebfa31fd142f91a22a50a3ba89  guix-build-04c4cc059e44/output/aarch64-linux-gnu/bitcoin-04c4cc059e44-aarch64-linux-gnu-debug.tar.gz
1b54fb59042b8abc7d2b7079de3f9b16770f6434c41049aebf4e0bbb4278596b  guix-build-04c4cc059e44/output/aarch64-linux-gnu/bitcoin-04c4cc059e44-aarch64-linux-gnu.tar.gz
fc14d2a51e6a1b12e8a7b96450e15809bf1db5dd1a8c5a378763641e20f871a4  guix-build-04c4cc059e44/output/arm-linux-gnueabihf/SHA256SUMS.part
bb0fe708f4fb50a917c29299de96d7afb7d7e387ab55dfc7f75d162afaa9e493  guix-build-04c4cc059e44/output/arm-linux-gnueabihf/bitcoin-04c4cc059e44-arm-linux-gnueabihf-debug.tar.gz
d0691b820933c849e5e52f3b2deca15b43866c7260fad403cfa396f224b5bf49  guix-build-04c4cc059e44/output/arm-linux-gnueabihf/bitcoin-04c4cc059e44-arm-linux-gnueabihf.tar.gz
0de94186e11f0e483a82bf3ce073eda190e6a0a0c84935299fa331c2fe62c9e5  guix-build-04c4cc059e44/output/arm64-apple-darwin/SHA256SUMS.part
8f2068572bd83964591d6d420c72f4ac9388aecd152e75e9319ae35238f83fc7  guix-build-04c4cc059e44/output/arm64-apple-darwin/bitcoin-04c4cc059e44-arm64-apple-darwin-codesigning.tar.gz
9e31de4e53573c2479fc1061dd0769861d186de30c52dc8a27e1cd5aa0519d07  guix-build-04c4cc059e44/output/arm64-apple-darwin/bitcoin-04c4cc059e44-arm64-apple-darwin-unsigned.tar.gz
bb493e8cdb9939e595f0d9de9c0df446e71f3bf9298577b70442e6440d65be00  guix-build-04c4cc059e44/output/arm64-apple-darwin/bitcoin-04c4cc059e44-arm64-apple-darwin-unsigned.zip
b764580cf796e3350179c054598228e350bb767e3f21654b8541474c226bb201  guix-build-04c4cc059e44/output/dist-archive/bitcoin-04c4cc059e44.tar.gz
f741c4d7b5d72241624216bd07ae8454f02decac70129cd92a72d9016de82ca1  guix-build-04c4cc059e44/output/powerpc64-linux-gnu/SHA256SUMS.part
fd2f76976f1cf2ebf995d0b24f7545d6a863673e3629940967617d3cae6da752  guix-build-04c4cc059e44/output/powerpc64-linux-gnu/bitcoin-04c4cc059e44-powerpc64-linux-gnu-debug.tar.gz
ecf9fcfaf75c5542bd2b3346efebc01299bdeeab9604916a8dc1174993216cfc  guix-build-04c4cc059e44/output/powerpc64-linux-gnu/bitcoin-04c4cc059e44-powerpc64-linux-gnu.tar.gz
ecb8cdb345525219203d4921ba3586315649d3b9c46417ebe8bc1ebb40aff52f  guix-build-04c4cc059e44/output/riscv64-linux-gnu/SHA256SUMS.part
d5cb27d211f7d5e639708b057e4cf3bb738d7c8b0b13c9023a6bee3a1af598c3  guix-build-04c4cc059e44/output/riscv64-linux-gnu/bitcoin-04c4cc059e44-riscv64-linux-gnu-debug.tar.gz
1b807b6e2fe23d63e9d84b1d27c4ec0bfae45f8595bb20ae015dbdf62b45462e  guix-build-04c4cc059e44/output/riscv64-linux-gnu/bitcoin-04c4cc059e44-riscv64-linux-gnu.tar.gz
ec7c9113b4acad38b112812ce9bc0bf6dcffd9e6eb9b6f89bbf08da42cb93ae1  guix-build-04c4cc059e44/output/x86_64-apple-darwin/SHA256SUMS.part
82bc0df4fcefe8ecd6dc9336ab17de24e4d42812de0e860f34c7b1ebe2fd8552  guix-build-04c4cc059e44/output/x86_64-apple-darwin/bitcoin-04c4cc059e44-x86_64-apple-darwin-codesigning.tar.gz
05320e706a0082b7cdd63aa8d004046aeb042f5742bae2c1531a38c0885d3f19  guix-build-04c4cc059e44/output/x86_64-apple-darwin/bitcoin-04c4cc059e44-x86_64-apple-darwin-unsigned.tar.gz
047df0d762d3f67ed814cc98a375781d3e4588d6fb0ae226b73278ef9339d9b9  guix-build-04c4cc059e44/output/x86_64-apple-darwin/bitcoin-04c4cc059e44-x86_64-apple-darwin-unsigned.zip
ef85f647d71c0d5999acf9c2ab84a13b2971475d64781d7d595122e129dc8565  guix-build-04c4cc059e44/output/x86_64-linux-gnu/SHA256SUMS.part
913e6c99c854298d4414441dfee8bb0e0cb378a6c5e7470080c2f68da24152bf  guix-build-04c4cc059e44/output/x86_64-linux-gnu/bitcoin-04c4cc059e44-x86_64-linux-gnu-debug.tar.gz
a5c01a598b1dff050ea820416266989207f62db9937e4836025def585bb9ae5f  guix-build-04c4cc059e44/output/x86_64-linux-gnu/bitcoin-04c4cc059e44-x86_64-linux-gnu.tar.gz
43f67909195c5b32f337659489758ec350eb555d31c33c952b58478e942a22d1  guix-build-04c4cc059e44/output/x86_64-w64-mingw32/SHA256SUMS.part
2f09e27676290afa2bb0172a6ce94a5e0188cd516f3771cb9a85ae9102cad2ef  guix-build-04c4cc059e44/output/x86_64-w64-mingw32/bitcoin-04c4cc059e44-win64-codesigning.tar.gz
5938f7b23de5701fa02720290b008c1b4c7f5ca95faa45051ab1dfb6c9c32256  guix-build-04c4cc059e44/output/x86_64-w64-mingw32/bitcoin-04c4cc059e44-win64-debug.zip
7d2008ba3affffb705b9271c0e29d88ccb1e3c1dd225ffc91963da2ecbc49fb2  guix-build-04c4cc059e44/output/x86_64-w64-mingw32/bitcoin-04c4cc059e44-win64-setup-unsigned.exe
8aa293975b097d2890fe9efb521a9a330d9f3332e74a4bec63a9102c2ecbf799  guix-build-04c4cc059e44/output/x86_64-w64-mingw32/bitcoin-04c4cc059e44-win64-unsigned.zip

purpleKarrot · 2025-07-03T16:01:02Z

ACK 04c4cc0

DrahtBot · 2025-07-04T08:10:02Z

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit `51ccc71` (master)	commit `45a458f` (pull/32865/merge)
*-aarch64-linux-gnu-debug.tar.gz	`b4f62a03fed68e6e...`	`17d5966123ada5cb...`
*-aarch64-linux-gnu.tar.gz	`67da2dd39446e055...`	`f7453b63f1556576...`
*-arm-linux-gnueabihf-debug.tar.gz	`224d178ed09b58ce...`	`10529d1e616e0215...`
*-arm-linux-gnueabihf.tar.gz	`2d8d8e1901fa7f17...`	`09039650386fa94a...`
*-arm64-apple-darwin-codesigning.tar.gz	`aaa96ec17bc009bf...`	`3838c6763cfb8a4b...`
*-arm64-apple-darwin-unsigned.tar.gz	`6351e4ec85221874...`	`13b314f13ed66030...`
*-arm64-apple-darwin-unsigned.zip	`5b5977fab0619831...`	`f815b8dcfd4446cc...`
*-powerpc64-linux-gnu-debug.tar.gz	`4a6d90213362c408...`	`7ce7745554f02d03...`
*-powerpc64-linux-gnu.tar.gz	`623b924620830e3f...`	`513e36358d096f3e...`
*-riscv64-linux-gnu-debug.tar.gz	`29acf8d38a47618f...`	`9cdd17e576c70584...`
*-riscv64-linux-gnu.tar.gz	`396297bf42899816...`	`56ea7076196578cf...`
*-x86_64-apple-darwin-codesigning.tar.gz	`3eaa5212fcca90db...`	`2a930ac29ca4a02b...`
*-x86_64-apple-darwin-unsigned.tar.gz	`2baee23a3e8aa15a...`	`e313a52242151801...`
*-x86_64-apple-darwin-unsigned.zip	`99e52233a0e9db27...`	`3b561705e62916ea...`
*-x86_64-linux-gnu-debug.tar.gz	`4246718f7c02ecdf...`	`327449434c43b21a...`
*-x86_64-linux-gnu.tar.gz	`57491003214db776...`	`83e913b03cd534c1...`
*.tar.gz	`d014f5b396d4a0d4...`	`82ccd3a5878b3a0c...`
SHA256SUMS.part	`5e57caac3e97ba45...`	`b47d852635150b67...`
guix_build.log	`c32f66181eb0e72c...`	`977b2aeb3198d668...`
guix_build.log.diff		`75d6680c4b3970c4...`

luke-jr · 2025-07-07T20:35:40Z

Concept NACK: These warnings don't appear to be just for developers, but also relevant to builders.

fanquake · 2025-07-07T20:50:11Z

Concept NACK: These warnings don't appear to be just for developers, but also relevant to builders.

Not sure I understand your NACK. The warnings will still be shown to builders; using AUTHOR_WARNING just gives us a proper mechanism to turn warnings into errors.

ryanofsky

Code review ACK bf8bf74.

IIUC, the last 3 commits passing -Werror=dev to cmake in CI, guix, and developer presents seem like clearly good changes, because they turn cmake internal warnings and message(AUTHOR_WARNING "...") statements into errors which stop the build so they can't be ignored.

The other commit 02955f3 is changing our current message(WARNING "...") statements into message(AUTHOR_WARNING "...") statements so these warnings will also become errors when -Werror=dev is used. This change seems a little kludgy because author warnings seem meant for people maintaining the build system, not just using it, but all of the warnings that are changed seem to be about ignored options provided by users. However, cmake doesn't provide an easy way to treat normal warnings as errors, so using AUTHOR_WARNING instead of WARNING seems helpful even if it doesn't make sense semantically.

ryanofsky · 2025-07-08T13:16:45Z

Not sure I understand your NACK. The warnings will still be shown to builders; using AUTHOR_WARNING just gives us a proper mechanism to turn warnings into errors.

I think luke's concern might be that author warnings can be suppressed with -Wno-dev and might be suppressed by default in certain environments, since author warnings are only supposed to be relevant to project maintainers, not to users.

In 02955f3 we are turning normal warnings about ignored options into the author warnings so the change does not really make sense semantically and could cause these warnings to be ignored or hidden in cases where they might not be ignored before.

But since cmake only provides an easy way to turn author warnings into errors, not other types of warnings, I do think this change is probably good overall, just maybe not ideal.

fanquake · 2025-07-09T09:01:24Z

@ryanofsky Yea, it's a shame CMake doesn't have something builtin to do this. I think this change, while not perfect, is better than the current state of having our own warning handling (where warnings are still easily missed (#31476), we have no option to turn them into errors, and someone still needs to pass -Wdev to get CMake warnings turned into errors). As well as the alternatives of, just adding even more options to the build system (#31669) which doesn't actually fix the issue generically, and still requires someone to use the CMake options if they want all errors, or adding even more custom warning handling (#31665).

purpleKarrot · 2025-07-09T09:16:01Z

@fanquake, what exactly would fix the issue upstream? A generic -Werror option (without the = part) that simply turns all warnings into errors?

fanquake · 2025-07-09T09:22:42Z

A generic -Werror option (without the = part) that simply turns all warnings into errors?

I think so. The downside in this PR is that we have to somewhat (even if semantically) misuse AUTHOR_WARNING(), to get the warning into error behaviour. Ideally we could just use WARNING(), and -Werror would turn those warnings into errors.

8f766f3 ci: enable -Werror=dev (fanquake) 7b420ca guix: configure with -Werror=dev (fanquake) 44097dd cmake: enable -Werror=dev in dev-mode preset (fanquake) Pull request description: Pass `-Werror=dev` in the CI, Guix and the `dev-mode` preset. https://cmake.org/cmake/help/latest/manual/cmake.1.html#cmdoption-cmake-Werror: > Make developer warnings errors. > Make warnings that are meant for the author of the CMakeLists.txt files errors. By default this will also turn on deprecated warnings as errors. Pulled out of #32865. ACKs for top commit: Sjors: re-ACK 8f766f3 hebasto: ACK 8f766f3, tested on Ubuntu 24.04. Tree-SHA512: 0fa321b77d2519b5249d90590664c4e5938ac86209b068658647adf97ab55ea4d54c913aae2f622385fe2f41d7c851cd5d7371905fdad38b66cb124371e16ac7

Co-authored-by: Daniel Pfeifer <daniel@pfeifer-mail.de>

DrahtBot added the Build system label Jul 3, 2025

maflcko reviewed Jul 3, 2025

View reviewed changes

ci/test/03_test_script.sh Outdated Show resolved Hide resolved

hebasto reviewed Jul 3, 2025

View reviewed changes

fanquake force-pushed the use_author_warning branch from 3549e0b to 73580d6 Compare July 3, 2025 12:45

DrahtBot added the CI failed label Jul 3, 2025

fanquake force-pushed the use_author_warning branch from 73580d6 to 04c4cc0 Compare July 3, 2025 13:22

fanquake marked this pull request as ready for review July 3, 2025 13:23

maflcko added the DrahtBot Guix build requested label Jul 3, 2025

DrahtBot mentioned this pull request Jul 3, 2025

cmake: Check user-defined APPEND_*FLAGS variables early #32367

Open

DrahtBot mentioned this pull request Jul 3, 2025

build: Make config warnings fatal if -DWCONFIGURE_ERROR=ON #31665

Closed

DrahtBot mentioned this pull request Jul 3, 2025

ci: detect outbound internet traffic generated while running tests #31349

Open

DrahtBot removed the CI failed label Jul 3, 2025

DrahtBot removed the DrahtBot Guix build requested label Jul 4, 2025

fanquake force-pushed the use_author_warning branch from 04c4cc0 to bf8bf74 Compare July 4, 2025 13:15

This was referenced Jul 5, 2025

test: Turn rpcauth.py test into functional test #32881

Merged

ci: Use optimized Debug build type in test-each-commit #32888

Merged

ryanofsky approved these changes Jul 8, 2025

View reviewed changes

fanquake force-pushed the use_author_warning branch from bf8bf74 to d948e91 Compare July 10, 2025 09:02

fanquake force-pushed the use_author_warning branch from d948e91 to ad180b7 Compare July 10, 2025 10:58

DrahtBot added the CI failed label Jul 10, 2025

fanquake mentioned this pull request Jul 10, 2025

Enable -Werror=dev in CI & Guix #32937

Merged

DrahtBot removed the CI failed label Jul 10, 2025

cmake: use AUTHOR_WARNING

bbaf3ec

Co-authored-by: Daniel Pfeifer <daniel@pfeifer-mail.de>

fanquake force-pushed the use_author_warning branch from ad180b7 to bbaf3ec Compare July 14, 2025 10:07

janb84 mentioned this pull request Aug 6, 2025

Removing Bitcoin core text where unnecessary #33126

Open

DrahtBot mentioned this pull request Aug 7, 2025

build: Set AUTHOR_WARNING on warnings #33144

Open

cmake: Use AUTHOR_WARNING for warnings #32865

Are you sure you want to change the base?

cmake: Use AUTHOR_WARNING for warnings #32865

Uh oh!

Conversation

fanquake commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

DrahtBot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

Uh oh!

Uh oh!

fanquake commented Jul 3, 2025

Uh oh!

hebasto Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

maflcko Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

fanquake Jul 4, 2025

Choose a reason for hiding this comment

Uh oh!

maflcko Jul 9, 2025

Choose a reason for hiding this comment

Uh oh!

hebasto Jul 10, 2025

Choose a reason for hiding this comment

Uh oh!

maflcko Aug 6, 2025

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jul 3, 2025

Uh oh!

fanquake commented Jul 3, 2025

Uh oh!

purpleKarrot commented Jul 3, 2025

Uh oh!

DrahtBot commented Jul 4, 2025

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

Uh oh!

luke-jr commented Jul 7, 2025

Uh oh!

fanquake commented Jul 7, 2025

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky commented Jul 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

fanquake commented Jul 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

purpleKarrot commented Jul 9, 2025

Uh oh!

fanquake commented Jul 9, 2025

Uh oh!

Uh oh!

cmake: Use `AUTHOR_WARNING` for warnings #32865

cmake: Use `AUTHOR_WARNING` for warnings #32865

fanquake commented Jul 3, 2025 •

edited

Loading

DrahtBot commented Jul 3, 2025 •

edited

Loading

ryanofsky commented Jul 8, 2025 •

edited

Loading

fanquake commented Jul 9, 2025 •

edited

Loading