Skip to content

Fix deduplication in cargo tree (fixes #9599) #16606

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Nutomic wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix deduplication in cargo tree (fixes #9599) #16606

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b27b409
Fix deduplication in cargo tree (fixes #9599)
Nutomic Feb 9, 2026
fd93d89
Update test outputs
Nutomic Feb 27, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions src/cargo/ops/tree/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,22 +327,9 @@ impl<'a> Graph<'a> {
.filter(|(_name, indexes)| {
indexes
.into_iter()
.map(|(node, _)| {
match node {
Node::Package {
package_id,
features,
..
} => {
// Do not treat duplicates on the host or target as duplicates.
Node::Package {
package_id: package_id.clone(),
features: features.clone(),
kind: CompileKind::Host,
}
}
_ => unreachable!(),
}
.map(|(node, _)| match node {
Node::Package { package_id, .. } => package_id.clone(),
_ => unreachable!(),
})
.collect::<HashSet<_>>()
.len()
Expand Down
38 changes: 15 additions & 23 deletions tests/testsuite/cargo_tree/deps.rs
View file
Open in desktop
Copy link
Member

@weihanglo weihanglo Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, for bugfixes, we usually follow a variant of atomic commit pattern:

  1. Commit a test that asserts the current buggy behavior (passes).
  2. In the next commit, fix the bug and update the test/snapshot.

Every commit passes, and the test/snapshot diff shows the behavior change.

Copy link
Author

@Nutomic Nutomic Feb 17, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I understand. The test in this file already asserts the current behaviour and passes. Do you simply want me to squash the two commits in this PR into one?

Copy link
Contributor

@arlosi arlosi Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any changes to the test (like adding optdep 2.0.0), should be made in the first commit.

The 2nd commit should then only show the impact caused by the bug fix to the test (only the stdout_data should change).

Copy link
Author

@Nutomic Nutomic Feb 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated, I hope its right now.

Original file line number Diff line number Diff line change
Expand Up @@ -1307,6 +1307,7 @@ foo v0.1.0 ([ROOT]/foo)
fn host_dep_feature() {
// New feature resolver with optional build dep
Package::new("optdep", "1.0.0").publish();
Package::new("optdep", "2.0.0").publish();
Package::new("bar", "1.0.0")
.add_dep(Dependency::new("optdep", "1.0").optional(true))
.publish();
Expand All @@ -1318,11 +1319,9 @@ fn host_dep_feature() {
name = "foo"
version = "0.1.0"

[build-dependencies]
bar = { version = "1.0", features = ["optdep"] }

[dependencies]
bar = "1.0"
optdep = "2.0"
bar = { version = "1.0", features = ["optdep"] }
"#,
)
.file("src/lib.rs", "")
Expand All @@ -1333,10 +1332,9 @@ fn host_dep_feature() {
p.cargo("tree")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo)
└── bar v1.0.0
└── optdep v1.0.0
[build-dependencies]
└── bar v1.0.0 (*)
├── bar v1.0.0
│ └── optdep v1.0.0
└── optdep v2.0.0

"#]])
.run();
Expand All @@ -1351,13 +1349,11 @@ bar v1.0.0
.run();

// invert
p.cargo("tree -i optdep")
p.cargo("tree -i optdep@1.0.0")
.with_stdout_data(str![[r#"
optdep v1.0.0
└── bar v1.0.0
└── foo v0.1.0 ([ROOT]/foo)
[build-dependencies]
└── foo v0.1.0 ([ROOT]/foo)

"#]])
.run();
Expand All @@ -1368,29 +1364,25 @@ optdep v1.0.0
p.cargo("tree")
.with_stdout_data(str![[r#"
foo v0.1.0 ([ROOT]/foo)
└── bar v1.0.0
[build-dependencies]
└── bar v1.0.0
└── optdep v1.0.0
├── bar v1.0.0
│ └── optdep v1.0.0
└── optdep v2.0.0

"#]])
.run();

p.cargo("tree -p bar")
.with_stdout_data(str![[r#"
bar v1.0.0

bar v1.0.0
└── optdep v1.0.0

"#]])
.run();

p.cargo("tree -i optdep")
p.cargo("tree -i optdep@1.0.0")
.with_stdout_data(str![[r#"
optdep v1.0.0
└── bar v1.0.0
[build-dependencies]
└── foo v0.1.0 ([ROOT]/foo)

"#]])
Expand All @@ -1399,11 +1391,11 @@ optdep v1.0.0
// Check that -d handles duplicates with features.
p.cargo("tree -d")
.with_stdout_data(str![[r#"
bar v1.0.0
└── foo v0.1.0 ([ROOT]/foo)
optdep v1.0.0
└── bar v1.0.0
└── foo v0.1.0 ([ROOT]/foo)

bar v1.0.0
[build-dependencies]
optdep v2.0.0
└── foo v0.1.0 ([ROOT]/foo)

"#]])
Expand Down