Skip to content
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

Bugfixes #494

Merged
merged 2 commits into from
May 27, 2016
Merged

Bugfixes #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
68ccf97
Fix coverage report generation after 5dd04a3 if LCOV_EXCLUDE is empty
tribal-tec May 26, 2016
b251766
Fix include paths in project header file for generated files within t…
tribal-tec May 25, 2016
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
4 changes: 4 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# git master

* [494](https://github.com/Eyescale/CMake/pull/494):
Fix include paths in project header file for generated files within the binary dir
* [493](https://github.com/Eyescale/CMake/pull/493):
Do not mess with LCOV_EXCLUDE which is set by outside users
* [484](https://github.com/Eyescale/CMake/pull/484):
Fix coverage report generation for the top-level project.
* [478](https://github.com/Eyescale/CMake/pull/478):
Expand Down
2 changes: 1 addition & 1 deletion CommonCoverage.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ function(add_coverage_targets TEST_TARGET)
# 'tests/*' excluded otherwise unit test source file coverage is produced
add_custom_target(${PROJECT_NAME}-lcov-remove
COMMAND ${LCOV} -q --remove lcov.info '*.l*' '*.y*' 'tests/*' 'CMake/test/*'
'*/install/*' 'moc_*' 'qrc_*' ${GENERATED_FILES} '${LCOV_EXCLUDE}'
'*/install/*' 'moc_*' 'qrc_*' ${GENERATED_FILES} ${LCOV_EXCLUDE}
Copy link

Choose a reason for hiding this comment

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

No, the quotes are needed at least for Brayns, we added them here: https://github.com/Eyescale/CMake/pull/469/files

Copy link
Member Author

Choose a reason for hiding this comment

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

With the quotes and LCOV_EXCLUDE is emtpy, the coverage report is empty as well, hence the modification. I think after the previous PR wrt LCOV_EXCLUDE not set anymore with test files this variable is most of the time empty.

Copy link

Choose a reason for hiding this comment

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

I got that, but if it is not empty then the quotes are required. Check with Brayns, but the most correct solution is to add a tmp variable which is either empty if LCOV_EXCLUDE is empty or '${LCOV_EXCLUDE}' quoted if it's not.

Copy link
Member Author

Choose a reason for hiding this comment

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

There is no target 'BRayns-coverage', so I don't know what you are referring to.

Copy link

Choose a reason for hiding this comment

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

I am not sure, maybe @favreau remembers the details? What I do know is that this quotes were added for a good reason, and it was observed trying to configure Brayns, but I can't reproduce the problem now. Only ZeroBuf seems to use this variable, but I think we had found more than one occurrences at the time.

--output-file lcov2.info
COMMENT "Cleaning up code coverage counters"
WORKING_DIRECTORY ${PROJECT_BINARY_DIR}
Expand Down
8 changes: 7 additions & 1 deletion CommonLibrary.cmake
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,13 @@ macro(generate_library_header NAME)
"#define ${NAME}_H\n")
foreach(PUBLIC_HEADER ${PUBLIC_HEADERS})
if(IS_ABSOLUTE ${PUBLIC_HEADER})
get_filename_component(PUBLIC_HEADER ${PUBLIC_HEADER} NAME)
set(__base "${PROJECT_BINARY_DIR}/include/${INCLUDE_NAME}/")
string(REGEX MATCH ${__base} __has_base ${PUBLIC_HEADER})
if(__has_base)
string(REPLACE ${__base} "" PUBLIC_HEADER ${PUBLIC_HEADER})
else()
get_filename_component(PUBLIC_HEADER ${PUBLIC_HEADER} NAME)
endif()
endif()
if(NOT PUBLIC_HEADER MATCHES "defines.+\\.h" AND
(PUBLIC_HEADER MATCHES ".*\\.h$" OR PUBLIC_HEADER MATCHES ".*\\.hpp$"))
Expand Down