cargo icon indicating copy to clipboard operation
cargo copied to clipboard

Warn when new duplicate dependencies are introduced

Open kornelski opened this issue 1 year ago • 5 comments

Problem

There are situations in Rust when dependencies used by multiple crates should have the same semver-compatible versions (when they export traits or types shared across crates).

The problem is that cargo update or other lock file-rebuilding commands can change a project with no (unwanted) duplicates into a project with duplicates.

Duplicate dependencies are sometimes necessary or even desirable, but they can also happen by accident without users being aware of the problem.

Currently Cargo reports "Added dep-name 1.x", but that log message doesn't get any special treatment when the dependency is another version of a dependency that has remained on previous version.

Duplicate crates can cause compilation errors that rustc can't present nicely, because rustc doesn't know crate versions and doesn't know what caused them to be added to the project.

Proposed Solution

Cargo printing a warning along the lines of "Added duplicate dep-name 1.x, because other-dep requires ^1. Another dep-name version is 0.y, because different-dep requires 0.*" (or even print the whole "path" of dependency requirements up to the root)

This could help users spot the problem earlier, with more precise root cause information. Currently investigation requires running cargo tree -d and cargo tree -i, which uses may not know about, and looking up this info manually is more laborious than having it presented automatically when dupes happen.

Even when dupes don't cause compilation errors, they can slow down builds and bloat executables, so it is in users interest to avoid having duplicate dependencies when it's not necessary.

### Tasks
- [ ] Add a draft title or issue reference here

kornelski avatar May 10 '24 14:05 kornelski

Thanks for the proposal. This is pretty similar (or a dup?) to https://github.com/rust-lang/cargo/issues/7285. We are also brewing a Cargo linting system so eventually people are able to set lint levels for that. This is tracked in https://github.com/rust-lang/cargo/issues/12235. For now, cargo-deny is a great community-maintained tool for such use case.

Going to close this in favor of those. Let us know if there is something I missed and this should keep open separately :)

weihanglo avatar May 10 '24 19:05 weihanglo

#7285 sounds like it's already solved by cargo tree -d. There's a way to check for those users who know about the problem and that solution.

The issue for me is about improving discovery of the problem for users who are not already aware and actively tracking the problem.

kornelski avatar May 10 '24 21:05 kornelski

True. Addressing the problem from different angles. Reopened.

weihanglo avatar May 10 '24 21:05 weihanglo

The use case given is focused on public dependencies. I wonder if we should not warn for duplicate dependencies generally (which could get noisy, especially for use cases like cargo generate-lockfile) but provide a more focused message for duplicate public paths to a dependency (minus direct depending on two versions with renames). I wonder if it'd make sense to even run this as part of more general lints (#12235) and not just on lockfile changes.

side note: other tools for helping the user with this problem were deferred in the RFC.

epage avatar Jun 10 '24 21:06 epage

I don't think the warning would be noisy. On the contrary, I worry it would be too easy to miss, because updating from a lockfile state that doesn't duplicate a dependency to a state with a duplicated dependency is typically a one-time event (it won't just fix itself next time the lockfile is generated).

I think it would also be useful for private dependencies:

  • Cargo features of the dupes can get out of sync, affecting compatibility beyond Rust APIs (e.g. the dupe can fall back to a wrong TLS library or duplicate an entire async runtime).
  • Dupes can increase build time and compile time.
  • If an update meant to get important security or compatibility fixes, it can be problematic if an old instance is unexpectedly left in the dependency tree.

Dupes could be acceptable even in these cases, but I think it'd be better if it was a conscious decision, and a warning could help catch issues early.

kornelski avatar Oct 11 '24 20:10 kornelski