Always do version check (#75)

* Always do version check * Update CHANGELOG
EmbarkStudios · Jan 24, 2024 · 7cd76b3 · 7cd76b3
1 parent 243bf01
commit 7cd76b3
Show file tree

Hide file tree

Showing 6 changed files with 309 additions and 279 deletions.
diff --git a/CHANGELOG.md b/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#75](https://github.com/EmbarkStudios/krates/pull/75) resolved [#74](https://github.com/EmbarkStudios/krates/issues/74) by just always checking version requirements for dependencies. Sigh.
+
 ## [0.16.4] - 2024-01-22
 ### Fixed
 - [PR#73](https://github.com/EmbarkStudios/krates/pull/73) resolved [#72](https://github.com/EmbarkStudios/krates/issues/72) by correctly parsing the new stable package ids where a specifier was not used.

diff --git a/src/builder.rs b/src/builder.rs
@@ -696,7 +696,6 @@ impl Builder {
             name: String,
             pkg: Kid,
             dep_kinds: Vec<DepKindInfo>,
-            multi: bool,
         }
 
         #[derive(Debug)]
@@ -757,7 +756,6 @@ impl Builder {
                             name: dn.name,
                             pkg: Kid::from(dn.pkg),
                             dep_kinds,
-                            multi: false,
                         }
                     })
                     .collect();
@@ -766,18 +764,6 @@ impl Builder {
                 // 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
-                // Starting at i=1 means that i and i-1 are always guaranteed to exist
-                for i in 1..deps.len() {
-                    if deps[i - 1].pkg.name() != deps[i].pkg.name() {
-                        continue;
-                    }
-
-                    deps[i - 1].multi = true;
-                    deps[i].multi = true;
-                }
-
                 let mut features = rn.features;
 
                 // Note that cargo metadata _currently_ always outputs these in
@@ -1151,18 +1137,20 @@ impl Builder {
                 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"));
+                // version to disambiguate references to them, unfortunately,
+                // though (it should be) extremely rare to do this, we always
+                // check that versions match once a crate is found, as nodes can
+                // not be resolved due to features, but will still be listed as
+                // a dependency
+                //
+                // 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.pkg.version().parse().expect("failed to parse semver");
 
                 let edges = rdep.dep_kinds.iter().filter_map(|dk| {
                         let mask = match dk.kind {
@@ -1190,7 +1178,7 @@ impl Builder {
                                     return false;
                                 }
 
-                                rdep_version.as_ref().map_or(true, |rdv| dep.req.matches(rdv))
+                                dep.req.matches(&rdep_version)
                             })
                             .unwrap_or_else(|| panic!("cargo metadata resolved a dependency for a dependency not specified by the crate: {rdep:?}"));
 

diff --git a/tests/pid-opaque.json b/tests/pid-opaque.json
diff --git a/tests/pid-stable.json b/tests/pid-stable.json
diff --git a/tests/pid/Cargo.toml b/tests/pid/Cargo.toml
@@ -14,5 +14,23 @@ tower_http_4 = { package = "tower-http", version = "0.4.4", features = [] }
 tower-http = { package = "tower-http", version = "0.5.0", features = [
     "sensitive-headers",
 ] }
-# Repro for https://github.com/EmbarkStudios/krates/pull/71
+# Repro for https://github.com/EmbarkStudios/krates/pull/71 (ie, make number of dependencies odd)
 libc = "0.2"
+
+# Repro for https://github.com/EmbarkStudios/krates/issues/74, depending on 2
+# versions of the same crate, but only one of which is selected due to features
+[dependencies.time]
+version = "0.2"
+features = ["std"]
+optional = true
+default-features = false
+
+[dependencies.time03]
+version = "0.3"
+features = ["parsing"]
+optional = true
+default-features = false
+package = "time"
+
+[features]
+default = ["time03"]