-
Couldn't load subscription status.
- Fork 368
fix: save glob-root for correct rebuilds #4839
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
ac207c4
f937cde
8e7ef9a
9c322d8
2c10948
7e62942
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,6 +218,9 @@ pub struct CachedCondaMetadata { | |
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub input_hash: Option<InputHash>, | ||
|
|
||
| #[serde(default, skip_serializing_if = "Option::is_none")] | ||
| pub glob_root: Option<PathBuf>, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This as well. |
||
|
|
||
| #[serde(flatten)] | ||
| pub metadata: MetadataKind, | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,7 +1,7 @@ | ||
| use std::{ | ||
| collections::{BTreeMap, BTreeSet, HashSet}, | ||
| hash::{Hash, Hasher}, | ||
| path::PathBuf, | ||
| path::{Path, PathBuf}, | ||
| sync::Mutex, | ||
| }; | ||
|
|
||
|
|
@@ -126,6 +126,10 @@ impl BuildBackendMetadataSpec { | |
| &self.variants, | ||
| ); | ||
|
|
||
| let glob_root = discovered_backend | ||
| .init_params | ||
| .glob_root_with_fallback(&source_checkout.path); | ||
|
|
||
| // Check if we should skip the metadata cache for this backend | ||
| let skip_cache = Self::should_skip_metadata_cache( | ||
| &discovered_backend.backend_spec, | ||
|
|
@@ -143,13 +147,9 @@ impl BuildBackendMetadataSpec { | |
| .map_err(CommandDispatcherError::Failed)?; | ||
|
|
||
| if !skip_cache { | ||
| if let Some(metadata) = Self::verify_cache_freshness( | ||
| &source_checkout, | ||
| &command_dispatcher, | ||
| metadata, | ||
| &additional_glob_hash, | ||
| ) | ||
| .await? | ||
| if let Some(metadata) = | ||
| Self::verify_cache_freshness(&command_dispatcher, metadata, &additional_glob_hash) | ||
| .await? | ||
| { | ||
| return Ok(BuildBackendMetadata { | ||
| metadata, | ||
|
|
@@ -198,6 +198,7 @@ impl BuildBackendMetadataSpec { | |
| source_checkout, | ||
| backend, | ||
| additional_glob_hash, | ||
| glob_root, | ||
| log_sink, | ||
| ) | ||
| .await?; | ||
|
|
@@ -270,7 +271,6 @@ impl BuildBackendMetadataSpec { | |
| } | ||
|
|
||
| async fn verify_cache_freshness( | ||
| source_checkout: &SourceCheckout, | ||
| command_dispatcher: &CommandDispatcher, | ||
| metadata: Option<CachedCondaMetadata>, | ||
| additional_glob_hash: &[u8], | ||
|
|
@@ -293,11 +293,19 @@ impl BuildBackendMetadataSpec { | |
| return Ok(Some(metadata)); | ||
| }; | ||
|
|
||
| let Some(cached_root) = metadata.glob_root.as_ref() else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here we regenerate if its not found. |
||
| tracing::debug!( | ||
| "cached `{metadata_kind}` response missing glob root; regenerating metadata" | ||
| ); | ||
| return Ok(None); | ||
| }; | ||
| let effective_root = cached_root.as_path(); | ||
|
|
||
| // Check if the input hash is still valid. | ||
| let new_hash = command_dispatcher | ||
| .glob_hash_cache() | ||
| .compute_hash(GlobHashKey::new( | ||
| source_checkout.path.clone(), | ||
| effective_root.to_path_buf(), | ||
| input_globs.globs.clone(), | ||
| additional_glob_hash.to_vec(), | ||
| )) | ||
|
|
@@ -322,6 +330,7 @@ impl BuildBackendMetadataSpec { | |
| source_checkout: SourceCheckout, | ||
| backend: Backend, | ||
| additional_glob_hash: Vec<u8>, | ||
| glob_root: PathBuf, | ||
| mut log_sink: UnboundedSender<String>, | ||
| ) -> Result<CachedCondaMetadata, CommandDispatcherError<BuildBackendMetadataError>> { | ||
| let backend_identifier = backend.identifier().to_string(); | ||
|
|
@@ -365,7 +374,7 @@ impl BuildBackendMetadataSpec { | |
| let input_globs = extend_input_globs_with_variant_files( | ||
| outputs.input_globs.clone(), | ||
| &self.variant_files, | ||
| &source_checkout, | ||
| &glob_root, | ||
| ); | ||
| tracing::debug!( | ||
| backend = %backend_identifier, | ||
|
|
@@ -376,6 +385,7 @@ impl BuildBackendMetadataSpec { | |
| let input_hash = Self::compute_input_hash( | ||
| command_dispatcher, | ||
| &source_checkout, | ||
| &glob_root, | ||
| input_globs, | ||
| additional_glob_hash, | ||
| ) | ||
|
|
@@ -384,6 +394,7 @@ impl BuildBackendMetadataSpec { | |
| Ok(CachedCondaMetadata { | ||
| id: random(), | ||
| input_hash: input_hash.clone(), | ||
| glob_root: Some(glob_root), | ||
| metadata: MetadataKind::Outputs { | ||
| outputs: outputs.outputs, | ||
| }, | ||
|
|
@@ -394,6 +405,7 @@ impl BuildBackendMetadataSpec { | |
| async fn compute_input_hash( | ||
| command_queue: CommandDispatcher, | ||
| source: &SourceCheckout, | ||
| glob_root: &Path, | ||
| input_globs: BTreeSet<String>, | ||
| additional_glob_hash: Vec<u8>, | ||
| ) -> Result<Option<InputHash>, CommandDispatcherError<BuildBackendMetadataError>> { | ||
|
|
@@ -407,7 +419,7 @@ impl BuildBackendMetadataSpec { | |
| let input_hash = command_queue | ||
| .glob_hash_cache() | ||
| .compute_hash(GlobHashKey::new( | ||
| &source.path, | ||
| glob_root.to_path_buf(), | ||
| input_globs.clone(), | ||
| additional_glob_hash, | ||
| )) | ||
|
|
@@ -439,14 +451,15 @@ impl BuildBackendMetadataSpec { | |
| fn extend_input_globs_with_variant_files( | ||
| mut input_globs: BTreeSet<String>, | ||
| variant_files: &Option<Vec<PathBuf>>, | ||
| source_checkout: &SourceCheckout, | ||
| glob_root: &Path, | ||
| ) -> BTreeSet<String> { | ||
| if let Some(variant_files) = variant_files { | ||
| for variant_file in variant_files { | ||
| let relative = match variant_file.strip_prefix(&source_checkout.path) { | ||
| let relative = match variant_file.strip_prefix(glob_root) { | ||
| Ok(stripped) => stripped.to_path_buf(), | ||
| Err(_) => diff_paths(variant_file, &source_checkout.path) | ||
| .unwrap_or_else(|| variant_file.clone()), | ||
| Err(_) => { | ||
| diff_paths(variant_file, glob_root).unwrap_or_else(|| variant_file.clone()) | ||
| } | ||
| }; | ||
| let glob = relative.to_string_lossy().replace("\\", "/"); | ||
| input_globs.insert(glob); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -365,14 +365,22 @@ impl SourceBuildCacheStatusSpec { | |
| }; | ||
|
|
||
| // Checkout the source for the package. | ||
| let source_checkout = command_dispatcher | ||
| let _source_checkout = command_dispatcher | ||
| .checkout_pinned_source(source.clone()) | ||
| .await | ||
| .map_err_with(SourceBuildCacheStatusError::SourceCheckout)?; | ||
|
|
||
| let Some(glob_root) = source_info.glob_root.as_ref().cloned() else { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same here. |
||
| tracing::debug!( | ||
| "cached build missing glob root information; marking '{}' as stale", | ||
| self.package | ||
| ); | ||
| return Ok(CachedBuildStatus::Stale(cached_build)); | ||
| }; | ||
|
|
||
| // Compute the modification time of the files that match the source input globs. | ||
| let glob_time = match GlobModificationTime::from_patterns( | ||
| &source_checkout.path, | ||
| &glob_root, | ||
| source_info.globs.iter().map(String::as_str), | ||
| ) { | ||
| Ok(glob_time) => glob_time, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the new field.