Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Open
jvdp1 wants to merge 4 commits into fortran-lang:master
base: master
Choose a base branch
Loading
from jvdp1:fix_cmake

Conversation

@jvdp1
Copy link
Member

@jvdp1 jvdp1 commented Oct 5, 2025

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?

@jvdp1 jvdp1 requested a review from perazz October 5, 2025 13:48
Copy link
Member Author

jvdp1 commented Oct 5, 2025

Copy link
Contributor

eduardz1 commented Oct 5, 2025

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()
...

Copy link
Member Author

jvdp1 commented Oct 5, 2025

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?

perazz reacted with thumbs up emoji

Copy link
Contributor

eduardz1 commented Oct 5, 2025

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 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@perazz perazz perazz approved these changes

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /