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

Allow overriding of the libgit2 pkgconfig at build time #1065

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ReillyBrogan
Copy link

It is often desirable to Linux distributions to de-bundle packages as much as possible so that system versions of libraries are used instead of statically compiled versions. This is problematic with libgit2-sys as the supported version range is so narrow (also a problem for most libgit2 bindings). A common solution to this is to have co-installable versions of libgit2 where the pkgconfig file is renamed (perhaps to something like libgit-1.7 or similar). Support this in libgit2-sys so that distributions can override this at build time for arbitrary applications that may use the libgit2 bindings.

It is often desirable to Linux distributions to de-bundle packages as much as possible so that system versions of libraries are used instead of statically compiled versions. This is problematic with libgit2-sys as the supported version range is so narrow (also a problem for most libgit2 bindings). A common solution to this is to have co-installable versions of libgit2 where the pkgconfig file is renamed (perhaps to something like `libgit-1.7` or similar). Support this in libgit2-sys so that distributions can override this at build time for arbitrary applications that may use the libgit2 bindings.

Signed-off-by: Reilly Brogan <[email protected]>
@ReillyBrogan
Copy link
Author

@ehuss could we get this merged in before the next release? It shouldn't regress anything and it's entirely opt-in behavior. Renaming pkg-config files is also fairly standard when it comes to making different versions of a library co-installable.

@weihanglo
Copy link
Member

Hi @ReillyBrogan. Thank you for the contribution. I have a mixed feeling with this change. While it does fix the issue, it is a bit uncomfortable that we are able to set an ad-hoc environment variable that affects your dependency randomly.

Do you have more references or examples about this kind of pkg-config lib name override? How do other Rust *-sys libraries address this? Any other libgit2 binding also use this workaround?

@ReillyBrogan
Copy link
Author

that affects your dependency randomly

Keep in mind that if this searches for libgit2-1.18 for example that whatever pkgconfig file it finds will still need to pass the version check and if it's not actually libgit2 the build will fail when it tries to link against it.

I don't think it's realistic for a user to randomly set this environmental variable, happen to have a pkg-config with a version string somewhere in the supported version range, and also have it link successfully. The only people who are going to be setting this are going to be knowing what they are doing, or it just won't work at all.

How do other Rust *-sys libraries address this

They don't need to. This situation is fairly unique to libgit2 as it is probably the most widely used library on Linux that sees regular soname changes and has semi-frequent CVEs. Sqlite3 and zstd bindings, for comparison, haven't seen a single soname bump during the entire time that Rust has existed as a language. If there was a comparable situation, the same kind of solution would probably be the best approach.

Any other libgit2 binding also use this workaround?

No, because usually they are built with cmake or meson which have workarounds allowing for overriding a given pkgconfig search, or the search itself is usually in a build file in the source code that can be easily patched. Cargo projects are a different case because the libraries are usually downloaded at build-time (IE, it's non-trivial to patch them effectively) and the pkg_config crate only allows for overriding the pkg-config search path entirely (which doesn't help you if the pkg-config files are in the same directory).

I've been gradually converting packages on the Linux distribution I work on to use system libs as much as possible and as far as I can tell it's generally accepted practice to use environmental variables to override things in dependency builds. libgit2-sys already supports LIBGIT2_NO_VENDOR to force it to try to use the system libgit2 or fail if it can't, this new environmental variable just extends that by allowing it to use another libgit2 install if necessary.

Copy link
Member

@weihanglo weihanglo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the reply. This workaround seems fair.
I wish we had any better standard solution for all *-sys crate, like gdesmott/system-deps#97.

Regardlessly, thank you :)

@weihanglo
Copy link
Member

Could you help document the environment variable somewhere in the root README.md or lib.rs of libgit2-sys?
(I think I have missed documenting LIBGIT2_NO_VENDOR 😅)

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.

None yet

2 participants