secp256k1 icon indicating copy to clipboard operation
secp256k1 copied to clipboard

cmake: Rework `tests` target for Coverage configuartion and multi-config generators

Open hebasto opened this issue 1 year ago • 1 comments

The "experimental" status of the CMake-based build system can be reconsidered once it is adopted in the Bitcoin Core project. Fixing all known bugs is essential before this reconsideration.

Currently, I'm aware of a single issue: when using a multi-config generator, such as "Ninja Multi-Config", the build system still builds the tests binary for the "Coverage" configuration, which is meaningless:

$ cmake -B build -G "Ninja Multi-Config"
$ cmake --build build --config Coverage
[18/19] Building C object src/CMakeFiles/tests.dir/Coverage/tests.c.o
/home/hebasto/git/secp256k1/secp256k1/src/tests.c:18:13: note: ‘#pragma message: Defining VERIFY for tests being built for coverage analysis support is meaningless.’
   18 |     #pragma message("Defining VERIFY for tests being built for coverage analysis support is meaningless.")
      |             ^~~~~~~
[19/19] Linking C executable src/Coverage/tests

This PR fixes the issue.

It is an alternative to https://github.com/bitcoin-core/secp256k1/pull/1251 and https://github.com/bitcoin-core/secp256k1/pull/1291.

The last commit aaditionally addresses that comment.

hebasto avatar Aug 18 '24 15:08 hebasto

@hebasto Can you summarize what this PR does and why is it preferable to #1291? The approach in #1291 seemed natural to me.

real-or-random avatar Aug 19 '24 14:08 real-or-random

Rebased and reworked.

The following is an example workflow on Ubuntu 24.04:

cmake -B build -DCMAKE_GENERATOR="Ninja Multi-Config"
cmake --build build --config Coverage  # Ninja uses all available cores by default
ctest --test-dir build -j $(nproc) -C Coverage
mkdir -p build/coverage
gcovr --merge-mode-functions=separate --exclude 'src/bench*' --html --html-details -o build/coverage/coverage.html
firefox build/coverage/coverage.html

hebasto avatar Aug 10 '25 17:08 hebasto

@hebasto Can you summarize what this PR does and why is it preferable to #1291? The approach in #1291 seemed natural to me.

Answered in https://github.com/bitcoin-core/secp256k1/issues/1716#issuecomment-3172915276.

hebasto avatar Aug 10 '25 21:08 hebasto

Closing as we came to the conclusion that supporting the combination of coverage and multi-config generators is not worth the effort, see https://github.com/bitcoin-core/secp256k1/issues/1716#issuecomment-3178155434.

real-or-random avatar Sep 02 '25 21:09 real-or-random