Breaking change: `cargo metadata` no longer distinguishes between "magical" features from optional deps and normal features

Manishearth commented 2 years ago

Problem

In the past, optional dependencies in Cargo would automatically generate a feature. This would not turn up in cargo metadata; tools were supposed to use the fact that they are optional dependencies to deduce this.

Now, with the advent of dep: syntax, optional dependencies automatically add a "foo" = ["dep:foo"] key. It is no longer possible to distinguish between magical optional features and explicit features. This is means that the user's intent is lost, and tools like cargo-all-features can no longer work well.

For example, cargo-all-features has a skip_optional_dependencies flag that allows one to only test permutations of explicitly defined features. This behavior has silently changed on 1.60, making the tool suddenly start taking forever in CI (see https://github.com/frewsxcv/cargo-all-features/issues/29)

Steps

Run cargo metadata on any crate with optional dependencies before and after 1.60, and see the difference.

Possible Solution(s)

No response

Notes

I personally consider this a breaking change to the metadata; but even if we're okay with that level of change I do think that there should be some way to distinguish between user-supplied and magically-created deps, and ideally some way to do this that allows one to distinguish between the old format and the new one

Version

cargo 1.58.0 (f01b232bc 2022-01-19)
release: 1.58.0
commit-hash: f01b232bc7f4d94f0c4603930a5a96277715eb8c
commit-date: 2022-01-19
host: x86_64-unknown-linux-gnu
libgit2: 1.3.0 (sys:0.13.23 vendored)
libcurl: 7.80.0-DEV (sys:0.4.51+curl-7.80.0 vendored ssl:OpenSSL/1.1.1l)
os: Debian rodete (rodete) [64-bit]

ehuss commented 2 years ago

I'm sorry that broke the usage of your tool. I understand it can be frustrating. The thinking behind the change was that the features table now more accurately reflects the actual features that the package has. There are some tools where that makes things easier, because before tools would need to awkwardly check both the features table and the dependencies.

I'd like to try to discuss what some alternatives might be. There are some concerns if we change things around too much that can be even more disruptive, so I'm not sure if there is a solution that would be worth pursuing. We chatted about this in our meeting and here are some notes:

I feel like distinguishing between implicit and explicit features isn't something I'd like to emphasize. Implicit features are real features, and there are legitimate reasons for treating them as first-class. For example, if you have a serde optional dependency, and you want that exposed as a feature, I'd want to avoid forcing the user to add more ceremony to do so.
Many packages can't opt-in to the new dep: syntax due to MSRV, so they can't "hide" optional dependencies yet.
No way to explicitly mark optional dependencies as "private" (except for public/private dependencies, which probably won't make progress any time soon).
There could be two fields in the metadata, one with the "[features]" table exactly as written in Cargo.toml (the old behavior), and one with the "computed" features (the new behavior). However, I'd like to drill into why we would do this, as I still don't think this is a way we really want to use to "hide" optional dependencies.
- There might be various other things where we want to expose "raw" data from Cargo.toml. Not sure what those would be, perhaps some related to workspace inheritance.
- Could also add a field that lists which features were "implicit". Or which were "explicit".
- This might be too much information. We could have some way to query exactly what you want.
Bump --format-version to 2 for this change. This I think would be pretty disruptive, and so I think we should be very careful about doing it.
It might be difficult to make too many changes due to limited dev time, and it might be several releases before it can land on stable.

If you have ideas or thoughts on how to proceed, please let me know.

Manishearth commented 2 years ago

I feel like distinguishing between implicit and explicit features isn't something I'd like to emphasize. Implicit features are real features, and there are legitimate reasons for treating them as first-class. For example, if you have a serde optional dependency, and you want that exposed as a feature, I'd want to avoid forcing the user to add more ceremony to do so.

So, I feel like yes, at a mechanical level they are similar, but the intent differs, depending on the context, and good tools need to know programmer intent.

Could also add a field that lists which features were "implicit". Or which were "explicit".

I think this would actually suffice! Part of the problem is that tools like cargo-all-features at the moment cannot distinguish between old-style and new-style output; to "fix" this in cargo-all-features I had to hack up a thing that would search for dep: features.

Such a list would make it pretty easy for such tools to proceed but remain compatible.

rust-lang / cargo