Allow dependency metadata entries for direct URL requirements (#7846)

charliermarsh · web-flow · commit cc734ea2b6c8 · 2024-10-22T22:01:23.000-04:00
## Summary This is part of making #7299 (comment) better. You can now use `tool.uv.dependency-metadata` for direct URL requirements. Unfortunately, you _must_ include a version, since we need one to perform resolution.
diff --git a/crates/uv-distribution-types/src/dependency_metadata.rs b/crates/uv-distribution-types/src/dependency_metadata.rs
@@ -1,5 +1,6 @@
 use rustc_hash::FxHashMap;
 use serde::{Deserialize, Serialize};
+use tracing::{debug, warn};
 use uv_normalize::{ExtraName, PackageName};
 use uv_pep440::{Version, VersionSpecifiers};
 use uv_pep508::Requirement;
@@ -20,22 +21,57 @@ impl DependencyMetadata {
     }
 
     /// Retrieve a [`StaticMetadata`] entry by [`PackageName`] and [`Version`].
-    pub fn get(&self, package: &PackageName, version: &Version) -> Option<ResolutionMetadata> {
+    pub fn get(
+        &self,
+        package: &PackageName,
+        version: Option<&Version>,
+    ) -> Option<ResolutionMetadata> {
         let versions = self.0.get(package)?;
 
-        // Search for an exact, then a global match.
-        let metadata = versions
-            .iter()
-            .find(|v| v.version.as_ref() == Some(version))
-            .or_else(|| versions.iter().find(|v| v.version.is_none()))?;
-
-        Some(ResolutionMetadata {
-            name: metadata.name.clone(),
-            version: version.clone(),
-            requires_dist: metadata.requires_dist.clone(),
-            requires_python: metadata.requires_python.clone(),
-            provides_extras: metadata.provides_extras.clone(),
-        })
+        if let Some(version) = version {
+            // If a specific version was requested, search for an exact match, then a global match.
+            let metadata = versions
+                .iter()
+                .find(|v| v.version.as_ref() == Some(version))
+                .inspect(|_| {
+                    debug!("Found dependency metadata entry for `{package}=={version}`",);
+                })
+                .or_else(|| versions.iter().find(|v| v.version.is_none()))
+                .inspect(|_| {
+                    debug!("Found global metadata entry for `{package}`",);
+                });
+            let Some(metadata) = metadata else {
+                warn!("No dependency metadata entry found for `{package}=={version}`");
+                return None;
+            };
+            debug!("Found dependency metadata entry for `{package}=={version}`",);
+            Some(ResolutionMetadata {
+                name: metadata.name.clone(),
+                version: version.clone(),
+                requires_dist: metadata.requires_dist.clone(),
+                requires_python: metadata.requires_python.clone(),
+                provides_extras: metadata.provides_extras.clone(),
+            })
+        } else {
+            // If no version was requested (i.e., it's a direct URL dependency), allow a single
+            // versioned match.
+            let [metadata] = versions.as_slice() else {
+                warn!("Multiple dependency metadata entries found for `{package}`");
+                return None;
+            };
+            let Some(version) = metadata.version.clone() else {
+                warn!("No version found in dependency metadata entry for `{package}`");
+                return None;
+            };
+            debug!("Found dependency metadata entry for `{package}` (assuming: `{version}`)");
+            Some(ResolutionMetadata {
+                name: metadata.name.clone(),
+                version,
+                requires_dist: metadata.requires_dist.clone(),
+                requires_python: metadata.requires_python.clone(),
+                provides_extras: metadata.provides_extras.clone(),
+            })
+        }
     }
 
     /// Retrieve all [`StaticMetadata`] entries.
diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs
@@ -354,7 +354,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
     ///
     /// While hashes will be generated in some cases, hash-checking is _not_ enforced and should
     /// instead be enforced by the caller.
-    pub async fn get_wheel_metadata(
+    async fn get_wheel_metadata(
         &self,
         dist: &BuiltDist,
         hashes: HashPolicy<'_>,
@@ -363,7 +363,7 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
         if let Some(metadata) = self
             .build_context
             .dependency_metadata()
-            .get(dist.name(), dist.version())
+            .get(dist.name(), Some(dist.version()))
         {
             return Ok(ArchiveMetadata::from_metadata23(metadata.clone()));
         }
@@ -425,14 +425,16 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
     ) -> Result<ArchiveMetadata, Error> {
         // If the metadata was provided by the user directly, prefer it.
         if let Some(dist) = source.as_dist() {
-            if let Some(version) = dist.version() {
-                if let Some(metadata) = self
-                    .build_context
-                    .dependency_metadata()
-                    .get(dist.name(), version)
-                {
-                    return Ok(ArchiveMetadata::from_metadata23(metadata.clone()));
-                }
+            if let Some(metadata) = self
+                .build_context
+                .dependency_metadata()
+                .get(dist.name(), dist.version())
+            {
+                // If we skipped the build, we should still resolve any Git dependencies to precise
+                // commits.
+                self.builder.resolve_revision(source, &self.client).await?;
+
+                return Ok(ArchiveMetadata::from_metadata23(metadata.clone()));
             }
         }
 
diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs
@@ -1497,6 +1497,40 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         ))
     }
 
+    /// Resolve a source to a specific revision.
+    pub(crate) async fn resolve_revision(
+        &self,
+        source: &BuildableSource<'_>,
+        client: &ManagedClient<'_>,
+    ) -> Result<(), Error> {
+        match source {
+            BuildableSource::Dist(SourceDist::Git(source)) => {
+                self.build_context
+                    .git()
+                    .fetch(
+                        &source.git,
+                        client.unmanaged.uncached_client(&source.url).clone(),
+                        self.build_context.cache().bucket(CacheBucket::Git),
+                        self.reporter.clone().map(Facade::from),
+                    )
+                    .await?;
+            }
+            BuildableSource::Url(SourceUrl::Git(source)) => {
+                self.build_context
+                    .git()
+                    .fetch(
+                        source.git,
+                        client.unmanaged.uncached_client(source.url).clone(),
+                        self.build_context.cache().bucket(CacheBucket::Git),
+                        self.reporter.clone().map(Facade::from),
+                    )
+                    .await?;
+            }
+            _ => {}
+        }
+        Ok(())
+    }
+
     /// Heal a [`Revision`] for a local archive.
     async fn heal_archive_revision(
         &self,
diff --git a/crates/uv-git/src/git.rs b/crates/uv-git/src/git.rs
@@ -590,7 +590,7 @@ pub(crate) fn fetch(
     }
 }
 
-/// Attempts to use `git` CLI installed on the system to fetch a repository,.
+/// Attempts to use `git` CLI installed on the system to fetch a repository.
 fn fetch_with_cli(
     repo: &mut GitRepository,
     url: &str,
diff --git a/crates/uv-git/src/resolver.rs b/crates/uv-git/src/resolver.rs
@@ -38,6 +38,50 @@ impl GitResolver {
         self.0.get(reference)
     }
 
+    /// Resolve a Git URL to a specific commit.
+    pub async fn resolve(
+        &self,
+        url: &GitUrl,
+        client: ClientWithMiddleware,
+        cache: PathBuf,
+        reporter: Option<impl Reporter + 'static>,
+    ) -> Result<GitSha, GitResolverError> {
+        debug!("Resolving source distribution from Git: {url}");
+
+        let reference = RepositoryReference::from(url);
+
+        // If we know the precise commit already, return it.
+        if let Some(precise) = self.get(&reference) {
+            return Ok(*precise);
+        }
+
+        // Avoid races between different processes, too.
+        let lock_dir = cache.join("locks");
+        fs::create_dir_all(&lock_dir).await?;
+        let repository_url = RepositoryUrl::new(url.repository());
+        let _lock = LockedFile::acquire(
+            lock_dir.join(cache_digest(&repository_url)),
+            &repository_url,
+        )
+        .await?;
+
+        // Fetch the Git repository.
+        let source = if let Some(reporter) = reporter {
+            GitSource::new(url.clone(), client, cache).with_reporter(reporter)
+        } else {
+            GitSource::new(url.clone(), client, cache)
+        };
+        let precise = tokio::task::spawn_blocking(move || source.resolve())
+            .await?
+            .map_err(GitResolverError::Git)?;
+
+        // Insert the resolved URL into the in-memory cache. This ensures that subsequent fetches
+        // resolve to the same precise commit.
+        self.insert(reference, precise);
+
+        Ok(precise)
+    }
+
     /// Fetch a remote Git repository.
     pub async fn fetch(
         &self,
diff --git a/crates/uv-git/src/source.rs b/crates/uv-git/src/source.rs
@@ -51,6 +51,68 @@ impl GitSource {
         }
     }
 
+    /// Resolve a Git source to a specific revision.
+    #[instrument(skip(self), fields(repository = %self.git.repository, rev = ?self.git.precise))]
+    pub fn resolve(self) -> Result<GitSha> {
+        // Compute the canonical URL for the repository.
+        let canonical = RepositoryUrl::new(&self.git.repository);
+
+        // The path to the repo, within the Git database.
+        let ident = cache_digest(&canonical);
+        let db_path = self.cache.join("db").join(&ident);
+
+        // Authenticate the URL, if necessary.
+        let remote = if let Some(credentials) = GIT_STORE.get(&canonical) {
+            Cow::Owned(credentials.apply(self.git.repository.clone()))
+        } else {
+            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.into()) => {
+                debug!("Using existing Git source `{}`", self.git.repository);
+                (db, 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.map(GitOid::from),
+                    &self.client,
+                )?;
+
+                (db, GitSha::from(actual_rev), task)
+            }
+        };
+
+        let short_id = db.to_short_id(actual_rev.into())?;
+
+        // Report the checkout operation to the reporter.
+        if let Some(task) = task {
+            if let Some(reporter) = self.reporter.as_ref() {
+                reporter.on_checkout_complete(remote.url(), short_id.as_str(), task);
+            }
+        }
+
+        Ok(actual_rev)
+    }
+
     /// Fetch the underlying Git repository at the given revision.
     #[instrument(skip(self), fields(repository = %self.git.repository, rev = ?self.git.precise))]
     pub fn fetch(self) -> Result<Fetch> {
diff --git a/crates/uv-resolver/src/redirect.rs b/crates/uv-resolver/src/redirect.rs
@@ -1,5 +1,4 @@
 use url::Url;
-
 use uv_git::{GitReference, GitResolver};
 use uv_pep508::VerbatimUrl;
 use uv_pypi_types::{ParsedGitUrl, ParsedUrl, VerbatimParsedUrl};
@@ -17,9 +16,8 @@ pub(crate) fn url_to_precise(url: VerbatimParsedUrl, git: &GitResolver) -> Verba
     let Some(new_git_url) = git.precise(git_url.clone()) else {
         debug_assert!(
             matches!(git_url.reference(), GitReference::FullCommit(_)),
-            "Unseen Git URL: {}, {:?}",
+            "Unseen Git URL: {}, {git_url:?}",
             url.verbatim,
-            git_url
         );
         return url;
     };
diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs
diff --git a/docs/concepts/projects.md b/docs/concepts/projects.md
diff --git a/docs/concepts/resolution.md b/docs/concepts/resolution.md

Original file line number	Diff line number	Diff line change
`@@ -590,7 +590,7 @@ pub(crate) fn fetch(`
`590`	`590`	`}`
`591`	`591`	`}`
`592`	`592`
`593`		-/// Attempts to use `git` CLI installed on the system to fetch a repository,.
	`593`	+/// Attempts to use `git` CLI installed on the system to fetch a repository.
`594`	`594`	`fn fetch_with_cli(`
`595`	`595`	`repo: &mut GitRepository,`
`596`	`596`	`url: &str,`