10000 cmake: resolve libraries found by pkg-config by pks-t · Pull Request #4642 · libgit2/libgit2 · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 2 commits into from
May 9, 2018

Conversation

pks-t
Copy link
Member
@pks-t pks-t commented Apr 27, 2018

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

@pks-t pks-t force-pushed the pks/cmake-resolve-pkgconfig branch from b1c8c13 to b8e1745 Compare April 27, 2018 14:19
@pks-t
Copy link
Member Author
pks-t commented Apr 27, 2018

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

@pks-t pks-t force-pushed the pks/cmake-resolve-pkgconfig branch 5 times, most recently from d99d060 to 807edf8 Compare April 30, 2018 08:46
@pks-t pks-t force-pushed the pks/cmake-resolve-pkgconfig branch 2 times, most recently from ab87498 to 683a2aa Compare May 4, 2018 17:17
pks-t added 2 commits May 9, 2018 13:50
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.
@pks-t pks-t force-pushed the pks/cmake-resolve-pkgconfig branch from 683a2aa to 8ab470f Compare May 9, 2018 11:51
@pks-t pks-t merged commit f9cf9a0 into libgit2:master May 9, 2018
@pks-t pks-t deleted the pks/cmake-resolve-pkgconfig branch May 9, 2018 12:52
@tiennou
Copy link
Contributor
tiennou commented May 29, 2018

@pks-t Can this be scheduled for backport-0.27.2 ?

@pks-t
Copy link
Member Author
pks-t commented May 29, 2018 via email

@Keno
Copy link
Keno commented Jun 21, 2018

Hi there,

this change seems to have broken cross compile from linux to Windows via mingw.
In my particular case, libssh2 depends on bcrypt via import library (mingw's libbcrypt.a),
which is listed in the pkg-config file. However, using this approach to find the library fails
to find that import library leading to libgit2 refusing to build.

@pks-t
Copy link
Member Author
pks-t commented Jun 29, 2018

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

libssh2_userauth_publickey_frommemory not found in static libssh2
3 participants
0