Make the precise field of a source an Enum

src/cargo/core/dependency.rs

@@ -352,9 +352,7 @@ impl Dependency {
         // Only update the `precise` of this source to preserve other
         // information about dependency's source which may not otherwise be
         // tested during equality/hashing.
-        me.source_id = me
-            .source_id
-            .with_precise(id.source_id().precise().map(|s| s.to_string()));
+        me.source_id = me.source_id.with_precise_from(id.source_id());
         self
     }
 

src/cargo/core/package_id.rs

@@ -160,14 +160,6 @@ impl PackageId {
         self.inner.source_id
     }
 
-    pub fn with_precise(self, precise: Option<String>) -> PackageId {
-        PackageId::pure(
-            self.inner.name,
-            self.inner.version.clone(),
-            self.inner.source_id.with_precise(precise),
-        )
-    }
-
     pub fn with_source_id(self, source: SourceId) -> PackageId {
         PackageId::pure(self.inner.name, self.inner.version.clone(), source)
     }

src/cargo/core/registry.rs

@@ -161,7 +161,7 @@ impl<'cfg> PackageRegistry<'cfg> {
 
             // If the previous source was not a precise source, then we can be
             // sure that it's already been updated if we've already loaded it.
-            Some((previous, _)) if previous.precise().is_none() => {
+            Some((previous, _)) if !previous.has_precise() => {
                 debug!("load/precise  {}", namespace);
                 return Ok(());
             }
@@ -170,7 +170,7 @@ impl<'cfg> PackageRegistry<'cfg> {
             // then we're done, otherwise we need to need to move forward
             // updating this source.
             Some((previous, _)) => {
-                if previous.precise() == namespace.precise() {
+                if previous.has_same_precise_as(namespace) {
                     debug!("load/match    {}", namespace);
                     return Ok(());
                 }
@@ -471,9 +471,9 @@ impl<'cfg> PackageRegistry<'cfg> {
         //
         // If we have a precise version, then we'll update lazily during the
         // querying phase. Note that precise in this case is only
-        // `Some("locked")` as other `Some` values indicate a `cargo update
+        // `"locked"` as other values indicate a `cargo update
         // --precise` request
-        if source_id.precise() != Some("locked") {
+        if !source_id.has_locked_precise() {
             self.sources.get_mut(source_id).unwrap().invalidate_cache();
         } else {
             debug!("skipping update due to locked registry");

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 {

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()?;
@@ -332,10 +354,10 @@ impl SourceId {
     pub fn display_registry_name(self) -> String {
         if let Some(key) = self.inner.registry_key.as_ref().map(|k| k.key()) {
             key.into()
-        } else if self.precise().is_some() {
+        } 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,37 +466,81 @@ 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 == Some(Precise::Locked)
+    }
+
+    /// Check if two sources have the same precise data field
+    pub fn has_same_precise_as(self, other: Self) -> bool {
+        self.inner.precise == other.inner.precise
     }
 
     /// Check if the precise data field stores information for this `name`
     /// 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: v,
+            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: 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()
+        })
+    }
+
+    /// Creates a new `SourceId` from this source with the `precise` from some other `SourceId`.
+    pub fn with_precise_from(self, v: Self) -> SourceId {
+        SourceId::wrap(SourceIdInner {
+            precise: v.inner.precise.clone(),
             ..(*self.inner).clone()
         })
     }
@@ -487,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>.
@@ -625,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])?;
                 }

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())
@@ -230,7 +230,7 @@ pub fn update_lockfile(ws: &Workspace<'_>, opts: &UpdateOptions<'_>) -> CargoRes
                     b[i..]
                         .iter()
                         .take_while(|b| a == b)
-                        .all(|b| a.source_id().precise() != b.source_id().precise())
+                        .all(|b| !a.source_id().has_same_precise_as(b.source_id()))
                 })
                 .cloned()
                 .collect()

src/cargo/ops/common_for_install_and_uninstall.rs

@@ -215,7 +215,7 @@ impl InstallTracker {
                 let precise_equal = if source_id.is_git() {
                     // Git sources must have the exact same hash to be
                     // considered "fresh".
-                    dupe_pkg_id.source_id().precise() == source_id.precise()
+                    dupe_pkg_id.source_id().has_same_precise_as(source_id)
                 } else {
                     true
                 };

src/cargo/ops/resolve.rs

@@ -389,12 +389,8 @@ pub fn resolve_with_previous<'cfg>(
                         }) {
                             Some(id_using_default) => {
                                 let id_using_master = id_using_default.with_source_id(
-                                    dep.source_id().with_precise(
-                                        id_using_default
-                                            .source_id()
-                                            .precise()
-                                            .map(|s| s.to_string()),
-                                    ),
+                                    dep.source_id()
+                                        .with_precise_from(id_using_default.source_id()),
                                 );
 
                                 let mut locked_dep = dep.clone();
@@ -796,7 +792,7 @@ fn master_branch_git_source(id: PackageId, resolve: &Resolve) -> Option<PackageI
             let new_source =
                 SourceId::for_git(source.url(), GitReference::Branch("master".to_string()))
                     .unwrap()
-                    .with_precise(source.precise().map(|s| s.to_string()));
+                    .with_precise_from(source);
             return Some(id.with_source_id(new_source));
         }
     }

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() {

src/cargo/sources/config.rs

@@ -145,7 +145,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
                 // Attempt to interpret the source name as an alt registry name
                 if let Ok(alt_id) = SourceId::alt_registry(self.config, name) {
                     debug!("following pointer to registry {}", name);
-                    break alt_id.with_precise(id.precise().map(str::to_string));
+                    break alt_id.with_precise_from(id);
                 }
                 bail!(
                     "could not find a configured source with the \
@@ -163,7 +163,7 @@ impl<'cfg> SourceConfigMap<'cfg> {
                 }
                 None if id == cfg.id => return id.load(self.config, yanked_whitelist),
                 None => {
-                    break cfg.id.with_precise(id.precise().map(|s| s.to_string()));
+                    break cfg.id.with_precise_from(id);
                 }
             }
             debug!("following pointer to {}", name);
@@ -199,7 +199,7 @@ a lock file compatible with `{orig}` cannot be generated in this situation
             );
         }
 
-        if old_src.requires_precise() && id.precise().is_none() {
+        if old_src.requires_precise() && !id.has_precise() {
             bail!(
                 "\
 the source {orig} requires a lock file to be present first before it can be