Skip to content

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

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Aug 18, 2024

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 #1251 and #1291.

The last commit aaditionally addresses that comment.

@real-or-random
Copy link
Contributor

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

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants