Bad error message when a strong dep features's depedency is an unused optional dependency #14016

epage · 2024-06-05T16:47:52Z

Problem

If nothing can activate an optional dependency, cargo acts as if the dependency doesn't exist which creates poor error messages.

Also, with how things are arranged, the unused optional dependency lint doesn't get reported which could at least reduce the burden on the error message.

Steps

Baseline Cargo.toml:

cargo-features = ["edition2024"]
[package]
name = "cargo-14010"
version = "0.1.0"
edition = "2024"

[dependencies]
serde = { version = "1.0.203", optional = true }

$ cargo  nightly check -Zcargo-lints
warning: unused optional dependency
 --> Cargo.toml:8:1
  |
8 | serde = { version = "1.0.203", optional = true }
  | -----
  |
  = note: `cargo::unused_optional_dependency` is set to `warn` by default
  = help: remove the dependency or activate it in a feature with `dep:serde`
    Finished `dev` profile [unoptimized   debuginfo] target(s) in 0.00s

Add the following to Cargo.toml:

[features]
serde = ["serde/derive"]

$ cargo  nightly check -Zcargo-lints
error: failed to parse manifest at `/home/epage/src/personal/dump/cargo-14010-1/Cargo.toml`

Caused by:
  feature `serde` includes `serde/derive`, but `serde` is not a dependency

Possible Solution(s)

Improve the error message
Implicitly create dep:serde if serde/derive is used
Deprecate serde/derive syntax, removing it in a new edition (Transition out weak feature syntax in an edition #10556)

Notes

This was split out of #14015 assuming we'll go with a different solution than it

Version

No response

The text was updated successfully, but these errors were encountered:

epage · 2024-06-05T16:49:21Z

#14010 (comment)

This makes me worried about dep_name/feature_name syntax on Edition 2024. It now requires also adding dep:dep_name at which point it isn't much different than dep_name?/feature_name.

I had missing this part in past discussions (e.g. this zulip topic). It seems like we should have either changed it in Edition 2024 to not need dep:dep_name or transition out the syntax according to #10556. I worry that changing the meaning of dep_name/feature_name would be too subtle. Maybe we should at least go forward with the deprecation of dep_name/feature_name as mentioned in the comments of #10556?

epage · 2024-06-05T16:49:34Z

#14010 (comment)

Also, another reason why deprecating implicit features doesn't play well with dep_name/feature_name is that
[features]
foo = ["dep:dep_name"]
bar = ["dep_name/feature_name"]
and
[features]
bar = ["dep:dep_name", "dep_name/feature_name"]
are valid while the following is not
[features]
bar = ["dep_name/feature_name"]
That inconsistency feels weird.

epage · 2024-06-11T15:43:43Z

We discussed this in today's team meeting and lean towards "dep_name/feature_name" implying "dep:dep_name" in Edition 2024 with it being normalized in the published Cargo.toml file.

fix(fix): Address problems with implicit -> explicit feature migration ### What does this PR try to resolve? Within the scope of `cargo fix` there are two problems - We wipe out existing feature activations if it has the same name as an optional dependency - The `Cargo.toml` isn't parseable because the unused optional dependency won't "exist" if just `dep_name/feature_name` is used Fixes #14010 ### How should we test and review this PR? As for the unused optional dependency not "existing" error, - #14015 is for improving the message for weak dep features - #14016 is for re-evaluating how we handle this for strong dep features Depending on what solution we go with for #14016, we might want to revisit the second migration within this PR. This is one reason I made the commit separate (in addition to just making it clearer whats happening as this gets into some finer details of features). ### Additional information

This doesn't revert the last commit of rust-lang#14018 so that it works properly on Editions 2021 and 2024. Fixes rust-lang#14016

fix: Ensure dep/feature activates the dependency on 2024 ### What does this PR try to resolve? Fixes #14016 ### How should we test and review this PR? This doesn't revert the last commit of #14018 so that it works properly on Editions 2021 and 2024. ### Additional information

Fixes rust-lang#14283 by re-opening rust-lang#14016 so we don't block people testing other Edition 2024 changes. This reverts commit 99fae91.

Revert "fix: Ensure dep/feature activates the dependency on 2024" ### What does this PR try to resolve? Fixes #14283 by re-opening #14016 so we don't block people testing other Edition 2024 changes. ### How should we test and review this PR? This reverts commit 99fae91. I initially held off on reverting in case we quickly had a clear direction to go. Since we're still doing some investigation, I decided to move forward with this. ### Additional information

epage · 2024-07-24T14:04:54Z

Re-opened due to #14295. See #14283 for more context.

`all-is-cubes-port` is used only by the glTF test, so the `gltf` feature should enable it and there's no need for enabling specific features. This is a workaround for <rust-lang/cargo#14016> affecting my testing against nightly & edition 2024. I'm not sure whether I prefer it overall.

epage · 2024-10-01T17:00:55Z

When we tried to fix this with #14221, we caused #14283.

The following content is adapted from #14283 and https://blog.rust-lang.org/inside-rust/2024/08/15/this-development-cycle-in-cargo-1.81.html#removing-implicit-features

Let's step back to the beginning of how all of this works.

package.edition = "2021"
[features]
feature = ["dep/feature"]
[dependencies]
dep = { version = "1", optional = true }

will cause a dep implicit feature to be created

As we want to suppress implicit features in 2024, we strip all optional dependencies like that. This led to a new problem:

package.edition = "2024"
[features]
feature = ["dep/feature"]
[dependencies]
dep = { version = "1", optional = true }

As dep is stripped, this fails because dep/feature can't be resolved. The user can workaround this by adding dep:dep (discoverability is poor).

We solved this by implicitly adding dep:dep to feature. However, I forgot to check dependencies.dep.optional, so if it isn't optional, it also fails but in a way the user can't workaround.

Logically, checking dependencies.dep.optional gets circular because

resolve_features needs to know about resolved dependencies to add dep:dep
resolve_dependencies needs to know about resolved features to know what dep:dep are present

We could break this cycle by making a lot of assumptions about the unresolved forms.

If we solved this now, we would be stuck with it forever. In examining the potential alternatives, it seemed better to design a more holistic solution rather than being stuck with the maintenance burden that solving this now would cause.

Other options include

Reworking what dep_name/feature_name de-sugars to, including making dep:dep_name work with optional dependencies
Change dep_name?/feature_name to work with required dependencies and switch to it

Whatever we design, we need to keep in mind that "dep_name/feature_name" is more like "dep_name"?, "dep:dep_name"?, "dep_name?/feature_name"

It activates a feature of the same name as dep_name, if present
It activates dep:dep_name, if optional
It activates the feature_name inside of dep_name

Due to problems we ran into with rust-lang#14016, we're removing implicit features from the 2024 edition to give ourselves more time to design it as we should. I could have added a new flag for this or made an EditionNext but I decided to remove it in the hopes to avoid any path dependency in solving this the next time.

fix: Remove implicit feature removal ### What does this PR try to resolve? Due to problems we ran into with #14016, we're removing implicit features from the 2024 edition to give ourselves more time to design it as we should. ### How should we test and review this PR? ### Additional information I could have added a new flag for this or made an EditionNext but I decided to remove it in the hopes to avoid any path dependency in solving this the next time.

epage added C-bug Category: bug A-features Area: features — conditional compilation A-editions Area: edition-specific issues S-needs-team-input Status: Needs input from team on whether/how to proceed. labels Jun 5, 2024

epage mentioned this issue Jun 5, 2024

Bad error message when a weak dep features's depedency is an unused optional dependency #14015

Closed

This was referenced Jun 5, 2024

2024 implicit feature migration deletes dependency features #14010

Closed

Tracking Issue for Rust 2024: Cargo remove implicit features rust-lang/rust#123750

Closed

fix(fix): Address problems with implicit -> explicit feature migration #14018

Merged

epage added a commit to epage/cargo that referenced this issue Jul 9, 2024

fix: Ensure dep/feature activates the dependency on 2024

cce530b

This doesn't revert the last commit of rust-lang#14018 so that it works properly on Editions 2021 and 2024. Fixes rust-lang#14016

epage mentioned this issue Jul 9, 2024

fix: Ensure dep/feature activates the dependency on 2024 #14221

Merged

epage added a commit to epage/cargo that referenced this issue Jul 10, 2024

fix: Ensure dep/feature activates the dependency on 2024

336e1cf

This doesn't revert the last commit of rust-lang#14018 so that it works properly on Editions 2021 and 2024. Fixes rust-lang#14016

bors closed this as completed in 99fae91 Jul 10, 2024

epage mentioned this issue Jul 22, 2024

Activation of features of non-optional dependencies breaks manifest for 2024 edition #14283

Closed

epage added a commit to epage/cargo that referenced this issue Jul 24, 2024

Revert "fix: Ensure dep/feature activates the dependency on 2024"

407a32c

Fixes rust-lang#14283 by re-opening rust-lang#14016 so we don't block people testing other Edition 2024 changes. This reverts commit 99fae91.

epage mentioned this issue Jul 24, 2024

Revert "fix: Ensure dep/feature activates the dependency on 2024" #14295

Merged

epage reopened this Jul 24, 2024

epage mentioned this issue Aug 7, 2024

Update feature docs for no implicit features in 2024 #14011

Closed

ehuss mentioned this issue Sep 11, 2024

Tracking Issue for remove-implicit-feature RFC 3491 #12826

Open

epage mentioned this issue Oct 1, 2024

fix: Remove implicit feature removal #14630

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bad error message when a strong dep features's depedency is an unused optional dependency #14016

Bad error message when a strong dep features's depedency is an unused optional dependency #14016

epage commented Jun 5, 2024

epage commented Jun 5, 2024

epage commented Jun 5, 2024

epage commented Jun 11, 2024

epage commented Jul 24, 2024

epage commented Oct 1, 2024

Bad error message when a strong dep features's depedency is an unused optional dependency #14016

Bad error message when a strong dep features's depedency is an unused optional dependency #14016

Comments

epage commented Jun 5, 2024

Problem

Steps

Possible Solution(s)

Notes

Version

epage commented Jun 5, 2024

epage commented Jun 5, 2024

epage commented Jun 11, 2024

epage commented Jul 24, 2024

epage commented Oct 1, 2024