Skip to content
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: replace check_library_exists_concat() #12070

Closed

Conversation

vszakats
Copy link
Member

@vszakats vszakats commented Oct 9, 2023

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: #11537 #11558
Fixes #11285
Fixes #11648
Closes #12070

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
@vszakats vszakats changed the title cmake: delete check_library_exists_concat() cmake: replace check_library_exists_concat() Oct 9, 2023
@vszakats
Copy link
Member Author

@nmoinvaz: Does this fix the issues for you?

@nmoinvaz
Copy link
Contributor

I am on an older version of the CMakeLists.txt but I have applied your changes and they work for me. Thanks for your help.

@vszakats vszakats closed this in 84a6579 Oct 15, 2023
@vszakats vszakats deleted the cmake-remove-lib-exists-concat branch October 15, 2023 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

CMake FetchContent with ZLIB fails (>=8.0.0)
2 participants