2024 implicit feature migration deletes dependency features about cargo HOT 8 CLOSED

When there is a ["foo/feature_name"] syntax in the [features] table, and dep:foo isn't found anywhere in the [features] table, it should be rewritten to be ["dep:foo", "foo/feature_name"]

Why does it need to be re-written, rather than ignored? I thought "foo/feature_name" was "dep:foo", "foo?/feature_name".

from cargo.

I think it is the same in spirit, but just serde/derive fails in the new edition with:

error: failed to parse manifest at `/Users/eric/Temp/foo/Cargo.toml`

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

To be honest, I don't understand that error "is not a dependency". I'm guessing since the implicit feature is missing, perhaps it just fails to look it up in the features hash map? I have not looked.

from cargo.

The error is because the implementation strips unused optional dependencies.

I'll look into this to make sure we correctly detect what should be unused dependencies and update things accordingly.

from cargo.

Huh, this is surprising

[package]
name = "cargo-14010"
version = "0.1.0"
edition = "2021"

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

[features]

{
  "serde": [
    "dep:serde"
  ]
}

foo = ["dep:serde"]

{
  "foo": [
    "dep:serde"
  ]
}

foo = ["serde/derive"]

{
  "foo": [
    "serde/derive"
  ],
  "serde": [
    "dep:serde"
  ]
}

foo = ["dep:serde", "serde?/derive"]

{
  "foo": [
    "dep:serde",
    "serde?/derive"
  ]
}

So "dep_name/feature_name" is different from "dep:dep_name", "dep_name?/feature_name" in that the implicit feature doesn't get suppressed.

I'm guessing we had looked at the code when implementing the lint and stuff but I partially copied it but didn't think it through but went off of my incorrect intuition.

from cargo.

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?

from cargo.

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.

from cargo.

So "dep_name/feature_name" is different from "dep:dep_name", "dep_name?/feature_name" in that the implicit feature doesn't get suppressed.
Yes, I suppose so.

And then the most important step is:
Then If the dep_name/dep_name_feature or dep_name?/dep_name_feature exists, then we should keep them when applying Cargo fix ?

from cargo.

I've split out the general cargo behavior questions in #14015 and #14016 so this can focus on just cargo fix.

My thought is that we can fix this as suggested (adding dep:dep_name) for now and if we deprecate dep_name/feature_name then the fix for that can run on top of the fix for this.

from cargo.