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

use cmake current source dir to get correct path #35

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

Conversation

callalilychen
Copy link

Using the repo as third party library using FetchContent failed:

 FetchContent_Declare(
       cmake-codecov
       GIT_REPOSITORY https://github.com/RWTH-HPC/CMake-codecov.git
)
FetchContent_MakeAvailable(cmake-codecov) 

with the following error message:

CMake Error at build/_deps/cmake-codecov-src/src/libfoo/CMakeLists.txt:13 (add_coverage):
  Unknown CMake command "add_coverage".

This PR fix the issue by using the correct cmake variable in CMakeLists.txt

@alehaa
Copy link
Contributor

alehaa commented Nov 7, 2021

Using FetchContent_Declare() is not the intended way to include this project in CMake. We should check compatibility first, so it doesn't break anything else. If we can ensure it works, a new section should be added in the README.

My concern is, including this module via FetchContent_Declare(), all test targets from /src will be included into the master project. This shouldn't happen, as the root CMakeLists.txt file was intended for testing only.

@callalilychen
Copy link
Author

callalilychen commented Nov 8, 2021

FetchContent_Declare() does not call add_subdirectory(), but FetchContent_MakeAvailable(cmake-codecov) will do it.
FetchContent_Declare() only clone the repository. I also released that FetchContent_MakeAvaiable() is not the best way to use the library like you said.
But I still think CMAKE_SOURCE_DIR is not the best variable to use, because CMAKE_SOURCE_DIR means the root source directory of the whole cmake project, which is not the current directory. To reference the current directory is better to use PROJECT_SOURCE_DIR, or CMAKE_CURRENT_DIR or CMAKE_CURRENT_LIST_DIR, which are more explicit.

I use the library now like this:
https://github.com/rainerschoe/ZeroCopyMessagePack/blob/master/CMakeLists.txt#L42-L51

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