Remove FullCommit variant

charliermarsh · charliermarsh · commit 40e6880e41b9 · 2025-01-21T14:18:05.000-05:00
diff --git a/crates/uv-git/src/git.rs b/crates/uv-git/src/git.rs
@@ -40,6 +40,14 @@ pub static GIT: LazyLock<Result<PathBuf, GitError>> = LazyLock::new(|| {
     })
 });
 
+/// Strategy when fetching refspecs for a [`GitReference`]
+enum RefspecStrategy {
+    /// All refspecs should be fetched, if any fail then the fetch will fail.
+    All,
+    /// Stop after the first successful fetch, if none succeed then the fetch will fail.
+    First,
+}
+
 /// A reference to commit or commit-ish.
 #[derive(Debug, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
 pub enum GitReference {
@@ -49,24 +57,14 @@ pub enum GitReference {
     Tag(String),
     /// From a reference that's ambiguously a branch or tag.
     BranchOrTag(String),
-    /// From a reference that's ambiguously a short commit, a branch, or a tag.
+    /// From a reference that's ambiguously a commit, branch, or tag.
     BranchOrTagOrCommit(String),
     /// From a named reference, like `refs/pull/493/head`.
     NamedRef(String),
-    /// From a specific revision, using a full 40-character commit hash.
-    FullCommit(GitOid),
     /// The default branch of the repository, the reference named `HEAD`.
     DefaultBranch,
 }
 
-/// Strategy when fetching refspecs for a [`GitReference`]
-enum RefspecStrategy {
-    // All refspecs should be fetched, if any fail then the fetch will fail
-    All,
-    // Stop after the first successful fetch, if none succeed then the fetch will fail
-    First,
-}
-
 impl GitReference {
     /// Creates a [`GitReference`] from an arbitrary revision string, which could represent a
     /// branch, tag, commit, or named ref.
@@ -87,7 +85,6 @@ impl GitReference {
             Self::Branch(rev) => Some(rev),
             Self::BranchOrTag(rev) => Some(rev),
             Self::BranchOrTagOrCommit(rev) => Some(rev),
-            Self::FullCommit(rev) => Some(rev.as_str()),
             Self::NamedRef(rev) => Some(rev),
             Self::DefaultBranch => None,
         }
@@ -100,7 +97,6 @@ impl GitReference {
             Self::Branch(rev) => rev,
             Self::BranchOrTag(rev) => rev,
             Self::BranchOrTagOrCommit(rev) => rev,
-            Self::FullCommit(rev) => rev.as_str(),
             Self::NamedRef(rev) => rev,
             Self::DefaultBranch => "HEAD",
         }
@@ -112,26 +108,93 @@ impl GitReference {
             Self::Branch(_) => "branch",
             Self::Tag(_) => "tag",
             Self::BranchOrTag(_) => "branch or tag",
-            Self::FullCommit(_) => "commit",
             Self::BranchOrTagOrCommit(_) => "branch, tag, or commit",
             Self::NamedRef(_) => "ref",
             Self::DefaultBranch => "default branch",
         }
     }
+}
 
-    /// Returns the precise [`GitOid`] of this reference, if it's a full commit.
-    pub(crate) fn as_sha(&self) -> Option<GitOid> {
-        if let Self::FullCommit(rev) = self {
-            Some(*rev)
-        } else {
-            None
+impl Display for GitReference {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}", self.as_str().unwrap_or("HEAD"))
+    }
+}
+
+/// A Git reference (like a tag or branch) or a specific commit.
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Hash, PartialOrd, Ord)]
+enum ReferenceOrOid<'reference> {
+    /// A Git reference, like a tag or branch.
+    Reference(&'reference GitReference),
+    /// A specific commit.
+    Oid(GitOid),
+}
+
+impl ReferenceOrOid<'_> {
+    /// Resolves the [`ReferenceOrOid`] to an object ID with objects the `repo` currently has.
+    fn resolve(&self, repo: &GitRepository) -> Result<GitOid> {
+        let refkind = self.kind_str();
+        let result = match self {
+            // Resolve the commit pointed to by the tag.
+            //
+            // `^0` recursively peels away from the revision to the underlying commit object.
+            // This also verifies that the tag indeed refers to a commit.
+            Self::Reference(GitReference::Tag(s)) => {
+                repo.rev_parse(&format!("refs/remotes/origin/tags/{s}^0"))
+            }
+
+            // Resolve the commit pointed to by the branch.
+            Self::Reference(GitReference::Branch(s)) => repo.rev_parse(&format!("origin/{s}^0")),
+
+            // Attempt to resolve the branch, then the tag.
+            Self::Reference(GitReference::BranchOrTag(s)) => repo
+                .rev_parse(&format!("origin/{s}^0"))
+                .or_else(|_| repo.rev_parse(&format!("refs/remotes/origin/tags/{s}^0"))),
+
+            // Attempt to resolve the branch, then the tag, then the commit.
+            Self::Reference(GitReference::BranchOrTagOrCommit(s)) => repo
+                .rev_parse(&format!("origin/{s}^0"))
+                .or_else(|_| repo.rev_parse(&format!("refs/remotes/origin/tags/{s}^0")))
+                .or_else(|_| repo.rev_parse(&format!("{s}^0"))),
+
+            // We'll be using the HEAD commit.
+            Self::Reference(GitReference::DefaultBranch) => {
+                repo.rev_parse("refs/remotes/origin/HEAD")
+            }
+
+            // Resolve a named reference.
+            Self::Reference(GitReference::NamedRef(s)) => repo.rev_parse(&format!("{s}^0")),
+
+            // Resolve a specific commit.
+            Self::Oid(s) => repo.rev_parse(&format!("{s}^0")),
+        };
+
+        result.with_context(|| anyhow::format_err!("failed to find {refkind} `{self}`"))
+    }
+
+    /// Returns the kind of this [`ReferenceOrOid`].
+    fn kind_str(&self) -> &str {
+        match self {
+            Self::Reference(reference) => reference.kind_str(),
+            Self::Oid(_) => "commit",
+        }
+    }
+
+    /// Converts the [`ReferenceOrOid`] to a `str` that can be used as a revision.
+    fn as_rev(&self) -> &str {
+        match self {
+            Self::Reference(r) => r.as_rev(),
+            Self::Oid(rev) => rev.as_str(),
         }
     }
 }
 
-impl Display for GitReference {
+impl Display for ReferenceOrOid<'_> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
-        write!(f, "{}", self.as_str().unwrap_or("HEAD"))
+        match self {
+            Self::Reference(reference) => write!(f, "{reference}"),
+            Self::Oid(oid) => write!(f, "{oid}"),
+        }
     }
 }
 
@@ -241,8 +304,9 @@ impl GitRemote {
         locked_rev: Option<GitOid>,
         client: &ClientWithMiddleware,
     ) -> Result<(GitDatabase, GitOid)> {
-        let locked_ref = locked_rev.map(GitReference::FullCommit);
-        let reference = locked_ref.as_ref().unwrap_or(reference);
+        let reference = locked_rev
+            .map(ReferenceOrOid::Oid)
+            .unwrap_or(ReferenceOrOid::Reference(reference));
         let enable_lfs_fetch = env::var(EnvVars::UV_GIT_LFS).is_ok();
 
         if let Some(mut db) = db {
@@ -334,45 +398,6 @@ impl GitDatabase {
     }
 }
 
-impl GitReference {
-    /// Resolves self to an object ID with objects the `repo` currently has.
-    pub(crate) fn resolve(&self, repo: &GitRepository) -> Result<GitOid> {
-        let refkind = self.kind_str();
-        let result = match self {
-            // Resolve the commit pointed to by the tag.
-            //
-            // `^0` recursively peels away from the revision to the underlying commit object.
-            // This also verifies that the tag indeed refers to a commit.
-            Self::Tag(s) => repo.rev_parse(&format!("refs/remotes/origin/tags/{s}^0")),
-
-            // Resolve the commit pointed to by the branch.
-            Self::Branch(s) => repo.rev_parse(&format!("origin/{s}^0")),
-
-            // Attempt to resolve the branch, then the tag.
-            Self::BranchOrTag(s) => repo
-                .rev_parse(&format!("origin/{s}^0"))
-                .or_else(|_| repo.rev_parse(&format!("refs/remotes/origin/tags/{s}^0"))),
-
-            // Attempt to resolve the branch, then the tag, then the commit.
-            Self::BranchOrTagOrCommit(s) => repo
-                .rev_parse(&format!("origin/{s}^0"))
-                .or_else(|_| repo.rev_parse(&format!("refs/remotes/origin/tags/{s}^0")))
-                .or_else(|_| repo.rev_parse(&format!("{s}^0"))),
-
-            // We'll be using the HEAD commit.
-            Self::DefaultBranch => repo.rev_parse("refs/remotes/origin/HEAD"),
-
-            // Resolve a named reference.
-            Self::NamedRef(s) => repo.rev_parse(&format!("{s}^0")),
-
-            // Resolve a direct commit reference.
-            Self::FullCommit(s) => repo.rev_parse(&format!("{s}^0")),
-        };
-
-        result.with_context(|| anyhow::format_err!("failed to find {refkind} `{self}`"))
-    }
-}
-
 impl GitCheckout {
     /// Creates an instance of [`GitCheckout`]. This doesn't imply the checkout
     /// is done. Use [`GitCheckout::is_fresh`] to check.
@@ -472,10 +497,10 @@ impl GitCheckout {
 /// * Dispatches `git fetch` using the git CLI.
 ///
 /// The `remote_url` argument is the git remote URL where we want to fetch from.
-pub(crate) fn fetch(
+fn fetch(
     repo: &mut GitRepository,
     remote_url: &Url,
-    reference: &GitReference,
+    reference: ReferenceOrOid<'_>,
     client: &ClientWithMiddleware,
 ) -> Result<()> {
     let oid_to_fetch = match github_fast_path(repo, remote_url, reference, client) {
@@ -499,15 +524,15 @@ pub(crate) fn fetch(
     match reference {
         // For branches and tags we can fetch simply one reference and copy it
         // locally, no need to fetch other branches/tags.
-        GitReference::Branch(branch) => {
+        ReferenceOrOid::Reference(GitReference::Branch(branch)) => {
             refspecs.push(format!("+refs/heads/{branch}:refs/remotes/origin/{branch}"));
         }
 
-        GitReference::Tag(tag) => {
+        ReferenceOrOid::Reference(GitReference::Tag(tag)) => {
             refspecs.push(format!("+refs/tags/{tag}:refs/remotes/origin/tags/{tag}"));
         }
 
-        GitReference::BranchOrTag(branch_or_tag) => {
+        ReferenceOrOid::Reference(GitReference::BranchOrTag(branch_or_tag)) => {
             refspecs.push(format!(
                 "+refs/heads/{branch_or_tag}:refs/remotes/origin/{branch_or_tag}"
             ));
@@ -519,7 +544,7 @@ pub(crate) fn fetch(
 
         // For ambiguous references, we can fetch the exact commit (if known); otherwise,
         // we fetch all branches and tags.
-        GitReference::BranchOrTagOrCommit(branch_or_tag_or_commit) => {
+        ReferenceOrOid::Reference(GitReference::BranchOrTagOrCommit(branch_or_tag_or_commit)) => {
             // The `oid_to_fetch` is the exact commit we want to fetch. But it could be the exact
             // commit of a branch or tag. We should only fetch it directly if it's the exact commit
             // of a short commit hash.
@@ -537,15 +562,15 @@ pub(crate) fn fetch(
             }
         }
 
-        GitReference::DefaultBranch => {
+        ReferenceOrOid::Reference(GitReference::DefaultBranch) => {
             refspecs.push(String::from("+HEAD:refs/remotes/origin/HEAD"));
         }
 
-        GitReference::NamedRef(rev) => {
+        ReferenceOrOid::Reference(GitReference::NamedRef(rev)) => {
             refspecs.push(format!("+{rev}:{rev}"));
         }
 
-        GitReference::FullCommit(rev) => {
+        ReferenceOrOid::Oid(rev) => {
             refspecs.push(format!("+{rev}:refs/commit/{rev}"));
         }
     }
@@ -587,7 +612,7 @@ pub(crate) fn fetch(
     };
     match reference {
         // With the default branch, adding context is confusing
-        GitReference::DefaultBranch => result,
+        ReferenceOrOid::Reference(GitReference::DefaultBranch) => result,
         _ => result.with_context(|| {
             format!(
                 "failed to fetch {} `{}`",
@@ -703,7 +728,7 @@ enum FastPathRev {
 fn github_fast_path(
     git: &mut GitRepository,
     url: &Url,
-    reference: &GitReference,
+    reference: ReferenceOrOid<'_>,
     client: &ClientWithMiddleware,
 ) -> Result<FastPathRev> {
     let Some(GitHubRepository { owner, repo }) = GitHubRepository::parse(url) else {
@@ -713,12 +738,12 @@ fn github_fast_path(
     let local_object = reference.resolve(git).ok();
 
     let github_branch_name = match reference {
-        GitReference::Branch(branch) => branch,
-        GitReference::Tag(tag) => tag,
-        GitReference::BranchOrTag(branch_or_tag) => branch_or_tag,
-        GitReference::DefaultBranch => "HEAD",
-        GitReference::NamedRef(rev) => rev,
-        GitReference::BranchOrTagOrCommit(rev) => {
+        ReferenceOrOid::Reference(GitReference::DefaultBranch) => "HEAD",
+        ReferenceOrOid::Reference(GitReference::Branch(branch)) => branch,
+        ReferenceOrOid::Reference(GitReference::Tag(tag)) => tag,
+        ReferenceOrOid::Reference(GitReference::BranchOrTag(branch_or_tag)) => branch_or_tag,
+        ReferenceOrOid::Reference(GitReference::NamedRef(rev)) => rev,
+        ReferenceOrOid::Reference(GitReference::BranchOrTagOrCommit(rev)) => {
             // `revparse_single` (used by `resolve`) is the only way to turn
             // short hash -> long hash, but it also parses other things,
             // like branch and tag names, which might coincidentally be
@@ -743,18 +768,18 @@ fn github_fast_path(
             }
             rev
         }
-        GitReference::FullCommit(rev) => {
+        ReferenceOrOid::Oid(rev) => {
             debug!("Skipping GitHub fast path; full commit hash provided: {rev}");
 
-            if let Some(ref local_object) = local_object {
+            if let Some(local_object) = local_object {
                 if rev == local_object {
                     return Ok(FastPathRev::UpToDate);
                 }
             }
 
             // If we know the reference is a full commit hash, we can just return it without
             // querying GitHub.
-            return Ok(FastPathRev::NeedsFetch(*rev));
+            return Ok(FastPathRev::NeedsFetch(rev));
         }
     };
 
diff --git a/crates/uv-git/src/lib.rs b/crates/uv-git/src/lib.rs
@@ -31,11 +31,10 @@ pub struct GitUrl {
 impl GitUrl {
     /// Create a new [`GitUrl`] from a repository URL and a reference.
     pub fn from_reference(repository: Url, reference: GitReference) -> Self {
-        let precise = reference.as_sha();
         Self {
             repository,
             reference,
-            precise,
+            precise: None,
         }
     }
 
@@ -113,9 +112,6 @@ impl From<GitUrl> for Url {
         } else {
             // Otherwise, add the branch or tag name.
             match git.reference {
-                GitReference::FullCommit(rev) => {
-                    url.set_path(&format!("{}@{}", url.path(), rev));
-                }
                 GitReference::Branch(rev)
                 | GitReference::Tag(rev)
                 | GitReference::BranchOrTag(rev)
diff --git a/crates/uv-pypi-types/src/requirement.rs b/crates/uv-pypi-types/src/requirement.rs
@@ -745,9 +745,6 @@ impl From<RequirementSource> for RequirementSourceWire {
                     | GitReference::NamedRef(rev) => {
                         url.query_pairs_mut().append_pair("rev", rev.as_str());
                     }
-                    GitReference::FullCommit(rev) => {
-                        url.query_pairs_mut().append_pair("rev", rev.as_str());
-                    }
                     GitReference::DefaultBranch => {}
                 }
 
diff --git a/crates/uv-resolver/src/lock/mod.rs b/crates/uv-resolver/src/lock/mod.rs
@@ -3511,7 +3511,6 @@ impl From<GitReference> for GitSourceKind {
             GitReference::BranchOrTag(rev) => GitSourceKind::Rev(rev.to_string()),
             GitReference::BranchOrTagOrCommit(rev) => GitSourceKind::Rev(rev.to_string()),
             GitReference::NamedRef(rev) => GitSourceKind::Rev(rev.to_string()),
-            GitReference::FullCommit(rev) => GitSourceKind::Rev(rev.to_string()),
             GitReference::DefaultBranch => GitSourceKind::DefaultBranch,
         }
     }
@@ -3564,9 +3563,6 @@ fn locked_git_url(git_dist: &GitSourceDist) -> Url {
         | GitReference::NamedRef(rev) => {
             url.query_pairs_mut().append_pair("rev", rev.as_str());
         }
-        GitReference::FullCommit(rev) => {
-            url.query_pairs_mut().append_pair("rev", rev.as_str());
-        }
         GitReference::DefaultBranch => {}
     }
 
diff --git a/crates/uv-resolver/src/redirect.rs b/crates/uv-resolver/src/redirect.rs
diff --git a/crates/uv-workspace/src/pyproject.rs b/crates/uv-workspace/src/pyproject.rs

Original file line number	Diff line number	Diff line change
`@@ -745,9 +745,6 @@ impl From<RequirementSource> for RequirementSourceWire {`
`745`	`745`	`\| GitReference::NamedRef(rev) => {`
`746`	`746`	`url.query_pairs_mut().append_pair("rev", rev.as_str());`
`747`	`747`	`}`
`748`		`- GitReference::FullCommit(rev) => {`
`749`		`- url.query_pairs_mut().append_pair("rev", rev.as_str());`
`750`		`- }`
`751`	`748`	`GitReference::DefaultBranch => {}`
`752`	`749`	`}`
`753`	`750`
Original file line number	Diff line number	Diff line change
`@@ -3511,7 +3511,6 @@ impl From<GitReference> for GitSourceKind {`
`3511`	`3511`	`GitReference::BranchOrTag(rev) => GitSourceKind::Rev(rev.to_string()),`
`3512`	`3512`	`GitReference::BranchOrTagOrCommit(rev) => GitSourceKind::Rev(rev.to_string()),`
`3513`	`3513`	`GitReference::NamedRef(rev) => GitSourceKind::Rev(rev.to_string()),`
`3514`		`- GitReference::FullCommit(rev) => GitSourceKind::Rev(rev.to_string()),`
`3515`	`3514`	`GitReference::DefaultBranch => GitSourceKind::DefaultBranch,`
`3516`	`3515`	`}`
`3517`	`3516`	`}`
`@@ -3564,9 +3563,6 @@ fn locked_git_url(git_dist: &GitSourceDist) -> Url {`
`3564`	`3563`	`\| GitReference::NamedRef(rev) => {`
`3565`	`3564`	`url.query_pairs_mut().append_pair("rev", rev.as_str());`
`3566`	`3565`	`}`
`3567`		`- GitReference::FullCommit(rev) => {`
`3568`		`- url.query_pairs_mut().append_pair("rev", rev.as_str());`
`3569`		`- }`
`3570`	`3566`	`GitReference::DefaultBranch => {}`
`3571`	`3567`	`}`
`3572`	`3568`