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
CMake FetchContent with ZLIB fails (>=8.0.0) #11285
Comments
I don't like this behaviour either (the same for the headers checks), but my understanding is that this is basically cloning the autotools behaviour, thus probably making the logic easier to maintain. |
If the code breaks the build, then what's the proposed fix? The configure build seems to work so if so this was an incomplete "mimic"... I doubt that trying to "clone behavior" is a very good idea long term, as the two systems are very different, using different paradigms and they are also often maintained and updated by a different set of people. |
Well, it's like that since I can remember. Maybe, just maybe, re-writing it from scratch would make sense (there are benefits and downsides of doing so). |
It is not really "Makefile.am files" though. They are files we share between the build systems to avoid having to maintain two separate lists of the same things. |
May very well be - I know nothing about autotools - and it makes sense to have single source of truth like that. |
I understand that my first suggestion of removing the The real issue after all is that this list does not only contains libraries but also CMake targets. Would removing targets from the list before calling |
We have encountered this same issue. We don't use If you guys are looking to continue to emulate autotools, here is a possible solution that removes targets from the list before performing macro(check_library_exists_concat LIBRARY SYMBOL VARIABLE)
# Enumerate CURL_LIBS and add each library to CMAKE_REQUIRED_LIBRARIES if it
# is not a CMake target. System libraries should not depend on CMake targets.
set(CMAKE_REQUIRED_LIBRARIES)
foreach(LIB ${CURL_LIBS})
if (NOT TARGET ${LIB})
set(CMAKE_REQUIRED_LIBRARIES ${LIB} ${CMAKE_REQUIRED_LIBRARIES})
endif()
endforeach()
# Using CMake targets in the call to check_library_exists() may cause it to fail
# unless the target is IMPORTED.
check_library_exists("${LIBRARY}" ${SYMBOL} "${CMAKE_LIBRARY_PATH}"
${VARIABLE})
set(CMAKE_REQUIRED_LIBRARIES)
if(${VARIABLE})
set(CURL_LIBS ${LIBRARY} ${CURL_LIBS})
endif()
endmacro() The comment for the # Some libraries depend on others to link correctly. I don't doubt the claim, but I'm not aware of a specific instance where this is the case. However, if you don't want to continue to emulate autotools then this might be a simpler solution: macro(check_library_exists_concat LIBRARY SYMBOL VARIABLE)
check_library_exists("${LIBRARY}" ${SYMBOL} "${CMAKE_LIBRARY_PATH}"
${VARIABLE})
if(${VARIABLE})
set(CURL_LIBS ${LIBRARY} ${CURL_LIBS})
endif()
endmacro() @bagder I can submit a PR if you have a specific way you want to go. |
The idea of `check_library_exists_concat()` is that it detects an optional component and adds it to the list of libs that we also use in subsequent component checks. This caused problems when detecting components, with unnecessary dependencies that were not yet built. CMake offers the `CMAKE_REQUIRED_LIBRARIES` variable to set the libs used for component checks, which we already use in most of the cases. That left 4 uses of `check_library_exists_concat()`. Only one of these actually needed the 'concat' feature (ldap/lber). Delete this function and replace it with standard `check_library_exists()` and manual management of our `CURL_LIBS` list we use when linking build targets. And special logic to handle the ldap/lber case. (We have a similar function for headers: `check_include_file_concat`. It works, but problematic for performance reasons and because it hides the actual headers necessary in `check_symbol_exists()` calls.) Ref: curl#11537 Fixes curl#11285 Fixes curl#11648 Closes #xxxxx
The idea of `check_library_exists_concat()` is that it detects an optional component and adds it to the list of libs that we also use in subsequent component checks. This caused problems when detecting components with unnecessary dependencies that were not yet built. CMake offers the `CMAKE_REQUIRED_LIBRARIES` variable to set libs used for component checks, which we already use in most cases. That left 4 uses of `check_library_exists_concat()`. Only one of these actually needed the 'concat' feature (ldap/lber). Delete this function and replace it with standard `check_library_exists()` and manual management of our `CURL_LIBS` list we use when linking build targets. And special logic to handle the ldap/lber case. (We have a similar function for headers: `check_include_file_concat()`. It works, but problematic for performance reasons and because it hides the actual headers required in `check_symbol_exists()` calls.) Ref: curl#11537 curl#11558 Fixes curl#11285 Fixes curl#11648 Closes curl#12070
I did this
Include curl with zlib support in my CMake project using FetchContent.
Minimal CMakeLists.txt:
I expected the following
The CMake configuration to run smoothly
curl/libcurl version
8.1.2 (Issue started with 8.0.0)
operating system
Linux pop-os 6.2.6-76060206-generic #202303130630
168375320722.04~77c1465 SMP PREEMPT_DYNAMIC Wed M x86_64 x86_64 x86_64 GNU/LinuxWhat I get
When configuring, I get the following error:
When digging a little, I found something I don't understand in the CMake macro
check_library_exists_concat
Do we need to concatenate the searched library with the current state of the
CURL_LIBS
variable when running thecheck_library_exists
?Removing it fixes my issue.
Thanks !
The text was updated successfully, but these errors were encountered: