diff --git a/crates/uv-git-types/src/oid.rs b/crates/uv-git-types/src/oid.rs index 2ccd376b731f1..9319f5e3402fb 100644 --- a/crates/uv-git-types/src/oid.rs +++ b/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) ); } } diff --git a/crates/uv-git/src/resolver.rs b/crates/uv-git/src/resolver.rs index 9335aed4d0ad5..fd90ff587e2bc 100644 --- a/crates/uv-git/src/resolver.rs +++ b/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, 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. diff --git a/crates/uv-git/src/source.rs b/crates/uv-git/src/source.rs index 2031c49e51e27..cb6d0a24f3941 100644 --- a/crates/uv-git/src/source.rs +++ b/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)> { + 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::() { + 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); } } diff --git a/crates/uv-static/src/env_vars.rs b/crates/uv-static/src/env_vars.rs index 58191fe64411e..aff56df459657 100644 --- a/crates/uv-static/src/env_vars.rs +++ b/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"; } diff --git a/crates/uv/tests/it/pip_install.rs b/crates/uv/tests/it/pip_install.rs index 872db659f671b..fa823cce0f5b7 100644 --- a/crates/uv/tests/it/pip_install.rs +++ b/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"))] diff --git a/docs/reference/environment.md b/docs/reference/environment.md index f86f52bcae384..61889ddb3ea6c 100644 --- a/docs/reference/environment.md +++ b/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.