Skip to content

Commit

Permalink
Fix duplicate versions of the same crate with different feature sets (#…
Browse files Browse the repository at this point in the history 
…98)

* Fix duplicate versions of the same crate with different feature sets

* Update CHANGELOG
  • Loading branch information
@Jake-Shadle
Jake-Shadle authored Nov 14, 2024
1 parent d1d0cf4 commit 0ca60a4
Show file tree
Hide file tree
Showing 7 changed files with 1,603 additions and 1,455 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

<!-- next-header -->
## [Unreleased] - ReleaseDate
### Fixed
- [PR#98](https://github.com/EmbarkStudios/krates/pull/98) resolved [#84](https://github.com/EmbarkStudios/krates/issues/84) and [#97](https://github.com/EmbarkStudios/krates/issues/97) by resolving `<crate>/<feature>` references to the correct crate in all cases, as it could have resolved to a crate with the same name but different version/feature set previously.

## [0.17.4] - 2024-11-14
### Fixed
- [PR#96](https://github.com/EmbarkStudios/krates/pull/96) fixed an issue where package specs didn't allow the `@` separator, resolving [cargo-deny#717](https://github.com/EmbarkStudios/cargo-deny/issues/717).
Expand Down
20 changes: 13 additions & 7 deletions src/builder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1054,10 +1054,10 @@ impl Builder {
visit_stack.push((pid, None));
}

// This _should_ never fail in normal cases, however if the
// an index implementation is not provided, it's possible for
// the resolved features to mention features that aren't in
// the actual crate manifest
// This _should_ never fail in normal cases, however if an
// index implementation is not provided, it's possible for the
// resolved features to mention features that aren't in the
// actual crate manifest
let fs = if let Some(fs) = krate.features.get(rnode.feature(feature)) {
fs
} else {
Expand All @@ -1079,9 +1079,15 @@ impl Builder {
}
};

let Some(ndep) = rnode.deps.iter().find(|rdep| {
dep_names_match(krate_name, &rdep.name) || krate_name == rdep.pkg.name()
}) else {
// Note that we don't care about the package name here, as cargo
// resolves feature by the manifest name for the package only, since
// it's possible to have multiple versions of the same crate that
// each need a unique name
let Some(ndep) = rnode
.deps
.iter()
.find(|rdep| dep_names_match(krate_name, &rdep.name))
else {
// We can have a feature that points to a crate that isn't resolved by cargo due to it being
// a dev-only dependency
continue;
Expand Down
16 changes: 16 additions & 0 deletions tests/feature-bug-2/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
[package]
name = "feature-bug-2"
version = "0.1.0"
edition = "2021"

[dependencies]
defmt = { version = "0.3.8", optional = true }
embedded-hal-02 = { version = "0.2.7", features = ["unproven"], package = "embedded-hal" }
embedded-hal = "1.0.0"

[features]
default = []
defmt = [
"dep:defmt",
"embedded-hal/defmt-03",
]
1 change: 1 addition & 0 deletions tests/feature-bug-2/src/lib.rs
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

16 changes: 16 additions & 0 deletions tests/features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -347,3 +347,19 @@ fn includes_target_specific_feature_dependencies() {
let dotgraph = krates::petgraph::dot::Dot::new(md.graph()).to_string();
insta::assert_snapshot!(dotgraph);
}

/// Ensures that dependencies on the same crate but on different version with
/// different features, correctly resolves them when doing feature resolution
/// See <https://github.com/EmbarkStudios/krates/issues/97>
#[test]
fn duplicate_versions() {
let mut cmd = krates::Cmd::new();
cmd.manifest_path("tests/feature-bug-2/Cargo.toml")
.all_features();

let builder = krates::Builder::new();
let md: krates::Krates<util::JustId> = builder.build(cmd, krates::NoneFilter).unwrap();

let dotgraph = krates::petgraph::dot::Dot::new(md.graph()).to_string();
insta::assert_snapshot!(dotgraph);
}
131 changes: 131 additions & 0 deletions tests/snapshots/features__duplicate_versions.snap
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
---
source: tests/features.rs
expression: dotgraph
snapshot_kind: text
---
digraph {
0 [ label = "crate bitflags 1.3.2" ]
1 [ label = "crate defmt 0.3.8" ]
2 [ label = "crate defmt-macros 0.3.9" ]
3 [ label = "crate defmt-parser 0.3.4" ]
4 [ label = "crate embedded-hal 0.2.7" ]
5 [ label = "crate embedded-hal 1.0.0" ]
6 [ label = "crate feature-bug-2 0.1.0 path+file:///krates/tests/feature-bug-2" ]
7 [ label = "crate nb 0.1.3" ]
8 [ label = "crate nb 1.1.0" ]
9 [ label = "crate proc-macro-error 1.0.4" ]
10 [ label = "crate proc-macro-error-attr 1.0.4" ]
11 [ label = "crate proc-macro2 1.0.89" ]
12 [ label = "crate quote 1.0.37" ]
13 [ label = "crate syn 1.0.109" ]
14 [ label = "crate syn 2.0.87" ]
15 [ label = "crate thiserror 1.0.69" ]
16 [ label = "crate thiserror-impl 1.0.69" ]
17 [ label = "crate unicode-ident 1.0.13" ]
18 [ label = "crate version_check 0.9.5" ]
19 [ label = "crate void 1.0.2" ]
20 [ label = "feature default" ]
21 [ label = "feature unstable" ]
22 [ label = "feature default" ]
23 [ label = "feature default" ]
24 [ label = "feature default" ]
25 [ label = "feature full" ]
26 [ label = "feature extra-traits" ]
27 [ label = "feature default" ]
28 [ label = "feature unstable" ]
29 [ label = "feature unproven" ]
30 [ label = "feature defmt-03" ]
31 [ label = "feature proc-macro" ]
32 [ label = "feature proc-macro" ]
33 [ label = "feature defmt-03" ]
34 [ label = "feature defmt" ]
35 [ label = "feature defmt-03" ]
36 [ label = "feature default" ]
37 [ label = "feature syn-error" ]
38 [ label = "feature syn" ]
39 [ label = "feature proc-macro" ]
40 [ label = "feature printing" ]
41 [ label = "feature parsing" ]
42 [ label = "feature derive" ]
43 [ label = "feature clone-impls" ]
1 -> 20 [ label = "" ]
1 -> 2 [ label = "" ]
2 -> 21 [ label = "" ]
2 -> 22 [ label = "" ]
2 -> 23 [ label = "" ]
2 -> 24 [ label = "" ]
2 -> 25 [ label = "" ]
2 -> 26 [ label = "" ]
2 -> 27 [ label = "" ]
3 -> 15 [ label = "" ]
4 -> 7 [ label = "" ]
4 -> 28 [ label = "" ]
4 -> 19 [ label = "" ]
5 -> 1 [ label = "" ]
6 -> 1 [ label = "" ]
6 -> 29 [ label = "" ]
6 -> 5 [ label = "" ]
6 -> 30 [ label = "" ]
7 -> 8 [ label = "" ]
9 -> 10 [ label = "" ]
9 -> 23 [ label = "" ]
9 -> 24 [ label = "" ]
9 -> 13 [ label = "" ]
9 -> 18 [ label = "(build)" ]
10 -> 23 [ label = "" ]
10 -> 24 [ label = "" ]
10 -> 18 [ label = "(build)" ]
11 -> 17 [ label = "" ]
12 -> 11 [ label = "" ]
12 -> 31 [ label = "" ]
13 -> 11 [ label = "" ]
13 -> 17 [ label = "" ]
14 -> 11 [ label = "" ]
14 -> 31 [ label = "" ]
14 -> 12 [ label = "" ]
14 -> 32 [ label = "" ]
14 -> 17 [ label = "" ]
15 -> 16 [ label = "" ]
16 -> 23 [ label = "" ]
16 -> 24 [ label = "" ]
16 -> 27 [ label = "" ]
20 -> 0 [ label = "" ]
21 -> 3 [ label = "" ]
29 -> 4 [ label = "" ]
29 -> 28 [ label = "" ]
30 -> 5 [ label = "" ]
30 -> 33 [ label = "" ]
34 -> 6 [ label = "" ]
34 -> 1 [ label = "" ]
34 -> 35 [ label = "" ]
36 -> 6 [ label = "" ]
28 -> 7 [ label = "" ]
37 -> 9 [ label = "" ]
37 -> 38 [ label = "" ]
38 -> 9 [ label = "" ]
38 -> 13 [ label = "" ]
22 -> 9 [ label = "" ]
22 -> 37 [ label = "" ]
31 -> 11 [ label = "" ]
23 -> 11 [ label = "" ]
23 -> 31 [ label = "" ]
32 -> 12 [ label = "" ]
32 -> 31 [ label = "" ]
24 -> 12 [ label = "" ]
24 -> 32 [ label = "" ]
39 -> 14 [ label = "" ]
39 -> 31 [ label = "" ]
40 -> 14 [ label = "" ]
40 -> 12 [ label = "" ]
41 -> 14 [ label = "" ]
25 -> 14 [ label = "" ]
26 -> 14 [ label = "" ]
42 -> 14 [ label = "" ]
27 -> 14 [ label = "" ]
27 -> 42 [ label = "" ]
27 -> 41 [ label = "" ]
27 -> 40 [ label = "" ]
27 -> 43 [ label = "" ]
27 -> 39 [ label = "" ]
43 -> 14 [ label = "" ]
}
Loading

0 comments on commit 0ca60a4

Please sign in to comment.