cmake: fix multiple include of CURL package #11913

dg0yt · 2023-09-22T05:51:01Z

Fixes errors on second find_package(CURL). This is a frequent case with transitive dependencies.

CMake Error at ...:
  add_library cannot create ALIAS target "CURL::libcurl" because another
  target with the same name already exists.

Test to reproduce:

cmake_minimum_required(VERSION 3.27)  # must be 3.18 or higher

project(curl)

set(CURL_DIR "example/lib/cmake/CURL/")
find_package(CURL CONFIG REQUIRED)
find_package(CURL CONFIG REQUIRED)  # fails

add_executable(main main.c)
target_link_libraries(main CURL::libcurl)

Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes
Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html
Ref: #12300
Assisted-by: Harry Mallon
Closes #11913

jzakrzewski · 2023-09-22T06:29:37Z

Out of curiosity: how did you hit this problem?

dg0yt · 2023-09-22T06:33:29Z

Failed update attempt in vcpkg:
microsoft/vcpkg#33886
e.g. https://github.com/microsoft/vcpkg/runs/16951375053

~~One hit~~ The demonstrator is GDAL: It uses curl directly, but it also uses PROJ, which adds another transitive usage. So both GDAL itself and PROJ config do find_package(CURL), at least for static library linkage.

dg0yt · 2023-09-22T06:35:42Z

vcpkg update with this change added: microsoft/vcpkg#33924

jzakrzewski · 2023-09-22T06:39:49Z

I see - thanks for explanation.

bagder · 2023-09-22T08:30:33Z

@jzakrzewski should we count that as a 👍 from you?

jzakrzewski · 2023-09-22T10:03:31Z

Frankly, I don't know if this is enough.
I'm not sure what is the correct thing to do, but it seems to me, that in this scenario the find module will overwrite some properties of the CURL::libcurl target if it already exists.
But I'm not sure. I'd have to test it an see what will actually happen.

CMake/curl-config.cmake.in

jzakrzewski · 2023-09-22T10:12:36Z

OK, I checked it. Since CURLTargets.cmake is already protected against multiple calls, this fix looks OK.

Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>

dg0yt · 2023-09-22T11:10:49Z

FTR this if guard is a frequent pattern in bug fixes while packaging, and standard in official Find modules, e.g.
https://github.com/Kitware/CMake/blob/cc49d22e624963cbb42a4f06cbcff15f1ad2082f/Modules/FindPNG.cmake#L132-L133

dg0yt · 2023-09-23T06:35:14Z

FTR there is another problem, with older version of CMake:

CMake Error at /mnt/vss/_work/1/s/scripts/buildsystems/vcpkg.cmake:639 (_add_library):
  add_library cannot create ALIAS target "CURL::libcurl" because target
  "CURL::libcurl_static" is IMPORTED.

(Found by a vcpkg CI test which CMake 3.7.2.)

Choose what you want:

Stop supporting old versions of Cmake for downstream usage.
Generally create an INTERFACE IMPORTED library, and put CURL::libcurl into the INTERFACE_LINK_LIBRARIES.
Use the latter only for older version of CMake.

This can be done in another PR.

vszakats · 2023-09-23T09:23:01Z

Speaking of ALIAS issues, there was also this one reported for older CMake:

[25/26] -- Found CURL: ***/build/parts/curl/install/lib64/cmake/CURL/CURLConfig.cmake (found version "8.3.0-DEV")
[25/26] CMake Error at ***/build/parts/curl/install/lib64/cmake/CURL/CURLConfig.cmake:62 (add_library):
[25/26]   add_library cannot create ALIAS target "CURL::libcurl" because target
[25/26]   "CURL::libcurl_static" is imported but not globally visible.

Ref: #11646
Ref: libssh2/libssh2#1154

dg0yt · 2023-09-23T09:33:04Z

https://cmake.org/cmake/help/latest/command/add_library.html#alias-libraries:
The first step is CMake 3.11 (globally visible targets), the second CMake 3.18 (all targets).

The downside of the the INTERFACE target is that it doesn't let you read properties like an ALIAS does.

dg0yt · 2023-10-07T04:40:48Z

Ping.

vszakats · 2023-10-07T07:46:19Z

Could you create a minimal build case example that reproduces this issue? We could include that in a future integration test.

Tried reproducing this for libssh2 (where I added such tests earlier this year) but without success: https://github.com/libssh2/libssh2/tree/master/tests/cmake

dg0yt · 2023-10-07T07:53:07Z

cmake_minimum_required(3.1)
project(test)
find_package(CURL CONFIG)
find_package(CURL CONFIG)

(And even the CONFIG isn't needed, but it skips the indirection via FindCURL.cmake.)

dg0yt · 2023-10-07T08:07:59Z

FTR in the vcpkg port I already replaced the ALIAS with IMPORTED, to satisfy older versions of CMake.
https://github.com/microsoft/vcpkg/blob/cc97b4536ae749ec0e4f643488b600b217540fb3/ports/curl/cmake-config.patch#L8-L13

vszakats · 2023-10-07T08:48:06Z

@dg0yt: Thanks for the example. My next trouble is that this is what I tried with libssh2 to reproduce this.

I tried it now with curl, and strangely it runs without issues. This is with CMake 3.27.6.

What might cause the difference?

(Not questioning the validy of the issue, I'd just like to learn what's happening.)

CMakeLists.txt:

cmake_minimum_required(VERSION 3.1)  # it fails with "(3.1)"
project(test)
find_package(CURL CONFIG)
find_package(CURL CONFIG)

$ rm -rf bld; cmake -B bld -DCURL_DIR=.../curl/bld-cmake/pkg/usr/local/lib/cmake/CURL

CMake Deprecation Warning at CMakeLists.txt:1 (cmake_minimum_required):
  Compatibility with CMake < 3.5 will be removed from a future version of
  CMake.

  Update the VERSION argument <min> value or use a ...<max> suffix to tell
  CMake that the project does not need compatibility with older versions.


-- The C compiler identification is AppleClang 14.0.0.14000029
-- The CXX compiler identification is AppleClang 14.0.0.14000029
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /Library/Developer/CommandLineTools/usr/bin/cc - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /Library/Developer/CommandLineTools/usr/bin/c++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Found OpenSSL: [...]
-- Found ZLIB: [...]
-- Configuring done (1.6s)
-- Generating done (0.0s)
-- Build files have been written to: .../curl/tests/cmake/bld

.../curl/bld-cmake/pkg/usr/local/lib/cmake/CURL/CURLConfig.cmake:

[...]
# Alias for either shared or static library
add_library(CURL::libcurl ALIAS CURL::libcurl_static)

dg0yt · 2023-10-07T08:52:51Z

Sorry, the example was too short and failed for the first line. I will extend it.

dg0yt · 2023-10-07T08:58:43Z

Sorry, the example was too short and failed for the first line. I will extend it.

FTR after fixing the first line, the example is still exposes the error for older versions of CMake (_add_library cannot create ALIAS target "CURL::libcurl" because target "CURL::libcurl_static" is imported but not globally visible.). Tested with CMake 3.16.

vszakats · 2023-10-07T09:29:25Z

Got it, so this is only hit by older CMake versions (e.g. 3.16).

Do we know which CMake version changed this behaviour?

Ref: https://cmake.org/cmake/help/latest/release/index.html

dg0yt · 2023-10-07T09:35:16Z

Do we know which CMake version changed this behaviour?

#11913 (comment)

I currently fail to reproduce the message from the initial post, but I verified that it is in the linked vpckg CI log from the initial curl update attempt, and this was using CMake 3.27.1. I need to verify that no other package did something fishy. There is nothing more telling than a producing minimal reproducable example ;-)

PatTheMav · 2023-10-16T14:05:25Z

Generally create an INTERFACE IMPORTED library, and put CURL::libcurl into the INTERFACE_LINK_LIBRARIES.

FWIW this is the canonical way to "alias" targets imported in Find Modules or CMake Packages. The drawback is loosing easy access to custom properties, but their existence should not even be known to consumers of the library (i.e., all properties required for building/linking should be exposed with INTERFACE properties that are propagated by CMake anyway).

But I also agree that this should be tackled in a separate PR, because the version incompatibility exists already (and is not introduced by the PR) and the PR fixes a breaking issue with any CMake project that transitively depends on cURL more than once.

hjmallon · 2023-11-09T18:33:50Z

There are discussions of two issues in this thread. The fix proposed in this MR fixes issue 1, which is very easy to reproduce.

Build CURL with cmake

cmake . -B build
cmake --build build -j10
cmake --install build --prefix build/install

You now have an installed version of CURL in build/install/lib.

Try to build the following project (change CURL_DIR to wherever you put the install)

cmake_minimum_required(VERSION 3.27)

project(curl)

set(CURL_DIR "BLAH/build/install/lib/cmake/CURL")
find_package(CURL CONFIG REQUIRED)
find_package(CURL CONFIG REQUIRED)

The second find_package(CURL CONFIG REQUIRED) fails with

CMake Error at BLAH/build/install/lib/cmake/CURL/CURLConfig.cmake:62 (add_library):
  add_library cannot create ALIAS target "CURL::libcurl" because another
  target with the same name already exists.
Call Stack (most recent call first):
  CMakeLists.txt:7 (find_package)

Issue 2 is totally separate as mentioned a few times in this thread

hjmallon · 2023-11-09T18:43:09Z

AHA https://cmake.org/cmake/help/v3.28/policy/CMP0107.html

New in version 3.18.

It is not allowed to create an ALIAS target with the same name as an another target.

In CMake 3.17 and below, an ALIAS target can overwrite silently an existing target with the same name.

vszakats · 2023-11-09T18:49:35Z

@hjmallon Thanks for your example in #12300.

I re-tested, and oddly enough this issue is dependent on cmake_minimum_required(VERSION <ver>). It works silently when <ver> is 3.17 or lower and breaks like this with 3.18 or higher (ref: #12300 (comment)):

CMake Error at .../lib/cmake/CURL/CURLConfig.cmake:62 (add_library):
  add_library cannot create ALIAS target "CURL::libcurl" because another
  target with the same name already exists.
Call Stack (most recent call first):
  CMakeLists.txt:8 (find_package)

Release notes of 3.18 doesn't say anything obvious to indicate this breaking change: https://cmake.org/cmake/help/latest/release/3.18.html
Though maybe this isn't even the place to look.

hjmallon · 2023-11-09T18:50:40Z

https://cmake.org/cmake/help/v3.28/policy/CMP0107.html

@vszakats : Look at my link above, it documents the change in behaviour

EDIT: Also in release notes

Creation of an ALIAS target overwriting an existing target now raises an error. See policy CMP0107.

vszakats · 2023-11-09T18:53:30Z

Oh, nice find! So it is listed in the 3.18 changelog after all, in "Other Changes".

Also extend our integration test double inclusion. It will still not catch this case, because that requires `cmake_minimum_required(VERSION 3.18)` or higher. Fixes: ``` CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:8 (add_library): add_library cannot create ALIAS target "libssh2::libssh2" because another target with the same name already exists. Call Stack (most recent call first): CMakeLists.txt:24 (find_package) CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:13 (add_library): add_library cannot create ALIAS target "Libssh2::libssh2" because another target with the same name already exists. Call Stack (most recent call first): CMakeLists.txt:24 (find_package) ``` Test to reproduce: ```cmake cmake_minimum_required(VERSION 3.27) # must be 3.18 or higher project(test) find_package(libssh2 CONFIG) find_package(libssh2 CONFIG) # fails add_executable(test main.c) target_link_libraries(test libssh2::libssh2) ``` Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html Assisted-by: Kai Pastor Assisted-by: Harry Mallon Ref: curl/curl#11913 Closes #xxxx

Also extend our integration test double inclusion. It will still not catch this case, because that requires `cmake_minimum_required(VERSION 3.18)` or higher. Fixes: ``` CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:8 (add_library): add_library cannot create ALIAS target "libssh2::libssh2" because another target with the same name already exists. Call Stack (most recent call first): CMakeLists.txt:24 (find_package) CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:13 (add_library): add_library cannot create ALIAS target "Libssh2::libssh2" because another target with the same name already exists. Call Stack (most recent call first): CMakeLists.txt:24 (find_package) ``` Test to reproduce: ```cmake cmake_minimum_required(VERSION 3.18) # must be 3.18 or higher project(test) find_package(libssh2 CONFIG) find_package(libssh2 CONFIG) # fails add_executable(test main.c) target_link_libraries(test libssh2::libssh2) ``` Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html Assisted-by: Kai Pastor Assisted-by: Harry Mallon Ref: curl/curl#11913 Closes #xxxx

Also extend our integration test double inclusion. It will still not catch this case, because that requires `cmake_minimum_required(VERSION 3.18)` or higher. Fixes: ``` CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:8 (add_library): add_library cannot create ALIAS target "libssh2::libssh2" because another target with the same name already exists. Call Stack (most recent call first): CMakeLists.txt:24 (find_package) CMake Error at .../lib/cmake/libssh2/libssh2-config.cmake:13 (add_library): add_library cannot create ALIAS target "Libssh2::libssh2" because another target with the same name already exists. Call Stack (most recent call first): CMakeLists.txt:24 (find_package) ``` Test to reproduce: ```cmake cmake_minimum_required(VERSION 3.18) # must be 3.18 or higher project(test) find_package(libssh2 CONFIG) find_package(libssh2 CONFIG) # fails add_executable(test main.c) target_link_libraries(test libssh2::libssh2) ``` Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html Assisted-by: Kai Pastor Assisted-by: Harry Mallon Ref: curl/curl#11913 Closes #1216

vszakats · 2023-11-11T23:23:21Z

Thank you all, merged now.

Fixes errors on second `find_package(CURL)`. This is a frequent case with transitive dependencies: ``` CMake Error at ...: add_library cannot create ALIAS target "CURL::libcurl" because another target with the same name already exists. ``` Test to reproduce: ```cmake cmake_minimum_required(VERSION 3.27) # must be 3.18 or higher project(curl) set(CURL_DIR "example/lib/cmake/CURL/") find_package(CURL CONFIG REQUIRED) find_package(CURL CONFIG REQUIRED) # fails add_executable(main main.c) target_link_libraries(main CURL::libcurl) ``` Ref: https://cmake.org/cmake/help/latest/release/3.18.html#other-changes Ref: https://cmake.org/cmake/help/v3.18/policy/CMP0107.html Ref: curl#12300 Assisted-by: Harry Mallon Closes curl#11913

Fix curl-config.cmake.in

3ab0406

github-actions bot added build cmake labels Sep 22, 2023

dg0yt mentioned this pull request Sep 22, 2023

[curl] update to 8.3.0 microsoft/vcpkg#33877

Closed

vszakats reviewed Sep 22, 2023

View reviewed changes

CMake/curl-config.cmake.in Outdated Show resolved Hide resolved

Update CMake/curl-config.cmake.in

8227409

Co-authored-by: Viktor Szakats <vszakats@users.noreply.github.com>

vszakats mentioned this pull request Sep 22, 2023

cmake: test multiple find_package() libssh2/libssh2#1189

Closed

vszakats mentioned this pull request Nov 9, 2023

cmake: Fix multiple include of CURL package #12300

Closed

vszakats changed the title ~~Fix curl-config.cmake.in~~ cmake: fix multiple include of CURL package Nov 9, 2023

vszakats approved these changes Nov 9, 2023

View reviewed changes

vszakats mentioned this pull request Nov 9, 2023

cmake: fix multiple include of libssh2 package libssh2/libssh2#1216

Closed

vszakats closed this in 45d2ff6 Nov 10, 2023

dg0yt deleted the patch-1 branch November 26, 2023 13:12

cmake: fix multiple include of CURL package #11913

cmake: fix multiple include of CURL package #11913

dg0yt commented Sep 22, 2023 •

edited by vszakats

jzakrzewski commented Sep 22, 2023

dg0yt commented Sep 22, 2023 •

edited

dg0yt commented Sep 22, 2023

jzakrzewski commented Sep 22, 2023

bagder commented Sep 22, 2023

jzakrzewski commented Sep 22, 2023

jzakrzewski commented Sep 22, 2023

dg0yt commented Sep 22, 2023

dg0yt commented Sep 23, 2023 •

edited

vszakats commented Sep 23, 2023

dg0yt commented Sep 23, 2023

dg0yt commented Oct 7, 2023

vszakats commented Oct 7, 2023

dg0yt commented Oct 7, 2023 •

edited

dg0yt commented Oct 7, 2023

vszakats commented Oct 7, 2023 •

edited

dg0yt commented Oct 7, 2023

dg0yt commented Oct 7, 2023

vszakats commented Oct 7, 2023

dg0yt commented Oct 7, 2023

PatTheMav commented Oct 16, 2023

hjmallon commented Nov 9, 2023 •

edited

hjmallon commented Nov 9, 2023

vszakats commented Nov 9, 2023 •

edited

hjmallon commented Nov 9, 2023 •

edited

vszakats commented Nov 9, 2023 •

edited

vszakats commented Nov 11, 2023

cmake: fix multiple include of CURL package #11913

cmake: fix multiple include of CURL package #11913

Conversation

dg0yt commented Sep 22, 2023 • edited by vszakats

jzakrzewski commented Sep 22, 2023

dg0yt commented Sep 22, 2023 • edited

dg0yt commented Sep 22, 2023

jzakrzewski commented Sep 22, 2023

bagder commented Sep 22, 2023

jzakrzewski commented Sep 22, 2023

jzakrzewski commented Sep 22, 2023

dg0yt commented Sep 22, 2023

dg0yt commented Sep 23, 2023 • edited

vszakats commented Sep 23, 2023

dg0yt commented Sep 23, 2023

dg0yt commented Oct 7, 2023

vszakats commented Oct 7, 2023

dg0yt commented Oct 7, 2023 • edited

dg0yt commented Oct 7, 2023

vszakats commented Oct 7, 2023 • edited

dg0yt commented Oct 7, 2023

dg0yt commented Oct 7, 2023

vszakats commented Oct 7, 2023

dg0yt commented Oct 7, 2023

PatTheMav commented Oct 16, 2023

hjmallon commented Nov 9, 2023 • edited

hjmallon commented Nov 9, 2023

vszakats commented Nov 9, 2023 • edited

hjmallon commented Nov 9, 2023 • edited

vszakats commented Nov 9, 2023 • edited

vszakats commented Nov 11, 2023

dg0yt commented Sep 22, 2023 •

edited by vszakats

dg0yt commented Sep 22, 2023 •

edited

dg0yt commented Sep 23, 2023 •

edited

dg0yt commented Oct 7, 2023 •

edited

vszakats commented Oct 7, 2023 •

edited

hjmallon commented Nov 9, 2023 •

edited

vszakats commented Nov 9, 2023 •

edited

hjmallon commented Nov 9, 2023 •

edited

vszakats commented Nov 9, 2023 •

edited