precise as enum not string

Eh2406 · Eh2406 · commit 407487f38762 · 2023-10-18T17:06:33.000Z
diff --git a/src/cargo/core/resolver/encode.rs b/src/cargo/core/resolver/encode.rs
@@ -776,7 +776,7 @@ pub fn encodable_package_id(
             }
         }
     }
-    let mut source = encodable_source_id(id_to_encode.with_precise(None), resolve_version);
+    let mut source = encodable_source_id(id_to_encode.without_precise(), resolve_version);
     if let Some(counts) = &state.counts {
         let version_counts = &counts[&id.name()];
         if version_counts[&id.version()] == 1 {
diff --git a/src/cargo/core/source_id.rs b/src/cargo/core/source_id.rs
@@ -3,7 +3,8 @@ use crate::sources::registry::CRATES_IO_HTTP_INDEX;
 use crate::sources::source::Source;
 use crate::sources::{DirectorySource, CRATES_IO_DOMAIN, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
 use crate::sources::{GitSource, PathSource, RegistrySource};
-use crate::util::{config, CanonicalUrl, CargoResult, Config, IntoUrl, ToSemver};
+use crate::util::interning::InternedString;
+use crate::util::{config, CanonicalUrl, CargoResult, Config, IntoUrl};
 use anyhow::Context;
 use serde::de;
 use serde::ser;
@@ -50,14 +51,37 @@ struct SourceIdInner {
     /// The source kind.
     kind: SourceKind,
     /// For example, the exact Git revision of the specified branch for a Git Source.
-    precise: Option<String>,
+    precise: Option<Precise>,
     /// Name of the remote registry.
     ///
     /// WARNING: this is not always set when the name is not known,
     /// e.g. registry coming from `--index` or Cargo.lock
     registry_key: Option<KeyOf>,
 }
 
+#[derive(Eq, PartialEq, Clone, Debug, Hash)]
+enum Precise {
+    Locked,
+    Updated {
+        name: InternedString,
+        from: semver::Version,
+        to: semver::Version,
+    },
+    GitUrlFragment(String),
+}
+
+impl fmt::Display for Precise {
+    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
+        match self {
+            Precise::Locked => "locked".fmt(f),
+            Precise::Updated { name, from, to } => {
+                write!(f, "{}={}->{}", name, from, to)
+            }
+            Precise::GitUrlFragment(s) => s.fmt(f),
+        }
+    }
+}
+
 /// The possible kinds of code source.
 /// Along with [`SourceIdInner`], this fully defines the source.
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
@@ -178,17 +202,15 @@ impl SourceId {
                 let precise = url.fragment().map(|s| s.to_owned());
                 url.set_fragment(None);
                 url.set_query(None);
-                Ok(SourceId::for_git(&url, reference)?.with_precise(precise))
+                Ok(SourceId::for_git(&url, reference)?.with_git_precise(precise))
             }
             "registry" => {
                 let url = url.into_url()?;
-                Ok(SourceId::new(SourceKind::Registry, url, None)?
-                    .with_precise(Some("locked".to_string())))
+                Ok(SourceId::new(SourceKind::Registry, url, None)?.with_locked_precise())
             }
             "sparse" => {
                 let url = string.into_url()?;
-                Ok(SourceId::new(SourceKind::SparseRegistry, url, None)?
-                    .with_precise(Some("locked".to_string())))
+                Ok(SourceId::new(SourceKind::SparseRegistry, url, None)?.with_locked_precise())
             }
             "path" => {
                 let url = url.into_url()?;
@@ -335,7 +357,7 @@ impl SourceId {
         } else if self.has_precise() {
             // We remove `precise` here to retrieve an permissive version of
             // `SourceIdInner`, which may contain the registry name.
-            self.with_precise(None).display_registry_name()
+            self.without_precise().display_registry_name()
         } else {
             url_display(self.url())
         }
@@ -444,19 +466,14 @@ impl SourceId {
         }
     }
 
-    /// Gets the value of the precise field.
-    pub fn precise(self) -> Option<&'static str> {
-        self.inner.precise.as_deref()
-    }
-
     /// Check if the precise data field has bean set
     pub fn has_precise(self) -> bool {
         self.inner.precise.is_some()
     }
 
     /// Check if the precise data field has bean set to "Locked"
     pub fn has_locked_precise(self) -> bool {
-        self.inner.precise.as_deref() == Some("Locked")
+        self.inner.precise == Some(Precise::Locked)
     }
 
     /// Check if two sources have the same precise data field
@@ -468,28 +485,54 @@ impl SourceId {
     /// from a call to [SourceId::with_precise_registry_version].
     ///
     /// If so return the version currently in the lock file and the version to be updated to.
-    /// If specified, our own source will have a precise version listed of the form
-    // `<pkg>=<p_req>-><f_req>` where `<pkg>` is the name of a crate on
-    // this source, `<p_req>` is the version installed and `<f_req>` is the
-    // version requested (argument to `--precise`).
     pub fn precise_registry_version(
         self,
-        name: &str,
-    ) -> Option<(semver::Version, semver::Version)> {
-        self.inner
-            .precise
-            .as_deref()
-            .and_then(|p| p.strip_prefix(name)?.strip_prefix('='))
-            .map(|p| {
-                let (current, requested) = p.split_once("->").unwrap();
-                (current.to_semver().unwrap(), requested.to_semver().unwrap())
-            })
+        pkg: &str,
+    ) -> Option<(&semver::Version, &semver::Version)> {
+        match &self.inner.precise {
+            Some(Precise::Updated { name, from, to }) if name == pkg => Some((from, to)),
+            _ => None,
+        }
+    }
+
+    pub fn precise_git_fragment(self) -> Option<&'static str> {
+        match &self.inner.precise {
+            Some(Precise::GitUrlFragment(s)) => Some(&s[..8]),
+            _ => None,
+        }
+    }
+
+    pub fn precise_git_oid(self) -> CargoResult<Option<git2::Oid>> {
+        Ok(match self.inner.precise.as_ref() {
+            Some(Precise::GitUrlFragment(s)) => {
+                Some(git2::Oid::from_str(s).with_context(|| {
+                    format!("precise value for git is not a git revision: {}", s)
+                })?)
+            }
+            _ => None,
+        })
     }
 
     /// Creates a new `SourceId` from this source with the given `precise`.
-    pub fn with_precise(self, v: Option<String>) -> SourceId {
+    pub fn with_git_precise(self, fragment: Option<String>) -> SourceId {
+        SourceId::wrap(SourceIdInner {
+            precise: fragment.map(|f| Precise::GitUrlFragment(f)),
+            ..(*self.inner).clone()
+        })
+    }
+
+    /// Creates a new `SourceId` from this source without a `precise`.
+    pub fn without_precise(self) -> SourceId {
         SourceId::wrap(SourceIdInner {
-            precise: v,
+            precise: None,
+            ..(*self.inner).clone()
+        })
+    }
+
+    /// Creates a new `SourceId` from this source without a `precise`.
+    pub fn with_locked_precise(self) -> SourceId {
+        SourceId::wrap(SourceIdInner {
+            precise: Some(Precise::Locked),
             ..(*self.inner).clone()
         })
     }
@@ -510,13 +553,21 @@ impl SourceId {
     /// The data can be read with [SourceId::precise_registry_version]
     pub fn with_precise_registry_version(
         self,
-        name: impl fmt::Display,
-        version: &semver::Version,
+        name: InternedString,
+        version: semver::Version,
         precise: &str,
     ) -> CargoResult<SourceId> {
-        semver::Version::parse(precise)
+        let precise = semver::Version::parse(precise)
             .with_context(|| format!("invalid version format for precise version `{precise}`"))?;
-        Ok(self.with_precise(Some(format!("{}={}->{}", name, version, precise))))
+
+        Ok(SourceId::wrap(SourceIdInner {
+            precise: Some(Precise::Updated {
+                name,
+                from: version,
+                to: precise,
+            }),
+            ..(*self.inner).clone()
+        }))
     }
 
     /// Returns `true` if the remote registry is the standard <https://crates.io>.
@@ -648,7 +699,8 @@ impl fmt::Display for SourceId {
                     write!(f, "?{}", pretty)?;
                 }
 
-                if let Some(ref s) = self.inner.precise {
+                if let Some(s) = &self.inner.precise {
+                    let s = s.to_string();
                     let len = cmp::min(s.len(), 8);
                     write!(f, "#{}", &s[..len])?;
                 }
diff --git a/src/cargo/ops/cargo_generate_lockfile.rs b/src/cargo/ops/cargo_generate_lockfile.rs
@@ -105,14 +105,14 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
                         if pid.source_id().is_registry() {
                             pid.source_id().with_precise_registry_version(
                                 pid.name(),
-                                pid.version(),
+                                pid.version().clone(),
                                 precise,
                             )?
                         } else {
-                            pid.source_id().with_precise(Some(precise.to_string()))
+                            pid.source_id().with_git_precise(Some(precise.to_string()))
                         }
                     }
-                    None => pid.source_id().with_precise(None),
+                    None => pid.source_id().without_precise(),
                 });
             }
             if let Ok(unused_id) =
@@ -147,7 +147,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
                 format!(
                     "{} -> #{}",
                     removed[0],
-                    &added[0].source_id().precise().unwrap()[..8]
+                    &added[0].source_id().precise_git_fragment().unwrap()
                 )
             } else {
                 format!("{} -> v{}", removed[0], added[0].version())
diff --git a/src/cargo/ops/vendor.rs b/src/cargo/ops/vendor.rs
@@ -258,7 +258,7 @@ fn sync(
         } else {
             // Remove `precise` since that makes the source name very long,
             // and isn't needed to disambiguate multiple sources.
-            source_id.with_precise(None).as_url().to_string()
+            source_id.without_precise().as_url().to_string()
         };
 
         let source = if source_id.is_crates_io() {
diff --git a/src/cargo/sources/git/source.rs b/src/cargo/sources/git/source.rs
@@ -89,13 +89,7 @@ impl<'cfg> GitSource<'cfg> {
 
         let remote = GitRemote::new(source_id.url());
         let manifest_reference = source_id.git_reference().unwrap().clone();
-        let locked_rev =
-            match source_id.precise() {
-                Some(s) => Some(git2::Oid::from_str(s).with_context(|| {
-                    format!("precise value for git is not a git revision: {}", s)
-                })?),
-                None => None,
-            };
+        let locked_rev = source_id.precise_git_oid()?;
         let ident = ident_shallow(
             &source_id,
             config
@@ -290,7 +284,9 @@ impl<'cfg> Source for GitSource<'cfg> {
             .join(short_id.as_str());
         db.copy_to(actual_rev, &checkout_path, self.config)?;
 
-        let source_id = self.source_id.with_precise(Some(actual_rev.to_string()));
+        let source_id = self
+            .source_id
+            .with_git_precise(Some(actual_rev.to_string()));
         let path_source = PathSource::new_recursive(&checkout_path, source_id, self.config);
 
         self.path_source = Some(path_source);
diff --git a/src/cargo/sources/registry/index.rs b/src/cargo/sources/registry/index.rs
@@ -663,7 +663,7 @@ impl<'cfg> RegistryIndex<'cfg> {
 
         // Handle `cargo update --precise` here.
         let precise = source_id.precise_registry_version(name.as_str());
-        let summaries = summaries.filter(|s| match &precise {
+        let summaries = summaries.filter(|s| match precise {
             Some((current, requested)) => {
                 if req.matches(current) {
                     // Unfortunately crates.io allows versions to differ only

Original file line number	Diff line number	Diff line change
`@@ -776,7 +776,7 @@ pub fn encodable_package_id(`
`776`	`776`	`}`
`777`	`777`	`}`
`778`	`778`	`}`
`779`		`- let mut source = encodable_source_id(id_to_encode.with_precise(None), resolve_version);`
	`779`	`+ let mut source = encodable_source_id(id_to_encode.without_precise(), resolve_version);`
`780`	`780`	`if let Some(counts) = &state.counts {`
`781`	`781`	`let version_counts = &counts[&id.name()];`
`782`	`782`	`if version_counts[&id.version()] == 1 {`