Support stable package id

CHANGELOG.md

@@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 <!-- next-header -->
 ## [Unreleased] - ReleaseDate
+### Fixed
+- [PR#65](https://github.com/EmbarkStudios/krates/pull/65) resolved [#64](https://github.com/EmbarkStudios/krates/issues/64) by adding support for the newly stabilized (currently nightly only) package id format.
+
+### Changed
+- [PR#65](https://github.com/EmbarkStudios/krates/pull/65) changed `Kid` from just a type alias for `cargo_metadata::PackageId` to an actual type that has accessors for the various components of the id. It also specifies its own `Ord` etc implementation so that those ids are sorted the exact same as the old version.
+
 ## [0.15.3] - 2024-01-12
 ### Fixed
 - [PR#63](https://github.com/EmbarkStudios/krates/pull/63) resolved [#62](https://github.com/EmbarkStudios/krates/issues/62) which was a bug introduced in [PR#61](https://github.com/EmbarkStudios/krates/pull/61)

examples/graph.rs

@@ -28,7 +28,7 @@ pub type Graph = krates::Krates<Simple>;
 impl From<krates::cm::Package> for Simple {
     fn from(pkg: krates::cm::Package) -> Self {
         Self {
-            id: pkg.id,
+            id: pkg.id.into(),
             //features: pkg.fee
         }
     }

src/lib.rs

@@ -48,9 +48,144 @@ pub use builder::{
 };
 pub use errors::Error;
 pub use pkgspec::PkgSpec;
+use std::fmt;
 
 /// A crate's unique identifier
-pub type Kid = cargo_metadata::PackageId;
+#[derive(Clone, Default)]
+pub struct Kid {
+    /// The full package id string as supplied by cargo
+    pub repr: String,
+    /// The subslices for each component in name -> version -> source order
+    components: [(usize, usize); 3],
+}
+
+impl Kid {
+    /// Gets the name of the package
+    #[inline]
+    pub fn name(&self) -> &str {
+        let (s, e) = self.components[0];
+        &self.repr[s..e]
+    }
+
+    /// Gets the semver of the package
+    #[inline]
+    pub fn version(&self) -> &str {
+        let (s, e) = self.components[1];
+        &self.repr[s..e]
+    }
+
+    /// Gets the source url of the package
+    #[inline]
+    pub fn source(&self) -> &str {
+        let (s, e) = self.components[2];
+        &self.repr[s..e]
+    }
+}
+
+#[allow(clippy::fallible_impl_from)]
+impl From<cargo_metadata::PackageId> for Kid {
+    fn from(pid: cargo_metadata::PackageId) -> Self {
+        let repr = pid.repr;
+
+        let gen = || {
+            let components = if repr.contains(' ') {
+                let name = (0, repr.find(' ')?);
+                let version = (name.1 + 1, repr[name.1 + 1..].find(' ')? + name.1 + 1);
+                // Note we skip the open and close parentheses as they are superfluous
+                // as every source has them, as well as not being present in the new
+                // stabilized format
+                //
+                // Note that we also chop off the commit id, it is not present in
+                // the stabilized format and is not used for package identification anyways
+                let source = (version.1 + 2, repr.rfind('#').unwrap_or(repr.len() - 1));
+
+                [name, version, source]
+            } else {
+                let vmn = repr.rfind('#')?;
+                let (name, version) = if let Some(split) = repr[vmn..].find('@') {
+                    ((vmn + 1, vmn + split), (vmn + split + 1, repr.len()))
+                } else {
+                    let begin = repr.rfind('/')? + 1;
+                    let end = if repr.starts_with("git+") {
+                        repr[begin..].find('?')? + begin
+                    } else {
+                        vmn
+                    };
+
+                    ((begin, end), (vmn + 1, repr.len()))
+                };
+
+                [name, version, (0, vmn)]
+            };
+
+            Some(components)
+        };
+
+        if let Some(components) = gen() {
+            Self { repr, components }
+        } else {
+            panic!("unable to parse package id '{repr}'");
+        }
+    }
+}
+
+impl fmt::Debug for Kid {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        let mut ds = f.debug_struct("Kid");
+
+        ds.field("name", &self.name())
+            .field("version", &self.version());
+
+        let src = self.source();
+        if src != "registry+https://github.com/rust-lang/crates.io-index" {
+            ds.field("source", &src);
+        }
+
+        ds.finish()
+    }
+}
+
+impl fmt::Display for Kid {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        f.write_str(&self.repr)
+    }
+}
+
+impl std::hash::Hash for Kid {
+    fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
+        state.write(self.repr.as_bytes());
+    }
+}
+
+impl Ord for Kid {
+    fn cmp(&self, o: &Self) -> std::cmp::Ordering {
+        let a = &self.repr;
+        let b = &o.repr;
+
+        for (ar, br) in self.components.into_iter().zip(o.components.into_iter()) {
+            let ord = a[ar.0..ar.1].cmp(&b[br.0..br.1]);
+            if ord != std::cmp::Ordering::Equal {
+                return ord;
+            }
+        }
+
+        std::cmp::Ordering::Equal
+    }
+}
+
+impl PartialOrd for Kid {
+    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
+        Some(self.cmp(other))
+    }
+}
+
+impl Eq for Kid {}
+
+impl PartialEq for Kid {
+    fn eq(&self, other: &Self) -> bool {
+        self.cmp(other) == std::cmp::Ordering::Equal
+    }
+}
 
 /// The set of features that have been enabled on a crate
 pub type EnabledFeatures = std::collections::BTreeSet<String>;
@@ -84,8 +219,6 @@ impl PartialEq<DK> for DepKind {
     }
 }
 
-use std::fmt;
-
 impl fmt::Display for DepKind {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {
@@ -509,29 +642,36 @@ where
     ///
     /// fn print(krates: &Krates, name: &str) {
     ///     let req = VersionReq::parse("=0.2").unwrap();
-    ///     for vs in krates.search_matches(name, req.clone()).map(|(_, krate)| &krate.version) {
-    ///         println!("found version {} matching {}!", vs, req);
+    ///     for vs in krates.search_matches(name, req.clone()).map(|km| &km.krate.version) {
+    ///         println!("found version {vs} matching {req}!");
     ///     }
     /// }
     /// ```
     pub fn search_matches(
         &self,
         name: impl Into<String>,
         req: semver::VersionReq,
-    ) -> impl Iterator<Item = (NodeId, &N)> {
+    ) -> impl Iterator<Item = KrateMatch<'_, N>> {
         let raw_nodes = &self.graph.raw_nodes()[0..self.krates_end];
 
         let name = name.into();
 
-        raw_nodes.iter().enumerate().filter_map(move |(id, node)| {
-            if let Node::Krate { krate, .. } = &node.weight {
-                if krate.name() == name && req.matches(krate.version()) {
-                    return Some((NodeId::new(id), krate));
+        raw_nodes
+            .iter()
+            .enumerate()
+            .filter_map(move |(index, node)| {
+                if let Node::Krate { krate, id, .. } = &node.weight {
+                    if krate.name() == name && req.matches(krate.version()) {
+                        return Some(KrateMatch {
+                            node_id: NodeId::new(index),
+                            krate,
+                            kid: id,
+                        });
+                    }
                 }
-            }
 
-            None
-        })
+                None
+            })
     }
 
     /// Get an iterator over all of the crates in the graph with the given name,
@@ -541,28 +681,44 @@ where
     /// use krates::Krates;
     ///
     /// fn print_all_versions(krates: &Krates, name: &str) {
-    ///     for vs in krates.krates_by_name(name).map(|(_, krate)| &krate.version) {
-    ///         println!("found version {}", vs);
+    ///     for vs in krates.krates_by_name(name).map(|km| &km.krate.version) {
+    ///         println!("found version {vs}");
     ///     }
     /// }
     /// ```
-    pub fn krates_by_name(&self, name: impl Into<String>) -> impl Iterator<Item = (NodeId, &N)> {
+    pub fn krates_by_name(
+        &self,
+        name: impl Into<String>,
+    ) -> impl Iterator<Item = KrateMatch<'_, N>> {
         let raw_nodes = &self.graph.raw_nodes()[0..self.krates_end];
 
         let name = name.into();
 
-        raw_nodes.iter().enumerate().filter_map(move |(id, node)| {
-            if let Node::Krate { krate, .. } = &node.weight {
-                if krate.name() == name {
-                    return Some((NodeId::new(id), krate));
+        raw_nodes
+            .iter()
+            .enumerate()
+            .filter_map(move |(index, node)| {
+                if let Node::Krate { krate, id, .. } = &node.weight {
+                    if krate.name() == name {
+                        return Some(KrateMatch {
+                            node_id: NodeId::new(index),
+                            krate,
+                            kid: id,
+                        });
+                    }
                 }
-            }
 
-            None
-        })
+                None
+            })
     }
 }
 
+pub struct KrateMatch<'graph, N> {
+    pub krate: &'graph N,
+    pub kid: &'graph Kid,
+    pub node_id: NodeId,
+}
+
 impl<N, E> std::ops::Index<NodeId> for Krates<N, E> {
     type Output = N;
 
@@ -586,3 +742,31 @@ impl<N, E> std::ops::Index<usize> for Krates<N, E> {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn converts_package_ids() {
+        let ids = [
+            ("registry+https://github.com/rust-lang/crates.io-index#[email protected]", "ab_glyph", "0.2.22", "registry+https://github.com/rust-lang/crates.io-index"),
+            ("git+https://github.com/EmbarkStudios/egui-stylist?rev=3900e8aedc5801e42c1bb747cfd025615bf3b832#0.2.0", "egui-stylist", "0.2.0", "git+https://github.com/EmbarkStudios/egui-stylist?rev=3900e8aedc5801e42c1bb747cfd025615bf3b832"),
+            ("path+file:///home/jake/code/ark/components/allocator#[email protected]", "ark-allocator", "0.1.0", "path+file:///home/jake/code/ark/components/allocator"),
+            ("git+https://github.com/EmbarkStudios/ash?branch=nv-low-latency2#0.38.0+1.3.269", "ash", "0.38.0+1.3.269", "git+https://github.com/EmbarkStudios/ash?branch=nv-low-latency2"),
+            ("git+https://github.com/EmbarkStudios/fsr-rs?branch=nv-low-latency2#[email protected]", "fsr", "0.1.7", "git+https://github.com/EmbarkStudios/fsr-rs?branch=nv-low-latency2"),
+            ("fuser 0.4.1 (git+https://github.com/cberner/fuser?branch=master#b2e7622942e52a28ffa85cdaf48e28e982bb6923)", "fuser", "0.4.1", "git+https://github.com/cberner/fuser?branch=master"),
+            ("fuser 0.4.1 (git+https://github.com/cberner/fuser?rev=b2e7622#b2e7622942e52a28ffa85cdaf48e28e982bb6923)", "fuser", "0.4.1", "git+https://github.com/cberner/fuser?rev=b2e7622"),
+            ("a 0.1.0 (path+file:///home/jake/code/krates/tests/ws/a)", "a", "0.1.0", "path+file:///home/jake/code/krates/tests/ws/a"),
+            ("bindgen 0.59.2 (registry+https://github.com/rust-lang/crates.io-index)", "bindgen", "0.59.2", "registry+https://github.com/rust-lang/crates.io-index"),
+        ];
+
+        for (repr, name, version, source) in ids {
+            let kid = super::Kid::from(cargo_metadata::PackageId {
+                repr: repr.to_owned(),
+            });
+
+            assert_eq!(kid.name(), name);
+            assert_eq!(kid.version(), version);
+            assert_eq!(kid.source(), source);
+        }
+    }
+}

src/pkgspec.rs

@@ -23,36 +23,18 @@ impl PkgSpec {
             }
         }
 
-        match self.url {
-            Some(ref u) => {
-                // Get the url from the identifier to avoid pointless
-                // allocations.
-                if let Some(mut url) = krate.id.repr.splitn(3, ' ').nth(2) {
-                    // Strip off the the enclosing parens
-                    url = &url[1..url.len() - 1];
-
-                    // Strip off the leading <source>+
-                    if let Some(ind) = url.find('+') {
-                        url = &url[ind + 1..];
-                    }
-
-                    // Strip off any fragments
-                    if let Some(ind) = url.rfind('#') {
-                        url = &url[..ind];
-                    }
+        let Some((url, src)) = self
+            .url
+            .as_ref()
+            .zip(krate.source.as_ref().map(|s| s.repr.as_str()))
+        else {
+            return true;
+        };
 
-                    // Strip off any query parts
-                    if let Some(ind) = url.rfind('?') {
-                        url = &url[..ind];
-                    }
+        let begin = src.find('+').map_or(0, |i| i + 1);
+        let end = src.find('?').or_else(|| src.find('#')).unwrap_or(src.len());
 
-                    u == url
-                } else {
-                    false
-                }
-            }
-            None => true,
-        }
+        url == &src[begin..end]
     }
 }
 

tests/features.rs

@@ -46,13 +46,13 @@ fn prunes_mixed_dependencies() {
 
 macro_rules! assert_features {
     ($graph:expr, $name:expr, $features:expr) => {
-        let (_, krate) = $graph.krates_by_name($name).next().unwrap();
+        let krates::KrateMatch { kid, .. } = $graph.krates_by_name($name).next().unwrap();
 
         let expected_features: std::collections::BTreeSet<_> =
             $features.into_iter().map(|s| s.to_owned()).collect();
 
         assert_eq!(
-            $graph.get_enabled_features(&krate.id).unwrap(),
+            $graph.get_enabled_features(kid).unwrap(),
             &expected_features
         );
     };