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

Request: Add a crate_feature to disable macros that depend on cargo env vars #783

Closed
acmcarther opened this issue Dec 24, 2016 · 4 comments
Labels
C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.

Comments

@acmcarther
Copy link

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 using Cargo. I'm actually experimenting with Bazel to build my Rust projects -- but clap 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!

@kbknapp
Copy link
Member

kbknapp commented Dec 27, 2016

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 default-features and are using crate_*! macros. The problem is that would potentially be a silent breaking change.

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?

@kbknapp kbknapp added C: macros M-breaking-change Meta: Implementing or merging this will introduce a breaking change. C-enhancement Category: Raise on the bar on expectations labels Dec 27, 2016
@kbknapp kbknapp added this to the 3.0 milestone Dec 27, 2016
@acmcarther
Copy link
Author

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.

@nabijaczleweli
Copy link
Contributor

nabijaczleweli commented Dec 27, 2016

do worry that there are people who have opted out of default-features and are using crate_*! macros.

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

homu added a commit that referenced this issue Dec 27, 2016
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
homu added a commit that referenced this issue Dec 27, 2016
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
@jsgf
Copy link

jsgf commented Feb 10, 2017

This might cover the same territory: rust-lang/rust#39656

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Raise on the bar on expectations M-breaking-change Meta: Implementing or merging this will introduce a breaking change.
Projects
None yet
Development

No branches or pull requests

4 participants