build: replace `WERROR` with `CMAKE_COMPILE_WARNING_AS

fanquake commented at 12:18 pm on February 4, 2026: member

CMAKE_COMPILE_WARNING_AS_ERROR was added to CMake in 3.24.

--compile-no-warning-as-error can be used, if needed, in future, to suppress the CMAKE_COMPILE_WARNING_AS_ERROR behaviour from a CI config.

Potential alternative to #33297. Closes #33284.

See https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html.

DrahtBot added the label Build system on Feb 4, 2026

DrahtBot commented at 12:18 pm on February 4, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	hebasto, purpleKarrot, maflcko

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

maflcko commented at 12:26 pm on February 4, 2026: member

I guess this will make ci/test/00_setup_env_native_previous_releases.sh:export CI_IMAGE_NAME_TAG="mirror.gcr.io/ubuntu:22.04" pass on all compile warnings, but maybe this is fine and we only care about hard compile failures there?

I think it would be better to hardcode -Werror in the flags of this CI config ?

fanquake force-pushed on Feb 4, 2026

DrahtBot added the label CI failed on Feb 4, 2026

maflcko commented at 12:37 pm on February 4, 2026: member

lgtm.

Would be nice to provide (and push) a diff to inject a warning (maybe a missing Wswitch or Wunused function?), so that the CI result can be confirmed for all tasks and local testing is easier.

in ci/test/00_setup_env_native_previous_releases.sh:23 in ab32ab1a51 outdated

18@@ -19,9 +19,9 @@ export BITCOIN_CONFIG="\
19  --preset=dev-mode \
20  -DREDUCE_EXPORTS=ON \
21  -DCMAKE_BUILD_TYPE=Debug \
22- -DCMAKE_C_FLAGS='-funsigned-char' \
23+ -DCMAKE_C_FLAGS='-funsigned-char -Werror' \

maflcko commented at 12:38 pm on February 4, 2026:

nit: Maybe document that this is only a temporary workaround until a later cmake version is used here?

fanquake commented at 1:23 pm on February 4, 2026:

Added.

fanquake force-pushed on Feb 4, 2026

fanquake commented at 1:27 pm on February 4, 2026: member

Would be nice to provide (and push)

Pushed.

Also checked (on alpine with gcc 15.x) that this would catch the same trailing whitespace issue recently seen with secp256k1:

0In file included from /bitcoin/src/secp256k1/src/util.h:10,
1                 from /bitcoin/src/secp256k1/src/field.h:10,
2                 from /bitcoin/src/secp256k1/src/group.h:10,
3                 from /bitcoin/src/secp256k1/src/precomputed_ecmult_gen.c:3:
4/bitcoin/src/secp256k1/src/../include/secp256k1.h:17:52: error: trailing whitespace [-Werror=trailing-whitespace=]
5   17 |  *    they describe, even if this violates rule 1.
6cc1: all warnings being treated as errors
7gmake[2]: *** [src/secp256k1/src/CMakeFiles/secp256k1_precomputed.dir/build.make:93: src/secp256k1/src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.o] Error 1

fanquake commented at 1:46 pm on February 4, 2026: member

Everything that was expected to fail, has failed, except for MSVC. I guess we don’t get warnings there for missing cases in a switch?

maflcko commented at 1:52 pm on February 4, 2026: member

lgtm ACK d9e5ba511ea0c60b1e0e16ad12357998389b4f1f

fanquake marked this as ready for review on Feb 4, 2026

fanquake force-pushed on Feb 4, 2026

DrahtBot removed the label CI failed on Feb 4, 2026

in ci/test/03_test_script.sh:90 in b6d915753c outdated

85@@ -86,7 +86,7 @@ if [ -z "$NO_DEPENDS" ]; then
86   BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} -DCMAKE_TOOLCHAIN_FILE=$DEPENDS_DIR/$HOST/toolchain.cmake"
87 fi
88 if [ -z "$NO_WERROR" ]; then
89-  BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} -DWERROR=ON"
90+  BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} -DCMAKE_COMPILE_WARNING_AS_ERROR=ON"
91 fi

willcl-ark commented at 3:21 pm on February 4, 2026:

As far as I can see nothing ever sets $NO_WERROR, so this is unconditionally hit and, if this is intended, could be clarified to:

0BITCOIN_CONFIG_ALL="${BITCOIN_CONFIG_ALL} -DCMAKE_COMPILE_WARNING_AS_ERROR=ON"

maflcko commented at 4:25 pm on February 4, 2026:

it was last used in fa47baa03bcfcf44fb2ed05f009a32d32f860c45. not sure if it will be needed again.

fanquake commented at 4:25 pm on February 4, 2026:

It’s like that, so the behaviour can be disabled.

hebasto commented at 4:02 pm on February 5, 2026:

The --compile-no-warning-as-error command line option can be used instead.

fanquake commented at 4:40 pm on February 5, 2026:

Given the availability of this option, have done as Will suggested.

fanquake force-pushed on Feb 5, 2026

fanquake requested review from purpleKarrot on Feb 5, 2026

hebasto commented at 3:37 pm on February 5, 2026: member

Concept ACK.

CMAKE_COMPILE_WARNING_AS_ERROR was added to CMake in 3.24.

Since its usage is not guarded by CMake policies, it does not depend on the minimum version defined here:: https://github.com/bitcoin/bitcoin/blob/eb97250421d35f8c172d43b69965af6273025702/CMakeLists.txt#L10

purpleKarrot commented at 3:59 pm on February 5, 2026: contributor

ACK f6386d8e06f382e95aba53a5f100d41b4d3744db

Love it. CMAKE_COMPILE_WARNING_AS_ERROR is set in the build scripts and logic is removed from CMakeLists.txt.

DrahtBot requested review from hebasto on Feb 5, 2026

DrahtBot requested review from maflcko on Feb 5, 2026

hebasto approved

hebasto commented at 4:21 pm on February 5, 2026: member

ACK f6386d8e06f382e95aba53a5f100d41b4d3744db, tested on Ubuntu 25.10. I’ve verified compiler flags in the secp256k1 subtree in verbose mode.

Still suggesting to ditch NO_WERROR as well.

nit: In the commit message s/DWERROR/WERROR/.

build: replace WERROR with CMAKE_COMPILE_WARNING_AS_ERROR

-Werror is added to the previous releases job, given it runs on Ubuntu
22.04, which uses an older CMake.

`--compile-no-warning-as-error` can be used, if needed, in future, to
suppress the `CMAKE_COMPILE_WARNING_AS_ERROR` behaviour from a CI
config.

CMAKE_COMPILE_WARNING_AS_ERROR was added to CMake in 3.24.
See https://cmake.org/cmake/help/latest/prop_tgt/COMPILE_WARNING_AS_ERROR.html.

Co-authored-by: willcl-ark <will8clark@gmail.com>

322c4ec442

fanquake force-pushed on Feb 5, 2026

fanquake commented at 4:42 pm on February 5, 2026: member

Fixed the commit message, took @willcl-ark change, update OP to mention --compile-no-warning-as-error as possible to use in CI configs going forward.

hebasto approved

hebasto commented at 4:49 pm on February 5, 2026: member

re-ACK 322c4ec4422a2e03fc81e61b1315b1245ba47d2f.

purpleKarrot commented at 5:33 pm on February 5, 2026: contributor

re-ACK 322c4ec4422a2e03fc81e61b1315b1245ba47d2f

Note that --compile-no-warning-as-error is intended to be used in build scripts to override COMPILE_WARNING_AS_ERROR properties set by the project in the case where project authors want to enforce -Werror behavior for local development, but builders want to track warnings separately from errors in CI. We currently do the opposite: We enable -Werror for CI.

maflcko commented at 6:15 pm on February 5, 2026: member

review ACK 322c4ec4422a2e03fc81e61b1315b1245ba47d2f 📩

Signature:

0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
2trusted comment: review ACK 322c4ec4422a2e03fc81e61b1315b1245ba47d2f 📩
3MpD+OAJVysUM5y9AMPahwglFE3ioFvSBF8eonILxzxxTCohUe1TCR5pV2Q8B/BVyP0azTtTF5jcjhXz/oJXwAQ==

hebasto merged this on Feb 5, 2026

hebasto closed this on Feb 5, 2026

fanquake deleted the branch on Feb 6, 2026

willcl-ark commented at 12:17 pm on February 6, 2026: member

post-merge ACK 322c4ec4422a2e03fc81e61b1315b1245ba47d2f

build: replace WERROR with CMAKE_COMPILE_WARNING_AS_ERROR #34504

Reviews

build: replace `WERROR` with `CMAKE_COMPILE_WARNING_AS_ERROR` #34504