-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Request: Add a crate_feature to disable macros that depend on cargo env vars #783
Comments
Wow, I wasn't aware of Bazel, that's interesting. Although I'm not against adding these to a feature, and simply adding said feature to the defaults but I do worry that there are people who have opted out of Because of that concern, I think the best way forward is to add this to the 3x timeline, which hopefully isn't too far off (waiting on Macros 1.1). I believe all you should have to do on your end is export those environmental variables while building, and it should work? |
No worries, thanks for the quick response! Exporting fake values during my build step solves my problem and is no trouble for any length of time. |
Valid, but unnecessary concern. Consider: [features]
no_cargo = [] Then: #[cfg(not(feature="no_cargo"))]
macro_rules! crate_authors {
// magic
} That way you need an explicit opt-in to remove the macros, granting full backwards compatibility |
Add no_cargo feature to disable Cargo-env-var-dependent macros For example, given: ```toml clap = { path = "t:/clap-rs" } ``` The macros `crate_version!()` and `crate_authors!()` exist, so the crate compiles without errors: ``` Compiling https v0.2.0 (file:///P:/Rust/http) Finished debug [unoptimized + debuginfo] target(s) in 6.93 secs Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs [Finished in 7.5s] ``` But, adding the `no_cargo` feature: ```toml clap = { path = "t:/clap-rs", features = ["no_cargo"] } ``` The macros are removed, so the crate fails to compile: ``` Compiling clap v2.19.2 (file:///T:/clap-rs) Compiling https v0.2.0 (file:///P:/Rust/http) error: macro undefined: 'crate_version!' --> src\options.rs:40:22 | 40 | .version(crate_version!()) | ^^^^^^^^^^^^^ error: macro undefined: 'crate_authors!' --> src\options.rs:41:21 | 41 | .author(crate_authors!()) | ^^^^^^^^^^^^^ error: aborting due to 2 previous errors error: Could not compile `https`. ``` Closes #783
Add no_cargo feature to disable Cargo-env-var-dependent macros For example, given: ```toml clap = { path = "t:/clap-rs" } ``` The macros `crate_version!()` and `crate_authors!()` exist, so the crate compiles without errors: ``` Compiling https v0.2.0 (file:///P:/Rust/http) Finished debug [unoptimized + debuginfo] target(s) in 6.93 secs Finished debug [unoptimized + debuginfo] target(s) in 0.0 secs [Finished in 7.5s] ``` But, adding the `no_cargo` feature: ```toml clap = { path = "t:/clap-rs", features = ["no_cargo"] } ``` The macros are removed, so the crate fails to compile: ``` Compiling clap v2.19.2 (file:///T:/clap-rs) Compiling https v0.2.0 (file:///P:/Rust/http) error: macro undefined: 'crate_version!' --> src\options.rs:40:22 | 40 | .version(crate_version!()) | ^^^^^^^^^^^^^ error: macro undefined: 'crate_authors!' --> src\options.rs:41:21 | 41 | .author(crate_authors!()) | ^^^^^^^^^^^^^ error: aborting due to 2 previous errors error: Could not compile `https`. ``` Closes #783
This might cover the same territory: rust-lang/rust#39656 |
The crate_*! macros make a fairly valid assumption that consumers use
cargo
to build Rust projects by expecting certain environment variables to be present. Unfortunately, I am one of the very few not usingCargo
. I'm actually experimenting with Bazel to build my Rust projects -- butclap
fails to compile because bazel is not providing the expected environment variables.As of posting, the macros causing me issues are "crate_version!" and "crate_authors!", which seem to be provided as a convenience to
clap
users and aren't used internally.If this proposal is rejected, I do have mitigation options -- faking those values or patching clap locally -- so please reject if you feel that not enough users are impacted.
Thanks!
The text was updated successfully, but these errors were encountered: