cmake: define dependencies as IMPORTED interface targets
#16973
Conversation
vszakats
changed the title
|
There is more legwork to do here (esp. with GSS, a special case), but the bigger worry, Old Linux compatibility seems to be fixed now. |
|
After tests with 15 different, old cmake versions, it turns out that anything older The refusal to read the Also, it isn't just Old Linux, but multiple jobs tested in AppVeyor too. |
|
Remaining issues:
|
|
It seems that CMake messes up lib order when using imported interface targets. A rather terrible surprise. Since 2018: After recently fixing lib order using non-modern cmake, it surprises me that "modern" cmake makes this actively impossible. The suggested solution on Stack Overflow could just as well be suggesting to Honorable mention to binutils |
|
So, what CMake is doing is moving OpenSSL::SSL, OpenSSL::Crypto and ZLIB::ZLIB There is also a core assumption in CMake, that a library can always be addressed CMake seems to have invented Most likely I'm missing something and this assessment is just as skewed as any other, I don't know what is the magic trick to resolve this Gordian Knot. edit: all this mostly (exclusively?) affects static linking. |
vszakats
force-pushed
the
cm-interface
branch
6 times, most recently
from
b003226 to
18c3b77
Compare
|
I guess one way to deal with the broken lib order emitted by CMake We already do this as part of generating A better approach would be telling CMake about lib interdependencies, |
An earlier upstream patch made it possible to drop this hack. There is an open PR in curl, that's aiming to modernize dependency detection by using imported interface targets and target properties. That PR revealed an issue, where this solution trigger CMake automatisms that makes it override the lib order as passed to CMake. This in turn reintroduces the issue of linker errors with gcc's picky binuntils `ld` linker, which is sensitive to this. I've found no solution to get back control over lib order when using imported interface targets. Yet the PR seems like a good direction to take, and improves static linking by automatically pulling in all lib dependencies when consuming libcurl. It overall seems like a useful improvement despite the regression in lib order. To work around the lib order regression, let's reintroduce the previous hack to make `ld` work with the lib order CMake emits. Ref: curl/curl#16973 Reverts 2ed700e
Fixes: ``` CMake Error at bld-curl/_pkg/lib/cmake/CURL/CURLConfig.cmake:62 (add_library): add_library cannot create ALIAS target "CURL::libcurl" because target "CURL::libcurl_shared" is imported but not globally visible. Call Stack (most recent call first): CMakeLists.txt:39 (find_package) ``` Seen with cmake 3.12 when running tests/cmake with: ```shell export CMAKE_CONSUMER=/path/to/CMake-3.12.0/bin/cmake ./test.sh find_package -DBUILD_STATIC_CURL=ON ``` I don't claim to understand what this error says, why it happens in certain CMake versions, and why such workaround is necessary for what seems like rather standard export/consume configuration. Cherry-picked from curl#16973
…ming libcurl with old cmake Fixes: ``` CMake Error at bld-curl/_pkg/lib/cmake/CURL/CURLConfig.cmake:62 (add_library): add_library cannot create ALIAS target "CURL::libcurl" because target "CURL::libcurl_shared" is imported but not globally visible. Call Stack (most recent call first): CMakeLists.txt:39 (find_package) ``` tests/cmake reproducer (requires #16973): ```shell export CMAKE_CONSUMER=/path/to/CMake-3.12.0/bin/cmake ./test.sh find_package ``` I don't understand what this error says, why it happens in certain CMake versions, and why a workaround is necessary for what seems like a standard export/consume configuration. This patch is based on internet suggestions and other projects ending up with this workaround. Cherry-picked from #16973 Closes #17140
vszakats
force-pushed
the
cm-interface
branch
from
3a2dc01 to
de0cfa4
Compare
|
augment review |
|
Sorry, Augment does not have access to the org this pull request's branch is from. |
vszakats
removed
on-hold
feature-window
|
At least not after rc2. |
|
libcurl.pc for x64-linux (vcpkg, static library linkage, pristine source): Pretty much redundancy. Even more with pkgconfig -static. |
Reported-by: Kai Pastor Bug: curl#16973 (comment) Follow-up to 16f073e curl#16973
This has been working like this for a long time, and unrelated to this PR, or am I missing something? |
Bug: curl/curl#16973 (comment) Follow-up to 82b09f9 #1322 Closes #1750
Reported-by: Kai Pastor Bug: #16973 (comment) Follow-up to 16f073e #16973 Closes #19758
|
It is not related to this PR. The expected behavior is at least that information from Requires* isn't duplicated in Libs*, and that [Libs/Requires].private isn't duplicated in [Libs/Requires]. (.private being empty is a reasonable approach when there is no shared lib.) |
Sounds reasonable, but touching this seems like asking for a lot of trouble, (I wonder how lib order (combined from modules and raw libs) is turning out |
Modules encode transitive usage requirements. Nothing is lost. It just works.
Studying downstream issues is a lot of trouble with complex link library lists. |
Show a message if the CMake version is lower than that when consuming libcurl via the CMake config. The minimum CMake version on consumption is for now the same as the minimum required (v3.7) to build curl itself. Ref: https://cmake.org/cmake/help/v3.7/variable/CMAKE_MINIMUM_REQUIRED_VERSION.html Ref: #18704 (discussion) Follow-up to 16f073e #16973 Closes #19776
3 participants
Rework the way curl's custom Find modules advertise their properties.
Before this patch, Find modules returned detected dependency properties
(header dirs, libs, libdirs, C flags, etc.) via global variables. curl's
main
CMakeLists.txtcopied their values into global lists, which itlater applied to targets. This solution worked internally, but it was
unsuited for the public, distributed
CURLConfig.cmakeand publishingcurl's Find modules with it, due to polluting the namespace of consumer
projects. It's also impractical to apply the many individual variables
to every targets depending on libcurl.
To allow using Find modules in consumer projects, this patch makes them
define as imported interface targets, named
CURL::<dependency>. Thenstore dependency information as target properties. It avoids namespace
pollution and makes the dependency information apply automatically
to all targets using
CURL::libcurl_static.Find modules continue to return
*_FOUNDand*_VERSIONvariables.For dependencies detected via
pkg-config, CMake 3.16+ is recommended.Older CMake versions have a varying degree of support for
propagating/handling library directories. This may cause issues in envs
where dependencies reside in non-system locations and detected via
pkg-config(e.g. macOS + Homebrew). UseCURL_USE_PKGCONFIG=OFFto fix these issues. Or upgrade to newer CMake, or link libcurl
dynamically.
Also:
pkg-configfor old cmakefind_library()integrationtests.
curlinfobuild after these changes.CURL_LIBRARIES_PRIVATE.CURLconfig.cmake: use curl's Find modules to detect dependencies inthe consumer env.
CMAKE_C_FLAGS.Follow-up to e865420 cmake: prefer
COMPILE_OPTIONSoverCMAKE_C_FLAGSfor custom C options #17047Ref: #14930
Ref: libssh2/libssh2#1535
Ref: libssh2/libssh2#1571
Ref: libssh2/libssh2#1581
Ref: libssh2/libssh2#1623
w/o sp https://github.com/curl/curl/pull/16973/files?w=1
curl-config.cmake#14930ldpicky linker issues due unexplicably boulversed lib orderLIB_NAMEmove to PR → cmake: useLIB_NAMEincurl-config.cmake.in#17195CURL_LIBRARIESor other var with raw dep libnames? [DONE, but not planning to merge, unless requested and actually useful in real cases. → doing it anyway, just in case.]LIBSSH2_PC_LIBS_PRIVATEmore carefully libssh2/libssh2#1621 and did not implement it for curl, because there is no obvious case when duplicate modules names are listed next to each other, and curl doesn't do the deduplicated that libssh2 did.