From bc1d7764e226000e15ec105c21cc13813dd2c13b Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 7 Aug 2024 18:35:05 -0400 Subject: [PATCH] Combine fetch and resolve steps in Git resolver (#5886) ## Summary Whenever we call `resolve`, we immediately call `fetch` after. And in some cases `resolve` actually calls `fetch` internally. It seems a lot simpler to just merge these into one method that returns a `Fetch` (which itself contains the fully-resolved URL). Closes https://github.com/astral-sh/uv/issues/5876. --- crates/uv-distribution/src/source/mod.rs | 38 ++------------------ crates/uv-git/src/resolver.rs | 45 ++---------------------- crates/uv/tests/lock.rs | 2 +- 3 files changed, 6 insertions(+), 79 deletions(-) diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs index d642e6454..1bab8075c 100644 --- a/crates/uv-distribution/src/source/mod.rs +++ b/crates/uv-distribution/src/source/mod.rs @@ -1121,29 +1121,12 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { return Err(Error::HashesNotSupportedGit(source.to_string())); } - // Resolve to a precise Git SHA. - let url = if let Some(url) = self - .build_context - .git() - .resolve( - resource.git, - client.unmanaged.uncached_client().client(), - self.build_context.cache().bucket(CacheBucket::Git), - self.reporter.clone().map(Facade::from), - ) - .await? - { - Cow::Owned(url) - } else { - Cow::Borrowed(resource.git) - }; - // Fetch the Git repository. let fetch = self .build_context .git() .fetch( - &url, + resource.git, client.unmanaged.uncached_client().client(), self.build_context.cache().bucket(CacheBucket::Git), self.reporter.clone().map(Facade::from), @@ -1208,29 +1191,12 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> { return Err(Error::HashesNotSupportedGit(source.to_string())); } - // Resolve to a precise Git SHA. - let url = if let Some(url) = self - .build_context - .git() - .resolve( - resource.git, - client.unmanaged.uncached_client().client(), - self.build_context.cache().bucket(CacheBucket::Git), - self.reporter.clone().map(Facade::from), - ) - .await? - { - Cow::Owned(url) - } else { - Cow::Borrowed(resource.git) - }; - // Fetch the Git repository. let fetch = self .build_context .git() .fetch( - &url, + resource.git, client.unmanaged.uncached_client().client(), self.build_context.cache().bucket(CacheBucket::Git), self.reporter.clone().map(Facade::from), diff --git a/crates/uv-git/src/resolver.rs b/crates/uv-git/src/resolver.rs index 82883a04e..8d92a56a5 100644 --- a/crates/uv-git/src/resolver.rs +++ b/crates/uv-git/src/resolver.rs @@ -37,9 +37,7 @@ impl GitResolver { self.0.get(reference) } - /// Download a source distribution from a Git repository. - /// - /// Assumes that the URL is a precise Git URL, with a full commit hash. + /// Fetch a remote Git repository. pub async fn fetch( &self, url: &GitUrl, @@ -68,50 +66,13 @@ impl GitResolver { .await? .map_err(GitResolverError::Git)?; - Ok(fetch) - } - - /// Given a remote source distribution, return a precise variant. - /// - /// For example, given a Git dependency with a reference to a branch or tag, return a URL - /// with a precise reference to the current commit of that branch or tag. - /// - /// This method takes into account various normalizations that are independent from the Git - /// layer. For example: removing `#subdirectory=pkg_dir`-like fragments, and removing `git+` - /// prefix kinds. - /// - /// Returns `Ok(None)` if the URL already has a precise reference (i.e., it includes a full - /// commit hash in the URL itself, as opposed to, e.g., a branch name). - pub async fn resolve( - &self, - url: &GitUrl, - client: ClientWithMiddleware, - cache: PathBuf, - reporter: Option, - ) -> Result, GitResolverError> { - // If the Git reference already contains a complete SHA, short-circuit. - if url.precise().is_some() { - return Ok(None); - } - - // If the Git reference is in the in-memory cache, return it. - { - let reference = RepositoryReference::from(url); - if let Some(precise) = self.get(&reference) { - return Ok(Some(url.clone().with_precise(*precise))); - } - } - - let fetch = self.fetch(url, client, cache, reporter).await?; - let git = fetch.into_git(); - // Insert the resolved URL into the in-memory cache. - if let Some(precise) = git.precise() { + if let Some(precise) = fetch.git().precise() { let reference = RepositoryReference::from(url); self.insert(reference, precise); } - Ok(Some(git)) + Ok(fetch) } /// Given a remote source distribution, return a precise variant, if possible. diff --git a/crates/uv/tests/lock.rs b/crates/uv/tests/lock.rs index 1b855502c..32e08e598 100644 --- a/crates/uv/tests/lock.rs +++ b/crates/uv/tests/lock.rs @@ -2436,7 +2436,7 @@ fn lock_git_sha() -> Result<()> { [[distribution]] name = "uv-public-pypackage" version = "0.1.0" - source = { git = "https://github.com/astral-test/uv-public-pypackage?rev=main#0dacfd662c64cb4ceb16e6cf65a157a8b715b979" } + source = { git = "https://github.com/astral-test/uv-public-pypackage?rev=main#b270df1a2fb5d012294e9aaf05e7e0bab1e6a389" } "### ); });