-
Notifications
You must be signed in to change notification settings - Fork 413
Add a LIBGIT2_NO_VENDOR environment variable to build.rs #939
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
This would be very useful to void linux, as we've had issues with things not linking to our system libgit2, +1 |
This needs a rebase |
I've rebased the patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To test for cargo features you'd usually use
if cfg!(feature = "zlib-ng-compat") { ... }
instead of accessing the environment variable, but since this environment variable could also be set from outside of cargo I'm generatingcargo:rerun-if-env-changed=
output to let cargo know it needs to rebuild if they're changed
I am not quite understand the change here. Could we explain a bit more what “you” means here? git2 maintainers of git2 users?CARGO_FEATURE_*
is something set by cargo for build script to inform external build tools. Cargo features behavior AFAIK is not affected by someone setting those env vars.
Fix one clippy warning
Should we do one thing per commit? (Or per pull request if you'd like to)
let ssh = env_var("CARGO_FEATURE_SSH").is_some(); | ||
let vendored = env_var("CARGO_FEATURE_VENDORED").is_some(); | ||
let zlib_ng_compat = env_var("CARGO_FEATURE_ZLIB_NG_COMPAT").is_some(); | ||
let no_vendor = env_var("LIBGIT2_NO_VENDOR").map_or(false, |s| s != "0"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should doc this new environment variable about why and when people should use it. Maybe in README.md
or elsewhere.
Closing as completed from #966. |
This does the following things:
if cfg!(feature = "zlib-ng-compat") { ... }
instead of accessing the environment variable, but since this environment variable could also be set from outside of cargo I'm generatingcargo:rerun-if-env-changed=
output to let cargo know it needs to rebuild if they're changedLIBGIT2_NO_VENDOR
environment variable, this variable is supposed to deny vendoring even if the cargo featurelibgit2-sys/vendored
is enabled by any crate. If the system libgit2 is not present or not compatible it errors the build to let the maintainer know something needs to be fixed. This option would be especially useful for Linux package maintainers who always want to link the system library to avoid "shadow copies" of libgit2 embedded in binaries that are not known to the security team.The
LIBGIT2_NO_VENDOR
variable is analogous toOPENSSL_NO_VENDOR
in openssl-sys: sfackler/rust-openssl#1253There's currently no
Error
struct in the build.rs so I'm using aResult<(), ()>
, if you prefer anything else just let me know.