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

Invalid Cargo.toml when using dep: namespaced features #171

Closed
Anders429 opened this issue May 19, 2022 · 6 comments · Fixed by #195
Closed

Invalid Cargo.toml when using dep: namespaced features #171

Anders429 opened this issue May 19, 2022 · 6 comments · Fixed by #195

Comments

@Anders429
Copy link

Below is an attempt at creating a minimal reproducible example of this bug.

In rustc 1.60.0, namespaced dependencies were added that made the following Cargo.toml manifest possible:

[package]
name = "foo"
version = "0.1.0"
edition = "2021"

[dependencies]
hashbrown = "0.12.0"
rayon = {version = "1.5.1", optional = true}

[dev-dependencies]
trybuild = "1.0.61"

[features]
rayon = ["dep:rayon", "hashbrown/rayon"]

However, when adding a simple trybuild test, like the following:

tests/trybuild.rs

#[test]
fn test() {
    trybuild::TestCases::new().compile_fail("tests/ui/test.rs");
}

With tests/ui/test.rs simply containing:

fn main() {}

Running the tests results in the following:

$ cargo t

[...]

running 1 test
error: failed to parse manifest at `[...]/target/tests/foo/Cargo.toml`

Caused by:
  optional dependency `rayon` is not included in any feature
  Make sure that `dep:rayon` is included in one of features in the [features] table.
test test ... FAILED

[...]

If you look at target/tests/foo/Cargo.toml, it is as follows:

[package]
name = "foo-tests"
version = "0.0.0"
edition = "2021"
publish = false

[features]
rayon = ["foo/rayon"]
[dependencies.foo]
path = "[...]/foo/"
default-features = false

[dependencies.hashbrown]
version = "0.12.0"

[dependencies.rayon]
version = "1.5.1"
optional = true

[[bin]]
name = "foo-tests"
path = "main.rs"

[[bin]]
name = "trybuild000"
path = "[...]/foo/tests/ui/test.rs"

[workspace]

It seems the problem is that rayon = ["foo/rayon"] causes cargo to not use an implicit rayon = ["dep:rayon"], which means dep:rayon is never included in any feature, hence the error. Changing the line to rayon = ["foo/rayon", "dep:rayon"] solves the problem and allows the crate to compile.

I'm not exactly sure how to go about solving this generally. It seems all of the dependencies and features are included to allow the test crate to use them. I wonder if it would work to copy the list of additional features to enable for each feature into the new Cargo.toml file that is generated. This would retain the dep: features, while also not enabling any extra features, since those same features are already being enabled by the original crate itself.

kupiakos added a commit to kupiakos/open-enum that referenced this issue Sep 10, 2022
You can either use `std::os::raw::c_int` or `libc::c_int`.

Unfortunately, the feature name is awkward to work around
dtolnay/trybuild#171.
kupiakos added a commit to kupiakos/open-enum that referenced this issue Sep 10, 2022
You can either use `std::os::raw::c_int` or `libc::c_int`.

Unfortunately, the feature name is awkward to work around
dtolnay/trybuild#171.
@jhpratt
Copy link

jhpratt commented Oct 6, 2022

I'd love it if this were fixed! I recently updated time to use weak and namespaced dependencies, and now I'm looking at switching back to compiletest_rs, which is unfortunate.

@jhpratt
Copy link

jhpratt commented Oct 6, 2022

Well that was quick. Thank you @dtolnay!

@jhpratt
Copy link

jhpratt commented Oct 6, 2022

Actually, it looks like this does miss one case. If a feature has the same name as the namespaced feature, it fails with the same error.

[features]
rand = ["dep:rand"]

[dependencies]
rand = { version = "0.8", optional = true }
Caused by:
  feature `rand` includes `dep:rand`, but `rand` is not an optional dependency
  A non-optional dependency of the same name is defined; consider adding `optional = true` to its definition.

@dtolnay
Copy link
Owner

dtolnay commented Oct 6, 2022

That should work. Is it maybe if an optional dependency has the same name as a non-optional dev-dependency?

@jhpratt
Copy link

jhpratt commented Oct 6, 2022

Yeah, that is the case. I was typing that off-hand without thinking that it could be more complicated. Specifically I was testing the current head of time-rs/time (with RUSTFLAGS="--cfg __ui_tests").

Edit: Just tried 1.0.68, and it works. Thanks again!

@Anders429
Copy link
Author

@dtolnay thanks, #195 resolves my use-case as well. Much appreciated! :)

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 a pull request may close this issue.

3 participants