avoid fetching an exact, cached commit, even if it isn't locked

crates/uv-git-types/src/oid.rs

@@ -5,31 +5,36 @@ use thiserror::Error;
 
 /// Unique identity of any Git object (commit, tree, blob, tag).
 ///
-/// Note this type does not validate whether the input is a valid hash.
+/// This type's `FromStr` implementation validates that it's exactly 40 hex characters, i.e. a
+/// full-length git commit.
+///
+/// If Git's SHA-256 support becomes more widespread in the future (in particular if GitHub ever
+/// adds support), we might need to make this an enum.
 #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
 pub struct GitOid {
-    len: usize,
     bytes: [u8; 40],
 }
 
 impl GitOid {
     /// Return the string representation of an object ID.
     pub fn as_str(&self) -> &str {
-        str::from_utf8(&self.bytes[..self.len]).unwrap()
+        str::from_utf8(&self.bytes).unwrap()
     }
 
     /// Return a truncated representation, i.e., the first 16 characters of the SHA.
     pub fn as_short_str(&self) -> &str {
-        self.as_str().get(..16).unwrap_or(self.as_str())
+        &self.as_str()[..16]
     }
 }
 
 #[derive(Debug, Error, PartialEq)]
 pub enum OidParseError {
-    #[error("Object ID can be at most 40 hex characters")]
-    TooLong,
     #[error("Object ID cannot be parsed from empty string")]
     Empty,
+    #[error("Object ID must be exactly 40 hex characters")]
+    WrongLength,
+    #[error("Object ID must be valid hex characters")]
+    NotHex,
 }
 
 impl FromStr for GitOid {
@@ -40,17 +45,17 @@ impl FromStr for GitOid {
             return Err(OidParseError::Empty);
         }
 
-        if s.len() > 40 {
-            return Err(OidParseError::TooLong);
+        if s.len() != 40 {
+            return Err(OidParseError::WrongLength);
         }
 
-        let mut out = [0; 40];
-        out[..s.len()].copy_from_slice(s.as_bytes());
+        if !s.chars().all(|ch| ch.is_ascii_hexdigit()) {
+            return Err(OidParseError::NotHex);
+        }
 
-        Ok(GitOid {
-            len: s.len(),
-            bytes: out,
-        })
+        let mut bytes = [0; 40];
+        bytes.copy_from_slice(s.as_bytes());
+        Ok(GitOid { bytes })
     }
 }
 
@@ -101,11 +106,20 @@ mod tests {
     #[test]
     fn git_oid() {
         GitOid::from_str("4a23745badf5bf5ef7928f1e346e9986bd696d82").unwrap();
+        GitOid::from_str("4A23745BADF5BF5EF7928F1E346E9986BD696D82").unwrap();
 
         assert_eq!(GitOid::from_str(""), Err(OidParseError::Empty));
         assert_eq!(
             GitOid::from_str(&str::repeat("a", 41)),
-            Err(OidParseError::TooLong)
+            Err(OidParseError::WrongLength)
+        );
+        assert_eq!(
+            GitOid::from_str(&str::repeat("a", 39)),
+            Err(OidParseError::WrongLength)
+        );
+        assert_eq!(
+            GitOid::from_str(&str::repeat("x", 40)),
+            Err(OidParseError::NotHex)
         );
     }
 }

crates/uv-git/src/resolver.rs

@@ -12,6 +12,7 @@ use tracing::debug;
 use uv_cache_key::{RepositoryUrl, cache_digest};
 use uv_fs::LockedFile;
 use uv_git_types::{GitHubRepository, GitOid, GitReference, GitUrl};
+use uv_static::EnvVars;
 use uv_version::version;
 
 use crate::{Fetch, GitSource, Reporter};
@@ -54,6 +55,10 @@ impl GitResolver {
         url: &GitUrl,
         client: ClientWithMiddleware,
     ) -> Result<Option<GitOid>, GitResolverError> {
+        if std::env::var_os(EnvVars::UV_NO_GITHUB_FAST_PATH).is_some() {
+            return Ok(None);
+        }
+
         let reference = RepositoryReference::from(url);
 
         // If the URL is already precise, return it.

crates/uv-git/src/source.rs

@@ -11,11 +11,11 @@ use reqwest_middleware::ClientWithMiddleware;
 use tracing::{debug, instrument};
 
 use uv_cache_key::{RepositoryUrl, cache_digest};
-use uv_git_types::GitUrl;
+use uv_git_types::{GitOid, GitReference, GitUrl};
 use uv_redacted::DisplaySafeUrl;
 
 use crate::GIT_STORE;
-use crate::git::GitRemote;
+use crate::git::{GitDatabase, GitRemote};
 
 /// A remote Git source that can be checked out locally.
 pub struct GitSource {
@@ -86,40 +86,59 @@ impl GitSource {
             Cow::Borrowed(self.git.repository())
         };
 
-        let remote = GitRemote::new(&remote);
-        let (db, actual_rev, task) = match (self.git.precise(), remote.db_at(&db_path).ok()) {
-            // If we have a locked revision, and we have a preexisting database
-            // which has that revision, then no update needs to happen.
-            (Some(rev), Some(db)) if db.contains(rev) => {
-                debug!("Using existing Git source `{}`", self.git.repository());
-                (db, rev, None)
+        // Fetch the commit, if we don't already have it. Wrapping this section in a closure makes
+        // it easier to short-circuit this in the cases where we do have the commit.
+        let (db, actual_rev, maybe_task) = || -> Result<(GitDatabase, GitOid, Option<usize>)> {
+            let git_remote = GitRemote::new(&remote);
+            let maybe_db = git_remote.db_at(&db_path).ok();
+
+            // If we have a locked revision, and we have a pre-existing database which has that
+            // revision, then no update needs to happen.
+            if let (Some(rev), Some(db)) = (self.git.precise(), &maybe_db) {
+                if db.contains(rev) {
+                    debug!("Using existing Git source `{}`", self.git.repository());
+                    return Ok((maybe_db.unwrap(), rev, None));
+                }
             }
 
-            // ... otherwise we use this state to update the git database. Note
-            // that we still check for being offline here, for example in the
-            // situation that we have a locked revision but the database
-            // doesn't have it.
-            (locked_rev, db) => {
-                debug!("Updating Git source `{}`", self.git.repository());
-
-                // Report the checkout operation to the reporter.
-                let task = self.reporter.as_ref().map(|reporter| {
-                    reporter.on_checkout_start(remote.url(), self.git.reference().as_rev())
-                });
-
-                let (db, actual_rev) = remote.checkout(
-                    &db_path,
-                    db,
-                    self.git.reference(),
-                    locked_rev,
-                    &self.client,
-                    self.disable_ssl,
-                    self.offline,
-                )?;
-
-                (db, actual_rev, task)
+            // If the revision isn't locked, but it looks like it might be an exact commit hash,
+            // and we do have a pre-existing database, then check whether it is, in fact, a commit
+            // hash. If so, treat it like it's locked.
+            if let Some(db) = &maybe_db {
+                if let GitReference::BranchOrTagOrCommit(maybe_commit) = self.git.reference() {
+                    if let Ok(oid) = maybe_commit.parse::<GitOid>() {
+                        if db.contains(oid) {
+                            // This reference is an exact commit. Treat it like it's
+                            // locked.
+                            debug!("Using existing Git source `{}`", self.git.repository());
+                            return Ok((maybe_db.unwrap(), oid, None));
+                        }
+                    }
+                }
             }
-        };
+
+            // ... otherwise, we use this state to update the Git database. Note that we still check
+            // for being offline here, for example in the situation that we have a locked revision
+            // but the database doesn't have it.
+            debug!("Updating Git source `{}`", self.git.repository());
+
+            // Report the checkout operation to the reporter.
+            let task = self.reporter.as_ref().map(|reporter| {
+                reporter.on_checkout_start(git_remote.url(), self.git.reference().as_rev())
+            });
+
+            let (db, actual_rev) = git_remote.checkout(
+                &db_path,
+                maybe_db,
+                self.git.reference(),
+                self.git.precise(),
+                &self.client,
+                self.disable_ssl,
+                self.offline,
+            )?;
+
+            Ok((db, actual_rev, task))
+        }()?;
 
         // Don’t use the full hash, in order to contribute less to reaching the
         // path length limit on Windows.
@@ -137,9 +156,9 @@ impl GitSource {
         db.copy_to(actual_rev, &checkout_path)?;
 
         // Report the checkout operation to the reporter.
-        if let Some(task) = task {
+        if let Some(task) = maybe_task {
             if let Some(reporter) = self.reporter.as_ref() {
-                reporter.on_checkout_complete(remote.url(), actual_rev.as_str(), task);
+                reporter.on_checkout_complete(remote.as_ref(), actual_rev.as_str(), task);
             }
         }
 

crates/uv-static/src/env_vars.rs

@@ -727,4 +727,7 @@ impl EnvVars {
 
     /// Equivalent to the `--project` command-line argument.
     pub const UV_PROJECT: &'static str = "UV_PROJECT";
+
+    /// Disable GitHub-specific requests that allow uv to skip `git fetch` in some circumstances.
+    pub const UV_NO_GITHUB_FAST_PATH: &'static str = "UV_NO_GITHUB_FAST_PATH";
 }

crates/uv/tests/it/pip_install.rs

@@ -2111,6 +2111,53 @@ fn install_git_public_https_missing_commit() {
     "###);
 }
 
+#[test]
+#[cfg(feature = "git")]
+fn install_git_public_https_exact_commit() {
+    let context = TestContext::new(DEFAULT_PYTHON_VERSION);
+
+    // `uv pip install` a Git dependency with an exact commit.
+    uv_snapshot!(context.filters(), context.pip_install()
+        // Normally Updating/Updated notifications are suppressed in tests (because their order can
+        // be nondeterministic), but here that's exactly what we want to test for.
+        .env_remove(EnvVars::UV_TEST_NO_CLI_PROGRESS)
+        // Whether fetching happens during resolution or later depends on whether the GitHub fast
+        // path is taken, which isn't reliable. Disable it, so that we get a stable order of events
+        // here.
+        .env(EnvVars::UV_NO_GITHUB_FAST_PATH, "true")
+        .arg("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389")
+        , @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+       Updating https://github.com/astral-test/uv-public-pypackage (b270df1a2fb5d012294e9aaf05e7e0bab1e6a389)
+        Updated https://github.com/astral-test/uv-public-pypackage (b270df1a2fb5d012294e9aaf05e7e0bab1e6a389)
+    Resolved 1 package in [TIME]
+       Building uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389
+          Built uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
+     + uv-public-pypackage==0.1.0 (from git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389)
+    ");
+
+    // Run the exact same command again, with that commit now in cache.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .env_remove(EnvVars::UV_TEST_NO_CLI_PROGRESS)
+        .env(EnvVars::UV_NO_GITHUB_FAST_PATH, "true")
+        .arg("uv-public-pypackage @ git+https://github.com/astral-test/uv-public-pypackage@b270df1a2fb5d012294e9aaf05e7e0bab1e6a389")
+        , @r"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Resolved 1 package in [TIME]
+    Audited 1 package in [TIME]
+    ");
+}
+
 /// Install a package from a private GitHub repository using a PAT
 #[test]
 #[cfg(all(not(windows), feature = "git"))]

docs/reference/environment.md

@@ -235,6 +235,10 @@ non-editable
 
 Ignore `.env` files when executing `uv run` commands.
 
+### `UV_NO_GITHUB_FAST_PATH`
+
+Disable GitHub-specific requests that allow uv to skip `git fetch` in some circumstances.
+
 ### `UV_NO_INSTALLER_METADATA`
 
 Skip writing `uv` installer metadata files (e.g., `INSTALLER`, `REQUESTED`, and `direct_url.json`) to site-packages `.dist-info` directories.