Skip to content

Conversation

maflcko
Copy link
Member

@maflcko maflcko commented Aug 6, 2025

Now that the cmake setting -Werror=dev is set since commit 6a13a61 for the CI, guix and the dev cmake preset, it could make sense to notify developers about any warnings.

So do that with a single AUTHOR_WARNING.

This can be tested by introducing a bug, like:

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 6017775fa7..5610e03c66 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -589,7 +589,7 @@ set(Python3_FIND_FRAMEWORK LAST CACHE STRING "")
 # improves compatibility with Python version managers that use shims.
 set(Python3_FIND_UNVERSIONED_NAMES FIRST CACHE STRING "")
 mark_as_advanced(Python3_FIND_FRAMEWORK Python3_FIND_UNVERSIONED_NAMES)
-find_package(Python3 3.10 COMPONENTS Interpreter)
+find_package(Python3 3.210 COMPONENTS Interpreter)
 if(NOT TARGET Python3::Interpreter)
   list(APPEND configure_warnings
     "Minimum required Python not found."

Fixes #31476.

@DrahtBot
Copy link
Contributor

DrahtBot commented Aug 6, 2025

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/33144.

Reviews

See the guideline for information on the review process.

Type Reviewers
ACK l0rinc

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:

  • #32865 (cmake: Use AUTHOR_WARNING for warnings by fanquake)

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.

@l0rinc
Copy link
Contributor

l0rinc commented Aug 6, 2025

Rebased, tested it with:

diff --git a/CMakeLists.txt b/CMakeLists.txt
--- a/CMakeLists.txt	(revision 27e4b8c6856194fa8db028b6f7356f83ea3d7e3a)
+++ b/CMakeLists.txt	(date 1754504896922)
@@ -186,6 +186,7 @@
 string(APPEND CMAKE_CXX_LINK_EXECUTABLE " ${APPEND_LDFLAGS}")

 set(configure_warnings)
+list(APPEND configure_warnings "Testing https://github.com/bitcoin/bitcoin/pull/33144")

 include(CheckLinkerSupportsPIE)
 check_linker_supports_pie(configure_warnings)

running:

rm -rfd build && cmake -B build 2>&1 | grep -C1 Warning

Prints before the change:

CMake Warning at CMakeLists.txt:709 (message):
  Testing https://github.com/bitcoin/bitcoin/pull/33144

After the change:

CMake Warning at CMakeLists.txt:709 (message):
  Testing https://github.com/bitcoin/bitcoin/pull/33144
--

CMake Warning (dev) at CMakeLists.txt:712 (message):
  Warnings have been encountered!
This warning is for project developers.  Use -Wno-dev to suppress it.

ACK fa6497b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: Cmake warnings should be errors
3 participants