cmake: Rework flags summary #1558

pull hebasto wants to merge 2 commits into bitcoin-core:master from hebasto:240628-cmake-summary changing 3 files +92 −28
  1. hebasto commented at 9:35 am on June 28, 2024: member

    This was requested in https://github.com/hebasto/bitcoin/issues/221. The implementation follows the same approach as in https://github.com/hebasto/bitcoin/pull/93.

    Here are a few excerpts from the summaries:

    • Linux:
     0Cross compiling ....................... FALSE
     1Valgrind .............................. ON
     2Preprocessor defined macros ........... ENABLE_MODULE_ELLSWIFT=1 ENABLE_MODULE_SCHNORRSIG=1 ENABLE_MODULE_EXTRAKEYS=1 ENABLE_MODULE_ECDH=1 ECMULT_WINDOW_SIZE=15 COMB_BLOCKS=11 COMB_TEETH=6 USE_ASM_X86_64=1 VALGRIND
     3C compiler ............................ GNU 13.2.0, /usr/bin/cc
     4CMAKE_BUILD_TYPE ...................... RelWithDebInfo
     5C compiler flags ...................... -O2 -g -std=c90 -fPIC -fvisibility=hidden -pedantic -Wall -Wcast-align -Wcast-align=strict -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wshadow -Wstrict-prototypes -Wundef
     6Linker flags .......................... -fPIC -O2 -g -Wl,-soname,libsecp256k1.so.2
     7
     8NOTE: The summary above may not exactly match the final applied build flags
     9      if any additional CMAKE_* or environment variables have been modified.
    10      To see the exact flags applied, build with the --verbose option.
    
    • Windows:
     0Cross compiling ....................... FALSE
     1Valgrind .............................. OFF
     2Preprocessor defined macros ........... ENABLE_MODULE_ELLSWIFT=1 ENABLE_MODULE_SCHNORRSIG=1 ENABLE_MODULE_EXTRAKEYS=1 ENABLE_MODULE_ECDH=1 ECMULT_WINDOW_SIZE=15 COMB_BLOCKS=11 COMB_TEETH=6 _CRT_SECURE_NO_WARNINGS
     3C compiler ............................ MSVC 19.40.33811.0, C:/Program Files/Microsoft Visual Studio/2022/Community/VC/Tools/MSVC/14.40.33807/bin/Hostx64/x64/cl.exe
     4Available build configurations ........ RelWithDebInfo, Release, Debug, MinSizeRel, Coverage
     5Default build configuration ........... Debug
     6
     7'RelWithDebInfo' build configuration:
     8  C compiler flags .................... /DWIN32 /D_WINDOWS /Zi /O2 /Ob1 /W3 /wd4146 /wd4244 /wd4267
     9  Linker flags ........................ /machine:x64 /debug /INCREMENTAL
    10
    11'Release' build configuration:
    12  C compiler flags .................... /DWIN32 /D_WINDOWS /O2 /Ob2 /W3 /wd4146 /wd4244 /wd4267
    13  Linker flags ........................ /machine:x64 /INCREMENTAL:NO
    14
    15'Debug' build configuration:
    16  C compiler flags .................... /DWIN32 /D_WINDOWS /Zi /Ob0 /Od /RTC1 /W3 /wd4146 /wd4244 /wd4267
    17  Linker flags ........................ /machine:x64 /debug /INCREMENTAL
    18
    19'MinSizeRel' build configuration:
    20  C compiler flags .................... /DWIN32 /D_WINDOWS /O1 /Ob1 /W3 /wd4146 /wd4244 /wd4267
    21  Linker flags ........................ /machine:x64 /INCREMENTAL:NO
    22
    23'Coverage' build configuration:
    24  C compiler flags .................... /DWIN32 /D_WINDOWS /Zi /O2 /Ob1  -O0 -DCOVERAGE=1 --coverage /W3 /wd4146 /wd4244 /wd4267
    25  Linker flags ........................ /machine:x64 /debug /INCREMENTAL --coverage
    26
    27NOTE: The summary above may not exactly match the final applied build flags
    28      if any additional CMAKE_* or environment variables have been modified.
    29      To see the exact flags applied, build with the --verbose option.
    
  2. real-or-random commented at 11:28 am on June 28, 2024: contributor

    Concept ACK

    The note is a bit verbose, but I don’t have a convincing suggestion to shorten it.

  3. real-or-random approved
  4. real-or-random commented at 11:37 am on June 28, 2024: contributor

    Sorry, when I wrote the previous comment, I assumed this PR is in the hebasto/bitcoin repo for cmake staging… :man_facepalming: :

    utACK aa4c5f3553af1259c53fb4bbf450a3560694b7c6 this matches the implementation in https://github.com/hebasto/bitcoin/pull/93 and has been revieed there

  5. real-or-random added the label build on Jun 28, 2024
  6. fanquake commented at 11:38 am on July 1, 2024: member
    Looks like this is at least missing link options like -compatibility_version 5.0.0 -current_version 5.1.0.
  7. fanquake commented at 11:42 am on July 1, 2024: member
    Also flags like -fPIC are shown in the C compiler flags, but not shown in the linker flags?
  8. real-or-random commented at 12:36 pm on July 1, 2024: contributor

    Good observations. These should be fixed if the rule is as in this comment:

    0# Print tools' flags on best-effort. Include the abstracted
    1# CMake flags that we touch ourselves.
    
  9. cmake, macOS: Provide all components of {compatibility,current}_version 5d31c3208e
  10. hebasto force-pushed on Jul 1, 2024
  11. hebasto commented at 2:08 pm on July 1, 2024: member
    Both @fanquake’s comments have been addressed.
  12. fanquake commented at 2:19 pm on July 1, 2024: member

    Looks like this is at least missing link options like -compatibility_version 5.0.0 -current_version 5.1.0.

    The same is still missing on Linux?

  13. in cmake/FlagsSummary.cmake:57 in 765681b8a9 outdated
    48+      endif()
    49+    endif()
    50+    indent_message("Linker flags .........................." "${linker_flags}" ${indent_num})
    51+  else()
    52+    string(STRIP "${CMAKE_STATIC_LINKER_FLAGS} ${CMAKE_STATIC_LINKER_FLAGS_${config_uppercase}}" archiver_options)
    53+    indent_message("Archiver options ......................" "${archiver_options}" ${indent_num})
    


    fanquake commented at 2:30 pm on July 1, 2024:

    This doesn’t seem to list the options passed to the archiver? i.e:

     0cmake -B build -DCMAKE_C_COMPILER=clang -DBUILD_SHARED_LIBS=OFF
     1...
     2Build artifacts:
     3  library type ........................ Static
     4...
     5C compiler ............................ Clang 17.0.6, /usr/bin/clang
     6CMAKE_BUILD_TYPE ...................... RelWithDebInfo
     7C compiler flags ...................... -O2 -g -std=c90 -fvisibility=hidden -pedantic -Wall -Wcast-align -Wconditional-uninitialized -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wreserved-identifier -Wshadow -Wstrict-prototypes -Wundef
     8Archiver options ...................... 
     9....
    10/usr/bin/llvm-ar qc libsecp256k1.a CMakeFiles/secp256k1.dir/secp256k1.c.o CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.o CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.o
    11/usr/bin/llvm-ranlib libsecp256k1.a
    

    Shouldn’t this be listing qc as the archiver options? Otherwise, what is the output expected to be there?


    hebasto commented at 2:43 pm on July 1, 2024:

    Shouldn’t this be listing qc as the archiver options?

    Isn’t qc rather a operation mode, not options?

    Otherwise, what is the output expected to be there?

    Archiver options are expected. For instance,

    0$ cmake -B build -DBUILD_SHARED_LIBS=OFF -DCMAKE_STATIC_LINKER_FLAGS=-o
    1...
    2Archiver options ...................... -o
    

    fanquake commented at 2:55 pm on July 1, 2024:

    Isn’t qc rather a operation mode, not options?

    That seems like an unhelpful distinction to try and draw? In any case, no, they are options, see “Options”: https://www.man7.org/linux/man-pages/man1/ar.1.html.


    hebasto commented at 8:22 pm on July 1, 2024:
    Thanks! Reworked.
  14. hebasto force-pushed on Jul 1, 2024
  15. hebasto force-pushed on Jul 1, 2024
  16. hebasto commented at 8:21 pm on July 1, 2024: member
    All recent @fanquake’s comments have been addressed.
  17. cmake: Rework flags summary 8ff8b25e29
  18. hebasto force-pushed on Jul 2, 2024
  19. hebasto commented at 7:48 am on July 2, 2024: member
    Fixed quoting of (potentially empty) string arguments.
  20. hebasto marked this as a draft on Sep 18, 2024
  21. purpleKarrot commented at 4:38 pm on September 29, 2025: contributor

    I’m not a fan of these flag summaries in the configure output. My main concerns are:

    • Maintenance burden:
      This PR adds 90 lines of non-config logic to a section that should remain declarative and free of additional logic.

    • Worsened user experience:
      In ccmake, the summary appears as a warning that must be dismissed. Instead of the usual c c g workflow, users are now forced to press c e c e g.

    • Lack of trustworthiness:
      The summary is only an approximation. Users who require the exact flags will need to use a different approach anyway.

    Hence, I would vote to remove the flag summary from the configure log. If there is a need for exact flags, we should provide a proper, trustworthy solution.

  22. real-or-random commented at 10:19 am on October 1, 2025: contributor

    Hence, I would vote to remove the flag summary from the configure log. If there is a need for exact flags, we should provide a proper, trustworthy solution.

    I tend to agree. But this seems like a thing where we could just follow whatever Bitcoin Core does. Is the summary still present in Core?

  23. hebasto commented at 10:50 am on October 1, 2025: member

    Hence, I would vote to remove the flag summary from the configure log.

    I tend to agree. But this seems like a thing where we could just follow whatever Bitcoin Core does. Is the summary still present in Core?

    It is. And people are still concerned about its correctness.

    cc @fanquake


    @purpleKarrot

    If there is a need for exact flags, we should provide a proper, trustworthy solution.

    Hard to disagree. Could you share any ideas for a possible approach?

  24. real-or-random commented at 7:56 am on October 2, 2025: contributor

    @purpleKarrot

    If there is a need for exact flags, we should provide a proper, trustworthy solution.

    Hard to disagree. Could you share any ideas for a possible approach?

    I think there’s not much we can do because the exact flags depend on the artifact that is built. The exact way is already there: cmake --build -v, perhaps piped to grep <artifact>. If people are not aware of this functionality, we should add it to contributor guides or simply print a message that recommends this command instead of a summary.

  25. purpleKarrot commented at 8:38 am on October 2, 2025: contributor

    we should add it to contributor guides

    I like that approach. We should recommend people to use the Ninja generator by default (eg by setting CMAKE_GENERATOR as an environment variable in their dotfiles) and we should hint that Ninja has some Extra tools.

    It is cool that cmake can create a build directory and that it provides an abstraction of the build system with cmake --build. But this functionality is mostly useful for shell scripts. For development, invoking the build system directly from inside the build directory is easier to type and more powerful:

    0set -x CMAKE_GENERATOR 'Ninja'  # put this in your dotfiles
    1mkdir build
    2cd build
    3cmake ..                        # assuming the source directory is the parent of the current working dir
    4ninja help                      # print all build targets
    5ninja secp256k1                 # build a single target
    6ninja -t list                   # list all of ninja's extra tools
    7ninja -t commands secp256k1     # print the commands that are needed to build a target
    
  26. hebasto commented at 10:31 am on October 2, 2025: member

    we should add it to contributor guides

    I like that approach. We should recommend people to use the Ninja generator by default (eg by setting CMAKE_GENERATOR as an environment variable in their dotfiles) and we should hint that Ninja has some Extra tools.

    I set Ninja as my default CMake generator long time ago. And I like Ninja Extra tools:

    0$ ninja -C build -t commands secp256k1
    1/usr/bin/cc -DCOMB_BLOCKS=43 -DCOMB_TEETH=6 -DECMULT_WINDOW_SIZE=15 -DENABLE_MODULE_ECDH=1 -DENABLE_MODULE_ELLSWIFT=1 -DENABLE_MODULE_EXTRAKEYS=1 -DENABLE_MODULE_MUSIG=1 -DENABLE_MODULE_SCHNORRSIG=1 -DUSE_ASM_X86_64=1 -DVALGRIND  -O2 -g  -std=c90 -fPIC -Wall -pedantic -Wcast-align -Wcast-align=strict -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wshadow -Wstrict-prototypes -Wundef -MD -MT src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.o -MF src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.o.d -o src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.o -c /home/hebasto/dev/secp256k1/secp256k1/src/precomputed_ecmult.c
    2/usr/bin/cc -DCOMB_BLOCKS=43 -DCOMB_TEETH=6 -DECMULT_WINDOW_SIZE=15 -DENABLE_MODULE_ECDH=1 -DENABLE_MODULE_ELLSWIFT=1 -DENABLE_MODULE_EXTRAKEYS=1 -DENABLE_MODULE_MUSIG=1 -DENABLE_MODULE_SCHNORRSIG=1 -DUSE_ASM_X86_64=1 -DVALGRIND  -O2 -g  -std=c90 -fPIC -Wall -pedantic -Wcast-align -Wcast-align=strict -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wshadow -Wstrict-prototypes -Wundef -MD -MT src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.o -MF src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.o.d -o src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.o -c /home/hebasto/dev/secp256k1/secp256k1/src/precomputed_ecmult_gen.c
    3/usr/bin/cc -DCOMB_BLOCKS=43 -DCOMB_TEETH=6 -DECMULT_WINDOW_SIZE=15 -DENABLE_MODULE_ECDH=1 -DENABLE_MODULE_ELLSWIFT=1 -DENABLE_MODULE_EXTRAKEYS=1 -DENABLE_MODULE_MUSIG=1 -DENABLE_MODULE_SCHNORRSIG=1 -DUSE_ASM_X86_64=1 -DVALGRIND -Dsecp256k1_EXPORTS  -O2 -g  -std=c90 -fPIC -Wall -pedantic -Wcast-align -Wcast-align=strict -Wextra -Wnested-externs -Wno-long-long -Wno-overlength-strings -Wno-unused-function -Wshadow -Wstrict-prototypes -Wundef -MD -MT src/CMakeFiles/secp256k1.dir/secp256k1.c.o -MF src/CMakeFiles/secp256k1.dir/secp256k1.c.o.d -o src/CMakeFiles/secp256k1.dir/secp256k1.c.o -c /home/hebasto/dev/secp256k1/secp256k1/src/secp256k1.c
    4: && /usr/bin/cc -fPIC -O2 -g  -shared -Wl,--dependency-file=src/CMakeFiles/secp256k1.dir/link.d -Wl,-soname,libsecp256k1.so.6 -o lib/libsecp256k1.so.6.0.1 src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult.c.o src/CMakeFiles/secp256k1_precomputed.dir/precomputed_ecmult_gen.c.o src/CMakeFiles/secp256k1.dir/secp256k1.c.o   && :
    5/home/hebasto/Downloads/cmake-4.1.1-linux-x86_64/bin/cmake -E cmake_symlink_library lib/libsecp256k1.so.6.0.1  lib/libsecp256k1.so.6 lib/libsecp256k1.so && :
    

    The only thing left is to convince other developers and keep the CI and OSS-Fuzz logs readable.


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin-core/secp256k1. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-10-13 22:15 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me