Avoid generating unused hashes during uv lock

charliermarsh · charliermarsh · commit 556f71fb3844 · 2025-01-05T19:45:51.000-05:00
diff --git a/crates/uv-distribution-types/src/hash.rs b/crates/uv-distribution-types/src/hash.rs
@@ -5,7 +5,7 @@ pub enum HashPolicy<'a> {
     /// No hash policy is specified.
     None,
     /// Hashes should be generated (specifically, a SHA-256 hash), but not validated.
-    Generate,
+    Generate(HashGeneration),
     /// Hashes should be validated against a pre-defined list of hashes. If necessary, hashes should
     /// be generated so as to ensure that the archive is valid.
     Validate(&'a [HashDigest]),
@@ -17,21 +17,28 @@ impl HashPolicy<'_> {
         matches!(self, Self::None)
     }
 
-    /// Returns `true` if the hash policy is `Generate`.
-    pub fn is_generate(&self) -> bool {
-        matches!(self, Self::Generate)
-    }
-
     /// Returns `true` if the hash policy is `Validate`.
     pub fn is_validate(&self) -> bool {
         matches!(self, Self::Validate(_))
     }
 
+    /// Returns `true` if the hash policy indicates that hashes should be generated.
+    pub fn is_generate(&self, dist: &crate::BuiltDist) -> bool {
+        match self {
+            HashPolicy::Generate(HashGeneration::Url) => dist.file().is_none(),
+            HashPolicy::Generate(HashGeneration::All) => {
+                dist.file().map_or(true, |file| file.hashes.is_empty())
+            }
+            HashPolicy::Validate(_) => false,
+            HashPolicy::None => false,
+        }
+    }
+
     /// Return the algorithms used in the hash policy.
     pub fn algorithms(&self) -> Vec<HashAlgorithm> {
         match self {
             Self::None => vec![],
-            Self::Generate => vec![HashAlgorithm::Sha256],
+            Self::Generate(_) => vec![HashAlgorithm::Sha256],
             Self::Validate(hashes) => {
                 let mut algorithms = hashes.iter().map(HashDigest::algorithm).collect::<Vec<_>>();
                 algorithms.sort();
@@ -45,12 +52,22 @@ impl HashPolicy<'_> {
     pub fn digests(&self) -> &[HashDigest] {
         match self {
             Self::None => &[],
-            Self::Generate => &[],
+            Self::Generate(_) => &[],
             Self::Validate(hashes) => hashes,
         }
     }
 }
 
+/// The context in which hashes should be generated.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum HashGeneration {
+    /// Generate hashes for direct URL distributions.
+    Url,
+    /// Generate hashes for direct URL distributions, along with any distributions that are hosted
+    /// on a registry that does _not_ provide hashes.
+    All,
+}
+
 pub trait Hashed {
     /// Return the [`HashDigest`]s for the archive.
     fn hashes(&self) -> &[HashDigest];
@@ -59,7 +76,7 @@ pub trait Hashed {
     fn satisfies(&self, hashes: HashPolicy) -> bool {
         match hashes {
             HashPolicy::None => true,
-            HashPolicy::Generate => self
+            HashPolicy::Generate(_) => self
                 .hashes()
                 .iter()
                 .any(|hash| hash.algorithm == HashAlgorithm::Sha256),
@@ -71,7 +88,7 @@ pub trait Hashed {
     fn has_digests(&self, hashes: HashPolicy) -> bool {
         match hashes {
             HashPolicy::None => true,
-            HashPolicy::Generate => self
+            HashPolicy::Generate(_) => self
                 .hashes()
                 .iter()
                 .any(|hash| hash.algorithm == HashAlgorithm::Sha256),
diff --git a/crates/uv-distribution/src/distribution_database.rs b/crates/uv-distribution/src/distribution_database.rs
@@ -405,22 +405,28 @@ impl<'a, Context: BuildContext> DistributionDatabase<'a, Context> {
             return Ok(ArchiveMetadata::from_metadata23(metadata.clone()));
         }
 
-        // If hash generation is enabled, and the distribution isn't hosted on an index, get the
+        // If hash generation is enabled, and the distribution isn't hosted on a registry, get the
         // entire wheel to ensure that the hashes are included in the response. If the distribution
         // is hosted on an index, the hashes will be included in the simple metadata response.
         // For hash _validation_, callers are expected to enforce the policy when retrieving the
         // wheel.
+        //
+        // Historically, for `uv pip compile --universal`, we also generate hashes for
+        // registry-based distributions when the relevant registry doesn't provide them. This was
+        // motivated by `--find-links`. We continue that behavior (under `HashGeneration::All`) for
+        // backwards compatibility, but it's a little dubious, since we're only hashing _one_
+        // distribution here (as opposed to hashing all distributions for the version), and it may
+        // not even be a compatible distribution!
+        //
         // TODO(charlie): Request the hashes via a separate method, to reduce the coupling in this API.
-        if hashes.is_generate() {
-            if dist.file().map_or(true, |file| file.hashes.is_empty()) {
-                let wheel = self.get_wheel(dist, hashes).await?;
-                let metadata = wheel.metadata()?;
-                let hashes = wheel.hashes;
-                return Ok(ArchiveMetadata {
-                    metadata: Metadata::from_metadata23(metadata),
-                    hashes,
-                });
-            }
+        if hashes.is_generate(dist) {
+            let wheel = self.get_wheel(dist, hashes).await?;
+            let metadata = wheel.metadata()?;
+            let hashes = wheel.hashes;
+            return Ok(ArchiveMetadata {
+                metadata: Metadata::from_metadata23(metadata),
+                hashes,
+            });
         }
 
         let result = self
diff --git a/crates/uv-requirements/src/source_tree.rs b/crates/uv-requirements/src/source_tree.rs
@@ -13,7 +13,7 @@ use url::Url;
 use uv_configuration::ExtrasSpecification;
 use uv_distribution::{DistributionDatabase, Reporter, RequiresDist};
 use uv_distribution_types::{
-    BuildableSource, DirectorySourceUrl, HashPolicy, SourceUrl, VersionId,
+    BuildableSource, DirectorySourceUrl, HashGeneration, HashPolicy, SourceUrl, VersionId,
 };
 use uv_fs::Simplified;
 use uv_normalize::{ExtraName, PackageName};
@@ -213,8 +213,8 @@ impl<'a, Context: BuildContext> SourceTreeResolver<'a, Context> {
         // manual match.
         let hashes = match self.hasher {
             HashStrategy::None => HashPolicy::None,
-            HashStrategy::Generate => HashPolicy::Generate,
-            HashStrategy::Verify(_) => HashPolicy::Generate,
+            HashStrategy::Generate(mode) => HashPolicy::Generate(*mode),
+            HashStrategy::Verify(_) => HashPolicy::Generate(HashGeneration::All),
             HashStrategy::Require(_) => {
                 return Err(anyhow::anyhow!(
                     "Hash-checking is not supported for local directories: {}",
diff --git a/crates/uv-types/src/hash.rs b/crates/uv-types/src/hash.rs
@@ -5,7 +5,8 @@ use url::Url;
 
 use uv_configuration::HashCheckingMode;
 use uv_distribution_types::{
-    DistributionMetadata, HashPolicy, Name, Resolution, UnresolvedRequirement, VersionId,
+    DistributionMetadata, HashGeneration, HashPolicy, Name, Resolution, UnresolvedRequirement,
+    VersionId,
 };
 use uv_normalize::PackageName;
 use uv_pep440::Version;
@@ -19,7 +20,7 @@ pub enum HashStrategy {
     #[default]
     None,
     /// Hashes should be generated (specifically, a SHA-256 hash), but not validated.
-    Generate,
+    Generate(HashGeneration),
     /// Hashes should be validated, if present, but ignored if absent.
     ///
     /// If necessary, hashes should be generated to ensure that the archive is valid.
@@ -35,7 +36,7 @@ impl HashStrategy {
     pub fn get<T: DistributionMetadata>(&self, distribution: &T) -> HashPolicy {
         match self {
             Self::None => HashPolicy::None,
-            Self::Generate => HashPolicy::Generate,
+            Self::Generate(mode) => HashPolicy::Generate(*mode),
             Self::Verify(hashes) => {
                 if let Some(hashes) = hashes.get(&distribution.version_id()) {
                     HashPolicy::Validate(hashes.as_slice())
@@ -56,7 +57,7 @@ impl HashStrategy {
     pub fn get_package(&self, name: &PackageName, version: &Version) -> HashPolicy {
         match self {
             Self::None => HashPolicy::None,
-            Self::Generate => HashPolicy::Generate,
+            Self::Generate(mode) => HashPolicy::Generate(*mode),
             Self::Verify(hashes) => {
                 if let Some(hashes) =
                     hashes.get(&VersionId::from_registry(name.clone(), version.clone()))
@@ -79,7 +80,7 @@ impl HashStrategy {
     pub fn get_url(&self, url: &Url) -> HashPolicy {
         match self {
             Self::None => HashPolicy::None,
-            Self::Generate => HashPolicy::Generate,
+            Self::Generate(mode) => HashPolicy::Generate(*mode),
             Self::Verify(hashes) => {
                 if let Some(hashes) = hashes.get(&VersionId::from_url(url)) {
                     HashPolicy::Validate(hashes.as_slice())
@@ -100,7 +101,7 @@ impl HashStrategy {
     pub fn allows_package(&self, name: &PackageName, version: &Version) -> bool {
         match self {
             Self::None => true,
-            Self::Generate => true,
+            Self::Generate(_) => true,
             Self::Verify(_) => true,
             Self::Require(hashes) => {
                 hashes.contains_key(&VersionId::from_registry(name.clone(), version.clone()))
@@ -112,7 +113,7 @@ impl HashStrategy {
     pub fn allows_url(&self, url: &Url) -> bool {
         match self {
             Self::None => true,
-            Self::Generate => true,
+            Self::Generate(_) => true,
             Self::Verify(_) => true,
             Self::Require(hashes) => hashes.contains_key(&VersionId::from_url(url)),
         }
diff --git a/crates/uv/src/commands/pip/compile.rs b/crates/uv/src/commands/pip/compile.rs
@@ -17,8 +17,8 @@ use uv_configuration::{
 use uv_configuration::{KeyringProviderType, TargetTriple};
 use uv_dispatch::{BuildDispatch, SharedState};
 use uv_distribution_types::{
-    DependencyMetadata, Index, IndexLocations, NameRequirementSpecification, Origin,
-    UnresolvedRequirementSpecification, Verbatim,
+    DependencyMetadata, HashGeneration, Index, IndexLocations, NameRequirementSpecification,
+    Origin, UnresolvedRequirementSpecification, Verbatim,
 };
 use uv_fs::Simplified;
 use uv_install_wheel::linker::LinkMode;
@@ -266,7 +266,7 @@ pub(crate) async fn pip_compile(
 
     // Generate, but don't enforce hashes for the requirements.
     let hasher = if generate_hashes {
-        HashStrategy::Generate
+        HashStrategy::Generate(HashGeneration::All)
     } else {
         HashStrategy::None
     };
diff --git a/crates/uv/src/commands/project/lock.rs b/crates/uv/src/commands/project/lock.rs
@@ -17,7 +17,7 @@ use uv_configuration::{
 use uv_dispatch::{BuildDispatch, SharedState};
 use uv_distribution::DistributionDatabase;
 use uv_distribution_types::{
-    DependencyMetadata, Index, IndexLocations, NameRequirementSpecification,
+    DependencyMetadata, HashGeneration, Index, IndexLocations, NameRequirementSpecification,
     UnresolvedRequirementSpecification,
 };
 use uv_git::ResolvedRepositoryReference;
@@ -472,7 +472,7 @@ async fn do_lock(
         .index_strategy(index_strategy)
         .build_options(build_options.clone())
         .build();
-    let hasher = HashStrategy::Generate;
+    let hasher = HashStrategy::Generate(HashGeneration::Url);
 
     // TODO(charlie): These are all default values. We should consider whether we want to make them
     // optional on the downstream APIs.
diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs
@@ -9307,7 +9307,7 @@ fn lock_find_links_local_wheel() -> Result<()> {
     ----- stderr -----
     Using CPython 3.12.[X] interpreter at: [PYTHON-3.12]
     Creating virtual environment at: .venv
-    Prepared 1 package in [TIME]
+    Prepared 2 packages in [TIME]
     Installed 2 packages in [TIME]
      + project==0.1.0 (from file://[TEMP_DIR]/workspace)
      + tqdm==1000.0.0
@@ -9518,7 +9518,7 @@ fn lock_find_links_http_wheel() -> Result<()> {
     ----- stdout -----
 
     ----- stderr -----
-    Prepared 1 package in [TIME]
+    Prepared 2 packages in [TIME]
     Installed 2 packages in [TIME]
      + packaging==23.2
      + project==0.1.0 (from file://[TEMP_DIR]/)
@@ -9744,7 +9744,7 @@ fn lock_local_index() -> Result<()> {
     ----- stdout -----
 
     ----- stderr -----
-    Prepared 1 package in [TIME]
+    Prepared 2 packages in [TIME]
     Installed 2 packages in [TIME]
      + project==0.1.0 (from file://[TEMP_DIR]/)
      + tqdm==1000.0.0
diff --git a/crates/uv/tests/it/sync.rs b/crates/uv/tests/it/sync.rs
@@ -5870,6 +5870,7 @@ fn sync_build_tag() -> Result<()> {
     ----- stdout -----
 
     ----- stderr -----
+    Prepared 1 package in [TIME]
     Installed 1 package in [TIME]
      + build-tag==1.0.0
     "###);
