diff --git a/CHANGELOG.md b/CHANGELOG.md index 5094f93b6..d61bd043a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Changed** A cache miss now lists every change to the command, the task's cache settings and the envs it depends on, instead of the first: when a task's `input` and `output` settings changed together, or a tracked env var and a file changed together, `vp run --verbose` and `vp run --last-details` show each change on its own line. When the command, a setting or a `cache.env` var changed, the files are now also compared with the task's previous cached run, so the miss shows whether reverting that change is enough. Files are still checked only up to the first changed one. A summary saved by an earlier version can't be shown by `vp run --last-details` ([#961](https://github.com/voidzero-dev/vite-task/pull/961), [#962](https://github.com/voidzero-dev/vite-task/pull/962)). - **Added** `vp run --dry-run` shows what a run would do without running anything: each command's line with its cache status, and the summary, as a real run prints them. The cache is checked as in a real run, the remote cache included, and nothing is written: no cache entry, no downloaded or restored outputs, and no change to what `vp run --last-details` shows. With `-v`, the detailed summary shows why each command would miss. Like other `vp run` flags, `--dry-run` goes before the task name; after it, it's passed to the task ([#960](https://github.com/voidzero-dev/vite-task/pull/960)). - **Added** `vp run --json` prints every package and task in the workspace, with each task's `dependsOn` resolved to task IDs, as one JSON document for scripts, editors and coding agents. It runs nothing and, like `vp run` without a task, loads every package's config. It prints the same document from any directory in the workspace: indented in a terminal, on one line when captured. Its `$schema` field points to a JSON Schema that describes each field, which `vp run` writes to the system temp directory ([#948](https://github.com/voidzero-dev/vite-task/pull/948)). - **Fixed** On Linux, where every user shares the temp folder `/tmp`, the helper files `vp run` writes there, such as the library that tracks file accesses, now go in a folder only the current user can write to, like `/tmp/fspy-1000`. Before, they went in a folder such as `/tmp/fspy` that belonged to whoever ran `vp run` first: running a different version as another user failed, and the same version used files that user could replace ([#947](https://github.com/voidzero-dev/vite-task/pull/947)). diff --git a/crates/vt/docs/task-cache.md b/crates/vt/docs/task-cache.md index f71891d95..87627ad5c 100644 --- a/crates/vt/docs/task-cache.md +++ b/crates/vt/docs/task-cache.md @@ -248,13 +248,19 @@ See [inputs.md](../../../docs/inputs.md) for full details. ### 8. Fingerprint Validation -When a cache entry exists, the fingerprint is validated to detect changes: +When a cache entry exists, the fingerprint is validated to detect changes. A miss lists every change it found, sorted by kind and then by name or path. The file check stops at the first changed file: ```rust -pub enum FingerprintMismatch { - SpawnFingerprint { old: SpawnFingerprint, new: SpawnFingerprint }, +pub enum Change { + Program, + Args, + Cwd, InputConfig, - InputChanged { kind: InputChangeKind, path: RelativePathBuf }, + OutputConfig, + UntrackedEnvAdded { name: Str }, + UntrackedEnvRemoved { name: Str }, + Env { mismatch: EnvMismatch, source: EnvSource }, + File { kind: InputChangeKind, path: RelativePathBuf }, } pub enum InputChangeKind { @@ -587,7 +593,7 @@ Each `&&` separated command is cached independently. If only terser config chang ``` crates/vt/src/session/ ├── cache/ -│ ├── mod.rs # ExecutionCache, CacheEntryKey/Value, FingerprintMismatch +│ ├── mod.rs # ExecutionCache, CacheEntryKey/Value, Change │ └── display.rs # Cache status display formatting ├── execute/ │ ├── mod.rs # execute_spawn, SpawnOutcome diff --git a/crates/vt/src/session/cache/display.rs b/crates/vt/src/session/cache/display.rs index 9a94497c1..2f85440e3 100644 --- a/crates/vt/src/session/cache/display.rs +++ b/crates/vt/src/session/cache/display.rs @@ -3,126 +3,52 @@ //! This module provides plain text formatting for cache status. //! Coloring is handled by the reporter to respect `NO_COLOR` environment variable. -use rustc_hash::FxHashSet; -use serde::{Deserialize, Serialize}; -use vt_plan::cache_metadata::SpawnFingerprint; use vt_str::Str; -use super::{ - CacheHitSource, CacheMiss, EnvMismatch, FingerprintMismatch, InputChangeKind, split_path, -}; +use super::{CacheHitSource, CacheMiss, Change, EnvSource, InputChangeKind, split_path}; use crate::session::event::{CacheStatus, RunKind}; -/// Describes a single atomic change between two spawn fingerprints. -/// -/// Used both for live cache status display and for persisted summary data. -#[derive(Serialize, Deserialize)] -pub enum SpawnFingerprintChange { - /// A fingerprinted env var was added, removed, or changed value. - Env(EnvMismatch), - - // Untracked env config changes - /// Untracked env pattern added - UntrackedEnvAdded { name: Str }, - /// Untracked env pattern removed - UntrackedEnvRemoved { name: Str }, - - // Command changes - /// Program changed - ProgramChanged, - /// Args changed - ArgsChanged, - - // Working directory change - /// Working directory changed - CwdChanged, -} - -/// Format a single spawn fingerprint change as human-readable text. -/// -/// Used by both the live cache status display and the persisted summary rendering. -pub fn format_spawn_change(change: &SpawnFingerprintChange) -> Str { +/// Format one change as the detailed summary lists it, e.g. +/// `env 'NODE_ENV' added` or `'src/a.ts' modified`. +pub fn format_change(change: &Change) -> Str { match change { - SpawnFingerprintChange::Env(mismatch) => vt_str::format!("{mismatch}"), - SpawnFingerprintChange::UntrackedEnvAdded { name } => { - vt_str::format!("untracked env '{name}' added") - } - SpawnFingerprintChange::UntrackedEnvRemoved { name } => { - vt_str::format!("untracked env '{name}' removed") - } - SpawnFingerprintChange::ProgramChanged => Str::from("program changed"), - SpawnFingerprintChange::ArgsChanged => Str::from("args changed"), - SpawnFingerprintChange::CwdChanged => Str::from("working directory changed"), + Change::Program => Str::from("program changed"), + Change::Args => Str::from("args changed"), + Change::Cwd => Str::from("working directory changed"), + Change::InputConfig => Str::from("input configuration changed"), + Change::OutputConfig => Str::from("output configuration changed"), + Change::UntrackedEnvAdded { name } => vt_str::format!("untracked env '{name}' added"), + Change::UntrackedEnvRemoved { name } => vt_str::format!("untracked env '{name}' removed"), + Change::Env { mismatch, .. } => vt_str::format!("{mismatch}"), + Change::File { kind, path } => format_input_change_str(*kind, path.as_str()), } } -/// Compare two spawn fingerprints and return all changes. -pub fn detect_spawn_fingerprint_changes( - old: &SpawnFingerprint, - new: &SpawnFingerprint, -) -> Vec { - let mut changes = Vec::new(); - let old_env = old.env_fingerprints(); - let new_env = new.env_fingerprints(); - - // Check for removed or changed envs - for (key, old_value) in &old_env.fingerprinted_envs { - if let Some(new_value) = new_env.fingerprinted_envs.get(key) { - if old_value != new_value { - changes - .push(SpawnFingerprintChange::Env(EnvMismatch::Changed { name: key.clone() })); - } - } else { - changes.push(SpawnFingerprintChange::Env(EnvMismatch::Removed { name: key.clone() })); +/// The inline reason for a miss: its first change. When the first change is +/// an env, every env change is named, as in `envs 'A', 'B' changed`, and a +/// single env from `cache.env` reads `env 'NODE_ENV' changed` however it +/// changed. Untracked env changes read `untracked env config changed`. +fn format_changes_inline(changes: &[Change]) -> Str { + match changes.first() { + None => Str::from("configuration changed"), + Some(Change::UntrackedEnvAdded { .. } | Change::UntrackedEnvRemoved { .. }) => { + Str::from("untracked env config changed") } - } - - // Check for added envs - for key in new_env.fingerprinted_envs.keys() { - if !old_env.fingerprinted_envs.contains_key(key) { - changes.push(SpawnFingerprintChange::Env(EnvMismatch::Added { name: key.clone() })); + Some(Change::Env { mismatch, source }) => { + let names: Vec<&Str> = changes + .iter() + .filter_map(|change| match change { + Change::Env { mismatch, .. } => Some(mismatch.name()), + _ => None, + }) + .collect(); + match (names.as_slice(), source) { + ([_], EnvSource::Tracked) => vt_str::format!("{mismatch}"), + _ => format_env_changed_inline(&names), + } } + Some(change) => format_change(change), } - - // Check untracked env config changes - let old_untracked: FxHashSet<_> = old_env.untracked_env_config.iter().collect(); - let new_untracked: FxHashSet<_> = new_env.untracked_env_config.iter().collect(); - for name in old_untracked.difference(&new_untracked) { - changes.push(SpawnFingerprintChange::UntrackedEnvRemoved { name: (*name).clone() }); - } - for name in new_untracked.difference(&old_untracked) { - changes.push(SpawnFingerprintChange::UntrackedEnvAdded { name: (*name).clone() }); - } - - // Check program changes - if old.program_fingerprint_debug() != new.program_fingerprint_debug() { - changes.push(SpawnFingerprintChange::ProgramChanged); - } - - // Check args changes - if old.args() != new.args() { - changes.push(SpawnFingerprintChange::ArgsChanged); - } - - // Check cwd changes - if old.cwd() != new.cwd() { - changes.push(SpawnFingerprintChange::CwdChanged); - } - - changes -} - -/// Names of the env vars involved in a set of spawn-fingerprint changes, in the -/// order detected. Only env changes are collected; untracked-env and non-env -/// changes are skipped. -fn env_change_names(changes: &[SpawnFingerprintChange]) -> Vec<&Str> { - changes - .iter() - .filter_map(|change| match change { - SpawnFingerprintChange::Env(mismatch) => Some(mismatch.name()), - _ => None, - }) - .collect() } /// Inline cache-miss reason naming the env var(s) that changed, e.g. @@ -171,39 +97,9 @@ pub fn format_cache_status_inline(cache_status: &CacheStatus, run_kind: RunKind) // This keeps the output clean for first-time executions None } - CacheStatus::Miss(CacheMiss::FingerprintMismatch(mismatch)) => { + CacheStatus::Miss(CacheMiss::Changed(comparison)) => { // Show "cache miss" with reason why cache couldn't be used - let reason = match mismatch { - FingerprintMismatch::SpawnFingerprint { old, new } => { - let changes = detect_spawn_fingerprint_changes(old, new); - match changes.first() { - Some(SpawnFingerprintChange::Env(_)) => { - format_env_changed_inline(&env_change_names(&changes)) - } - Some( - SpawnFingerprintChange::UntrackedEnvAdded { .. } - | SpawnFingerprintChange::UntrackedEnvRemoved { .. }, - ) => Str::from("untracked env config changed"), - Some(SpawnFingerprintChange::ProgramChanged) => { - Str::from("program changed") - } - Some(SpawnFingerprintChange::ArgsChanged) => Str::from("args changed"), - Some(SpawnFingerprintChange::CwdChanged) => { - Str::from("working directory changed") - } - None => Str::from("configuration changed"), - } - } - FingerprintMismatch::InputConfig => Str::from("input configuration changed"), - FingerprintMismatch::OutputConfig => Str::from("output configuration changed"), - FingerprintMismatch::InputChanged { kind, path } => { - format_input_change_str(*kind, path.as_str()) - } - FingerprintMismatch::TrackedEnvChanged(mismatch) - | FingerprintMismatch::TrackedEnvQueryChanged { mismatch, .. } => { - vt_str::format!("{mismatch}") - } - }; + let reason = format_changes_inline(&comparison.changes); Some(vt_str::format!("○ cache miss: {reason}{}", suffix("executing"))) } CacheStatus::Miss(CacheMiss::RemoteReadFailed(error)) => { @@ -237,18 +133,48 @@ pub fn format_input_change_str(kind: InputChangeKind, path: &str) -> Str { #[cfg(test)] mod tests { use super::*; + use crate::session::cache::{Comparison, EnvMismatch}; + + fn env(mismatch: EnvMismatch, source: EnvSource) -> Change { + Change::Env { mismatch, source } + } + + fn miss(changes: Vec) -> CacheStatus { + CacheStatus::Miss(CacheMiss::Changed(Comparison::new(changes))) + } #[test] - fn env_spawn_changes_report_names_only() { - let added = SpawnFingerprintChange::Env(EnvMismatch::Added { name: Str::from("MY_ENV") }); - let removed = - SpawnFingerprintChange::Env(EnvMismatch::Removed { name: Str::from("MY_ENV") }); - let changed = - SpawnFingerprintChange::Env(EnvMismatch::Changed { name: Str::from("MY_ENV") }); + fn env_changes_report_names_only() { + let name = || Str::from("MY_ENV"); + let source = EnvSource::Fingerprinted; + let added = env(EnvMismatch::Added { name: name() }, source); + let removed = env(EnvMismatch::Removed { name: name() }, source); + let changed = env(EnvMismatch::Changed { name: name() }, source); + + assert_eq!(format_change(&added).as_str(), "env 'MY_ENV' added"); + assert_eq!(format_change(&removed).as_str(), "env 'MY_ENV' removed"); + assert_eq!(format_change(&changed).as_str(), "env 'MY_ENV' changed"); + } - assert_eq!(format_spawn_change(&added).as_str(), "env 'MY_ENV' added"); - assert_eq!(format_spawn_change(&removed).as_str(), "env 'MY_ENV' removed"); - assert_eq!(format_spawn_change(&changed).as_str(), "env 'MY_ENV' changed"); + #[test] + fn inline_reason_is_the_first_change_in_kind_order() { + let fingerprinted = + |name| env(EnvMismatch::Added { name: Str::from(name) }, EnvSource::Fingerprinted); + let status = miss(vec![fingerprinted("B"), Change::Args, fingerprinted("A")]); + assert_eq!( + format_cache_status_inline(&status, RunKind::Dry).as_deref(), + Some("○ cache miss: args changed") + ); + let status = miss(vec![fingerprinted("B"), fingerprinted("A")]); + assert_eq!( + format_cache_status_inline(&status, RunKind::Dry).as_deref(), + Some("○ cache miss: envs 'A', 'B' changed") + ); + let status = miss(vec![fingerprinted("A")]); + assert_eq!( + format_cache_status_inline(&status, RunKind::Dry).as_deref(), + Some("○ cache miss: env 'A' changed") + ); } #[test] @@ -283,19 +209,12 @@ mod tests { #[test] fn inline_tracked_env_mismatch_preserves_kind() { - let added = CacheStatus::Miss(CacheMiss::FingerprintMismatch( - FingerprintMismatch::TrackedEnvQueryChanged { - query: crate::session::execute::fingerprint::TrackedEnvQuery::Glob(Str::from( - "PROBE_*", - )), - mismatch: EnvMismatch::Added { name: Str::from("PROBE_C") }, - }, - )); - let removed = CacheStatus::Miss(CacheMiss::FingerprintMismatch( - FingerprintMismatch::TrackedEnvChanged(EnvMismatch::Removed { - name: Str::from("PROBE_A"), - }), - )); + let added = + miss(vec![env(EnvMismatch::Added { name: Str::from("PROBE_C") }, EnvSource::Tracked)]); + let removed = miss(vec![env( + EnvMismatch::Removed { name: Str::from("PROBE_A") }, + EnvSource::Tracked, + )]); assert_eq!( format_cache_status_inline(&added, RunKind::Real).as_deref(), diff --git a/crates/vt/src/session/cache/mod.rs b/crates/vt/src/session/cache/mod.rs index cdffa93e0..e364d5413 100644 --- a/crates/vt/src/session/cache/mod.rs +++ b/crates/vt/src/session/cache/mod.rs @@ -15,16 +15,12 @@ use std::{ }; // Re-export display functions for convenience -pub use display::format_cache_status_inline; -pub use display::{ - SpawnFingerprintChange, detect_spawn_fingerprint_changes, format_input_change_str, - format_spawn_change, -}; +pub use display::{format_cache_status_inline, format_change}; use rusqlite::{Connection, OptionalExtension as _}; use serde::{Deserialize, Serialize}; use tokio::sync::Mutex; use tokio_util::sync::CancellationToken; -use vt_glob::path::PathGlobSet; +use vt_casefold::EnvName; use vt_graph::config::ResolvedGlobConfig; use vt_path::{AbsolutePath, RelativePathBuf}; use vt_plan::{ @@ -38,13 +34,10 @@ use wincode::{ error::{ReadResult, WriteResult}, }; -use self::remote::{ReadError, RemoteClients, RemoteUploads, Restore, UploadError}; +use self::remote::{ReadError, RemoteClients, RemoteUploads, Resolved, Restore, UploadError}; use super::{ event::RunKind, - execute::{ - fingerprint::{PostRunFingerprint, TrackedEnvQuery}, - pipe::StdOutput, - }, + execute::{fingerprint::PostRunFingerprint, pipe::StdOutput}, }; const TASK_CACHE_PREALLOCATION_SIZE_LIMIT: usize = 256 * 1024 * 1024; @@ -145,19 +138,114 @@ pub enum CacheHitSource { } #[derive(Debug, Clone)] -#[expect( - clippy::large_enum_variant, - reason = "FingerprintMismatch contains SpawnFingerprint which is intentionally large; boxing would add unnecessary indirection for a short-lived enum" -)] pub enum CacheMiss { NotFound, - FingerprintMismatch(FingerprintMismatch), + /// The cache has a run of the task to compare with, and it differs. + Changed(Comparison), /// Reading the remote cache failed, and the local cache has no entry for /// the task. RemoteReadFailed(Arc), } -#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +/// How the current execution differs from the cached run a lookup compared +/// it with. +#[derive(Debug, Clone)] +pub struct Comparison { + /// Every difference found, sorted by kind, then by name or path, never + /// empty. Command, config and env changes are complete. The file check + /// stops at the first changed file it finds, so at most one file is + /// listed. + pub changes: Vec, +} + +impl Comparison { + /// Sort `changes` and drop duplicates, such as an env var that two + /// tracked queries match. Of two duplicates, the one found first is kept. + fn new(mut changes: Vec) -> Self { + changes.sort_by(|a, b| a.order_key().cmp(&b.order_key())); + changes.dedup_by(|a, b| a.order_key() == b.order_key()); + Self { changes } + } +} + +/// One difference between the current execution and the cached run it's +/// compared with. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum Change { + /// The program the command runs changed. + Program, + /// The command's arguments changed. + Args, + /// The directory the command runs in changed. + Cwd, + /// The task's `input` setting changed. + InputConfig, + /// The task's `output` setting changed. + OutputConfig, + /// A name was added to the task's `untrackedEnv` setting. + UntrackedEnvAdded { name: Str }, + /// A name was removed from the task's `untrackedEnv` setting. + UntrackedEnvRemoved { name: Str }, + /// An env var was added, removed, or changed value. + Env { mismatch: EnvMismatch, source: EnvSource }, + /// An input file or folder was added, removed, or modified. + File { kind: InputChangeKind, path: RelativePathBuf }, +} + +impl Change { + /// The order changes are listed in: by kind in the order of the + /// variants, then by name or path, then by how it changed. Env names + /// compare by the platform's rules, so on Windows a tool's `getEnv('a')` + /// and a query matching `A` name the same env. + fn order_key(&self) -> (u8, OrderName<'_>, u8) { + let exact = OrderName::Exact; + match self { + Self::Program => (0, exact(""), 0), + Self::Args => (1, exact(""), 0), + Self::Cwd => (2, exact(""), 0), + Self::InputConfig => (3, exact(""), 0), + Self::OutputConfig => (4, exact(""), 0), + Self::UntrackedEnvAdded { name } => (5, exact(name), 0), + Self::UntrackedEnvRemoved { name } => (5, exact(name), 1), + Self::Env { mismatch, .. } => { + let how = match mismatch { + EnvMismatch::Added { .. } => 0, + EnvMismatch::Removed { .. } => 1, + EnvMismatch::Changed { .. } => 2, + }; + (6, OrderName::Env(EnvName::new(mismatch.name().as_str())), how) + } + Self::File { kind, path } => { + let how = match kind { + InputChangeKind::ContentModified => 0, + InputChangeKind::Added => 1, + InputChangeKind::Removed => 2, + }; + (7, exact(path.as_str()), how) + } + } + } +} + +/// A change's name or path, as [`Change::order_key`] compares it. +#[derive(PartialEq, Eq, PartialOrd, Ord)] +enum OrderName<'a> { + Exact(&'a str), + Env(EnvName<&'a str>), +} + +/// Where the cache records an env var. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +pub enum EnvSource { + /// The task's `cache.env` names it, so its value is part of the cache + /// key. + Fingerprinted, + /// A runner-aware tool read it with `tracked: true`, by name or with a + /// query. + Tracked, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] pub enum InputChangeKind { /// File content changed but path is the same ContentModified, @@ -206,6 +294,19 @@ impl EnvMismatch { _ => None, } } + + /// Every env added, removed or changed between `stored` and `current`, in + /// name order: those in `stored` first, then those only in `current`. + pub fn diff<'a, T: Eq>( + stored: &'a BTreeMap, + current: &'a BTreeMap, + ) -> impl Iterator + 'a { + let added = current.keys().filter(|name| !stored.contains_key(*name)); + stored + .keys() + .chain(added) + .filter_map(|name| Self::compare(name, stored.get(name), current.get(name))) + } } impl Display for EnvMismatch { @@ -218,47 +319,6 @@ impl Display for EnvMismatch { } } -#[derive(Debug, Clone, Serialize)] -pub enum FingerprintMismatch { - /// Found a previous cache entry key for the same task, but the spawn fingerprint differs. - /// This happens when the command itself or an env changes. - SpawnFingerprint { - /// The fingerprint from the cached entry - old: SpawnFingerprint, - /// The fingerprint of the current execution - new: SpawnFingerprint, - }, - /// Found a previous cache entry key for the same task, but `input_config` differs. - InputConfig, - /// Found a previous cache entry key for the same task, but `output_config` differs. - OutputConfig, - - InputChanged { - kind: InputChangeKind, - path: RelativePathBuf, - }, - /// A runner-aware tool-tracked env var changed between runs. - TrackedEnvChanged(EnvMismatch), - /// A runner-aware tool-tracked bulk env query's match-set changed between runs. - TrackedEnvQueryChanged { - query: TrackedEnvQuery, - mismatch: EnvMismatch, - }, -} - -impl From for FingerprintMismatch { - fn from(mismatch: crate::session::execute::fingerprint::PostRunMismatch) -> Self { - use crate::session::execute::fingerprint::PostRunMismatch; - match mismatch { - PostRunMismatch::Input { kind, path } => Self::InputChanged { kind, path }, - PostRunMismatch::TrackedEnv(mismatch) => Self::TrackedEnvChanged(mismatch), - PostRunMismatch::TrackedEnvQuery { query, mismatch } => { - Self::TrackedEnvQueryChanged { query, mismatch } - } - } - } -} - /// Split a relative path into `(parent_dir, filename)`. /// Returns `None` for the parent if the path has no `/` separator. pub fn split_path(path: &str) -> (Option<&str>, &str) { @@ -309,6 +369,15 @@ fn enable_wal(conn: &Connection) -> rusqlite::Result<()> { Ok(()) } +/// The miss to report when both the local and the remote cache missed: the +/// local one, unless the local cache has no entry for the task. +fn prefer_local_miss(local_miss: CacheMiss, remote_miss: CacheMiss) -> CacheMiss { + match local_miss { + CacheMiss::NotFound => remote_miss, + local_miss => local_miss, + } +} + impl ExecutionCache { #[tracing::instrument(level = "debug", skip_all)] pub fn load_from_path(path: &AbsolutePath) -> anyhow::Result { @@ -371,18 +440,9 @@ impl ExecutionCache { cancel_token: &CancellationToken, ) -> anyhow::Result> { let cache_key = CacheEntryKey::from_metadata(cache_metadata); - // Part of the key, so the same as when the entry was created. - let input_negative_globs = PathGlobSet::new(&cache_metadata.input_config.negative_globs)?; let local_miss = match self - .try_hit_local( - cache_metadata, - &cache_key, - globbed_inputs, - &input_negative_globs, - workspace_root, - run_kind, - ) + .try_hit_local(cache_metadata, &cache_key, globbed_inputs, workspace_root, run_kind) .await? { Ok(value) => return Ok(Ok(CacheHit { value, source: CacheHitSource::Local })), @@ -401,27 +461,20 @@ impl ExecutionCache { ) => remote_config, None => return Ok(Err(local_miss)), }; - let remote_miss = match self + let hit = self .try_hit_remote( remote_config, cache_metadata, &cache_key, globbed_inputs, - &input_negative_globs, workspace_root, cache_dir, run_kind, + local_miss, cancel_token, ) - .await? - { - Ok(value) => return Ok(Ok(CacheHit { value, source: CacheHitSource::Remote })), - Err(miss) => miss, - }; - Ok(Err(match local_miss { - CacheMiss::NotFound => remote_miss, - local_miss => local_miss, - })) + .await?; + Ok(hit.map(|value| CacheHit { value, source: CacheHitSource::Remote })) } async fn try_hit_local( @@ -429,7 +482,6 @@ impl ExecutionCache { cache_metadata: &CacheMetadata, cache_key: &CacheEntryKey, globbed_inputs: &BTreeMap, - input_negative_globs: &PathGlobSet<'_>, workspace_root: &AbsolutePath, run_kind: RunKind, ) -> anyhow::Result> { @@ -437,13 +489,15 @@ impl ExecutionCache { // Try to find the cache entry by key (spawn fingerprint + input config) if let Some(cache_value) = self.get_by_cache_key(cache_key).await? { - if let Some(mismatch) = cache_value.validate( - &cache_metadata.unfiltered_envs, + let changes = cache_value.validate( + cache_key, + cache_key, globbed_inputs, - input_negative_globs, + &cache_metadata.unfiltered_envs, workspace_root, - )? { - return Ok(Err(CacheMiss::FingerprintMismatch(mismatch))); + )?; + if !changes.is_empty() { + return Ok(Err(CacheMiss::Changed(Comparison::new(changes)))); } // Associate the execution key to the cache entry key if not already, // so that next time we can find it and report what changed @@ -459,9 +513,23 @@ impl ExecutionCache { self.get_cache_key_by_execution_key(execution_cache_key).await? { // `get_by_cache_key` above returned None for the *current* cache key, - // so the associated key must differ. - let mismatch = old_cache_key.into_mismatch(cache_key); - return Ok(Err(CacheMiss::FingerprintMismatch(mismatch))); + // so the associated key must differ. Its entry is only compared + // with, so one that can't be read leaves just the key changes. + let old_value = self.get_by_cache_key(&old_cache_key).await.unwrap_or_else(|err| { + tracing::debug!(?err, "failed to read the task's previous cache entry"); + None + }); + let changes = match old_value { + Some(old_value) => old_value.validate( + &old_cache_key, + cache_key, + globbed_inputs, + &cache_metadata.unfiltered_envs, + workspace_root, + )?, + None => old_cache_key.changes_to(cache_key), + }; + return Ok(Err(CacheMiss::Changed(Comparison::new(changes)))); } Ok(Err(CacheMiss::NotFound)) @@ -470,9 +538,15 @@ impl ExecutionCache { /// Fetch the entry from the remote cache that `remote_config` configures. /// An exact entry that passes validation is a hit once its output archive /// is downloaded, or right away in a dry run. - /// A fallback entry, a failed validation, or a failed read is a miss. An - /// error while validating counts as a failed read, so the remote entry - /// never fails the task. + /// A failed validation or a failed read is a miss. An error while + /// validating counts as a failed read, so the remote entry never fails the + /// task. A miss is `local_miss`, the local cache's, unless the local cache + /// has no entry for the task. + /// + /// A fallback is only the key of the task's previous entry, so that entry + /// is fetched too, to compare with as in the local cache, unless the local + /// cache's miss wins anyway. If that fetch fails, the miss is a failed + /// read, like a failed first fetch. #[expect(clippy::too_many_arguments, reason = "forwarded from `try_hit`")] async fn try_hit_remote( &self, @@ -480,29 +554,52 @@ impl ExecutionCache { cache_metadata: &CacheMetadata, cache_key: &CacheEntryKey, globbed_inputs: &BTreeMap, - input_negative_globs: &PathGlobSet<'_>, workspace_root: &AbsolutePath, cache_dir: &AbsolutePath, run_kind: RunKind, + local_miss: CacheMiss, cancel_token: &CancellationToken, ) -> anyhow::Result> { + let execution_cache_key = &cache_metadata.execution_cache_key; let fetched = self .remote_clients - .fetch(remote_config, cache_key, &cache_metadata.execution_cache_key, cancel_token) + .fetch(remote_config, cache_key, execution_cache_key, cancel_token) .await; let validate = |cache_value: &CacheEntryValue| { cache_value.validate( - &cache_metadata.unfiltered_envs, + cache_key, + cache_key, globbed_inputs, - input_negative_globs, + &cache_metadata.unfiltered_envs, workspace_root, ) }; - let Restore { value: cache_value, blob_id } = - match remote::resolve(fetched, cache_key, validate) { - Ok(restore) => restore, - Err(miss) => return Ok(Err(miss)), - }; + let Restore { value: cache_value, blob_id } = match remote::resolve(fetched, validate) { + Ok(Resolved::Exact(restore)) => restore, + Ok(Resolved::Fallback(old_cache_key)) if matches!(local_miss, CacheMiss::NotFound) => { + let old_value = match self + .remote_clients + .fetch_value(remote_config, &old_cache_key, execution_cache_key, cancel_token) + .await + { + Ok(old_value) => old_value, + Err(err) => return Ok(Err(err.into_miss())), + }; + let changes = match old_value { + Some(old_value) => old_value.validate( + &old_cache_key, + cache_key, + globbed_inputs, + &cache_metadata.unfiltered_envs, + workspace_root, + )?, + None => old_cache_key.changes_to(cache_key), + }; + return Ok(Err(CacheMiss::Changed(Comparison::new(changes)))); + } + Ok(Resolved::Fallback(_)) => return Ok(Err(local_miss)), + Err(remote_miss) => return Ok(Err(prefer_local_miss(local_miss, remote_miss))), + }; // `resolve` checks that the entry has an archive exactly when it has a // blob to download, so the hit a dry run shows says whether it restores // outputs. @@ -518,7 +615,7 @@ impl ExecutionCache { .await { Ok(archive_name) => Some(archive_name), - Err(err) => return Ok(Err(err.into_miss())), + Err(err) => return Ok(Err(prefer_local_miss(local_miss, err.into_miss()))), } } None => None, @@ -856,6 +953,26 @@ mod tests { assert_eq!(count_b, 0); } + fn tracked_env_changed(name: &str) -> Change { + Change::Env { + mismatch: EnvMismatch::Changed { name: Str::from(name) }, + source: EnvSource::Tracked, + } + } + + /// Env names compare by the platform's rules: on Windows, a tool's + /// `getEnv('probe_env')` and a query that matches `PROBE_ENV` name the + /// same env, so its change is listed once. + #[test] + fn env_changes_dedupe_by_platform_rules() { + let comparison = Comparison::new(vec![ + tracked_env_changed("probe_env"), + tracked_env_changed("PROBE_ENV"), + tracked_env_changed("probe_env"), + ]); + assert_eq!(comparison.changes.len(), if cfg!(windows) { 1 } else { 2 }); + } + /// `synchronous` is `NORMAL` only when the database is in WAL mode (see `enable_wal` for why). #[test] fn sync_is_relaxed_only_in_wal_mode() { diff --git a/crates/vt/src/session/cache/remote.rs b/crates/vt/src/session/cache/remote.rs index 7afdac876..9fbd11f49 100644 --- a/crates/vt/src/session/cache/remote.rs +++ b/crates/vt/src/session/cache/remote.rs @@ -39,7 +39,7 @@ use wincode::{ }; use super::{ - CACHE_SCHEMA_VERSION, CacheEntryKey, CacheEntryValue, CacheMiss, FingerprintMismatch, + CACHE_SCHEMA_VERSION, CacheEntryKey, CacheEntryValue, CacheMiss, Change, Comparison, TaskCacheConfig, archive, deserialize_cache, serialize_cache, }; @@ -116,35 +116,38 @@ pub(super) struct Restore { pub blob_id: Option, } -/// Turn the result of a fetch into an entry to restore or a miss. An exact -/// match's entries are checked newest first, and the first that passes is -/// restored. `validate` checks an entry against the current execution. If no -/// entry passes, the miss is the newest entry's: a fingerprint mismatch, or a -/// read failure if it doesn't decode, its blob doesn't match its value, or it -/// can't be validated. A fallback's miss reason compares its key with -/// `cache_key`, and no match is a miss without an entry. A failed fetch is a -/// read failure. -#[expect( - clippy::result_large_err, - reason = "`CacheMiss` is intentionally large, and a lookup returns it once" -)] +/// What a fetch found that isn't a miss yet. +#[derive(Debug)] +pub(super) enum Resolved { + /// An exact entry that passed validation. + Exact(Restore), + /// No exact entry, but the key of the entry last stored for the task. The + /// miss compares the execution with that entry. + Fallback(CacheEntryKey), +} + +/// Turn the result of a fetch into an entry to restore, a fallback key, or a +/// miss. An exact match's entries are checked newest first, and the first +/// that passes is restored. `validate` checks an entry against the current +/// execution. If no entry passes, the miss is the newest entry's: the changes +/// it found, or a read failure if it doesn't decode, its blob doesn't match +/// its value, or it can't be validated. No match is a miss without an entry. +/// A failed fetch is a read failure. pub(super) fn resolve( fetched: Result, ReadError>, - cache_key: &CacheEntryKey, - mut validate: impl FnMut(&CacheEntryValue) -> anyhow::Result>, -) -> Result { + mut validate: impl FnMut(&CacheEntryValue) -> anyhow::Result>, +) -> Result { let entries = match fetched.map_err(ReadError::into_miss)? { Some(Fetched::Exact { entries }) => entries, Some(Fetched::Fallback { key }) => { - let key = decode_key(&key).map_err(ReadError::into_miss)?; - return Err(CacheMiss::FingerprintMismatch(key.into_mismatch(cache_key))); + return Ok(Resolved::Fallback(decode_key(&key).map_err(ReadError::into_miss)?)); } None => return Err(CacheMiss::NotFound), }; let mut newest_miss = None; for entry in entries { match check_entry(entry, &mut validate) { - Ok(restore) => return Ok(restore), + Ok(restore) => return Ok(Resolved::Exact(restore)), Err(miss) => { newest_miss.get_or_insert(miss); } @@ -155,21 +158,17 @@ pub(super) fn resolve( } /// Check one entry of an exact match, as [`resolve`] describes. -#[expect( - clippy::result_large_err, - reason = "`CacheMiss` is intentionally large, and a lookup checks only a few entries" -)] fn check_entry( Entry { value, blob_id }: Entry, - validate: &mut impl FnMut(&CacheEntryValue) -> anyhow::Result>, + validate: &mut impl FnMut(&CacheEntryValue) -> anyhow::Result>, ) -> Result { let value = decode_value(&value).map_err(|err| ReadError::CorruptValue(err).into_miss())?; if value.output_archive.is_some() != blob_id.is_some() { return Err(ReadError::MismatchedBlob.into_miss()); } match validate(&value) { - Ok(None) => Ok(Restore { value, blob_id }), - Ok(Some(mismatch)) => Err(CacheMiss::FingerprintMismatch(mismatch)), + Ok(changes) if changes.is_empty() => Ok(Restore { value, blob_id }), + Ok(changes) => Err(CacheMiss::Changed(Comparison::new(changes))), Err(err) => Err(ReadError::Validate(err).into_miss()), } } @@ -226,6 +225,25 @@ impl RemoteClients { .map_err(ReadError::Fetch) } + /// Fetch the value of the newest entry stored under exactly `cache_key`, + /// to compare with, without its blob. `None` if there is no such entry. + pub(super) async fn fetch_value( + &self, + remote_config: &ResolvedRemoteCacheConfig, + cache_key: &CacheEntryKey, + execution_cache_key: &ExecutionCacheKey, + cancel_token: &CancellationToken, + ) -> Result, ReadError> { + match self.fetch(remote_config, cache_key, execution_cache_key, cancel_token).await? { + Some(Fetched::Exact { entries }) => entries + .first() + .map(|newest| decode_value(&newest.value)) + .transpose() + .map_err(ReadError::CorruptValue), + Some(Fetched::Fallback { .. }) | None => Ok(None), + } + } + /// Download the blob `blob_id` from the remote cache that `remote_config` /// configures into `cache_dir`, checking that it decodes as an output /// archive as it arrives. It's downloaded to a `.tmp` file, which is @@ -503,11 +521,10 @@ fn decode_key(bytes: &[u8]) -> Result { } #[cfg(test)] -mod tests { +pub(super) mod tests { use std::{collections::BTreeMap, net::TcpListener, time::Duration}; use tokio::sync::oneshot; - use vt_glob::path::PathGlobSet; use vt_graph::config::ResolvedGlobConfig; use vt_path::{AbsolutePathBuf, RelativePathBuf}; use vt_plan::{ @@ -527,7 +544,8 @@ mod tests { /// A key whose spawn fingerprint runs `vtt build`. `SpawnFingerprint`'s /// fields are private to `vt_plan`, so it's decoded from types with the /// same encoding. Update them if `SpawnFingerprint` changes. - fn cache_key(input_config: ResolvedGlobConfig) -> CacheEntryKey { + /// A cache entry key with `input_config`, for tests. + pub(in crate::session::cache) fn cache_key(input_config: ResolvedGlobConfig) -> CacheEntryKey { #[derive(SchemaWrite)] enum ProgramFingerprintLayout { OutsideWorkspace { program_name: Str }, @@ -596,10 +614,10 @@ mod tests { /// The path of an added input, which `miss` must be. fn added_input(miss: &CacheMiss) -> &str { match miss { - CacheMiss::FingerprintMismatch(FingerprintMismatch::InputChanged { - kind: InputChangeKind::Added, - path, - }) => path.as_str(), + CacheMiss::Changed(comparison) => match comparison.changes.as_slice() { + [Change::File { kind: InputChangeKind::Added, path }] => path.as_str(), + changes => panic!("expected an added input, got {changes:?}"), + }, miss => panic!("expected an added input, got {miss:?}"), } } @@ -608,19 +626,15 @@ mod tests { /// the current inputs. fn validate_against( globbed_inputs: BTreeMap, - ) -> impl FnMut(&CacheEntryValue) -> anyhow::Result> { + ) -> impl FnMut(&CacheEntryValue) -> anyhow::Result> { move |value| { let workspace_root = vt_path::current_dir().unwrap(); - value.validate( - &FxHashMap::default(), - &globbed_inputs, - &PathGlobSet::default(), - &workspace_root, - ) + let key = cache_key(ResolvedGlobConfig::default_auto()); + value.validate(&key, &key, &globbed_inputs, &FxHashMap::default(), &workspace_root) } } - fn not_validated(_: &CacheEntryValue) -> anyhow::Result> { + fn not_validated(_: &CacheEntryValue) -> anyhow::Result> { panic!("only exact entries that decode and match their blob are validated") } @@ -634,9 +648,8 @@ mod tests { /// The read failure of an exact match whose only entry is `value`, without /// a blob. It must be "remote cache value is corrupt". fn corrupt_value(value: Vec) -> Arc { - let key = cache_key(ResolvedGlobConfig::default_auto()); let fetched = Ok(Some(Fetched::Exact { entries: vec![Entry { value, blob_id: None }] })); - match resolve(fetched, &key, not_validated).unwrap_err() { + match resolve(fetched, not_validated).unwrap_err() { CacheMiss::RemoteReadFailed(error) => { assert_eq!(vt_str::format!("{error}"), "remote cache value is corrupt"); error @@ -645,12 +658,21 @@ mod tests { } } + /// The entry `resolved` restores, which must be an exact entry. + fn restored(resolved: Resolved) -> Restore { + match resolved { + Resolved::Exact(restore) => restore, + Resolved::Fallback(key) => { + panic!("expected an entry to restore, got a fallback {key:?}") + } + } + } + #[test] fn exact_entry_that_validates_is_restored() { - let key = cache_key(ResolvedGlobConfig::default_auto()); - let restore = - resolve(Ok(Some(exact(&cache_value()))), &key, validate_against(BTreeMap::new())) - .unwrap(); + let restore = restored( + resolve(Ok(Some(exact(&cache_value()))), validate_against(BTreeMap::new())).unwrap(), + ); assert_eq!(restore.blob_id.as_deref(), Some("1")); assert_eq!(restore.value.std_outputs[0].content, b"built\n"); assert_eq!(restore.value.duration, Duration::from_millis(5)); @@ -659,19 +681,13 @@ mod tests { #[test] fn exact_entry_that_fails_validation_is_a_mismatch() { let current_inputs = BTreeMap::from([(RelativePathBuf::new("src/a.txt").unwrap(), 1)]); - let miss = resolve( - Ok(Some(exact(&cache_value()))), - &cache_key(ResolvedGlobConfig::default_auto()), - validate_against(current_inputs), - ) - .unwrap_err(); + let miss = + resolve(Ok(Some(exact(&cache_value()))), validate_against(current_inputs)).unwrap_err(); + let CacheMiss::Changed(comparison) = &miss else { panic!("{miss:?}") }; assert!( matches!( - &miss, - CacheMiss::FingerprintMismatch(FingerprintMismatch::InputChanged { - kind: InputChangeKind::Added, - path, - }) if path.as_str() == "src/a.txt" + comparison.changes.as_slice(), + [Change::File { kind: InputChangeKind::Added, path }] if path.as_str() == "src/a.txt" ), "{miss:?}" ); @@ -679,17 +695,16 @@ mod tests { #[test] fn newest_entry_that_validates_is_restored() { - let key = cache_key(ResolvedGlobConfig::default_auto()); let entries = vec![entry(&cache_value(), "2"), entry(&cache_value(), "1")]; - let restore = - resolve(Ok(Some(Fetched::Exact { entries })), &key, validate_against(BTreeMap::new())) - .unwrap(); + let restore = restored( + resolve(Ok(Some(Fetched::Exact { entries })), validate_against(BTreeMap::new())) + .unwrap(), + ); assert_eq!(restore.blob_id.as_deref(), Some("2")); } #[test] fn older_entry_that_validates_is_restored_after_newer_ones_fail() { - let key = cache_key(ResolvedGlobConfig::default_auto()); let entries = vec![ entry(&with_inputs(&["src/c.txt"]), "4"), Entry { value: b"not a cache value".to_vec(), blob_id: Some(Str::from("3")) }, @@ -698,11 +713,13 @@ mod tests { ]; let mut validated = 0; let mut validate = validate_against(inputs(&["src/a.txt"])); - let restore = resolve(Ok(Some(Fetched::Exact { entries })), &key, |value| { - validated += 1; - validate(value) - }) - .unwrap(); + let restore = restored( + resolve(Ok(Some(Fetched::Exact { entries })), |value| { + validated += 1; + validate(value) + }) + .unwrap(), + ); assert_eq!(restore.blob_id.as_deref(), Some("2")); // The corrupt entry isn't validated, and the entry after the restored // one isn't checked. @@ -711,26 +728,19 @@ mod tests { #[test] fn newest_entry_explains_the_miss_when_none_validates() { - let key = cache_key(ResolvedGlobConfig::default_auto()); let entries = vec![entry(&cache_value(), "2"), entry(&with_inputs(&["src/b.txt"]), "1")]; - let miss = resolve( - Ok(Some(Fetched::Exact { entries })), - &key, - validate_against(inputs(&["src/a.txt"])), - ) - .unwrap_err(); + let miss = + resolve(Ok(Some(Fetched::Exact { entries })), validate_against(inputs(&["src/a.txt"]))) + .unwrap_err(); assert_eq!(added_input(&miss), "src/a.txt"); let entries = vec![ Entry { value: b"not a cache value".to_vec(), blob_id: None }, entry(&cache_value(), "1"), ]; - let miss = resolve( - Ok(Some(Fetched::Exact { entries })), - &key, - validate_against(inputs(&["src/a.txt"])), - ) - .unwrap_err(); + let miss = + resolve(Ok(Some(Fetched::Exact { entries })), validate_against(inputs(&["src/a.txt"]))) + .unwrap_err(); assert_eq!(read_failure(miss), "remote cache value is corrupt"); } @@ -742,44 +752,29 @@ mod tests { .post_run_fingerprint .tracked_env_queries .insert(TrackedEnvQuery::Glob(Str::from("PROBE_[")), BTreeMap::new()); - let miss = resolve( - Ok(Some(exact(&value))), - &cache_key(ResolvedGlobConfig::default_auto()), - validate_against(BTreeMap::new()), - ) - .unwrap_err(); + let miss = resolve(Ok(Some(exact(&value))), validate_against(BTreeMap::new())).unwrap_err(); assert_eq!(read_failure(miss), "remote cache entry couldn't be validated"); } #[test] - fn fallback_is_a_mismatch_with_the_stored_key() { - let stored_key = encode_key(&cache_key(ResolvedGlobConfig::default_auto())).unwrap(); - let mut input_config = ResolvedGlobConfig::default_auto(); - input_config.positive_globs.insert(Str::from("src/**")); - let miss = resolve( - Ok(Some(Fetched::Fallback { key: stored_key })), - &cache_key(input_config), - not_validated, - ) - .unwrap_err(); - assert!( - matches!(miss, CacheMiss::FingerprintMismatch(FingerprintMismatch::InputConfig)), - "{miss:?}" - ); + fn fallback_is_the_stored_key() { + let key = cache_key(ResolvedGlobConfig::default_auto()); + let stored_key = encode_key(&key).unwrap(); + let resolved = + resolve(Ok(Some(Fetched::Fallback { key: stored_key })), not_validated).unwrap(); + assert!(matches!(&resolved, Resolved::Fallback(stored) if *stored == key), "{resolved:?}"); } #[test] fn no_match_is_a_miss_without_an_entry() { - let key = cache_key(ResolvedGlobConfig::default_auto()); - let miss = resolve(Ok(None), &key, not_validated).unwrap_err(); + let miss = resolve(Ok(None), not_validated).unwrap_err(); assert!(matches!(miss, CacheMiss::NotFound), "{miss:?}"); } #[test] fn failed_fetch_is_a_read_failure() { - let key = cache_key(ResolvedGlobConfig::default_auto()); let fetched = Err(ReadError::Fetch(vt_remote_cache::Error::InvalidEndpoint(None))); - let miss = resolve(fetched, &key, not_validated).unwrap_err(); + let miss = resolve(fetched, not_validated).unwrap_err(); assert_eq!(read_failure(miss), "remote cache fetch failed"); } @@ -841,12 +836,11 @@ mod tests { #[test] fn blob_that_does_not_match_the_value_is_a_read_failure() { - let key = cache_key(ResolvedGlobConfig::default_auto()); let without_archive = CacheEntryValue { output_archive: None, ..cache_value() }; for (value, blob_id) in [(cache_value(), None), (without_archive, Some(Str::from("1")))] { let value = encode_value(&value).unwrap(); let fetched = Ok(Some(Fetched::Exact { entries: vec![Entry { value, blob_id }] })); - let miss = resolve(fetched, &key, not_validated).unwrap_err(); + let miss = resolve(fetched, not_validated).unwrap_err(); assert_eq!(read_failure(miss), "remote cache entry's blob doesn't match its value"); } } @@ -871,7 +865,7 @@ mod tests { garbage.extend(b"not a cache key"); for stored_key in [b"not a cache key".to_vec(), other_schema, other_format, garbage] { let fetched = Ok(Some(Fetched::Fallback { key: stored_key })); - let miss = resolve(fetched, &key, not_validated).unwrap_err(); + let miss = resolve(fetched, not_validated).unwrap_err(); assert_eq!(read_failure(miss), "remote cache key is corrupt"); } } diff --git a/crates/vt/src/session/cache/validation.rs b/crates/vt/src/session/cache/validation.rs index bcf4008ef..77040bd00 100644 --- a/crates/vt/src/session/cache/validation.rs +++ b/crates/vt/src/session/cache/validation.rs @@ -2,62 +2,170 @@ use std::{collections::BTreeMap, ffi::OsStr, sync::Arc}; -use rustc_hash::FxHashMap; +use rustc_hash::{FxHashMap, FxHashSet}; use vt_casefold::EnvName; use vt_glob::path::PathGlobSet; use vt_path::{AbsolutePath, RelativePathBuf}; +use vt_plan::cache_metadata::SpawnFingerprint; -use super::{CacheEntryKey, CacheEntryValue, FingerprintMismatch, InputChangeKind}; +use super::{CacheEntryKey, CacheEntryValue, Change, EnvMismatch, EnvSource, InputChangeKind}; +use crate::session::execute::glob::compute_globbed_inputs; impl CacheEntryValue { - /// Validate explicit inputs, then inferred inputs and tracked environment - /// values against `unfiltered_envs`, the execution's - /// `CacheMetadata::unfiltered_envs`. `input_negative_globs`, compiled from - /// the entry key's input config, leave entries out of listings. Returns - /// the first mismatch, or `None` when the entry is valid. + /// Compare this cached run, stored under `stored_key`, with the execution, + /// whose key is `current_key`: the key's changes, explicit inputs, then + /// inferred inputs, and tracked environment values against + /// `unfiltered_envs`, the execution's `CacheMetadata::unfiltered_envs`. + /// Returns every key and env change and the first changed file found, or + /// nothing when the entry is valid. + /// + /// The entry stored under the execution's key has no key changes. The + /// task's previous entry, stored under another key, is compared to tell + /// whether reverting the key changes is enough: its explicit inputs are + /// globbed with its own key's input config, so the listed changes are + /// exactly what reverting needs. `globbed_inputs` are the execution's, + /// reused when the input configs are the same. /// /// # Errors /// - /// Propagates errors from post-run fingerprint validation. + /// Propagates errors from post-run fingerprint validation. Once a file + /// changed, the entry misses whatever the envs are, so an error comparing + /// them only leaves the env changes out. The previous entry is only + /// compared with, so an error comparing it leaves just the key changes. pub(crate) fn validate( &self, + stored_key: &CacheEntryKey, + current_key: &CacheEntryKey, + globbed_inputs: &BTreeMap, unfiltered_envs: &FxHashMap>, Arc>, + workspace_root: &AbsolutePath, + ) -> anyhow::Result> { + let mut changes = stored_key.changes_to(current_key); + match self.run_changes( + stored_key, + current_key, + globbed_inputs, + unfiltered_envs, + workspace_root, + ) { + Ok(run_changes) => changes.extend(run_changes), + Err(err) if stored_key != current_key => { + tracing::debug!(?err, "failed to compare with the previous cached run"); + } + Err(err) => return Err(err), + } + Ok(changes) + } + + /// The changes to the run's files and tracked envs: see [`Self::validate`]. + fn run_changes( + &self, + stored_key: &CacheEntryKey, + current_key: &CacheEntryKey, globbed_inputs: &BTreeMap, - input_negative_globs: &PathGlobSet<'_>, + unfiltered_envs: &FxHashMap>, Arc>, workspace_root: &AbsolutePath, - ) -> anyhow::Result> { - if let Some(mismatch) = detect_globbed_input_change(&self.globbed_inputs, globbed_inputs) { - return Ok(Some(mismatch)); + ) -> anyhow::Result> { + let input_config = &stored_key.input_config; + let own_globbed_inputs; + let globbed_inputs = if *input_config == current_key.input_config { + globbed_inputs + } else { + own_globbed_inputs = compute_globbed_inputs( + workspace_root, + &input_config.positive_globs, + &input_config.negative_globs, + )?; + &own_globbed_inputs + }; + // Listings leave out the entries the stored key's input exclusions match. + let input_negative_globs = PathGlobSet::new(&input_config.negative_globs)?; + let mut changes: Vec = + detect_globbed_input_change(&self.globbed_inputs, globbed_inputs).into_iter().collect(); + if changes.is_empty() { + changes.extend( + self.post_run_fingerprint + .first_input_change(workspace_root, &input_negative_globs)?, + ); } + let env_changes = self.post_run_fingerprint.env_changes(unfiltered_envs); + add_checked(&mut changes, env_changes, "tracked envs")?; + Ok(changes) + } +} - self.post_run_fingerprint - .validate(workspace_root, input_negative_globs, unfiltered_envs) - .map(|mismatch| mismatch.map(FingerprintMismatch::from)) +/// Add to `changes` what a later check of `what` found, one result per change +/// or per thing it couldn't check, or fail with its first error. Once a +/// change is known, the entry misses whatever else changed, so an error then +/// only leaves out what that check would have found. +fn add_checked( + changes: &mut Vec, + mut checked: Vec>, + what: &str, +) -> anyhow::Result<()> { + // Changes first, so they're known before any error. + checked.sort_by_key(Result::is_err); + for checked in checked { + match checked { + Ok(change) => changes.push(change), + Err(err) if !changes.is_empty() => { + tracing::debug!(?err, "failed to check the {what} of a changed entry"); + } + Err(err) => return Err(err), + } } + Ok(()) } impl CacheEntryKey { - /// Explain a fallback miss by comparing the stored key with the current key. - /// The keys must differ. Checks spawn, input, and output configuration in order. - pub(crate) fn into_mismatch(self, current: &Self) -> FingerprintMismatch { + /// Every difference between this stored key and `current`: the spawn + /// fingerprint, then the input and output configuration. Explains a miss + /// that found the task's previous entry under this key. + pub(crate) fn changes_to(&self, current: &Self) -> Vec { // Destructure to ensure we handle all fields when new ones are added. - let Self { - spawn_fingerprint: old_spawn_fingerprint, - input_config: old_input_config, - output_config: old_output_config, - } = self; - if old_spawn_fingerprint != current.spawn_fingerprint { - FingerprintMismatch::SpawnFingerprint { - old: old_spawn_fingerprint, - new: current.spawn_fingerprint.clone(), - } - } else if old_input_config != current.input_config { - FingerprintMismatch::InputConfig - } else { - debug_assert_ne!(old_output_config, current.output_config); - FingerprintMismatch::OutputConfig + let Self { spawn_fingerprint, input_config, output_config } = self; + let mut changes = spawn_changes(spawn_fingerprint, ¤t.spawn_fingerprint); + if *input_config != current.input_config { + changes.push(Change::InputConfig); + } + if *output_config != current.output_config { + changes.push(Change::OutputConfig); } + changes + } +} + +/// Every difference between two spawn fingerprints. +fn spawn_changes(old: &SpawnFingerprint, new: &SpawnFingerprint) -> Vec { + let mut changes = Vec::new(); + let old_env = old.env_fingerprints(); + let new_env = new.env_fingerprints(); + changes.extend( + EnvMismatch::diff(&old_env.fingerprinted_envs, &new_env.fingerprinted_envs) + .map(|mismatch| Change::Env { mismatch, source: EnvSource::Fingerprinted }), + ); + + // Check untracked env config changes + let old_untracked: FxHashSet<_> = old_env.untracked_env_config.iter().collect(); + let new_untracked: FxHashSet<_> = new_env.untracked_env_config.iter().collect(); + for name in old_untracked.difference(&new_untracked) { + changes.push(Change::UntrackedEnvRemoved { name: (*name).clone() }); + } + for name in new_untracked.difference(&old_untracked) { + changes.push(Change::UntrackedEnvAdded { name: (*name).clone() }); } + + if old.program_fingerprint_debug() != new.program_fingerprint_debug() { + changes.push(Change::Program); + } + if old.args() != new.args() { + changes.push(Change::Args); + } + if old.cwd() != new.cwd() { + changes.push(Change::Cwd); + } + + changes } /// Compare stored and current globbed inputs, returning the first changed path. @@ -65,7 +173,7 @@ impl CacheEntryKey { fn detect_globbed_input_change( stored: &BTreeMap, current: &BTreeMap, -) -> Option { +) -> Option { let mut stored_iter = stored.iter(); let mut current_iter = current.iter(); let mut s = stored_iter.next(); @@ -75,21 +183,15 @@ fn detect_globbed_input_change( match (s, c) { (None, None) => return None, (Some((sp, _)), None) => { - return Some(FingerprintMismatch::InputChanged { - kind: InputChangeKind::Removed, - path: sp.clone(), - }); + return Some(Change::File { kind: InputChangeKind::Removed, path: sp.clone() }); } (None, Some((cp, _))) => { - return Some(FingerprintMismatch::InputChanged { - kind: InputChangeKind::Added, - path: cp.clone(), - }); + return Some(Change::File { kind: InputChangeKind::Added, path: cp.clone() }); } (Some((sp, sh)), Some((cp, ch))) => match sp.cmp(cp) { std::cmp::Ordering::Equal => { if sh != ch { - return Some(FingerprintMismatch::InputChanged { + return Some(Change::File { kind: InputChangeKind::ContentModified, path: sp.clone(), }); @@ -98,18 +200,106 @@ fn detect_globbed_input_change( c = current_iter.next(); } std::cmp::Ordering::Less => { - return Some(FingerprintMismatch::InputChanged { - kind: InputChangeKind::Removed, - path: sp.clone(), - }); + return Some(Change::File { kind: InputChangeKind::Removed, path: sp.clone() }); } std::cmp::Ordering::Greater => { - return Some(FingerprintMismatch::InputChanged { - kind: InputChangeKind::Added, - path: cp.clone(), - }); + return Some(Change::File { kind: InputChangeKind::Added, path: cp.clone() }); } }, } } } + +#[cfg(test)] +mod tests { + use std::{ffi::OsString, time::Duration}; + + use vt_graph::config::ResolvedGlobConfig; + use vt_str::Str; + + use super::*; + use crate::session::{ + cache::remote::tests::cache_key, execute::fingerprint::PostRunFingerprint, + }; + + #[cfg(unix)] + fn non_utf8_os_string() -> OsString { + use std::os::unix::ffi::OsStringExt; + + OsString::from_vec(vec![0xFF]) + } + + #[cfg(windows)] + fn non_utf8_os_string() -> OsString { + use std::os::windows::ffi::OsStringExt; + + OsString::from_wide(&[0xD800]) + } + + /// A tracked env that can't be compared fails the lookup only when the + /// entry would otherwise be valid. After a changed file, it's a miss. + #[test] + fn env_that_cannot_be_compared_fails_only_a_valid_entry() { + let path = RelativePathBuf::new("a.txt").unwrap(); + let value = CacheEntryValue { + post_run_fingerprint: PostRunFingerprint { + tracked_envs: BTreeMap::from([(Str::from("PROBE_ENV"), None)]), + ..PostRunFingerprint::default() + }, + std_outputs: Arc::new([]), + duration: Duration::ZERO, + globbed_inputs: BTreeMap::from([(path.clone(), 1)]), + output_archive: None, + }; + let unfiltered_envs = FxHashMap::from_iter([( + EnvName::new(Arc::::from(OsStr::new("PROBE_ENV"))), + Arc::::from(non_utf8_os_string()), + )]); + let workspace_root = vt_path::current_dir().unwrap(); + let key = cache_key(ResolvedGlobConfig::default_auto()); + let validate = + |inputs| value.validate(&key, &key, &inputs, &unfiltered_envs, &workspace_root); + + assert!(validate(BTreeMap::from([(path.clone(), 1)])).is_err()); + let changes = validate(BTreeMap::from([(path.clone(), 2)])).unwrap(); + assert_eq!(changes, [Change::File { kind: InputChangeKind::ContentModified, path }]); + } + + /// A tracked env that can't be compared after another one changed keeps + /// that change: it's a miss, not a failed lookup. + #[test] + fn env_that_cannot_be_compared_keeps_another_envs_change() { + let value = CacheEntryValue { + post_run_fingerprint: PostRunFingerprint { + tracked_envs: BTreeMap::from([ + (Str::from("CHANGED_ENV"), None), + (Str::from("PROBE_ENV"), None), + ]), + ..PostRunFingerprint::default() + }, + std_outputs: Arc::new([]), + duration: Duration::ZERO, + globbed_inputs: BTreeMap::new(), + output_archive: None, + }; + let env = |name: &str, value: OsString| { + (EnvName::new(Arc::::from(OsStr::new(name))), Arc::::from(value)) + }; + let unfiltered_envs = FxHashMap::from_iter([ + env("CHANGED_ENV", OsString::from("set")), + env("PROBE_ENV", non_utf8_os_string()), + ]); + let workspace_root = vt_path::current_dir().unwrap(); + let key = cache_key(ResolvedGlobConfig::default_auto()); + let changes = value + .validate(&key, &key, &BTreeMap::new(), &unfiltered_envs, &workspace_root) + .unwrap(); + assert_eq!( + changes, + [Change::Env { + mismatch: EnvMismatch::Added { name: Str::from("CHANGED_ENV") }, + source: EnvSource::Tracked, + }] + ); + } +} diff --git a/crates/vt/src/session/event.rs b/crates/vt/src/session/event.rs index 01273a852..63fe6b9f1 100644 --- a/crates/vt/src/session/event.rs +++ b/crates/vt/src/session/event.rs @@ -150,10 +150,6 @@ pub enum RunKind { } #[derive(Debug, Clone)] -#[expect( - clippy::large_enum_variant, - reason = "CacheMiss variant is intentionally large and infrequently cloned" -)] pub enum CacheStatus { Disabled(CacheDisabledReason), Miss(CacheMiss), diff --git a/crates/vt/src/session/execute/fingerprint.rs b/crates/vt/src/session/execute/fingerprint.rs index 35c0cdb6f..34c5ae6ff 100644 --- a/crates/vt/src/session/execute/fingerprint.rs +++ b/crates/vt/src/session/execute/fingerprint.rs @@ -24,7 +24,7 @@ use wincode::{SchemaRead, SchemaWrite}; use super::rebuildable::Rebuildable; use crate::{ collections::HashMap, - session::cache::{EnvMismatch, InputChangeKind}, + session::cache::{Change, EnvMismatch, EnvSource, InputChangeKind}, }; #[derive( @@ -69,18 +69,6 @@ pub struct PostRunFingerprint { pub tracked_env_queries: BTreeMap>, } -/// A mismatch between the stored post-run fingerprint and the current state. -#[derive(Debug, Clone)] -pub enum PostRunMismatch { - /// An inferred input file or directory changed. - Input { kind: InputChangeKind, path: RelativePathBuf }, - /// A tool-tracked env var changed value, appeared, or disappeared. - TrackedEnv(EnvMismatch), - /// A tool-tracked bulk env query's match-set changed between runs. Carries - /// the first differing entry in env-name order. - TrackedEnvQuery { query: TrackedEnvQuery, mismatch: EnvMismatch }, -} - /// A path the task read, as the task left it. #[derive(SchemaWrite, SchemaRead, PartialEq, Eq, Debug, Clone, Serialize, Deserialize)] pub struct InferredInput { @@ -202,25 +190,18 @@ impl PostRunFingerprint { Ok(Self { inferred_inputs, tracked_envs, tracked_env_queries }) } - /// Validates the fingerprint against current filesystem state and the - /// unfiltered env context used by runner-aware IPC. `unfiltered_envs` must - /// be the same plan env context that served the original `getEnv` request, - /// not the filtered env passed to the spawned process. - /// `input_negative_globs` are the task's input negative globs, part of the - /// cache key and so the same as when the fingerprint was created. - /// - /// Returns `Some(mismatch)` if anything changed, `None` if all valid. - /// Returns an error if a tracked env is currently present but cannot be - /// represented as UTF-8; treating that value as unset would make cache - /// validation unsound. - #[tracing::instrument(level = "debug", skip_all, name = "validate_post_run_fingerprint")] - pub fn validate( + /// The first inferred input that differs from the filesystem now, if any. + /// The inputs are checked in parallel, so when several changed, which one + /// is found isn't defined. `input_negative_globs` are the task's input + /// negative globs, part of the cache key and so the same as when the + /// fingerprint was created. + #[tracing::instrument(level = "debug", skip_all)] + pub fn first_input_change( &self, base_dir: &AbsolutePath, input_negative_globs: &PathGlobSet<'_>, - unfiltered_envs: &FxHashMap>, Arc>, - ) -> anyhow::Result> { - let input_mismatch = + ) -> anyhow::Result> { + let input_change = self.inferred_inputs.par_iter().find_map_any(|(input_relative_path, input)| { let InferredInput { fingerprint: path_fingerprint, may_be_missing } = input; let input_full_path = Arc::::from(base_dir.join(input_relative_path)); @@ -260,13 +241,27 @@ impl PostRunFingerprint { } else { input_relative_path.clone() }; - Some(Ok(PostRunMismatch::Input { kind, path })) + Some(Ok(Change::File { kind, path })) } }); - if let Some(result) = input_mismatch { - return result.map(Some); - } + input_change.transpose() + } + /// Every tracked env var and tracked query match that differs from the + /// unfiltered env context used by runner-aware IPC, one result per change, + /// or per env or query that can't be compared, so one that can't doesn't + /// hide the others' changes. `unfiltered_envs` must be the same plan env + /// context that served the original `getEnv` request, not the filtered + /// env passed to the spawned process. + /// + /// A tracked env that's currently present but can't be represented as + /// UTF-8 is an error; treating that value as unset would make cache + /// validation unsound. + pub fn env_changes( + &self, + unfiltered_envs: &FxHashMap>, Arc>, + ) -> Vec> { + let mut checked = Vec::new(); for (name, stored_value) in &self.tracked_envs { let current_value = unfiltered_envs .get(EnvName::from_ref(OsStr::new(name.as_str()))) @@ -276,43 +271,40 @@ impl PostRunFingerprint { })?; Ok::<_, anyhow::Error>(EnvValueHash::new(value_str)) }) - .transpose()?; - if let Some(mismatch) = - EnvMismatch::compare(name, stored_value.as_ref(), current_value.as_ref()) - { - return Ok(Some(PostRunMismatch::TrackedEnv(mismatch))); + .transpose(); + match current_value { + Ok(current_value) => checked.extend( + EnvMismatch::compare(name, stored_value.as_ref(), current_value.as_ref()) + .map(Ok), + ), + Err(err) => checked.push(Err(err)), } } for (query, stored_matches) in &self.tracked_env_queries { - let current_matches = match match_env_query(query, unfiltered_envs)? { - EnvQueryValidation::Matches(matches) => matches, - EnvQueryValidation::NonUtf8Value(mismatch) => { - return Ok(Some(PostRunMismatch::TrackedEnvQuery { - query: query.clone(), - mismatch, - })); - } - }; - if let Some(mismatch) = first_env_glob_mismatch(stored_matches, ¤t_matches) { - return Ok(Some(PostRunMismatch::TrackedEnvQuery { - query: query.clone(), - mismatch, - })); + match match_env_query(query, unfiltered_envs) { + Ok(current_matches) => checked.extend( + env_query_mismatches(stored_matches, ¤t_matches).into_iter().map(Ok), + ), + Err(err) => checked.push(Err(err)), } } - Ok(None) + checked + .into_iter() + .map(|mismatch| { + mismatch.map(|mismatch| Change::Env { mismatch, source: EnvSource::Tracked }) + }) + .collect() } } /// Build the current match-set for `query` by enumerating the given env -/// snapshot and keeping matching UTF-8 names. If a matching env has a non-UTF-8 -/// value, return a changed mismatch so the stale cache entry is not replayed. +/// snapshot and keeping matching UTF-8 names. fn match_env_query( query: &TrackedEnvQuery, envs: &FxHashMap>, Arc>, -) -> anyhow::Result { +) -> anyhow::Result { Ok(match query { TrackedEnvQuery::Glob(pattern) => { let glob = vt_glob::env::EnvGlob::new(pattern.as_str())?; @@ -327,8 +319,8 @@ fn match_env_query( fn collect_matching_envs( envs: &FxHashMap>, Arc>, is_match: impl Fn(&str) -> bool, -) -> EnvQueryValidation { - let mut matches = BTreeMap::new(); +) -> EnvQueryMatches { + let mut matches = EnvQueryMatches::default(); for (name, value) in envs { let Some(name_str) = name.inner().to_str() else { continue; @@ -336,52 +328,43 @@ fn collect_matching_envs( if !is_match(name_str) { continue; } - let Some(value_str) = value.to_str() else { - return EnvQueryValidation::NonUtf8Value(EnvMismatch::Changed { - name: Str::from(name_str), - }); - }; - matches.insert(Str::from(name_str), EnvValueHash::new(value_str)); + if let Some(value_str) = value.to_str() { + matches.values.insert(Str::from(name_str), EnvValueHash::new(value_str)); + } else { + matches.non_utf8_values.insert(Str::from(name_str)); + } } - EnvQueryValidation::Matches(matches) + matches } -enum EnvQueryValidation { - Matches(BTreeMap), - NonUtf8Value(EnvMismatch), +/// The envs a tracked query matches now. +#[derive(Default)] +struct EnvQueryMatches { + /// The matched envs with UTF-8 values. + values: BTreeMap, + /// The matched envs whose values aren't UTF-8. The JS client errors when + /// it queries such a value, so each one is a change, which keeps a stale + /// cache entry from being replayed. + non_utf8_values: BTreeSet, } -/// Find the first deterministic difference between stored and current env -/// glob match-sets. -fn first_env_glob_mismatch( +/// Every difference between the stored match-set of a tracked env query and +/// the current one, in env-name order, after the envs whose values aren't +/// UTF-8. +fn env_query_mismatches( stored: &BTreeMap, - current: &BTreeMap, -) -> Option { - let mut stored_iter = stored.iter(); - let mut current_iter = current.iter(); - let mut s = stored_iter.next(); - let mut c = current_iter.next(); - - loop { - match (s, c) { - (None, None) => return None, - (Some((name, _)), None) => return Some(EnvMismatch::Removed { name: name.clone() }), - (None, Some((name, _))) => return Some(EnvMismatch::Added { name: name.clone() }), - (Some((sn, sv)), Some((cn, cv))) => match sn.cmp(cn) { - std::cmp::Ordering::Equal => { - if sv != cv { - return Some(EnvMismatch::Changed { name: sn.clone() }); - } - s = stored_iter.next(); - c = current_iter.next(); - } - std::cmp::Ordering::Less => return Some(EnvMismatch::Removed { name: sn.clone() }), - std::cmp::Ordering::Greater => { - return Some(EnvMismatch::Added { name: cn.clone() }); - } - }, - } - } + current: &EnvQueryMatches, +) -> Vec { + let mut mismatches: Vec = current + .non_utf8_values + .iter() + .map(|name| EnvMismatch::Changed { name: name.clone() }) + .collect(); + mismatches.extend( + EnvMismatch::diff(stored, ¤t.values) + .filter(|mismatch| !current.non_utf8_values.contains(mismatch.name())), + ); + mismatches } /// Determine the kind of change between two differing path fingerprints. @@ -643,6 +626,13 @@ mod tests { use super::*; + /// The tracked envs' changes, or the first env that can't be compared. + fn env_changes( + fingerprint: &PostRunFingerprint, + unfiltered_envs: &FxHashMap>, Arc>, + ) -> anyhow::Result> { + fingerprint.env_changes(unfiltered_envs).into_iter().collect() + } #[cfg(unix)] fn non_utf8_os_string() -> OsString { use std::os::unix::ffi::OsStringExt; @@ -669,9 +659,7 @@ mod tests { Arc::::from(non_utf8_os_string()), ); - let workspace_root = vt_path::current_dir().expect("cwd"); - let err = fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &unfiltered_envs) + let err = env_changes(&fingerprint, &unfiltered_envs) .expect_err("non-UTF-8 tracked env values must error"); assert!(err.to_string().contains("tracked env value for PROBE_ENV is not valid UTF-8")); @@ -690,19 +678,12 @@ mod tests { Arc::::from(non_utf8_os_string()), ); - let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &unfiltered_envs) - .expect("validation succeeds"); - - match mismatch { - Some(PostRunMismatch::TrackedEnvQuery { - query, - mismatch: EnvMismatch::Changed { name }, - }) => { - assert_eq!(query, TrackedEnvQuery::Glob(Str::from("PROBE_*"))); - assert_eq!(name.as_str(), "PROBE_BAD"); - } + let changes = env_changes(&fingerprint, &unfiltered_envs).expect("validation succeeds"); + + match changes.as_slice() { + [ + Change::Env { mismatch: EnvMismatch::Changed { name }, source: EnvSource::Tracked }, + ] => assert_eq!(name.as_str(), "PROBE_BAD"), other => panic!("expected changed tracked env query mismatch, got {other:?}"), } } @@ -726,12 +707,9 @@ mod tests { Arc::::from(OsStr::new("wildcard if interpreted as glob")), ); - let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &unfiltered_envs) - .expect("validation succeeds"); + let changes = env_changes(&fingerprint, &unfiltered_envs).expect("validation succeeds"); - assert!(mismatch.is_none()); + assert!(changes.is_empty()); } #[test] @@ -747,12 +725,9 @@ mod tests { Arc::::from(OsStr::new("value")), ); - let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &unfiltered_envs) - .expect("validation succeeds"); + let changes = env_changes(&fingerprint, &unfiltered_envs).expect("validation succeeds"); - assert!(mismatch.is_none()); + assert!(changes.is_empty()); } #[test] @@ -767,12 +742,9 @@ mod tests { Arc::::from(OsStr::new("value")), ); - let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &unfiltered_envs) - .expect("validation succeeds"); + let changes = env_changes(&fingerprint, &unfiltered_envs).expect("validation succeeds"); - assert_eq!(mismatch.is_none(), cfg!(windows)); + assert_eq!(changes.is_empty(), cfg!(windows)); } #[test] @@ -790,12 +762,9 @@ mod tests { Arc::::from(OsStr::new("value")), ); - let workspace_root = vt_path::current_dir().expect("cwd"); - let mismatch = fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &unfiltered_envs) - .expect("validation succeeds"); + let changes = env_changes(&fingerprint, &unfiltered_envs).expect("validation succeeds"); - assert_eq!(mismatch.is_none(), cfg!(windows)); + assert_eq!(changes.is_empty(), cfg!(windows)); } #[test] @@ -828,22 +797,20 @@ mod tests { let written = create(FxHashSet::from_iter([path])); let read_only = create(FxHashSet::default()); let validate = |fingerprint: &PostRunFingerprint| { - fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &FxHashMap::default()) - .unwrap() + fingerprint.first_input_change(&workspace_root, &PathGlobSet::default()).unwrap() }; std::fs::remove_file(full_path.as_path()).unwrap(); assert!(validate(&written).is_none()); assert!(matches!( validate(&read_only), - Some(PostRunMismatch::Input { kind: InputChangeKind::Removed, .. }) + Some(Change::File { kind: InputChangeKind::Removed, .. }) )); std::fs::write(full_path.as_path(), "garbage").unwrap(); assert!(matches!( validate(&written), - Some(PostRunMismatch::Input { kind: InputChangeKind::ContentModified, .. }) + Some(Change::File { kind: InputChangeKind::ContentModified, .. }) )); } @@ -1133,11 +1100,8 @@ mod tests { })) ); - let validate = || { - fingerprint - .validate(&workspace_root, &PathGlobSet::default(), &FxHashMap::default()) - .unwrap() - }; + let validate = + || fingerprint.first_input_change(&workspace_root, &PathGlobSet::default()).unwrap(); assert!(validate().is_none()); std::fs::remove_dir_all(workspace_root.join("pkg/gen").as_path()).unwrap(); std::fs::remove_file(workspace_root.join("pkg/out.txt").as_path()).unwrap(); @@ -1145,7 +1109,7 @@ mod tests { std::fs::write(workspace_root.join("pkg/notes.txt").as_path(), "x").unwrap(); match validate() { - Some(PostRunMismatch::Input { kind: InputChangeKind::Added, path }) => { + Some(Change::File { kind: InputChangeKind::Added, path }) => { assert_eq!(path.as_str(), "pkg/notes.txt"); } other => panic!("expected `pkg/notes.txt` added, got {other:?}"), @@ -1191,11 +1155,8 @@ mod tests { })) ); - let validate = || { - fingerprint - .validate(&workspace_root, &input_negative_globs, &FxHashMap::default()) - .unwrap() - }; + let validate = + || fingerprint.first_input_change(&workspace_root, &input_negative_globs).unwrap(); std::fs::create_dir(workspace_root.join("pkg/out").as_path()).unwrap(); std::fs::create_dir(workspace_root.join("pkg/.cache").as_path()).unwrap(); std::fs::remove_file(workspace_root.join("pkg/tool.log").as_path()).unwrap(); @@ -1203,7 +1164,7 @@ mod tests { std::fs::write(workspace_root.join("pkg/notes.txt").as_path(), "x").unwrap(); match validate() { - Some(PostRunMismatch::Input { kind: InputChangeKind::Added, path }) => { + Some(Change::File { kind: InputChangeKind::Added, path }) => { assert_eq!(path.as_str(), "pkg/notes.txt"); } other => panic!("expected `pkg/notes.txt` added, got {other:?}"), diff --git a/crates/vt/src/session/execute/mod.rs b/crates/vt/src/session/execute/mod.rs index 2ce04b862..22e004d4b 100644 --- a/crates/vt/src/session/execute/mod.rs +++ b/crates/vt/src/session/execute/mod.rs @@ -568,7 +568,8 @@ enum CacheLookup<'a> { /// Cache hit — the cached entry to replay, where it came from, and the /// metadata that looked it up. Hit { hit: CacheHit, metadata: &'a CacheMetadata }, - /// Cache miss — the detailed reason (`NotFound` or `FingerprintMismatch`). + /// Cache miss — the detailed reason (`NotFound`, `Changed`, or a failed + /// remote read). Miss { miss: CacheMiss, globbed_inputs: BTreeMap }, /// Caching is disabled for this task (no cache metadata). Disabled, diff --git a/crates/vt/src/session/mod.rs b/crates/vt/src/session/mod.rs index b0ade2985..85fe6be7f 100644 --- a/crates/vt/src/session/mod.rs +++ b/crates/vt/src/session/mod.rs @@ -15,7 +15,7 @@ use std::{ use anyhow::Context as _; use cache::ExecutionCache; -pub use cache::{CacheMiss, FingerprintMismatch}; +pub use cache::{CacheMiss, Change, Comparison}; use clap::Parser as _; use event::RunKind; use once_cell::sync::OnceCell; diff --git a/crates/vt/src/session/reporter/summary.rs b/crates/vt/src/session/reporter/summary.rs index 8e5608216..38f5c05d1 100644 --- a/crates/vt/src/session/reporter/summary.rs +++ b/crates/vt/src/session/reporter/summary.rs @@ -21,13 +21,8 @@ use vt_str::Str; use super::{CACHE_MISS_STYLE, COMMAND_STYLE, ColorizeExt}; use crate::session::{ - cache::{ - CacheHitSource, CacheMiss, EnvMismatch, FingerprintMismatch, InputChangeKind, - SpawnFingerprintChange, detect_spawn_fingerprint_changes, format_input_change_str, - format_spawn_change, - }, + cache::{CacheHitSource, CacheMiss, Change, format_change}, event::{CacheDisabledReason, CacheNotUpdatedReason, CacheStatus, CacheUpdateStatus, RunKind}, - execute::fingerprint::TrackedEnvQuery, }; // ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ @@ -146,17 +141,11 @@ pub enum SpawnOutcome { pub enum SavedCacheMissReason { /// No previous cache entry for this task. NotFound, - /// Spawn fingerprint changed (command, envs, cwd, etc.). - SpawnFingerprintChanged(Vec), - /// Task configuration changed (`input_config` or `glob_base`). - ConfigChanged, - /// An input file or folder changed. - InputChanged { kind: InputChangeKind, path: Str }, - /// A runner-aware tool reported a tracked env var that changed between runs. - TrackedEnvChanged(EnvMismatch), - /// A runner-aware tool reported a tracked bulk env query whose match-set changed - /// between runs. Carries the first differing entry. - TrackedEnvQueryChanged { query: TrackedEnvQuery, mismatch: EnvMismatch }, + /// The task's cached run differs. + Changed { + /// Each change, in the order they're listed. + changes: Vec, + }, /// Reading the remote cache failed, and the local cache had no entry. RemoteReadFailed(SavedError), } @@ -269,26 +258,7 @@ impl SavedCacheMissReason { fn from_cache_miss(cache_miss: &CacheMiss) -> Self { match cache_miss { CacheMiss::NotFound => Self::NotFound, - CacheMiss::FingerprintMismatch(mismatch) => match mismatch { - FingerprintMismatch::SpawnFingerprint { old, new } => { - Self::SpawnFingerprintChanged(detect_spawn_fingerprint_changes(old, new)) - } - FingerprintMismatch::InputConfig | FingerprintMismatch::OutputConfig => { - Self::ConfigChanged - } - FingerprintMismatch::InputChanged { kind, path } => { - Self::InputChanged { kind: *kind, path: Str::from(path.as_str()) } - } - FingerprintMismatch::TrackedEnvChanged(mismatch) => { - Self::TrackedEnvChanged(mismatch.clone()) - } - FingerprintMismatch::TrackedEnvQueryChanged { query, mismatch } => { - Self::TrackedEnvQueryChanged { - query: query.clone(), - mismatch: mismatch.clone(), - } - } - }, + CacheMiss::Changed(comparison) => Self::Changed { changes: comparison.changes.clone() }, CacheMiss::RemoteReadFailed(error) => { Self::RemoteReadFailed(SavedError::new(error.as_ref())) } @@ -524,7 +494,8 @@ impl TaskResult { } /// Format the cache status detail line for the full summary, with the - /// causes to show below it. Only a remote read failure has causes. The + /// lines below it: the other changes of a miss, each aligned under the + /// first, and the causes. Only a remote read failure has causes. The /// caller shows [`Self::ipc_server_error`] instead, if there is one. /// /// Examples: @@ -534,14 +505,14 @@ impl TaskResult { /// - "→ Cache hit, but the outputs couldn't be restored" /// - "→ Cache miss: no previous cache entry found" /// - "→ Cache disabled in task configuration" - fn format_cache_detail(&self, run_kind: RunKind) -> (Str, &[Str]) { + fn format_cache_detail(&self, run_kind: RunKind) -> (Str, Vec, &[Str]) { // Tool-reported cache disable — the tool said it shouldn't be cached. if let Self::Spawned { outcome: SpawnOutcome::Success { tool_disabled_cache: true, .. }, .. } = self { - return (Str::from("→ Not cached: the task opted out of caching"), &[]); + return (Str::from("→ Not cached: the task opted out of caching"), Vec::new(), &[]); } // Tracking came up short, so the inferred inputs and outputs would @@ -555,6 +526,7 @@ impl TaskResult { Str::from( "→ Not cached: this task used more files than automatic tracking can record. Configure `input` and `output` manually to enable caching.", ), + Vec::new(), &[], ); } @@ -567,6 +539,7 @@ impl TaskResult { Str::from( "→ Not cached: `input` auto-inference isn't supported on this OS. Configure `input` manually to enable caching.", ), + Vec::new(), &[], ); } @@ -593,34 +566,23 @@ impl TaskResult { SavedCacheMissReason::NotFound => { Str::from("→ Cache miss: no previous cache entry found") } - SavedCacheMissReason::SpawnFingerprintChanged(changes) => { - let formatted: Vec = changes.iter().map(format_spawn_change).collect(); - if formatted.is_empty() { - Str::from("→ Cache miss: configuration changed") - } else { - let joined = - formatted.iter().map(Str::as_str).collect::>().join("; "); - vt_str::format!("→ Cache miss: {joined}") - } - } - SavedCacheMissReason::ConfigChanged => { - Str::from("→ Cache miss: input configuration changed") - } - SavedCacheMissReason::InputChanged { kind, path } => { - let desc = format_input_change_str(*kind, path.as_str()); - vt_str::format!("→ Cache miss: {desc}") - } - SavedCacheMissReason::TrackedEnvChanged(mismatch) - | SavedCacheMissReason::TrackedEnvQueryChanged { mismatch, .. } => { - vt_str::format!("→ Cache miss: {mismatch}") + SavedCacheMissReason::Changed { changes } => { + let mut lines = changes.iter().map(format_change); + let first = + lines.next().unwrap_or_else(|| Str::from("configuration changed")); + return (vt_str::format!("→ Cache miss: {first}"), lines.collect(), &[]); } SavedCacheMissReason::RemoteReadFailed(error) => { - return (vt_str::format!("→ Cache miss: {}", error.message), &error.causes); + return ( + vt_str::format!("→ Cache miss: {}", error.message), + Vec::new(), + &error.causes, + ); } }, }, }; - (detail, &[]) + (detail, Vec::new(), &[]) } /// The [`Style`] for the cache detail line. @@ -846,8 +808,12 @@ pub fn format_full_summary(summary: &LastRunSummary, run_kind: RunKind) -> Vec = summary.as_str().lines().collect(); + assert!( + lines.windows(2).any(|pair| pair + == [ + " → Cache miss: input configuration changed", + " output configuration changed", + ]), + "{summary}" + ); + } + /// A dry run whose tasks all have caching disabled still gets its /// summary line. The other dry-run summaries are in the `dry_run` e2e /// snapshots. diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots.toml index cab8e88d8..606177290 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots.toml +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots.toml @@ -329,3 +329,95 @@ steps = [ "glob-test", ], comment = "cache miss: input removed" }, ] + +[[e2e]] +name = "input_and_output_config_changed" +comment = """ +When a task's `input` and `output` settings change together, the miss lists both. +""" +steps = [ + [ + "vt", + "run", + "test", + ], + [ + "vtt", + "replace-file-content", + "vite-task.json", + "\"env\": [\"MY_ENV\"]", + "\"env\": [\"MY_ENV\"], \"input\": [\"test.txt\"], \"output\": []", + ], + { argv = [ + "vt", + "run", + "-v", + "test", + ], comment = "The inline reason is the first change. The summary lists both." }, +] + +[[e2e]] +name = "env_and_file_changed" +comment = """ +When an env in `cache.env` changes, the miss also compares the task's files with its previous cached run, so it names what reverting needs. Dry runs record nothing, so each step compares with the same cached run. +""" +steps = [ + { argv = [ + "vt", + "run", + "test", + ], envs = [ + [ + "MY_ENV", + "1", + ], + ] }, + [ + "vtt", + "replace-file-content", + "test.txt", + "initial", + "changed", + ], + { argv = [ + "vt", + "run", + "--dry-run", + "-v", + "test", + ], envs = [ + [ + "MY_ENV", + "2", + ], + ], comment = "The env and the file both changed." }, + { argv = [ + "vt", + "run", + "--dry-run", + "test", + ], envs = [ + [ + "MY_ENV", + "1", + ], + ], comment = "Reverting the env isn't enough." }, + [ + "vtt", + "replace-file-content", + "test.txt", + "changed", + "initial", + ], + { argv = [ + "vt", + "run", + "--dry-run", + "test", + ], envs = [ + [ + "MY_ENV", + "1", + ], + ], comment = "Reverting the file too is a hit." }, +] diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots/env_and_file_changed.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots/env_and_file_changed.md new file mode 100644 index 000000000..d1d99a7c4 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots/env_and_file_changed.md @@ -0,0 +1,61 @@ +# env_and_file_changed + +When an env in `cache.env` changes, the miss also compares the task's files with its previous cached run, so it names what reverting needs. Dry runs record nothing, so each step compares with the same cached run. + +## `MY_ENV=1 vt run test` + +``` +$ vtt print-file test.txt +initial content +``` + +## `vtt replace-file-content test.txt initial changed` + +``` +``` + +## `MY_ENV=2 vt run --dry-run -v test` + +The env and the file both changed. + +``` +$ vtt print-file test.txt ○ cache miss: env 'MY_ENV' changed + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + Vite+ Task Runner • Execution Summary +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +Statistics: 1 task • 0 cache hits • 1 cache miss +Performance: 0% cache hit rate + +Task Details: +──────────────────────────────────────────────── + [1] cache-miss-reasons#test: $ vtt print-file test.txt + → Cache miss: env 'MY_ENV' changed + 'test.txt' modified +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +``` + +## `MY_ENV=1 vt run --dry-run test` + +Reverting the env isn't enough. + +``` +$ vtt print-file test.txt ○ cache miss: 'test.txt' modified +``` + +## `vtt replace-file-content test.txt changed initial` + +``` +``` + +## `MY_ENV=1 vt run --dry-run test` + +Reverting the file too is a hit. + +``` +$ vtt print-file test.txt ◉ cache hit + +--- +vt run: cache hit, nothing was run. +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots/input_and_output_config_changed.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots/input_and_output_config_changed.md new file mode 100644 index 000000000..4ea6f0b5e --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/cache_miss_reasons/snapshots/input_and_output_config_changed.md @@ -0,0 +1,39 @@ +# input_and_output_config_changed + +When a task's `input` and `output` settings change together, the miss lists both. + +## `vt run test` + +``` +$ vtt print-file test.txt +initial content +``` + +## `vtt replace-file-content vite-task.json '"env": ["MY_ENV"]' '"env": ["MY_ENV"], "input": ["test.txt"], "output": []'` + +``` +``` + +## `vt run -v test` + +The inline reason is the first change. The summary lists both. + +``` +$ vtt print-file test.txt ○ cache miss: input configuration changed, executing +initial content + + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + Vite+ Task Runner • Execution Summary +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +Statistics: 1 task • 0 cache hits • 1 cache miss +Performance: 0% cache hit rate + +Task Details: +──────────────────────────────────────────────── + [1] cache-miss-reasons#test: $ vtt print-file test.txt ✓ + → Cache miss: input configuration changed + output configuration changed +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/envs_input.txt b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/envs_input.txt new file mode 100644 index 000000000..9c59e24b8 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/envs_input.txt @@ -0,0 +1 @@ +first diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_env_and_envs.mjs b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_env_and_envs.mjs new file mode 100644 index 000000000..56a8b0257 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/scripts/fetch_env_and_envs.mjs @@ -0,0 +1,9 @@ +import { readFileSync } from 'node:fs'; +import { getEnv, getEnvs } from '@voidzero-dev/vite-task-client'; + +console.log(`TRACKED_ENV=${getEnv('TRACKED_ENV', { tracked: true }) ?? '(unset)'}`); +const matches = getEnvs('PROBE_*', { tracked: true }); +for (const [key, value] of Object.entries(matches).sort(([a], [b]) => a.localeCompare(b))) { + console.log(`${key}=${value}`); +} +console.log(readFileSync('envs_input.txt', 'utf8').trim()); diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml index 422f46aea..b61424bb7 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots.toml @@ -670,3 +670,51 @@ steps = [ ], ], comment = "cache hit: changed match set was requested with tracked: false" }, ] + +[[e2e]] +name = "every_tracked_env_change_is_listed" +comment = """ +A miss lists every tracked env and query match that changed, not just the first, along with the first changed file. The inline reason names the env changes together. +""" +ignore = true +steps = [ + { argv = [ + "vt", + "run", + "fetch-env-and-envs", + ], envs = [ + [ + "TRACKED_ENV", + "first", + ], + [ + "PROBE_A", + "a", + ], + ] }, + [ + "vtt", + "write-file", + "envs_input.txt", + "second", + ], + { argv = [ + "vt", + "run", + "-v", + "fetch-env-and-envs", + ], envs = [ + [ + "TRACKED_ENV", + "second", + ], + [ + "PROBE_A", + "a", + ], + [ + "PROBE_B", + "b", + ], + ], comment = "TRACKED_ENV changed, PROBE_B was added, and the input file was modified." }, +] diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/every_tracked_env_change_is_listed.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/every_tracked_env_change_is_listed.md new file mode 100644 index 000000000..62ef015a8 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/snapshots/every_tracked_env_change_is_listed.md @@ -0,0 +1,45 @@ +# every_tracked_env_change_is_listed + +A miss lists every tracked env and query match that changed, not just the first, along with the first changed file. The inline reason names the env changes together. + +## `TRACKED_ENV=first PROBE_A=a vt run fetch-env-and-envs` + +``` +$ node scripts/fetch_env_and_envs.mjs +TRACKED_ENV=first +PROBE_A=a +first +``` + +## `vtt write-file envs_input.txt second` + +``` +``` + +## `TRACKED_ENV=second PROBE_A=a PROBE_B=b vt run -v fetch-env-and-envs` + +TRACKED_ENV changed, PROBE_B was added, and the input file was modified. + +``` +$ node scripts/fetch_env_and_envs.mjs ○ cache miss: envs 'PROBE_B', 'TRACKED_ENV' changed, executing +TRACKED_ENV=second +PROBE_A=a +PROBE_B=b +second + + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + Vite+ Task Runner • Execution Summary +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +Statistics: 1 task • 0 cache hits • 1 cache miss +Performance: 0% cache hit rate + +Task Details: +──────────────────────────────────────────────── + [1] ipc-client-test#fetch-env-and-envs: $ node scripts/fetch_env_and_envs.mjs ✓ + → Cache miss: env 'PROBE_B' added + env 'TRACKED_ENV' changed + 'envs_input.txt' modified +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +``` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json index 1ff9e059e..aa67086e9 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/ipc_client_test/vite-task.json @@ -81,6 +81,13 @@ "command": "node scripts/fetch_envs.mjs", "cache": true }, + "fetch-env-and-envs": { + "command": "node scripts/fetch_env_and_envs.mjs", + "cache": { + "input": ["envs_input.txt"], + "output": [] + } + }, "fetch-envs-prefix": { "command": "node scripts/fetch_envs.mjs --prefix", "cache": true diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots.toml b/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots.toml index 1d6e5913d..b69e8bceb 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots.toml +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots.toml @@ -290,6 +290,74 @@ steps = [ ], ] +[[e2e]] +name = "fallback_compares_the_cached_run" +cfg = 'not(target_env = "musl")' +ignore = true +steps = [ + [ + "remote-cache-server", + "start", + ], + { argv = [ + "remote-cache-server", + "run", + "--github-actions", + "vt", + "run", + "build", + ], envs = [ + [ + "VP_REMOTE_CACHE", + "read-write", + ], + [ + "VP_RUN_INTERNAL_HIDE_PENDING_UPLOADS", + "1", + ], + ] }, + [ + "vtt", + "replace-file-content", + "vite-task.json", + "built", + "rebuilt", + ], + [ + "vtt", + "write-file", + "src/a.txt", + "changed", + ], + { argv = [ + "remote-cache-server", + "run", + "vt", + "run", + "--dry-run", + "-v", + "build", + ], comment = "The local cache has the previous run, and its miss reason wins, so the remote fallback's entry isn't fetched." }, + [ + "vt", + "cache", + "clean", + ], + { argv = [ + "remote-cache-server", + "run", + "vt", + "run", + "--dry-run", + "-v", + "build", + ], comment = "The fallback is only a key, so its entry is fetched too, and the miss lists its changed file." }, + [ + "remote-cache-server", + "stop", + ], +] + [[e2e]] name = "corrupt_archive" cfg = 'not(target_env = "musl")' diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback.md index c74d23543..4cd9e8ad9 100644 --- a/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback.md +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback.md @@ -32,6 +32,7 @@ The entry stored for this task has a different key. The miss reason compares it $ vtt write-file dist/output.txt rebuilt ○ cache miss: args changed, executing [remote-cache] POST /fetch 200 fallback +[remote-cache] POST /fetch 200 exact [1] ``` ## `remote-cache-server stop` diff --git a/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback_compares_the_cached_run.md b/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback_compares_the_cached_run.md new file mode 100644 index 000000000..6bdeeca13 --- /dev/null +++ b/crates/vt_bin/tests/e2e_snapshots/fixtures/remote_cache/snapshots/fallback_compares_the_cached_run.md @@ -0,0 +1,82 @@ +# fallback_compares_the_cached_run + +## `remote-cache-server start` + +``` +``` + +## `VP_REMOTE_CACHE=read-write VP_RUN_INTERNAL_HIDE_PENDING_UPLOADS=1 remote-cache-server run --github-actions vt run build` + +``` +$ vtt write-file dist/output.txt built + +[remote-cache] POST /fetch 404 +[remote-cache] POST /store 200 +``` + +## `vtt replace-file-content vite-task.json built rebuilt` + +``` +``` + +## `vtt write-file src/a.txt changed` + +``` +``` + +## `remote-cache-server run vt run --dry-run -v build` + +The local cache has the previous run, and its miss reason wins, so the remote fallback's entry isn't fetched. + +``` +$ vtt write-file dist/output.txt rebuilt ○ cache miss: args changed + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + Vite+ Task Runner • Execution Summary +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +Statistics: 1 task • 0 cache hits • 1 cache miss +Performance: 0% cache hit rate + +Task Details: +──────────────────────────────────────────────── + [1] remote-cache#build: $ vtt write-file dist/output.txt rebuilt + → Cache miss: args changed + 'src/a.txt' modified +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +[remote-cache] POST /fetch 200 fallback +``` + +## `vt cache clean` + +``` +``` + +## `remote-cache-server run vt run --dry-run -v build` + +The fallback is only a key, so its entry is fetched too, and the miss lists its changed file. + +``` +$ vtt write-file dist/output.txt rebuilt ○ cache miss: args changed + +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + Vite+ Task Runner • Execution Summary +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + +Statistics: 1 task • 0 cache hits • 1 cache miss +Performance: 0% cache hit rate + +Task Details: +──────────────────────────────────────────────── + [1] remote-cache#build: $ vtt write-file dist/output.txt rebuilt + → Cache miss: args changed + 'src/a.txt' modified +━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ +[remote-cache] POST /fetch 200 fallback +[remote-cache] POST /fetch 200 exact [1] +``` + +## `remote-cache-server stop` + +``` +``` diff --git a/docs/rfcs/dry-run.md b/docs/rfcs/dry-run.md index 2e870dc06..1e1738b9c 100644 --- a/docs/rfcs/dry-run.md +++ b/docs/rfcs/dry-run.md @@ -515,6 +515,7 @@ type RunPlan = { - **Exit codes.** A dry run exits 0 when planning succeeds, whatever it predicts, unless a step's cache can't be checked. When planning fails, the error goes to stderr and the exit code is non-zero. - **Summary.** A dry run's summary follows a real run's rules, with "nothing was run" in place of the successful count and the time saved. A single task gets a summary line only when it would hit: `vp run: cache hit, nothing was run.` There is no `--last-details` hint, since that shows the last real run. The detailed summary has no ✓ or exit codes, and a hit's line is just `→ Cache hit`. - **Cache errors.** When a step's cache can't be checked, for example because the cache database can't be read, the error is printed under that step's command, and the dry run stops and fails as a real run does. A cache folder that can't be opened, for example because it's read-only, fails the dry run as it fails a real run. +- **Remote baseline.** When the remote cache has no run with the step's command, config and env, it returns only the key of the step's most recent cached run, so `vp run` fetches that run too, to compare with. If that fetch fails, the step is an `error`, as when the first fetch fails. - **`--last-details`.** After a run with `-v`, it shows every change. ## Changes to existing behaviour