Skip to content

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Aug 9, 2023

Fixes:

[build] CMake Error at ***/cmake/libssh2/libssh2-config.cmake:7 (add library):
[build]   add library cannot create ALIAS target "libssh2::libssh2" because target
[build]   "libssh2::libssh2_static" is imported but not globally visible.

Reported-by: bilal614 on Github
Suggested-by: balikalina on Github
Ref: curl/curl#11629 (comment)
Ref: curl/curl#11646
Fixes: #1150
Closes #1154

Fixes:
```
[build] Make Error at /usr/local/lib/cmake/libssh2/libssh2-config.cmake:7 (add library):
[build]   add library cannot create ALIAS target "libssh2::libssh2" because target
[build]   "libssh2: :libssh2 static" is imported but not globally visible.
```

Reported-by: bilal614 on Github
Suggested-by: balikalina on Github
Ref: curl/curl#11629 (comment)
Ref: curl/curl#11646
Fixes: libssh2#1150
Closes #xxxx
@vszakats
Copy link
Member Author

vszakats commented Aug 9, 2023

/cc @bilal614

@bilal614
Copy link

bilal614 commented Aug 10, 2023

I tried out the proposed changes did not work exactly. However as you suggested(also highlighted in here https://gitlab.kitware.com/cmake/cmake/-/issues/18996) using the EXPORT_NAME with the following changes fixes the issue :

--- a/cmake/libssh2-config.cmake.in
+++ b/cmake/libssh2-config.cmake.in
@@ -3,8 +3,8 @@
 
 include("${CMAKE_CURRENT_LIST_DIR}/libssh2-targets.cmake")
 
-# Alias for either shared or static library
-add_library(libssh2::libssh2 ALIAS libssh2::@LIB_SELECTED@)
+# export libssh2 alias as target name
+set_property(TARGET ${LIB_SELECTED} PROPERTY EXPORT_NAME "libssh2::libssh2")
 
 # Compatibility alias
-add_library(Libssh2::libssh2 ALIAS libssh2::@LIB_SELECTED@)
+set_property(TARGET ${LIB_SELECTED} PROPERTY EXPORT_NAME "Libssh2::libssh2")

Appreciate the time and help on the issue, thanks.

@vszakats
Copy link
Member Author

vszakats commented Aug 10, 2023

Thanks for your tests @bilal614.

Does this still need the change in lib/CMakeLists.txt or can we drop that?
Are you sure the second set_property() call is additive and doesn't override the first one? (We want to make libssh2 accessible via both libssh2:libssh2 and Libssh2:libssh2.)

@vszakats vszakats changed the title cmake: fix libssh2 lib export alias [WIP] cmake: fix libssh2 lib export alias Aug 10, 2023
@vszakats
Copy link
Member Author

vszakats commented Aug 13, 2023

Disclaimer: I'm no expert in importing/exporting targets in CMake, and never used these features. After reading the docs and creating a local dependent project to test it (assuming this is correct; I'm not at all sure), I could neither find an error with current master, nor reproduce the reported issue. A CMake forum thread says this error might happen with older versions (3.16.3), but not with new ones (3.24).

This fails with both the initial and the alternative patch above.

CMakeLists.txt

cmake_minimum_required(VERSION 3.7)

project(test-dependent)

find_package(libssh2 REQUIRED CONFIG)
message(STATUS "|${LIBSSH2_FOUND}|")  # Empty even if found. Why?

add_executable(test-dependent-static "test_dependent.c")
target_link_libraries(test-dependent-static PRIVATE libssh2::libssh2_static)

add_executable(test-dependent-shared "test_dependent.c")
target_link_libraries(test-dependent-shared PRIVATE libssh2::libssh2_shared)

# Alias for either shared or static library
add_executable(test-dependent-selected-ns "test_dependent.c")
target_link_libraries(test-dependent-selected-ns PRIVATE libssh2::libssh2)

# Compatibility alias
add_executable(test-dependent-compat "test_dependent.c")
target_link_libraries(test-dependent-compat PRIVATE Libssh2::libssh2)

test_dependent.c

#include "libssh2.h"
#include <stdio.h>

int main(void)
{
    printf("libssh2_version(0): |%s|\n", libssh2_version(0));
    return 0;
}

run.dh

cmake -B bld -DCMAKE_PREFIX_PATH="<path-to-libssh2>/usr/local/lib/cmake/libssh2"
cd bld
make VERBOSE=1

I'm closing this, feel free to discuss below.

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

Successfully merging this pull request may close these issues.

2 participants