exclude inactive optional dependencies #537

[TitanNano]

cargo metadata currently outputs all dependencies, even optional dependencies that are not active. By comparing dep:* features with dependencies that have been declared as optional, we can eliminate inactive optional dependencies.

For #537

[bholley]

Thanks for the patch! @mystor WDYT?

[mystor]

I don't think this change actually improves the situation here, and I'm not sure if there is actually an issue to start with.

When running cargo metadata from within cargo-vet, we intentionally enable all features within the root crate when auditing by default (we have a flag to disable that, if that's undesirable for your project: https://mozilla.github.io/cargo-vet/commands.html#--no-all-features).

cargo metadata already doesn't include dependencies which which are not required by features. For example, the cargo_metadata crate (which we use) has an optional dependency on derive_builder, however that feature is not enabled by cargo-vet. The non-required crate doesn't appear anywhere in our Cargo.lock and isn't required to be mentioned in our supply-chain by cargo-vet

I think cargo metadata is already handling optional dependencies correctly, and only including them when needed. Do you have a concrete example where an optional dependency is incorrectly being included in the metadata? It would be useful to understand the problem we're trying to solve.

[mystor]

I think this change is incorrect. Looking at the reqwest crate, the default enabled features are ["default-tls"], which enables "native-tls-crate", which in turn turns on the dependency on this crate (https://github.com/seanmonstar/reqwest/blob/798df70fb0a4cd786ecc8cf770b33f1d56528524/Cargo.toml#L112). In the test crate we directly depend on reqwest with default features from the test crate (

cargo-vet/tests/test-project/Cargo.toml

Line 10 in 668c5bb

reqwest = "0.11.10"

), so that crate and those features should definitely be enabled, and reqwest should be considered to be a parent crate of native-tls.

I have a suspicion that these dependency edges aren't showing up because the code is incorrectly assuming that crate names match the corresponding optional dependency feature names, which is not necessarily the case.

[TitanNano]

This does indeed look like a false positive.

[TitanNano]

I think cargo metadata is already handling optional dependencies correctly, and only including them when needed. Do you have a concrete example where an optional dependency is incorrectly being included in the metadata? It would be useful to understand the problem we're trying to solve.

I have outlined one of our issues in a comment on the related issue: #537 (comment)

For example, sqlx is pulling in various drivers even though only one of them has been enabled via a feature.

There is also this cargo issue which indicates that this is indeed a problem, rust-lang/cargo#10801. This all said, I can see that your example is correct, and I haven't yet identified what the difference to the sqlx example is.