Skip to content
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.

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

Fix duplicate versions of the same crate with different feature sets #98

Merged
merged 2 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
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
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
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@

16 changes: 16 additions & 0 deletions tests/features.rs
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
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