Skip to content

split lcov and genhtml cmake opts for the Coverage profile #224

New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Draft
wants to merge 9 commits into
base: 29.x-knots
Choose a base branch
from
+20 −1
Draft

split lcov and genhtml cmake opts for the Coverage profile #224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7aa60c6
split rcov and genhtml cmake opts for the Coverage profile
tevio Oct 16, 2025
39f21c9
remove unnessecary duplicated set commands
tevio Oct 17, 2025
c115fc1
document the option
tevio Oct 19, 2025
e2d638f
remove duplicated lines and inline genhtml seperate_arguments
tevio Oct 19, 2025
b71b968
remove ENV overrides, shorten opt name, only allow setting via cmake …
tevio Oct 19, 2025
2c2d23e
make HTML_OPTS an override to LCOV_OPTS, for genhtml
tevio Oct 21, 2025
e3e917f
adapt commentary and make notes more concise
tevio Oct 24, 2025
97a898f
Default genhtml opts to LCOV_OPTS; respect explicitly empty HTML_OPTS
tevio Oct 24, 2025
b581b44
simplify logic for genhtml opt passing options, remove unnecessary guard
tevio Nov 2, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion cmake/script/CoverageInclude.cmake.in
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,15 @@ separate_arguments(LCOV_OPTS)
set(LCOV_COMMAND ${LCOV_EXECUTABLE} --gcov-tool ${CMAKE_CURRENT_LIST_DIR}/cov_tool_wrapper.sh ${LCOV_OPTS})

find_program(GENHTML_EXECUTABLE genhtml REQUIRED)
set(GENHTML_COMMAND ${GENHTML_EXECUTABLE} --show-details ${LCOV_OPTS})

# HTML_OPTS is optionally passed via -D flag.
# Default: inherit LCOV_OPTS. If HTML_OPTS is provided (even if empty), use it instead.
set(GENHTML_OPTS ${LCOV_OPTS})
Copy link

@kwsantiago kwsantiago Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we can't be consistent with the naming, either GENHTML_OPTS or HTML_OPTS

if(DEFINED HTML_OPTS)
separate_arguments(HTML_OPTS)
Copy link

@kwsantiago kwsantiago Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens if LCOV_OPTS is not defined? Does cmake create an empty variable or error?

set(GENHTML_OPTS ${HTML_OPTS})
endif()
set(GENHTML_COMMAND ${GENHTML_EXECUTABLE} --show-details ${GENHTML_OPTS})

find_program(GREP_EXECUTABLE grep REQUIRED)
find_program(AWK_EXECUTABLE awk REQUIRED)
Expand Down
11 changes: 11 additions & 0 deletions doc/developer-notes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -508,6 +508,17 @@ enabled by setting `LCOV_OPTS="--rc branch_coverage=1"`:
cmake -DLCOV_OPTS="--rc branch_coverage=1" -P build/Coverage.cmake
```

HTML_OPTS can override the genhtml options (which default to LCOV_OPTS). If HTML_OPTS is omitted, LCOV_OPTS are implicitly passed to genhtml. If HTML_OPTS is provided but empty (e.g. -DHTML_OPTS=""), no options are passed to genhtml and LCOV_OPTS are not inherited.
Copy link

@kwsantiago kwsantiago Oct 27, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May want to add an example showing combined use of both LCOV_OPTS and HTML_OPTS to clarify precedence.

Copy link

@kwsantiago kwsantiago Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if LCOV_OPTS is also not set? Does it default to empty?


Examples:
```
# Override genhtml options explicitly
cmake -DHTML_OPTS="--title 'Knots Coverage'" -P build/Coverage.cmake

# Provide an empty override: pass nothing to genhtml (do not inherit LCOV_OPTS)
cmake -DHTML_OPTS="" -P build/Coverage.cmake
```

To enable test parallelism:
```
cmake -DJOBS=$(nproc) -P build/Coverage.cmake
Expand Down