build: create Depends build type for depends and use it by default for depends builds #31920

1. 
   theuni commented at 9:08 pm on February 20, 2025: member

   As discussed a good bit with fanquake and hebasto. Would be nice to have for v29, but it’s very late, so no worries if it doesn’t make it.

   Fundamentally, this creates a “Depends” build type which represents the flags that were used to build depends as opposed to colliding with the other types.

   This allows the forwarding of optional flags into the CMake build for the “Depends” build type, but the user can now optionally use an existing (Debug/RelWitDebInfo/etc.) type to ignore the optimization flags set by depends and use the ones from that type instead.

   As an example, a user may do: make -C depends cmake –toolchain depends/arm64-apple-darwin/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug

   This would compile depends with the default optimization flags for depends (-O2) but build Core with the default debug optimization flags from CMake

   Depends note: For hosts, $host_*FLAGS variables now represent the mandatory flags for compiling and will be forwarded to all Core builds via the toolchain file, regardless of the build type. For most platforms they should be empty, but are useful at least for darwin.

   When setting (for example) CFLAGS from the command line when building depends, these flags will be stored in $host_release_CFLAGS (or debug), and will only be forwarded to the Depends build type.

2. depends: hard-code necessary c(xx)flags rather than setting them per-host

   The per-host flag values now represent the mandatory flags that cannot be
   overridden by the environment. Additionally, these flags (-pipe and -std=xx)
   will no longer be passed into the CMake build.

   40a01e9d30
3. depends: set env-provided flags to per-release

   Users can now safely set flags and have them override the optional
   per-release-type flags.
   
   This also means that user-provided flags will now be forwarded to CMake's
   build-type flags variables so that they'll correctly override the existing
   values.

   f7906de7e8
4. build: create Depends build type for depends and use it by default

   Rather than modifying the existing build types and adding flags for each,
   create a new type specifically for depends.
   
   This allows the forwarding of optional flags into the CMake build for the
   "Depends" build type, but the user can now optionally use an existing
   (Debug/RelWitDebInfo/etc.) type to ignore the optimization flags set by depends
   and use the ones from that type instead.
   
   As an example, a user may do:
   make -C depends
   cmake --toolchain depends/arm64-apple-darwin/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug
   
   This would compile depends with the default optimization flags for depends
   (-O2) but build Core with the default optimization flags from CMake.
   
   Depends note:
   For hosts, $host_*FLAGS variables now represent the mandatory flags for
   compiling and will be forwarded to all Core builds in the toolchain file,
   regardless of the build type. For most platforms they should be empty, but
   are useful at least for darwin.
   
   When setting (for example) CFLAGS from the command line when building depends,
   these flags will be stored in $host_release_CFLAGS (or _debug_), and will only
   be forwarded to the Depends build type.

   dfc074cd5c
5. 
   DrahtBot commented at 9:08 pm on February 20, 2025: 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/31920.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

6. DrahtBot added the label Build system on Feb 20, 2025
7. 
   maflcko commented at 9:16 pm on February 20, 2025: member

   How does this interact with the depends DEBUG build? It looks like this is a breaking change for people (and the ci) picking the Debug build type afterwards. It would be good to clarify this and also adjust the CI scripts, if needed.
8. 
   ryanofsky commented at 9:39 pm on February 20, 2025: contributor

   Current integration between depends and cmake builds definitely does not seem ideal, but it’s hard for me to figure out if this PR makes the situation better. Maybe the PR description can be updated to state some advantages of this new approach? Or some problems with the current approach that this new approach solves?

   I listed a few aspects of the cmake/depends integration that I thought seemed broken and counterintuitive in #30813 (comment) (after “Specifically in the current system…”), and it seems like none of these things are changing after this PR, so I can imagine this PR solving a different set of problems, but I’m not sure which problems.

9. DrahtBot added the label CI failed on Feb 21, 2025
10. 
    DrahtBot commented at 1:54 am on February 21, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/37567255430

    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.

11. 
    theuni commented at 8:18 pm on February 21, 2025: member

    Current integration between depends and cmake builds definitely does not seem ideal, but it’s hard for me to figure out if this PR makes the situation better. Maybe the PR description can be updated to state some advantages of this new approach? Or some problems with the current approach that this new approach solves?

    I listed a few aspects of the cmake/depends integration that I thought seemed broken and counterintuitive in #30813 (comment) (after “Specifically in the current system…”), and it seems like none of these things are changing after this PR, so I can imagine this PR solving a different set of problems, but I’m not sure which problems.

    Yeah, I don’t think this is intended to address any of those concerns. This is intended to define a sane way of forwarding compile flags from depends. I do agree feature options aren’t ideal at the moment though.

    How does this interact with the depends DEBUG build? It looks like this is a breaking change for people (and the ci) picking the Debug build type afterwards. It would be good to clarify this and also adjust the CI scripts, if needed.

    With this, building depends with DEBUG simply means that the “Depends” type now holds the debug flags (currently -O1 -g from depends).

    Here’s a quick before/after reference:

     0make -C depends
     1cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake
     2resulting cxxflags before: -pipe -std=c++20 -O2 -O2 -g
     3resulting cxxflags after:  -O2
     4
     5make -C depends
     6cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug
     7resulting cxxflags before: -pipe -std=c++20 -O0 -ftrapv -g3
     8resulting cxxflags after:  -O0 -ftrapv -g3
     9
    10make -C depends DEBUG=1
    11cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake
    12resulting cxxflags before: -pipe -std=c++20 -O2 -g
    13resulting cxxflags after:  -O1 -g
    14
    15make -C depends DEBUG=1
    16cmake -B build --toolchain depends/x86_64-pc-linux-gnu/toolchain.cmake -DCMAKE_BUILD_TYPE=Debug
    17resulting cxxflags before: -pipe -std=c++20 -O0 -ftrapv -O1 -g3 -g3
    18resulting cxxflags after:  -O0 -ftrapv -g3
    

    Basically, the “Depends” build type now comes from the debug/release flags, and the rest are mandatory that get passed to all build types.

    I think this is a much saner system, but it did change/break a few things, so it’s not ready to go in as-is. Will convert to draft. Curious to see if there are any Concept ACKs here though. It’s not ideal, but at least it’s not arbitrary clobbering as we have now.

    The issues specifically are:

    • -g is now off for a default depends Core build because it’s off for the “release” depends builds
    • only the “Depends” build works for Linux with DEBUG=1 because -D_GLIBCXX_DEBUG is then required unconditionally (Debug/Release/RelWithDebInfo won’t work)
    • Guix would probably need to be explicitly set to RelWithDebInfo so that it’s allowed to diverge from depends
    • ci builds that use the Debug type would need to be updated as that’s probably not necessary anymore (this one is actually a good thing)

Contributors
theuni DrahtBot maflcko ryanofsky

Labels
Build system CI failed