rust-lang / cargo

The Rust package manager
https://doc.rust-lang.org/cargo
Apache License 2.0
12.73k stars 2.42k forks source link

`cargo metadata` should be able to ignore dev-dependencies for better feature resolution #10718

Open sffc opened 2 years ago

sffc commented 2 years ago

Problem

cargo metadata includes both normal dependencies and dev-dependencies, distinguishing them with either "kind": null or "kind": "dev" in the dependency list for a package.

The problem is when there is a package that is used as both a normal dependency and as a dev-dependency with different sets of features. For example, consider the following packages:

# package_a/Cargo.toml
[dependencies]
package_b = { path = "../package_b" }

# package_b/Cargo.toml
[dependencies]
package_c = { path = "../package_c" }
[dev-dependencies]
package_d = { path = "../package_d", features = ["dev_only"] }

# package_c/Cargo.toml
[dependencies]
package_d = { path = "../package_d" }

# package_d/Cargo.toml
[dependencies]
some_big_dep = { version = "0.1", optional = true }
[features]
dev_only = ["some_big_dep"]

In this setup, cargo metadata will happily enable the "dev_only" feature of package_d, resulting in some_big_dep and all of its dependencies being included in the output.

This is undesirable because:

  1. It makes the metadata.json file significantly larger than it would be if only regular dependencies were included
  2. In order to manually drop packages that are only reachable via dev-dependencies, consumers of cargo metadata need to do a lot of work to track which features are used when; this is something cargo metadata should do automatically.

Proposed Solution

Add the following flag to cargo metadata:

This is similar to the --edges flag in cargo tree. If desired, the no-normal, no-build, and no-dev options could be included in order for more complete parity between cargo metadata and cargo tree.

If cargo metadata --dep-kinds normal were run on the example above, then some_big_dep should be fully dropped from the output, as if I deleted it from the Cargo.toml file.

Notes

The --no-default-features and --features flags on cargo metadata have extremely limited use with the limitation of not being able to ignore dev-dependencies. Projects that are careful about their dependencies and feature sets are the ones that benefit most from those flags, but any dev-dependency anywhere in the tree may still enable an undesired feature.

CC @Manishearth

epage commented 2 years ago

I'm having a hard time seeing how we would implement --dep-kinds as proposed without filtering the dependency data after they have been resolved which defeats the purpose of what is happening (yes, less data in output but the data would have been gathered and features will still be unified).

What it sounds like you are wanting is for cargo metadata to not imply --all-targets and instead have target behavior more akin to cargo check with all the associated flags (e.g. --tests). One idea for opt-ing out of the implied --all-targets is a --default-targets flag (since --no-all-targets sounds weird).

sffc commented 2 years ago

Update: It appears that this is a problem only for workspace members. #7754 tracks that separately.

Manishearth commented 2 years ago

I do think a flag to not imply all-targets would be good either way

in general i find cargo-tree's flag approach to be really good here (wondering if we can just get cargo-tree to have a machine readable output that's similar to metadata's one)

epage commented 2 years ago

Out of curiosity, I looked at what cargo tree does.

Setting dep kinds has two different effects

In my earlier answer, I had assumed we would completely respect the specified dep kinds for feature unification. Only partially respecting them is then doable.

As for the removing of dependency edges, we've had several requests for different application-specific queries / filtering of data for cargo metadata. Instead of implementing each one off request we've wondered about a more general way to filter results, say something like graphql. This might not be able to handle every type of request and some we'll either punt and say the caller needs to do while others we might still bake in. Not saying this to say "no" to this approach but to bring up some of the things I'm thinking about with this.