-
Notifications
You must be signed in to change notification settings - Fork 2.5k
cmake: resolve libraries found by pkg-config #4642
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
Conversation
b1c8c13
to
b8e1745
Compare
Amended the PR to fall back to static libraries in case no dynamic ones exist. Furthermore, we now output the list of resolved libraries, which should help |
d99d060
to
807edf8
Compare
ab87498
to
683a2aa
Compare
Libraries found by CMake modules are usually handled with their full path. This makes linking against those libraries a lot more robust when it comes to libraries in non-standard locations, as otherwise we might mix up libraries from different locations when link directories are given. One excemption are libraries found by PKG_CHECK_MODULES. Instead of returning libraries with their complete path, it will return the variable names as well as a set of link directories. In case where multiple sets of the same library are installed in different locations, this can lead the compiler to link against the wrong libraries in the end, when link directories of other dependencies are added. To fix this shortcoming, we need to manually resolve library paths returned by CMake against their respective library directories. This is an easy task to do with `FIND_LIBRARY`.
With the recent change of always resolving pkg-config libraries to their full path, we do not have to manage the LIBGIT2_LIBDIRS variable anymore. The only other remaining user of LIBGIT2_LIBDIRS is winhttp, which is a CMake-style library target and can thus be resolved by CMake automatically. Remove the variable to simplify our build system a bit.
683a2aa
to
8ab470f
Compare
@pks-t Can this be scheduled for backport-0.27.2 ? |
Sure, I actually intended to backport it to the bugfix release.
Will do so towards the end of this week and get v0.27.2 back on
track now that the security release is finally done.
|
Hi there, this change seems to have broken cross compile from linux to Windows via mingw. |
So the initial configuration of the build fails because CMake is now unable to find the transitive dependency libbcrypt.a, which is specified in libssh2's pkg-config file? To me this sounds like the pkg-config file is erroneous, then, as it would have to specify both the linking directory of libssh2 itself as well as the linking directory of libbcrypt.a. Otherwise it is expected that we are not able to resolve libbcrypt.a, and bailing out is the correct thing to do. Do both of these libraries live in different directories? Please correct me if I misunderstood your issue. |
Libraries found by CMake modules are usually handled with their full
path. This makes linking against those libraries a lot more robust when
it comes to libraries in non-standard locations, as otherwise we might
mix up libraries from different locations when link directories are
given.
One excemption are libraries found by PKG_CHECK_MODULES. Instead of
returning libraries with their complete path, it will return the
variable names as well as a set of link directories. In case where
multiple sets of the same library are installed in different locations,
this can lead the compiler to link against the wrong libraries in the
end, when link directories of other dependencies are added.
To fix this shortcoming, we need to manually resolve library paths
returned by CMake against their respective library directories. This is
an easy task to do with
FIND_LIBRARY
.I'm not too sure about the second patch yet. I think we don't need
LIBGIT2_LIBDIRS
anymore due to the change, and getting rid of it is definitely a nice thing. Let's see what CI has to say about it.This fixes #4120 and partially solves the issue in #4604