rust-bindgen
rust-bindgen copied to clipboard
Disable feature logging and clap on default
This crate is very popular and depends by a lot of other crates, which uses the default feature set of this crate.
However, by default, bindgen pulls in clap and logging, two features that never get used when using as a build-dev but just waste more time to compile.
Thus I remove it from the default feature set.
Signed-off-by: Jiahao XU [email protected]
Hmm, I am not sure we can do this.
cargo clean
cargo build
cargo run
results in:
error: target `bindgen` in package `bindgen` requires the features: `clap`
Consider enabling them by passing, e.g., `--features="clap"`
I do not think we can require people to pass --features
flags in order to get the bindgen
binary.
I think this PR would need https://github.com/rust-lang/cargo/issues/1982#issuecomment-766433889 to be resolved first.
https://github.com/rust-lang/rust-bindgen/blob/18367d5af64c64035104e86c2bca6c3667389d79/Cargo.toml#L53-L54
@kulp I don't think that can is a valid reason to keep the existing default feature set.
Looking at the dependents of bindgen
on crates.io, it is clear that almost all softwares on page 1-30 that use bindgen
's default feature set, including tokio::io-uring
, zstd
and etc.
This means bindgen
is slowing the entire ecosystem down by requiring them to build unused dependencies, which IMHO is definitely more important than breaking people building/running bindgen
binary on their own.
It will be great if cargo
supports enabling specific feature set when building binaries, but even without it, I would say we should remove clap
from default feature set.
I think this PR would need https://github.com/rust-lang/cargo/issues/1982#issuecomment-766433889 to be resolved first.
Thanks!
I am glad that there is an issue to resolve this!
@emilio @kulp would it be the worst idea in the world considering a possible split of bindgen
into the regular build-script bindgen
as a library and a bindgen-cli
crate that uses the former? It would be possible to put both crates under the same workspace.
That seems reasonable, though may need some wrangling to get the tests working, since they use the cli args.
@emilio I'm going to do a PoC next week and submit it to see if we can find a sweet spot for this.
Edit: opened https://github.com/rust-lang/rust-bindgen/pull/2284 as an alternative to this.
:umbrella: The latest upstream changes (presumably #2284) made this pull request unmergeable. Please resolve the merge conflicts.
Closing this in favor of #2284. Feel free to reopen if you consider that something from here is missing there.