Invalidate cache when --config-settings change (#7139)

## Summary If `--config-settings` are provided, we cache the built wheels under one more subdirectory. We _don't_ invalidate the actual source (i.e., trigger a re-download) or metadata, though -- those can be reused even when `--config-settings` change. Closes #7028.
astral-sh · Sep 10, 2024 · a8bd021 · a8bd021
1 parent fdf2ff5
commit a8bd021
Show file tree

Hide file tree

Showing 12 changed files with 148 additions and 9 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/crates/uv-dispatch/src/lib.rs b/crates/uv-dispatch/src/lib.rs
@@ -223,6 +223,7 @@ impl<'a> BuildContext for BuildDispatch<'a> {
             &BuildOptions::default(),
             self.hasher,
             self.index_locations,
+            self.config_settings,
             self.cache(),
             venv,
             &markers,

diff --git a/crates/uv-distribution/Cargo.toml b/crates/uv-distribution/Cargo.toml
@@ -13,6 +13,7 @@ license = { workspace = true }
 workspace = true
 
 [dependencies]
+cache-key = { workspace = true }
 distribution-filename = { workspace = true }
 distribution-types = { workspace = true }
 install-wheel-rs = { workspace = true }

diff --git a/crates/uv-distribution/src/index/built_wheel_index.rs b/crates/uv-distribution/src/index/built_wheel_index.rs
@@ -7,6 +7,7 @@ use distribution_types::{
 use platform_tags::Tags;
 use uv_cache::{Cache, CacheBucket, CacheShard, WheelCache};
 use uv_cache_info::CacheInfo;
+use uv_configuration::ConfigSettings;
 use uv_fs::symlinks;
 use uv_types::HashStrategy;
 
@@ -16,15 +17,22 @@ pub struct BuiltWheelIndex<'a> {
     cache: &'a Cache,
     tags: &'a Tags,
     hasher: &'a HashStrategy,
+    build_configuration: &'a ConfigSettings,
 }
 
 impl<'a> BuiltWheelIndex<'a> {
     /// Initialize an index of built distributions.
-    pub fn new(cache: &'a Cache, tags: &'a Tags, hasher: &'a HashStrategy) -> Self {
+    pub fn new(
+        cache: &'a Cache,
+        tags: &'a Tags,
+        hasher: &'a HashStrategy,
+        build_configuration: &'a ConfigSettings,
+    ) -> Self {
         Self {
             cache,
             tags,
             hasher,
+            build_configuration,
         }
     }
 
@@ -53,6 +61,13 @@ impl<'a> BuiltWheelIndex<'a> {
 
         let cache_shard = cache_shard.shard(revision.id());
 
+        // If there are build settings, we need to scope to a cache shard.
+        let cache_shard = if self.build_configuration.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(self.build_configuration))
+        };
+
         Ok(self.find(&cache_shard))
     }
     /// Return the most compatible [`CachedWheel`] for a given source distribution at a local path.
@@ -83,6 +98,13 @@ impl<'a> BuiltWheelIndex<'a> {
 
         let cache_shard = cache_shard.shard(revision.id());
 
+        // If there are build settings, we need to scope to a cache shard.
+        let cache_shard = if self.build_configuration.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(self.build_configuration))
+        };
+
         Ok(self
             .find(&cache_shard)
             .map(|wheel| wheel.with_cache_info(cache_info)))
@@ -125,6 +147,13 @@ impl<'a> BuiltWheelIndex<'a> {
 
         let cache_shard = cache_shard.shard(revision.id());
 
+        // If there are build settings, we need to scope to a cache shard.
+        let cache_shard = if self.build_configuration.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(self.build_configuration))
+        };
+
         Ok(self
             .find(&cache_shard)
             .map(|wheel| wheel.with_cache_info(cache_info)))
@@ -144,6 +173,13 @@ impl<'a> BuiltWheelIndex<'a> {
             WheelCache::Git(&source_dist.url, &git_sha.to_short_string()).root(),
         );
 
+        // If there are build settings, we need to scope to a cache shard.
+        let cache_shard = if self.build_configuration.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(self.build_configuration))
+        };
+
         self.find(&cache_shard)
     }
 

diff --git a/crates/uv-distribution/src/source/mod.rs b/crates/uv-distribution/src/source/mod.rs
@@ -426,6 +426,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         // freshness, since entries have to be fresher than the revision itself.
         let cache_shard = cache_shard.shard(revision.id());
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // If the cache contains a compatible wheel, return it.
         if let Some(built_wheel) = BuiltWheelMetadata::find_in_cache(tags, &cache_shard) {
             return Ok(built_wheel.with_hashes(revision.into_hashes()));
@@ -519,6 +527,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
             });
         }
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // Otherwise, we either need to build the metadata.
         // If the backend supports `prepare_metadata_for_build_wheel`, use it.
         if let Some(metadata) = self
@@ -671,6 +687,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         // freshness, since entries have to be fresher than the revision itself.
         let cache_shard = cache_shard.shard(revision.id());
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // If the cache contains a compatible wheel, return it.
         if let Some(built_wheel) = BuiltWheelMetadata::find_in_cache(tags, &cache_shard) {
             return Ok(built_wheel);
@@ -781,6 +805,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
             });
         }
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // Otherwise, we need to build a wheel.
         let task = self
             .reporter
@@ -897,6 +929,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
         // freshness, since entries have to be fresher than the revision itself.
         let cache_shard = cache_shard.shard(revision.id());
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // If the cache contains a compatible wheel, return it.
         if let Some(built_wheel) = BuiltWheelMetadata::find_in_cache(tags, &cache_shard) {
             return Ok(built_wheel);
@@ -1020,6 +1060,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
             ));
         }
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // Otherwise, we need to build a wheel.
         let task = self
             .reporter
@@ -1131,6 +1179,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
 
         let _lock = lock_shard(&cache_shard).await?;
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // If the cache contains a compatible wheel, return it.
         if let Some(built_wheel) = BuiltWheelMetadata::find_in_cache(tags, &cache_shard) {
             return Ok(built_wheel);
@@ -1257,6 +1313,14 @@ impl<'a, T: BuildContext> SourceDistributionBuilder<'a, T> {
             ));
         }
 
+        // If there are build settings, we need to scope to a cache shard.
+        let config_settings = self.build_context.config_settings();
+        let cache_shard = if config_settings.is_empty() {
+            cache_shard
+        } else {
+            cache_shard.shard(cache_key::cache_digest(config_settings))
+        };
+
         // Otherwise, we need to build a wheel.
         let task = self
             .reporter

diff --git a/crates/uv-installer/src/plan.rs b/crates/uv-installer/src/plan.rs
@@ -15,7 +15,7 @@ use platform_tags::Tags;
 use pypi_types::{Requirement, RequirementSource, ResolverMarkerEnvironment};
 use uv_cache::{Cache, CacheBucket, WheelCache};
 use uv_cache_info::{CacheInfo, Timestamp};
-use uv_configuration::{BuildOptions, Reinstall};
+use uv_configuration::{BuildOptions, ConfigSettings, Reinstall};
 use uv_distribution::{
     BuiltWheelIndex, HttpArchivePointer, LocalArchivePointer, RegistryWheelIndex,
 };
@@ -57,14 +57,15 @@ impl<'a> Planner<'a> {
         build_options: &BuildOptions,
         hasher: &HashStrategy,
         index_locations: &IndexLocations,
+        config_settings: &ConfigSettings,
         cache: &Cache,
         venv: &PythonEnvironment,
         markers: &ResolverMarkerEnvironment,
         tags: &Tags,
     ) -> Result<Plan> {
         // Index all the already-downloaded wheels in the cache.
         let mut registry_index = RegistryWheelIndex::new(cache, tags, index_locations, hasher);
-        let built_index = BuiltWheelIndex::new(cache, tags, hasher);
+        let built_index = BuiltWheelIndex::new(cache, tags, hasher, config_settings);
 
         let mut cached = vec![];
         let mut remote = vec![];

diff --git a/crates/uv/src/commands/pip/install.rs b/crates/uv/src/commands/pip/install.rs
@@ -408,6 +408,7 @@ pub(crate) async fn pip_install(
         link_mode,
         compile,
         &index_locations,
+        config_settings,
         &hasher,
         &markers,
         &tags,

diff --git a/crates/uv/src/commands/pip/operations.rs b/crates/uv/src/commands/pip/operations.rs
@@ -21,7 +21,8 @@ use pypi_types::ResolverMarkerEnvironment;
 use uv_cache::Cache;
 use uv_client::{BaseClientBuilder, RegistryClient};
 use uv_configuration::{
-    BuildOptions, Concurrency, Constraints, ExtrasSpecification, Overrides, Reinstall, Upgrade,
+    BuildOptions, Concurrency, ConfigSettings, Constraints, ExtrasSpecification, Overrides,
+    Reinstall, Upgrade,
 };
 use uv_dispatch::BuildDispatch;
 use uv_distribution::DistributionDatabase;
@@ -349,6 +350,7 @@ pub(crate) async fn install(
     link_mode: LinkMode,
     compile: bool,
     index_urls: &IndexLocations,
+    config_settings: &ConfigSettings,
     hasher: &HashStrategy,
     markers: &ResolverMarkerEnvironment,
     tags: &Tags,
@@ -376,6 +378,7 @@ pub(crate) async fn install(
             build_options,
             hasher,
             index_urls,
+            config_settings,
             cache,
             venv,
             markers,

diff --git a/crates/uv/src/commands/pip/sync.rs b/crates/uv/src/commands/pip/sync.rs
@@ -360,6 +360,7 @@ pub(crate) async fn pip_sync(
         link_mode,
         compile,
         &index_locations,
+        config_settings,
         &hasher,
         &markers,
         &tags,

diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs
@@ -892,6 +892,7 @@ pub(crate) async fn sync_environment(
         link_mode,
         compile_bytecode,
         index_locations,
+        config_setting,
         &hasher,
         &markers,
         tags,
@@ -1125,6 +1126,7 @@ pub(crate) async fn update_environment(
         *link_mode,
         *compile_bytecode,
         index_locations,
+        config_setting,
         &hasher,
         &markers,
         tags,

diff --git a/crates/uv/src/commands/project/sync.rs b/crates/uv/src/commands/project/sync.rs
@@ -305,6 +305,7 @@ pub(super) async fn do_sync(
         link_mode,
         compile_bytecode,
         index_locations,
+        config_setting,
         &hasher,
         &markers,
         tags,

diff --git a/crates/uv/tests/pip_install.rs b/crates/uv/tests/pip_install.rs
@@ -2764,9 +2764,37 @@ fn config_settings() {
         .join("__editable___setuptools_editable_0_1_0_finder.py");
     assert!(finder.exists());
 
-    // Install the editable package with `--editable_mode=compat`.
-    let context = TestContext::new("3.12");
+    // Reinstalling with `--editable_mode=compat` should be a no-op; changes in build configuration
+    // don't invalidate the environment.
+    uv_snapshot!(context.filters(), context.pip_install()
+        .arg("-e")
+        .arg(context.workspace_root.join("scripts/packages/setuptools_editable"))
+        .arg("-C")
+        .arg("editable_mode=compat")
+        , @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Audited 1 package in [TIME]
+    "###
+    );
+
+    // Uninstall the package.
+    uv_snapshot!(context.filters(), context.pip_uninstall()
+        .arg("setuptools-editable"), @r###"
+    success: true
+    exit_code: 0
+    ----- stdout -----
+
+    ----- stderr -----
+    Uninstalled 1 package in [TIME]
+     - setuptools-editable==0.1.0 (from file://[WORKSPACE]/scripts/packages/setuptools_editable)
+    "###);
 
+    // Install the editable package with `--editable_mode=compat`. We should ignore the cached
+    // build configuration and rebuild.
     uv_snapshot!(context.filters(), context.pip_install()
         .arg("-e")
         .arg(context.workspace_root.join("scripts/packages/setuptools_editable"))
@@ -2779,9 +2807,8 @@ fn config_settings() {
 
     ----- stderr -----
     Resolved 2 packages in [TIME]
-    Prepared 2 packages in [TIME]
-    Installed 2 packages in [TIME]
-     + iniconfig==2.0.0
+    Prepared 1 package in [TIME]
+    Installed 1 package in [TIME]
      + setuptools-editable==0.1.0 (from file://[WORKSPACE]/scripts/packages/setuptools_editable)
     "###
     );