CMake: isssue with an "add_library" added in #1033

[jvdp1]

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]

@perazz @eduardz1

[eduardz1]

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

[jvdp1]

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?

[eduardz1]

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