RFC: depends: add release type to CMake builds #29962

1. 
   theuni commented at 5:08 pm on April 25, 2024: member

   RFC because I’m not sure if this is the right thing to do in combination with our CFLAGS/CXXFLAGS/etc env overrides.

   I believe it was suggested by laanwj at some point.

2. depends: add release type to CMake builds 9f3dc55fac
3. 
   DrahtBot commented at 5:08 pm on April 25, 2024: contributor

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

   Code Coverage

   For detailed information about the code coverage, see the test coverage report.

   Reviews

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #29960 (depends: pass verbose through to cmake based makefiles by m3dwards)

   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.

4. 
   theuni commented at 5:12 pm on April 25, 2024: member

   Ping @hebasto
5. 
   hebasto commented at 5:21 pm on April 25, 2024: member

   The defaults for the “Makefile” generator are:

   • “RelWithDebInfo” : -O2 -g -DNDEBUG
   • “Debug’: -g

   That differs from our flags, which are -O2 and -O1 -g for Linux respectively.

   And CMake can change its defaults at any time (

   However, it would be nice to be explicit about the used build type, for example, -DCMAKE_BUILD_TYPE=None.

6. 
   theuni commented at 5:39 pm on April 25, 2024: member

   Right, but a few more things to consider:

   An upstream build could be following the docs and doing something like:

   0# Works correctly for both single and multi-config generators
   1target_compile_definitions(exe1 PRIVATE
   2  $<$<CONFIG:Debug>:DEBUG_BUILD>
   3)
   

   In this case, we wouldn’t currently pick up the extra debug opts.

   The other thing to consider that is that we could be using CMAKE_<LANG>_FLAGS_<CONFIG> for these options rather than CFLAGS/CXXFLAGS, potentially overriding the defaults you mentioned above but still being explicit about the build type.

   Note that I don’t necessarily believe these arguments and don’t feel strongly either way, I just wanted to raise the discussion.

7. 
   laanwj commented at 5:58 pm on April 25, 2024: member

   i think -O2 -g -DNDEBUG (RelWIthDebInfo) for the release makes sense, it means that the distributed binary will be optimized, with no extra runtime checks, but still get (as far as possible) descriptive debug information such as line numbers and symbols in case it’s necessary for troubleshooting, getting a traceback, figuring out where some crash address comes from, and so on.

   Edit: Oh, to not forget, debug metadata is also useful to figure out the details of differences between guix-built binaries, like where do they exactly come from. This is what i tend to most often use it for.

8. laanwj added the label Build system on Apr 25, 2024
9. DrahtBot added the label Needs rebase on May 6, 2024
10. 
    DrahtBot commented at 3:06 am on May 6, 2024: contributor

    🐙 This pull request conflicts with the target branch and needs rebase.

11. 
    fanquake commented at 3:33 pm on May 31, 2024: member

    This “seems” like the right thing to do, but I’m also not sure. Could you rebase here, to incorporate other recent CMake changes, and so it’s easier to test?
12. 
    fanquake commented at 2:04 pm on July 25, 2024: member

    We seem to have gravitated towards using DCMAKE_BUILD_TYPE=None + our flags for CMake packages in depends, (except for libevent), so maybe that is the best thing to do here, for now?
13. 
    hebasto commented at 2:11 pm on July 25, 2024: member

    We seem to have gravitated towards using DCMAKE_BUILD_TYPE=None + our flags for CMake packages in depends, (except for libevent), so maybe that is the best thing to do here, for now?

    I think this approach is quite straightforward and gives us full control on optimization/debug flags.

14. theuni closed this on Jul 26, 2024

    ---

15. 
    fanquake commented at 12:56 pm on July 26, 2024: member

    To be clear, I was not saying to not make this change, but we could set the None build type globally in both cases.

Contributors
theuni DrahtBot hebasto laanwj fanquake

Labels
Build system Needs rebase