Fix duplicate dependencies (#70)

* Fix duplicate dependencies

* Fix lint

* Simplify

* Update CHANGELOG

CHANGELOG.md

@@ -8,6 +8,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Fixed
+- [PR#70](https://github.com/EmbarkStudios/krates/pull/67) resolved [#68](https://github.com/EmbarkStudios/krates/issues/68) and [#69](https://github.com/EmbarkStudios/krates/issues/69) by additionally checking the version of resolve dependencies if there were 2 or more of the same name referenced by the same crate.
+
 ## [0.16.1] - 2024-01-20
 ### Fixed
 - [PR#67](https://github.com/EmbarkStudios/krates/pull/67) resolved [#66](https://github.com/EmbarkStudios/krates/issues/66) by ignore features that reference crates that aren't resolved, instead of panicing, as there should only be one case where that occurs.

src/builder.rs

@@ -696,6 +696,7 @@ impl Builder {
             name: String,
             pkg: Kid,
             dep_kinds: Vec<DepKindInfo>,
+            multi: bool,
         }
 
         #[derive(Debug)]
@@ -738,7 +739,7 @@ impl Builder {
                     );
                 }
 
-                let deps = rn
+                let mut deps: Vec<_> = rn
                     .deps
                     .into_iter()
                     .map(|dn| {
@@ -756,10 +757,26 @@ impl Builder {
                             name: dn.name,
                             pkg: Kid::from(dn.pkg),
                             dep_kinds,
+                            multi: false,
                         }
                     })
                     .collect();
 
+                // These _should_ always already be sorted, but again, might be
+                // due to implementation details rather than guaranteed
+                deps.sort_by(|a, b| a.pkg.cmp(&b.pkg));
+
+                // Note any dependencies that have the same name, we need to
+                // disambiguate them when resolving features
+                for ch in deps.chunks_mut(2) {
+                    if ch.len() != 2 || ch[0].pkg.name() != ch[1].pkg.name() {
+                        continue;
+                    }
+
+                    ch[0].multi = true;
+                    ch[1].multi = true;
+                }
+
                 let mut features = rn.features;
 
                 // Note that cargo metadata _currently_ always outputs these in
@@ -1132,6 +1149,20 @@ impl Builder {
                 let maybe_real_name = pkg.name();
                 let strong = features.is_some();
 
+                // If there are multiple versions of the same package we use the
+                // version to disambiguate references to them, but that is extremely
+                // rare so we only do it in the case there are actually multiple crates
+                // with the same name. Note that cargo _should_ fail to resolve
+                // nodes if the same package is referenced with two `^` (compatible)
+                // semvers, ie, you can't reference both ">= 0.2.12" and "=0.2.7" of
+                // a package even if they could never point to the same package
+                // This _may_ mean there could be a situation where a single crate
+                // _could_ be referenced with 0.0.x versions, but...I'll fix that
+                // if someone reports an issue
+                let rdep_version = rdep
+                    .multi
+                    .then(|| rdep.pkg.version().parse().expect("failed to parse semver"));
+
                 let edges = rdep.dep_kinds.iter().filter_map(|dk| {
                         let mask = match dk.kind {
                             DepKind::Normal => 0x1,
@@ -1154,7 +1185,11 @@ impl Builder {
 
                                 // Crates can rename the dependency package themselves
                                 let dep_name = dep.rename.as_deref().unwrap_or(&dep.name);
-                                dep_names_match(dep_name, &rdep.name) || maybe_real_name == dep_name
+                                if !dep_names_match(dep_name, &rdep.name) && maybe_real_name != dep_name {
+                                    return false;
+                                }
+
+                                rdep_version.as_ref().map_or(true, |rdv| dep.req.matches(rdv))
                             })
                             .unwrap_or_else(|| panic!("cargo metadata resolved a dependency for a dependency not specified by the crate: {rdep:?}"));
 

tests/misc.rs

@@ -190,18 +190,6 @@ fn direct_dependencies() {
     insta::assert_snapshot!(dd);
 }
 
-#[test]
-#[cfg(feature = "with-crates-index")]
-fn bug_repro() {
-    let mut kb = krates::Builder::new();
-    kb.with_crates_io_index(None, krates::index::IndexKind::Sparse)
-        .unwrap();
-
-    let grafs = util::build("bug.json", kb).unwrap();
-
-    insta::assert_snapshot!(grafs.dotgraph());
-}
-
 /// Validates that there is no difference between the OG "opaque" package id
 /// format and the newly stabilized one
 #[test]
@@ -211,3 +199,19 @@ fn opaque_matches_stable() {
 
     similar_asserts::assert_eq!(opaque.dotgraph(), stable.dotgraph());
 }
+
+/// Validates we can correctly find package ids for duplicated packages in both
+/// the opaque and stable formats
+///
+/// <https://github.com/EmbarkStudios/krates/issues/68>
+/// <https://github.com/EmbarkStudios/krates/issues/69>
+#[test]
+fn finds_duplicates() {
+    let opaque = util::build("pid-opaque.json", krates::Builder::new()).unwrap();
+    let stable = util::build("pid-stable.json", krates::Builder::new()).unwrap();
+
+    let opaque = opaque.dotgraph();
+    similar_asserts::assert_eq!(opaque, stable.dotgraph());
+
+    insta::assert_snapshot!(opaque);
+}

tests/pid/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "pid"
+version = "0.1.0"
+edition = "2021"
+
+[dependencies]
+# does not have a feature called wasm-bindgen
+getrandom = { version = "0.2.7", features = ["js"] }
+# does not have a feature called js
+getrandom_old = { package = "getrandom", version = "0.1.16", features = [
+    "wasm-bindgen",
+] }
+tower_http_4 = { package = "tower-http", version = "0.4.4", features = [] }
+tower-http = { package = "tower-http", version = "0.5.0", features = [
+    "sensitive-headers",
+] }

tests/pid/src/lib.rs

@@ -0,0 +1 @@
+