diff --git a/CHANGELOG.md b/CHANGELOG.md index 2bca0d5da..2a649f3f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** A task no longer fails when file-access tracking cannot start, for example because the tracking library cannot be written to the temporary directory. The task runs untracked and is reported as not cached ([#752](https://github.com/voidzero-dev/vite-task/pull/752)). - **Fixed** A mistyped or misplaced setting in a task now gets an error that names it. For example, `cache: { outputs: [...] }` now fails with an `unknown field` error that names `outputs` and lists the accepted settings, instead of `data did not match any variant of untagged enum UserTaskDefinition` ([vite-plus#2635](https://github.com/voidzero-dev/vite-plus/issues/2635), [#815](https://github.com/voidzero-dev/vite-task/pull/815)). - **Fixed** In cached tasks on Linux and macOS, programs that pass `-1` as a directory descriptor to calls such as `openat` no longer abort. For example, `ps -p ` from procps-ng 4.0.6 or later aborted when the process had already exited ([#862](https://github.com/voidzero-dev/vite-task/pull/862)). - **Fixed** `vp run` no longer panics before running a script that contains non-ASCII characters, such as `echo こんにちは`, and no longer cuts off such commands where it displays them ([#861](https://github.com/voidzero-dev/vite-task/pull/861)). diff --git a/crates/fspy/src/command.rs b/crates/fspy/src/command.rs index 2f7801d2e..d8ebf3c87 100644 --- a/crates/fspy/src/command.rs +++ b/crates/fspy/src/command.rs @@ -1,5 +1,6 @@ use std::{ ffi::{OsStr, OsString}, + io, path::{Path, PathBuf}, process::Stdio, }; @@ -169,13 +170,18 @@ impl Command { /// /// # Errors /// - /// Returns [`SpawnError`] if program resolution fails or the process cannot be spawned. + /// Returns [`SpawnError`] if program resolution fails, the tracking + /// machinery cannot be initialized (e.g. the preload library cannot be + /// materialized), or the process cannot be spawned. pub async fn spawn( mut self, cancellation_token: CancellationToken, ) -> Result { self.resolve_program()?; - SPY_IMPL.spawn(self, cancellation_token).await + match &*SPY_IMPL { + Ok(spy) => spy.spawn(self, cancellation_token).await, + Err(e) => Err(SpawnError::Injection(io::Error::new(e.kind(), e.to_string()))), + } } /// Resolve program name to full path using `PATH` and cwd. diff --git a/crates/fspy/src/lib.rs b/crates/fspy/src/lib.rs index 0bab440a4..4a894dd0a 100644 --- a/crates/fspy/src/lib.rs +++ b/crates/fspy/src/lib.rs @@ -57,8 +57,8 @@ pub struct TrackedChild { pub process_handle: std::os::windows::io::OwnedHandle, } -pub(crate) static SPY_IMPL: LazyLock = LazyLock::new(|| { +pub(crate) static SPY_IMPL: LazyLock> = LazyLock::new(|| { let tmp_dir = temp_dir().join("fspy"); let _ = create_dir(&tmp_dir); - SpyImpl::init_in(&tmp_dir).expect("Failed to initialize global spy") + SpyImpl::init_in(&tmp_dir) }); diff --git a/crates/vt/src/napi_client.rs b/crates/vt/src/napi_client.rs index c74ac8bed..46a0fe26c 100644 --- a/crates/vt/src/napi_client.rs +++ b/crates/vt/src/napi_client.rs @@ -13,9 +13,8 @@ use vt_path::{AbsolutePath, AbsolutePathBuf}; /// /// # Panics /// -/// Panics if the materialization fails on first call — this mirrors fspy's -/// `SPY_IMPL` and the same reasoning applies: if we can't write into the -/// system temp dir, the runner can't run tasks anyway. +/// Panics if the materialization fails on first call: if we can't write +/// into the system temp dir, the runner can't run tasks anyway. #[must_use] pub fn napi_client_path() -> &'static AbsolutePath { static PATH: LazyLock = LazyLock::new(|| { diff --git a/crates/vt/src/session/event.rs b/crates/vt/src/session/event.rs index 3d25842ef..a02e4577e 100644 --- a/crates/vt/src/session/event.rs +++ b/crates/vt/src/session/event.rs @@ -110,6 +110,10 @@ pub enum CacheNotUpdatedReason { /// (its `input` config includes auto-inference). Task ran but cannot /// be cached without tracked path accesses. FspyUnsupported, + /// fspy is compiled in, but the tracked spawn failed (e.g. the preload + /// library could not be materialized), so the task ran untracked and + /// cannot be cached without tracked path accesses. + FspyUnavailable, /// The runner's IPC server failed during execution, so the collected /// reports may be incomplete. Caching such a run would risk stale /// inputs/outputs on the next hit. Carries the underlying error for diff --git a/crates/vt/src/session/execute/cache_update.rs b/crates/vt/src/session/execute/cache_update.rs index 0909839ce..55e24e784 100644 --- a/crates/vt/src/session/execute/cache_update.rs +++ b/crates/vt/src/session/execute/cache_update.rs @@ -125,10 +125,10 @@ pub(super) async fn update_cache( } if fspy_outcome.is_none() && fspy.is_some() { - // Task requested fspy auto-inference but this binary was built without - // `cfg(fspy)`. Task ran, but we can't compute a valid cache entry + // Task requested fspy auto-inference but produced no tracked + // accesses. Task ran, but we can't compute a valid cache entry // without tracked path accesses. - return (CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnsupported), None); + return (CacheUpdateStatus::NotUpdated(fspy_missing_reason(outcome)), None); } // Collect tool-reported tracked envs for the post-run fingerprint. Env @@ -196,6 +196,17 @@ pub(super) async fn update_cache( } } +/// Why a run that requested fspy auto-inference produced no tracked +/// accesses: either this binary was built without `cfg(fspy)`, or the +/// tracked spawn failed and the task ran untracked (see [`super::spawn`]). +const fn fspy_missing_reason(outcome: &ChildOutcome) -> CacheNotUpdatedReason { + if outcome.fspy_unavailable { + CacheNotUpdatedReason::FspyUnavailable + } else { + CacheNotUpdatedReason::FspyUnsupported + } +} + /// Summarize the run's fspy observations. `Some` iff tracking was both /// requested (`tracking.fspy.is_some()`) and compiled in (`cfg(fspy)`). On a /// `cfg(not(fspy))` build this is always `None`, and [`update_cache`] diff --git a/crates/vt/src/session/execute/spawn.rs b/crates/vt/src/session/execute/spawn.rs index 7e2abb59e..7206d80d7 100644 --- a/crates/vt/src/session/execute/spawn.rs +++ b/crates/vt/src/session/execute/spawn.rs @@ -45,6 +45,10 @@ pub struct ChildOutcome { /// `Err` when a tracked process could not record everything it did. #[cfg(fspy)] pub path_accesses: Option>, + /// `true` when fspy tracking was requested but the tracked spawn failed + /// and the child ran untracked (see [`spawn`]). Always `false` on builds + /// without `cfg(fspy)`. + pub fspy_unavailable: bool, } /// Spawn a command with the requested fspy and stdio configuration. @@ -58,6 +62,11 @@ pub struct ChildOutcome { /// /// On builds without `cfg(fspy)`, the `fspy` argument is ignored and the tokio /// path is always taken. +/// +/// When fspy is requested but the tracked spawn itself fails (e.g. the preload +/// library cannot be materialized), the command falls back to an untracked +/// spawn with [`ChildOutcome::fspy_unavailable`] set, so the run is not +/// cached — a task must never fail to run because its tracing did. #[tracing::instrument(level = "debug", skip_all)] pub async fn spawn( cmd: &SpawnCommand, @@ -71,9 +80,29 @@ where K: AsRef, V: AsRef, { + let extra_envs: Vec<(K, V)> = extra_envs.into_iter().collect(); + + #[cfg(fspy)] + let mut fspy_unavailable = false; + #[cfg(not(fspy))] + let fspy_unavailable = false; + #[cfg(fspy)] if fspy { - return spawn_fspy(cmd, stdio, cancellation_token, extra_envs).await; + match spawn_fspy( + cmd, + stdio, + cancellation_token.clone(), + extra_envs.iter().map(|(k, v)| (k, v)), + ) + .await + { + Ok(handle) => return Ok(handle), + Err(err) => { + tracing::warn!("fspy spawn failed, falling back to untracked spawn: {err:#}"); + fspy_unavailable = true; + } + } } #[cfg(not(fspy))] let _ = fspy; @@ -82,12 +111,17 @@ where tokio_cmd.args(cmd.args.iter().map(vt_str::Str::as_str)); tokio_cmd.env_clear(); tokio_cmd.envs(cmd.spawn_envs.iter()); - tokio_cmd.envs(extra_envs); + tokio_cmd.envs(extra_envs.iter().map(|(k, v)| (k, v))); tokio_cmd.current_dir(&*cmd.cwd); apply_stdio(&mut tokio_cmd, stdio); - spawn_tokio(tokio_cmd, cancellation_token) + spawn_tokio(tokio_cmd, cancellation_token, fspy_unavailable) } +/// Spawn through fspy's tracked [`fspy::Command`]. +/// +/// A failure here must not fail the task: [`spawn`] falls back to an +/// untracked spawn and marks the outcome [`ChildOutcome::fspy_unavailable`] +/// so the run is not cached. #[cfg(fspy)] async fn spawn_fspy( cmd: &SpawnCommand, @@ -148,6 +182,7 @@ where Ok(ChildOutcome { exit_status: termination.status, path_accesses: Some(termination.path_accesses), + fspy_unavailable: false, }) } .boxed_local(); @@ -158,6 +193,7 @@ where fn spawn_tokio( mut cmd: tokio::process::Command, cancellation_token: CancellationToken, + fspy_unavailable: bool, ) -> anyhow::Result { let mut child = cmd.spawn()?; @@ -192,6 +228,7 @@ fn spawn_tokio( exit_status, #[cfg(fspy)] path_accesses: None, + fspy_unavailable, }) } .boxed_local(); diff --git a/crates/vt/src/session/reporter/summary.rs b/crates/vt/src/session/reporter/summary.rs index e5d330ad0..72849f394 100644 --- a/crates/vt/src/session/reporter/summary.rs +++ b/crates/vt/src/session/reporter/summary.rs @@ -123,6 +123,12 @@ pub enum SpawnOutcome { /// Task ran successfully but cache was not updated. #[serde(default)] fspy_unsupported: bool, + /// `true` when the task required fspy auto-inference but the tracked + /// spawn failed (e.g. the preload library could not be materialized), + /// so the task ran untracked. Task ran successfully but cache was not + /// updated. + #[serde(default)] + fspy_unavailable: bool, /// The IPC server error that caused the cache to be skipped, if any. ipc_server_error: Option, /// `true` when the task made more file accesses than tracking had @@ -335,29 +341,10 @@ impl TaskResult { package_path: &AbsolutePath, workspace_path: &AbsolutePath, ) -> Self { - let input_modified = match cache_update_status { - CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::InputModified { path }) => { - Some(InputModified::new(path, package_path, workspace_path)) - } - _ => None, - }; - let fspy_unsupported = matches!( - cache_update_status, - CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnsupported) - ); - let ipc_server_error = match cache_update_status { - CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::IpcServerError(err)) => { - Some(SavedError::new(err)) - } - _ => None, - }; - let tool_disabled_cache = matches!( - cache_update_status, - CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::ToolRequested) - ); - let tracking_incomplete = matches!( + let details = SuccessDetails::from_cache_update_status( cache_update_status, - CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::TrackingIncomplete) + package_path, + workspace_path, ); match cache_status { @@ -372,29 +359,13 @@ impl TaskResult { CacheStatus::Disabled(CacheDisabledReason::InProcessExecution) => Self::InProcess, CacheStatus::Disabled(CacheDisabledReason::NoCacheMetadata) => Self::Spawned { cache_status: SpawnedCacheStatus::Disabled, - outcome: spawn_outcome_from_execution( - exit_status, - saved_error, - input_modified, - fspy_unsupported, - ipc_server_error, - tool_disabled_cache, - tracking_incomplete, - ), + outcome: spawn_outcome_from_execution(exit_status, saved_error, details), }, CacheStatus::Miss(cache_miss) => Self::Spawned { cache_status: SpawnedCacheStatus::Miss(SavedCacheMissReason::from_cache_miss( cache_miss, )), - outcome: spawn_outcome_from_execution( - exit_status, - saved_error, - input_modified, - fspy_unsupported, - ipc_server_error, - tool_disabled_cache, - tracking_incomplete, - ), + outcome: spawn_outcome_from_execution(exit_status, saved_error, details), }, } } @@ -410,16 +381,72 @@ impl TaskResult { } } -/// Build a [`SpawnOutcome`] from process exit status and optional pre-converted error. -/// A failed upload is set later, with [`TaskResult::set_upload_error`]. -fn spawn_outcome_from_execution( - exit_status: Option, - saved_error: Option<&SavedError>, +/// The cache-not-updated details a run carries into its [`SpawnOutcome::Success`]. +#[derive(Default)] +#[expect( + clippy::struct_excessive_bools, + reason = "each flag is a distinct cache-not-updated outcome mirroring a CacheNotUpdatedReason variant" +)] +struct SuccessDetails { input_modified: Option, fspy_unsupported: bool, + fspy_unavailable: bool, ipc_server_error: Option, tool_disabled_cache: bool, tracking_incomplete: bool, +} + +impl SuccessDetails { + fn from_cache_update_status( + cache_update_status: &CacheUpdateStatus, + package_path: &AbsolutePath, + workspace_path: &AbsolutePath, + ) -> Self { + let input_modified = match cache_update_status { + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::InputModified { path }) => { + Some(InputModified::new(path, package_path, workspace_path)) + } + _ => None, + }; + let fspy_unsupported = matches!( + cache_update_status, + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnsupported) + ); + let fspy_unavailable = matches!( + cache_update_status, + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::FspyUnavailable) + ); + let ipc_server_error = match cache_update_status { + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::IpcServerError(err)) => { + Some(SavedError::new(err)) + } + _ => None, + }; + let tool_disabled_cache = matches!( + cache_update_status, + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::ToolRequested) + ); + let tracking_incomplete = matches!( + cache_update_status, + CacheUpdateStatus::NotUpdated(CacheNotUpdatedReason::TrackingIncomplete) + ); + Self { + input_modified, + fspy_unsupported, + fspy_unavailable, + ipc_server_error, + tool_disabled_cache, + tracking_incomplete, + } + } +} + +/// Build a [`SpawnOutcome`] from process exit status and optional pre-converted error. +/// A failed upload is set later, with [`TaskResult::set_upload_error`]. +fn spawn_outcome_from_execution( + exit_status: Option, + saved_error: Option<&SavedError>, + details: SuccessDetails, ) -> SpawnOutcome { match (exit_status, saved_error) { // Spawn error — process never ran @@ -427,11 +454,12 @@ fn spawn_outcome_from_execution( // Process exited successfully, possible infra error (Some(status), _) if status.success() => SpawnOutcome::Success { infra_error: saved_error.cloned(), - input_modified, - fspy_unsupported, - ipc_server_error, - tool_disabled_cache, - tracking_incomplete, + input_modified: details.input_modified, + fspy_unsupported: details.fspy_unsupported, + fspy_unavailable: details.fspy_unavailable, + ipc_server_error: details.ipc_server_error, + tool_disabled_cache: details.tool_disabled_cache, + tracking_incomplete: details.tracking_incomplete, upload_error: None, }, // Process exited with non-zero code @@ -451,6 +479,7 @@ fn spawn_outcome_from_execution( infra_error: None, input_modified: None, fspy_unsupported: false, + fspy_unavailable: false, ipc_server_error: None, tool_disabled_cache: false, tracking_incomplete: false, @@ -611,6 +640,18 @@ impl TaskResult { &[], ); } + // The tracked spawn failed, so the task ran untracked. + if let Self::Spawned { + outcome: SpawnOutcome::Success { fspy_unavailable: true, .. }, .. + } = self + { + return ( + Str::from( + "→ Not cached: file access tracking failed to start, so the task ran untracked. Configure `input` manually to enable caching.", + ), + &[], + ); + } let detail = match self { Self::CacheHit { saved_duration_ms, source } => { @@ -1207,6 +1248,7 @@ mod tests { infra_error: None, input_modified: None, fspy_unsupported: false, + fspy_unavailable: false, ipc_server_error: None, tracking_incomplete: false, tool_disabled_cache: false, @@ -1242,6 +1284,7 @@ mod tests { infra_error: None, input_modified: None, fspy_unsupported: false, + fspy_unavailable: false, ipc_server_error: None, tracking_incomplete: false, tool_disabled_cache: false,