build: define CMAKE_COMPILE_WARNING_AS

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

CMAKE_COMPILE_WARNING_AS_ERROR is not a cache variable by default in CMake, so it has no value in the configure summary when not set, and even when set cannot be toggled in ccmake. Define it as an option() to make it a cache BOOL with a default of OFF.

From the original MR to cmake, this was deliberately not set as a cache variable: https://gitlab.kitware.com/cmake/cmake/-/merge_requests/7187 (see Brad King’s reply to the collapsed comments from Marc Chevrier).

Most CMAKE_* variables which are expected to be toggled by users are (as far as I can research) cache variables by default. Those that are considered likely to be set by the project (e.g. CMAKE_CXX_STANDARD or CMAKE_POSITION_INDEPENDENT_CODE) are not, along with read-only variables, script/internal variables, platform sppecific variables, template variables. CMAKE_COMPILE_WARNING_AS_ERROR may be a slight outlier here.

I count ~ 600 documented CMAKE_* variables, of which ~ 60 are default cache variables.

I could only see a few of these like:

CMAKE_COMPILE_WARNING_AS_ERROR
CMAKE_CXX_STANDARD,
CMAKE_POSITION_INDEPENDENT_CODE
CMAKE_INTERPROCEDURAL_OPTIMIZATION

…that we (or any project) might want to expose as user-togglable, and would have to add as an option() in CMakeLists.txt.

DrahtBot added the label Build system on Feb 17, 2026

DrahtBot commented at 12:28 pm on February 17, 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	ajtowns, fanquake, hebasto

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

Conflicts

Reviewers, this pull request conflicts with the following ones:

#33974 (cmake: Check dependencies after build option interaction by hebasto)

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.

build: define CMAKE_COMPILE_WARNING_AS_ERROR as a cache option

CMAKE_COMPILE_WARNING_AS_ERROR is not a cache variable by default in
CMake, so it has no value in the configure summary when not set, and
even when set cannot be toggled in ccmake. Define it as an option() to
make it a cache BOOL with a default of OFF.

231dd04b8d

willcl-ark force-pushed on Feb 17, 2026

DrahtBot added the label CI failed on Feb 17, 2026

DrahtBot removed the label CI failed on Feb 17, 2026

willcl-ark commented at 1:38 pm on February 17, 2026: member

Comment from MR for easier visibility:

hebasto commented at 3:06 pm on February 17, 2026: member

cc @purpleKarrot

purpleKarrot commented at 4:58 pm on February 17, 2026: contributor

I have mixed feelings about this. Generally, my position is that projects should not set any CMAKE_ variables at all and instead respect that those variables may be set from the outside by the builder/packager, via command line or environment.

However, CMAKE_COMPILE_WARNING_AS_ERROR does not really fit in the usual pattern. It is not initialized by an environment variable, but instead CMake supports the --compile-no-warning-as-error option to allow builders/packagers to ignore the setting made by the project (See also my comment here).

So I might actually be persuaded to agree that we unconditionally set CMAKE_COMPILE_WARNING_AS_ERROR (or preferably the COMPILE_WARNING_AS_ERROR target property explicitly on select targets). This would still allow clients to turn off the setting via the aforementioned command line flag.

willcl-ark commented at 9:06 pm on February 17, 2026: member

Thanks for your thoughts @purpleKarrot, some of my own after playing with these approaches further:

So I might actually be persuaded to agree that we unconditionally set CMAKE_COMPILE_WARNING_AS_ERROR (or preferably the COMPILE_WARNING_AS_ERROR target property explicitly on select targets). This would still allow clients to turn off the setting via the aforementioned command line flag.

I think that if we do either of “unconditionally set CMAKE_COMPILE_WARNING_AS_ERROR (or preferably the COMPILE_WARNING_AS_ERROR target property explicitly on select targets)”, the side-effects are:

our configure output may be innaccurate (Treat compiler warnings as errors ..... ON when it’s disabled via --compile-no-warning-as-error), maybe we drop this?
we will not be able to modify the setting on-the-fly with ccmake, perhaps not particularly important, but it did partly motivate this PR.
In the case of an unconditional set, this would shadow any user passed flag (so cmake -B build -DCMAKE_COMPILE_WARNING_AS_ERROR=OFF woudl not work as expected), although we could use a guard here if we wanted this fixed.

As I see it, setting as a cache variable feels quite elegant, vs those tradeoffs…

Am I correct in thinking that you would prefer to set it per-target to i) be maximally modular and ii) avoid setting it on third-party vendored code? Or is there another reason?

maflcko commented at 9:29 pm on February 17, 2026: member

I don’t think CMAKE_COMPILE_WARNING_AS_ERROR should be enabled by default. There doesn’t exist a single version of gcc that can compile Bitcoin Core without false positive warnings, so turning it on by default has questionable benefits among making it impossible to compile Bitcoin Core for users that are not familiar with the cmake knobs.

#27343#event-22092375856
#29359

purpleKarrot commented at 10:34 pm on February 17, 2026: contributor

@willcl-ark, I said “I might be persuaded to agree”. But if you are in doubt, I will stick to my position.

willcl-ark commented at 9:41 pm on February 18, 2026: member

Thanks @purpleKarrot for sharing more on your position, that is helpful context to read, and I know your stance is a principled one!

I get the CMake philosophy that certain CMAKE_* variables belong to the builder, not the project, and agree that’s good advice for libraries (and pure source code projects) where downstream consumers need to control compilation. But IMO Bitcoin Core’s (current) situation is different enough to justify the approach here…

We are (in our current form) primarily an “end-user application” where we (not downstream) do the packaging. We are not (yet) a pure source code project. Though we do have a nascent library in the kernel.

We encourage users to build from source, and discoverability helps here. Being able to see and toggle this setting in ccmake or the configure summary helps both developers and self-builders understand what’s available. Since the default is OFF, self-builders are completely unaffected unless they explicitly opt in. Our release builds are produced via Guix with explicit, controlled flags, so this doesn’t change the release path either.

I noted, I do want to be mindful of the fact that we are gradually producing and releasing a kernel library, and I take @purpleKarrot’s point about the problems caused when sub-projects try to set things that conflict with a parent project’s build configuration:

They try to enable compiler warnings (which conflicts with the global setting in the CI script), they try to integrate valgrind or sanitizers (which conflicts with ctest_memcheck()), they try to provide coverage (which conflicts with ctest_coverage()) and so on. Everything they try to simplify is actually broken.

However, since we default here to OFF and respect both -DCMAKE_COMPILE_WARNING_AS_ERROR=OFF and --compile-no-warning-as-error, I don’t think it introduces that problem? (If anything, option() is better behaved here than an unconditional set() would be, since the cache variable won’t override a value already passed on the command line).

If we want to fix the regressions we noticed, then I favour the approach proposed here. If we are happy to accept them, then I think we should (drop the configure output and) take your approach. We only need to decide which.

fanquake commented at 10:01 am on February 19, 2026: member

cc @ajtowns - does this solve your issue?

ajtowns commented at 6:06 am on February 20, 2026: contributor

Yeah, seems to work for me. ACK 231dd04b8dcb85ed1bc9191b839fb6cd64acea86

fanquake commented at 3:00 pm on February 20, 2026: member

ACK 231dd04b8dcb85ed1bc9191b839fb6cd64acea86

hebasto approved

hebasto commented at 7:14 pm on February 20, 2026: member

ACK 231dd04b8dcb85ed1bc9191b839fb6cd64acea86, tested on Fedora 43.

#34504 (comment):

This change seems to prevent the option from showing up in ccmake ? Previously I could run ccmake build/ to turn WERROR on, but now I don’t seem to see the new CMAKE_COMPILE_WARNING_AS_ERROR option at all.

I’m glad to see people using ccmake. We should keep them in mind while maintaining the build system.

hebasto merged this on Feb 20, 2026

hebasto closed this on Feb 20, 2026

build: define CMAKE_COMPILE_WARNING_AS_ERROR as a cache option #34605

Reviews

Conflicts