-
Couldn't load subscription status.
- Fork 196
CMake: isssue with an "add_library" added in #1033 #1036
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
Conversation
Following the changes in #1033, the following Cmake directives are broken:
... if("${method}" STREQUAL "fetch") message(STATUS "Retrieving ${_lib} from ${_url}") include(FetchContent) FetchContent_Declare( "${_lib}" GIT_REPOSITORY "${_url}" GIT_TAG "HEAD" ) FetchContent_MakeAvailable("${_lib}") add_library("${_lib}::${_lib}" INTERFACE IMPORTED) target_link_libraries("${_lib}::${_lib}" INTERFACE "${_lib}") # We need the module directory in the subproject before we finish the configure stage FetchContent_GetProperties("${_lib}" SOURCE_DIR "${_pkg}_SOURCE_DIR") FetchContent_GetProperties("${_lib}" BINARY_DIR "${_pkg}_BINARY_DIR") if(NOT EXISTS "${${_pkg}_BINARY_DIR}/mod_files") make_directory("${${_pkg}_BINARY_DIR}/mod_files") endif() break() endif() ...Commenting out the directive
add_library(${PROJECT_NAME}::${target_name} ALIAS ${target_name})introduced in #1033 solved my issue. However, I am not an experxt in CMake. So, is this required?
I won't be able to test it in the next two weeks but you should be able to remove the two lines "add_library" and "target_link_libraries", the error is due to the target existing already. I see that the code you wrote is probably part of some function or some automated installation. In case you still want to keep these two lines for some repositories then change it to something like
... if (NOT TARGET "${_lib}::{_lib}") add_library("${_lib}::${_lib}" INTERFACE IMPORTED) target_link_libraries("${_lib}::${_lib}" INTERFACE "${_lib}") endif() ...
I won't be able to test it in the next two weeks
Thank you for your quick answer. It is not urgent on my side
but you should be able to remove the two lines "add_library" and "target_link_libraries", the error is due to the target existing already.
Only the second add_library was added in #1033, as far as I can see. So, I wonder why it was added there (there might be a reason that I am not aware of).
I see that the code you wrote is probably part of some function or some automated installation.
Indeed, it is based on the stdlib Findtest-drive.cmake
In case you still want to keep these two lines for some repositories then change it to something like
... if (NOT TARGET "${_lib}::{_lib}") add_library("${_lib}::${_lib}" INTERFACE IMPORTED) target_link_libraries("${_lib}::${_lib}" INTERFACE "${_lib}") endif() ...
It is also a possibility. But I will have to adapt many projects :( Of course, it is possible. But in that case, should the stdlib README be adapted?
I won't be able to test it in the next two weeks
Thank you for your quick answer. It is not urgent on my side
but you should be able to remove the two lines "add_library" and "target_link_libraries", the error is due to the target existing already.
Only the second
add_librarywas added in #1033, as far as I can see. So, I wonder why it was added there (there might be a reason that I am not aware of).I see that the code you wrote is probably part of some function or some automated installation.
Indeed, it is based on the
stdlibFindtest-drive.cmakeIn case you still want to keep these two lines for some repositories then change it to something like
... if (NOT TARGET "${_lib}::{_lib}") add_library("${_lib}::${_lib}" INTERFACE IMPORTED) target_link_libraries("${_lib}::${_lib}" INTERFACE "${_lib}") endif() ...It is also a possibility. But I will have to adapt many projects :( Of course, it is possible. But in that case, should the
stdlibREADMEbe adapted?
I didn't notice how it was done in in Findtest-drive.cmake, sorry! I added the alias because it makes it more seamless to integrate with FetchContent, removing the need to create the target manually, but you're right that the Findtest-drive.cmake should have been updated as well. Can somebody do that? Otherwise I can open a PR to fix it in about 2/3 weeks.
Maybe I wasn't clear enough before but if you use this code only for the stdlib you can simply remove these lines from your code
add_library("${_lib}::${_lib}" INTERFACE IMPORTED) target_link_libraries("${_lib}::${_lib}" INTERFACE "${_lib}")
Also it's not strictly related but I suggest using a commit hash for your GIT_TAG so you don't risk accidentally breaking the build
Add if(NOT TARGET) guards around add_library calls to prevent conflicts when the target already exists (e.g., when using stdlib with FetchContent after PR fortran-lang#1033 which adds ALIAS targets automatically). This allows users to use the Find module pattern with libraries that already provide namespaced ALIAS targets.
Guard add_library calls in Findtest-drive.cmake
Co-authored-by: Federico Perini <federico.perini@gmail.com>
Following the changes in #1033, the following Cmake directives are broken:
Commenting out the directive
add_library(${PROJECT_NAME}::${target_name} ALIAS ${target_name})introduced in #1033 solved my issue. However, I am not an experxt in CMake. So, is this required?