From d99983a630a4ebd232f0aeec56e9dc862d58ed60 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Tue, 8 Apr 2025 22:00:40 -0400 Subject: [PATCH] Avoid querying GitHub on repeated install invocations (#12767) ## Summary If you run `cargo run pip install "pip-test-package @ git+https://github.com/pypa/pip-test-package@5547fa909e83df8bd743d3978d6667497983a4b7"` repeatedly, then every time, we'll take the "GitHub fast path" every time, even if the package is already cached. This PR adds logic to skip the fast path if the reference looks like a commit that we've already checked out. Closes https://github.com/astral-sh/uv/issues/12760. --- crates/uv-distribution/src/source/mod.rs | 120 ++++++++++++++--------- crates/uv-git-types/src/oid.rs | 2 +- crates/uv-git/src/resolver.rs | 7 +- 3 files changed, 80 insertions(+), 49 deletions(-) diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index c08142a4d..32338581b 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -1552,58 +1552,84 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { return Err(Error::HashesNotSupportedGit(source.to_string())); } - // If this is GitHub URL, attempt to resolve to a precise commit using the GitHub API. - match self - .build_context - .git() - .github_fast_path( - resource.git, - client - .unmanaged - .uncached_client(resource.git.repository()) - .clone(), - ) - .await + // If the reference appears to be a commit, and we've already checked it out, avoid taking + // the GitHub fast path. + let cache_shard = resource + .git + .reference() + .as_str() + .and_then(|reference| GitOid::from_str(reference).ok()) + .map(|oid| { + self.build_context.cache().shard( + CacheBucket::SourceDistributions, + WheelCache::Git(resource.url, oid.as_short_str()).root(), + ) + }); + if cache_shard + .as_ref() + .is_some_and(|cache_shard| cache_shard.is_dir()) { - Ok(Some(precise)) => { - // There's no need to check the cache, since we can't use cached metadata if there are - // sources, and we can't know if there are sources without fetching the - // `pyproject.toml`. - // - // For the same reason, there's no need to write to the cache, since we won't be able to - // use it on subsequent runs. - match self - .github_metadata(precise, source, resource, client) - .await - { - Ok(Some(metadata)) => { - // Validate the metadata, but ignore it if the metadata doesn't match. - match validate_metadata(source, &metadata) { - Ok(()) => { - debug!("Found static metadata via GitHub fast path for: {source}"); - return Ok(ArchiveMetadata { - metadata: Metadata::from_metadata23(metadata), - hashes: HashDigests::empty(), - }); - } - Err(err) => { - debug!("Ignoring `pyproject.toml` from GitHub for {source}: {err}"); + debug!("Skipping GitHub fast path for: {source} (shard exists)"); + } else { + debug!("Attempting GitHub fast path for: {source}"); + + // If this is GitHub URL, attempt to resolve to a precise commit using the GitHub API. + match self + .build_context + .git() + .github_fast_path( + resource.git, + client + .unmanaged + .uncached_client(resource.git.repository()) + .clone(), + ) + .await + { + Ok(Some(precise)) => { + // There's no need to check the cache, since we can't use cached metadata if there are + // sources, and we can't know if there are sources without fetching the + // `pyproject.toml`. + // + // For the same reason, there's no need to write to the cache, since we won't be able to + // use it on subsequent runs. + match self + .github_metadata(precise, source, resource, client) + .await + { + Ok(Some(metadata)) => { + // Validate the metadata, but ignore it if the metadata doesn't match. + match validate_metadata(source, &metadata) { + Ok(()) => { + debug!( + "Found static metadata via GitHub fast path for: {source}" + ); + return Ok(ArchiveMetadata { + metadata: Metadata::from_metadata23(metadata), + hashes: HashDigests::empty(), + }); + } + Err(err) => { + debug!( + "Ignoring `pyproject.toml` from GitHub for {source}: {err}" + ); + } } } - } - Ok(None) => { - // Nothing to do. - } - Err(err) => { - debug!("Failed to fetch `pyproject.toml` via GitHub fast path for: {source} ({err})"); + Ok(None) => { + // Nothing to do. + } + Err(err) => { + debug!("Failed to fetch `pyproject.toml` via GitHub fast path for: {source} ({err})"); + } } } - } - Ok(None) => { - // Nothing to do. - } - Err(err) => { - debug!("Failed to resolve commit via GitHub fast path for: {source} ({err})"); + Ok(None) => { + // Nothing to do. + } + Err(err) => { + debug!("Failed to resolve commit via GitHub fast path for: {source} ({err})"); + } } } diff --git a/crates/uv-git-types/src/oid.rs b/crates/uv-git-types/src/oid.rs index 51ad40daa..2ccd376b7 100644 --- a/crates/uv-git-types/src/oid.rs +++ b/crates/uv-git-types/src/oid.rs @@ -20,7 +20,7 @@ impl GitOid { /// Return a truncated representation, i.e., the first 16 characters of the SHA. pub fn as_short_str(&self) -> &str { - &self.as_str()[..16] + self.as_str().get(..16).unwrap_or(self.as_str()) } } diff --git a/crates/uv-git/src/resolver.rs b/crates/uv-git/src/resolver.rs index 3e909efe9..9d9b216b7 100644 --- a/crates/uv-git/src/resolver.rs +++ b/crates/uv-git/src/resolver.rs @@ -56,6 +56,11 @@ impl GitResolver { ) -> Result, GitResolverError> { let reference = RepositoryReference::from(url); + // If the URL is already precise, return it. + if let Some(precise) = url.precise() { + return Ok(Some(precise)); + } + // If we know the precise commit already, return it. if let Some(precise) = self.get(&reference) { return Ok(Some(*precise)); @@ -72,7 +77,7 @@ impl GitResolver { let url = format!("https://api.github.com/repos/{owner}/{repo}/commits/{rev}"); - debug!("Attempting GitHub fast path for: {url}"); + debug!("Querying GitHub for commit at: {url}"); let mut request = client.get(&url); request = request.header("Accept", "application/vnd.github.3.sha"); request = request.header(