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

build: LIBGIT2_NO_VENDOR to force to use system libgit2 #966

Merged
merged 1 commit into from
Jul 29, 2023

Conversation

weihanglo
Copy link
Member

Specify LIBGIT2_NO_VENDOR to force to use system libgit2.

Due to the additive nature of Cargo features, if some crate in the
dependency graph activates vendored feature, there is no way to
revert it back. This env var serves as a workaround for this purpose.

An alternative is having no_vendored feature. We still need to
modify build.rs to make both vendored and no_vendored play
nice with each other (or bail out as they are mutual-exclusive).
However, there is no way to activate a Cargo feature via environment
variable (see rust-lang/cargo#4829). Altering environment variables
may be the only way to interact with some external build systems.

It is also pretty common that people don't want to vendor anything and
rather to see the build fail than vendoring.

Some prior arts:

  • OPENSSL_NO_VENDOR in openssl crate, which serves the exact
    purpose of LIBGIT2_NO_VENDOR 1.
  • Crate ssh2 has a similar LIBSSH2_SYS_USE_PKG_CONFIG, but it
    doesn't force to do so. It still falls back to vendored lib 2.
  • Crate curl has a feature force-system-lib-on-osx to trump all
    others features. It was created primarily for Rust releases 3.

Footnotes

  1. https://github.com/sfackler/rust-openssl/blob/50787ed35bf9efa9dd3cbb1993a2564014b67489/openssl/src/lib.rs#L65

  2. https://github.com/alexcrichton/ssh2-rs/blob/d9a1dfac4b8c09c5437eb477606b82aa4f67b092/libssh2-sys/build.rs#L22-L33

  3. https://github.com/alexcrichton/curl-rust/blob/431babf1dffe205641793353d3d57fdd36fe8534/curl-sys/build.rs#L15-L20

@weihanglo
Copy link
Member Author

This is spun off from #939 with more contexts.

In addition, we may want to build a convention for this kind of *-sys. Some ideas people have shared with me:

  • CARGO_BUILD_NO_VENDOR which if set prevents any vendoring
  • CARGO_BUILD_MYCRATE_NO_VENDOR which if set prevents vendoring in mycrate.
  • no-vendor feature in sys crates trumps vendored feature.

@weihanglo
Copy link
Member Author

More motivations why this is something we want:

  • We have no control over transitive dependencies that accidentally activate the vendored feature. And there is no way to turn it off.
  • If the build environment has the wrong system libgit2, we want a build to always fail because that is a real mistake. For example, libgit2 expects a range 1.6.4..1.7.0 and it silently goes vendoring if nothing found. We may want an explicit error like "your system needs a newer libgit2 to proceed".

@ehuss
Copy link
Contributor

ehuss commented Jul 28, 2023

OK, sorry about the delay. Can you rebase and fix the conflicts?

Specify `LIBGIT2_NO_VENDOR` to force to use system libgit2.

Due to the additive nature of Cargo features, if some crate in the
dependency graph activates `vendored` feature, there is no way to
revert it back. This env var serves as a workaround for this purpose.

An alternative is having `no_vendored` feature. We still need to
modify `build.rs` to make both `vendored` and `no_vendored` play
nice with each other (or bail out as they are mutual-exclusive).
However, there is no way to activate a Cargo feature via environment
variable (see rust-lang/cargo#4829). Altering environment variables
may be the only way to interact with some external build systems.

It is also pretty common that people don't want to vendor anything and
rather to see the build fail than vendoring.

Some prior arts:

* `OPENSSL_NO_VENDOR` in `openssl` crate, which serves the exact
  purpose of `LIBGIT2_NO_VENDOR` [^1].
* Crate `ssh2` has a similar `LIBSSH2_SYS_USE_PKG_CONFIG`, but it
  doesn't force to do so. It still falls back to vendored lib [^2].
* Crate `curl` has a feature `force-system-lib-on-osx` to trump all
  others features. It was created primarily for Rust releases [^3].

More motivations why this is something we want:

* We have no control over transitive dependencies that accidentally
  activate the vendored feature. And there is no way to turn it off.
* If the build environment has the wrong system libgit2, we want
  build to always fail because that is a real mistake. For example,
  libgit2 expects a range 1.6.4..1.7.0 and it silently goes vendoring
  if nothing found. We may want an explicit error like "your system
  needs a newer libgit2 to proceed".

[^1]: https://github.com/sfackler/rust-openssl/blob/50787ed35bf9efa9dd3cbb1993a2564014b67489/openssl/src/lib.rs#L65
[^2]: https://github.com/alexcrichton/ssh2-rs/blob/d9a1dfac4b8c09c5437eb477606b82aa4f67b092/libssh2-sys/build.rs#L22-L33
[^3]: https://github.com/alexcrichton/curl-rust/blob/431babf1dffe205641793353d3d57fdd36fe8534/curl-sys/build.rs#L15-L20
@weihanglo
Copy link
Member Author

Thanks for the review. Updated. Let me know if you have any concern.

Copy link
Contributor

@ehuss ehuss left a comment

Choose a reason for hiding this comment

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

Thanks!

@ehuss ehuss added this pull request to the merge queue Jul 29, 2023
Merged via the queue into rust-lang:master with commit 40c8cf3 Jul 29, 2023
2 of 8 checks passed
@weihanglo weihanglo deleted the libgit2-no-vendor branch July 29, 2023 22:36
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.

2 participants