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

Resolution of dependencies should not be affected by the dependencies of procedural-macro-library. #5760

Closed
andylokandy opened this issue Jul 21, 2018 · 1 comment

Comments

@andylokandy
Copy link

andylokandy commented Jul 21, 2018

Currently, the dependencies of procedural-macro-library would change the resolution of normal dependency of root package, which accidentally broke some crates and made them unusable in no_std environment.

For example:

Case A:

[package]
name = "demo"
version = "0.1.0"

[dependencies]
num-traits = { version = "0.2", default-features = false} 
// lib.rs

#![no_std]

extern crate num_traits;
> rustup target add thumbv7m-none-eabi
> cargo build --target thumbv7m-none-eabi
    Finished dev [unoptimized + debuginfo] target(s) in 0.02s

It compiled as expected; the default features is turned off, so that the std feature of num-traits is also removed.

Case B:

[package]
name = "demo"
version = "0.1.0"

[dependencies]
num-derive = "0.2"
// lib.rs

#![no_std]

#[macro_use]
extern crate num_derive;
PS K:\Code\Playground\fail> cargo build --target thumbv7m-none-eabi
   Compiling demo v0.1.0 (file:///K:/Code/Playground/fail)
   (...unused_imports warning...)

    Finished dev [unoptimized + debuginfo] target(s) in 0.66s

It also compiled as expected, though the num_derive depends on non-no-std libraries, like syn, quote and the num_traits with std feature. It's because num_derive is proc-macro-library and it's not going to be linked, which means the num_derive should only work at compile time. Thus, it is not constrained by no_std environment. In other words, the no_std environment should not be affected by proc-macro and no_link crates as well.

Case C:

[package]
name = "demo"
version = "0.1.0"

[dependencies]
num-derive = "0.2"
num-traits = { version = "0.2", default-features = false} 
// lib.rs

#![no_std]

#[macro_use]
extern crate num_derive;
extern crate num_traits;
> cargo build --target thumbv7m-none-eabi
   Compiling num-traits v0.2.5
error[E0463]: can't find crate for `std`====================>           ] 9/11: num-traits
  --> C:\Users\Andy\.cargo\registry\src\github.aaakk.us.kg-1ecc6299db9ec823\num-traits-0.2.5\src\lib.rs:23:1
   |
23 | extern crate std;
   | ^^^^^^^^^^^^^^^^^ can't find crate
   |
   = note: the `thumbv7m-none-eabi` target may not be installed

error: aborting due to previous error

For more information about this error, try `rustc --explain E0463`.
error: Could not compile `num-traits`.

To learn more, run the command again with --verbose.

In this case, rustc complians that the num-traits with std feature is used in no_std environment. Which is weird because we have required Cargo to remove it! If we build it by verbose, we can see the std feature is passed to rustc!

I guess it is because the resolution of proc-marco-library, num-derive, changed the feature resolution of num-traits in root. This problem may also arise in no_link library.

@andylokandy andylokandy changed the title Resolution of the dependencies of procedural-macro-library should not affects the normal library. Resolution of dependencies should not be affected by the dependencies of procedural-macro-library. Jul 21, 2018
@Eh2406
Copy link
Contributor

Eh2406 commented Jul 21, 2018

Thanks for letting us know about this problem. I agree that it is a problem and one that needs to be fixed. The current discussion of how is at #5730. Unfortunately it is a rather big braking change to how features were originally designed to work, so it will take some time to figure out.

But let's have the conversation in #5730 and mark this as a duplicate.

0ndorio added a commit to 0ndorio/hashbrown that referenced this issue Oct 30, 2019
Disables the default features of `ahash` and reintroduces them
through a new feature called `ahash-compile-time-rng`, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
`hashbrown` with `ahash` as default hasher and to disable the
`compile-time-rng` at the same time.

This might be useful for any dependend crate targeting `no_std`,
which contains `rand` or `rand_core` somewhere inside the dependency
tree as a bug in cargo accidently enables the underlying `getrandom`
feature if `compile-time-rng` is enabled [1].

... fixes rust-lang#124

[1] rust-lang/cargo#5760
0ndorio added a commit to 0ndorio/hashbrown that referenced this issue Oct 30, 2019
Disables the default features of `ahash` and reintroduces them
through a new feature called `ahash-compile-time-rng`, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
`hashbrown` with `ahash` as default hasher and to disable the
`compile-time-rng` at the same time.

This might be useful for any dependent crate targeting `no_std`,
which contains `rand` or `rand_core` somewhere inside the dependency
tree as a bug in cargo accidentally enables the underlying `getrandom`
feature if `compile-time-rng` is enabled [1].

... fixes rust-lang#124

[1] rust-lang/cargo#5760
0ndorio added a commit to 0ndorio/hashbrown that referenced this issue Oct 30, 2019
Disables the default features of `ahash` and reintroduces them
through a new feature called `ahash-compile-time-rng`, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
`hashbrown` with `ahash` as default hasher and to disable the
`compile-time-rng` at the same time.

This might be useful for any dependent crate targeting `no_std`,
which contains `rand` or `rand_core` somewhere inside the dependency
tree as a bug in cargo accidentally enables the underlying `getrandom`
feature if `compile-time-rng` is enabled [1].

... fixes rust-lang#124

[1] rust-lang/cargo#5760
bors added a commit to rust-lang/hashbrown that referenced this issue Oct 31, 2019
Introduce `ahash-compile-time-rng` feature.

**Content**

Disables the default features of `ahash` and reintroduces them
through a new feature called `ahash-compile-time-rng`, which is
enabled by default.

The new feature makes it possible for depended crates to rely on
`hashbrown` with `ahash` as default hasher and to disable the
`compile-time-rng` at the same time.

This might be useful for any depended crate targeting `no_std`,
which contains `rand` or `rand_core` somewhere inside the dependency
tree as a bug in cargo accidentally enables the underlying `getrandom`
feature if `compile-time-rng` is enabled [1].

... fixes #124

[1] rust-lang/cargo#5760

---

**Warnings**

 (1) Compiling `ahash` with disabled `compile-time-rng` feature is currently broken and requires tkaitchuck/aHash#25 to be merged in advance.

 (2) This introduces a hidden behavior change for all dependent crates, using hashbrown with `default-features = false` and `features = 'ahash'`. This happens as the `ahash` feature no longer implicitly enables the `compile-time-rng` feature of `ahash`.

---

Is the naming of the feature okay?  Do I need to add any additional  changes?
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

No branches or pull requests

2 participants