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.

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

Ensure no left over state is left in the required flags variable #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Twon
Copy link
Contributor

@Twon Twon commented Mar 28, 2021

I'm raising this PR to fix a bug when using the code coverage modules with a build that includes the HDF5 library. HDF5 uses the CMAKE_REQUIRED_FLAGS with try_compile, however, because it appends to CMAKE_REQUIRED_FLAGS its is picking up the leftover code coverage flags from the coverage module. All of the calls to try_compile now fail because gcov is not added as a linker flag as well:

-- try_compile: {COMPILE_DEFINITIONS: -DSTDC_HEADERS -O0 -g -fprofile-arcs -ftest-coverage -DHAVE_SYS_TIME_H -DHAVE_UNISTD_H -DHAVE_SYS_TYPES_H -DHAVE_SYS_SOCKET_H, LINK_LIBRARIES: m;dl, OUTPUT_VARIABLE: Change Dir: /home/toro/build/CMakeFiles/CMakeTmp

Run Build Command(s):/usr/local/bin/ninja cmTC_df3b4 && [1/2] Building C object CMakeFiles/cmTC_df3b4.dir/HDFTests.c.o
[2/2] Linking C executable cmTC_df3b4
FAILED: cmTC_df3b4 
: && /opt/ccache-3.7.9/bin/cc   -static-libstdc++ -static-libgcc -pthread -lrt -ldl CMakeFiles/cmTC_df3b4.dir/HDFTests.c.o  -o cmTC_df3b4  -lm  -ldl && :
/home/antony/packages/hdf5/original/config/cmake_ext_mod/HDFTests.c:115: error: undefined reference to '__gcov_init'
/home/antony/packages/hdf5/original/config/cmake_ext_mod/HDFTests.c:115: error: undefined reference to '__gcov_exit'
CMakeFiles/cmTC_df3b4.dir/HDFTests.c.o(.data+0x60): error: undefined reference to '__gcov_merge_add'
collect2: error: ld returned 1 exit status
ninja: build stopped: subcommand failed.

While is this arguably an error with the way HDF5 uses CMAKE_REQUIRED_FLAGS I think it makes sense to fix this here, because there may be other libraries out in the wild that makes similar assumptions about the usage of CMAKE_REQUIRED_FLAGS. Additionally, as this is the location the values originate from, and they are not longer needed after the check this seems the correct place to address the issue by resetting the values to their default state.

@alehaa
Copy link
Contributor

alehaa commented Mar 28, 2021

How did you include CMake-codecov? Afaik variables shouldn't cascade back to the callee's CMake file scope, unless the variable is cached explicitly - which is not the case.

@Twon
Copy link
Contributor Author

Twon commented Mar 28, 2021

In this case, the top-level project follows the super-build pattern and is not much more than a project that includes other projects as packages (by way of downloading other repositories from github and including them). All projects are included at a parallel level below the top-level super-build project by way of an include statement, not an add_subdirectory call which is what I believe is required to cause the cascading of variable scopes. One project in the dependencies then includes the code coverage module as such:

  get_package_path(CODECOV_PATH cmake-codecov) # Custom function returning the absolute path to the package code.
  list(APPEND CMAKE_MODULE_PATH ${CODECOV_PATH}/cmake)
  find_package(codecov)

I hope that provides enough information but let me know if that leaves unanswered questions? And thank you for replying so quickly, appreciate this awesome library 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants