cmake: Check -Wno-* compiler options for leveldb target #31366

1. 
   hebasto commented at 3:12 pm on November 25, 2024: member

   Otherwise, https://cirrus-ci.com/task/4830737755537408:

   0At global scope:
   1cc1plus: note: unrecognized command-line option ‘-Wno-conditional-uninitialized’ may have been intended to silence earlier diagnostics
   

2. hebasto added the label Build system on Nov 25, 2024
3. 
   DrahtBot commented at 3:12 pm on November 25, 2024: contributor

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

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	TheCharlatan
   Stale ACK	theuni

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

4. theuni approved
5. 
   theuni commented at 6:08 pm on November 25, 2024: member

   utACK 0f73d9dd49b371c28ba9e74ef3519ef8f6e80ebc

   A more helpful commit message would’ve been useful though: “Check for -Wfoo rather than -Wno-foo because the latter may not cause the test to fail”.

6. cmake: Check `-Wno-*` compiler options for `leveldb` target

   Check for -Wfoo rather than -Wno-foo because the latter may not cause
   the test to fail.

   9e4a4b4832
7. hebasto force-pushed on Nov 25, 2024
8. 
   hebasto commented at 8:25 pm on November 25, 2024: member

   A more helpful commit message would’ve been useful though: “Check for -Wfoo rather than -Wno-foo because the latter may not cause the test to fail”.

   Thanks! The commit message has been amended per your feedback.

9. DrahtBot added the label CI failed on Nov 25, 2024
10. DrahtBot removed the label CI failed on Nov 26, 2024
11. TheCharlatan approved
12. 
    TheCharlatan commented at 9:51 am on November 26, 2024: contributor

    ACK 9e4a4b4832219d9d11da441779ab8a3b1304bd8b
13. DrahtBot requested review from theuni on Nov 26, 2024
14. 
    fanquake commented at 12:20 pm on November 27, 2024: member

    In Autotools we filtered out the unwanted flags, ifor CMake, the -Wno-* variants were added (but in a way that didn’t quite work), and this PR fixes their usage by adding checks for the flags. However that means we duplicate checks that we do for the same flags earlier in the buildsystem, and we end up with something like:

    clang++ ... -Wconditional-uninitialized -Wsuggest-override ... -Wno-conditional-uninitialized -Wno-suggest-override

    when building leveldb. Wondering if we could have less duplication, and a more sensical compile command, by reverting to what we did in Autotools, and just filter out the unwanted flags?

15. 
    hebasto commented at 1:08 pm on November 27, 2024: member

    However that means we duplicate checks that we do for the same flags earlier in the buildsystem…

    Checks are cached. You can observe only a single instance of:

    0-- Performing Test CXX_SUPPORTS__WSUGGEST_OVERRIDE
    1-- Performing Test CXX_SUPPORTS__WSUGGEST_OVERRIDE - Success
    

    in the configuration output.

16. 
    fanquake commented at 1:10 pm on November 27, 2024: member

    The code is duplicated.
17. 
    maflcko commented at 1:20 pm on November 27, 2024: member

    Personally I think it is fine append -Wno... in 5 lines of code. I’d presume filtering would require even more code?
18. 
    fanquake commented at 1:22 pm on November 27, 2024: member

    I’d presume filtering would require even more code?

    I don’t see why it’d be more than 1 line.

Contributors
hebasto DrahtBot theuni TheCharlatan fanquake maflcko

Review Requested
theuni

Labels
Build system