Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Changelog

- **Added** `vp run --dry-run --json` prints what a run would do as one JSON document for scripts, CI and coding agents: each task the run would run, the tasks it waits for, and each step's program, arguments and directory, with whether it would hit the cache and, if not, every change found. Like `vp run --json`, it's indented in a terminal and on one line when captured, and its `$schema` field points to a JSON Schema that describes each field. A task that runs `vp run` shows that run's own plan as its step. `--json` with a task requires `--dry-run` ([#966](https://github.com/voidzero-dev/vite-task/pull/966), [#967](https://github.com/voidzero-dev/vite-task/pull/967)).
- **Added** `vp run --dry-run --json` prints what a run would do as one JSON document for scripts, CI and coding agents: each task the run would run, the tasks it waits for, and each step's program, arguments and directory, with whether it would hit the cache and, if not, every change found. Like `vp run --json`, it's indented in a terminal and on one line when captured, and its `$schema` field points to a JSON Schema that describes each field. A task that runs `vp run` shows that run's own plan as its step. A step that waits on one that would write files first, by running or by restoring outputs, is `unknown`, since its files may change before it starts. `--json` with a task requires `--dry-run` ([#966](https://github.com/voidzero-dev/vite-task/pull/966), [#967](https://github.com/voidzero-dev/vite-task/pull/967), [#968](https://github.com/voidzero-dev/vite-task/pull/968)).
- **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. Without `-v`, files are checked only up to the first changed one; with `-v`, every input of a miss is checked, and every changed file is listed in path order. When the command, a setting or a `cache.env` var changed, only those changes are listed, since there's no cached run with the new ones to compare files with. 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), [#963](https://github.com/voidzero-dev/vite-task/pull/963)).
- **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)).
- **Fixed** `vp run` now exits with code 1 when a task's cache can't be checked, for example because the cache database can't be read. It already stopped there, but exited 0 ([#960](https://github.com/voidzero-dev/vite-task/pull/960)).
Expand Down
27 changes: 21 additions & 6 deletions crates/vt/schemas/run-plan.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@
"type": "string"
},
"cache": {
"description": "Whether the cache would be used. Absent for a built-in such as `echo`.\n\nThe check looks at the files as they are now.",
"description": "Whether the cache would be used. Absent for a built-in such as `echo`.\n\nThe check looks at the files as they are now. `unknown` marks a step whose files an earlier step may change before it starts.",
"allOf": [
{
"$ref": "#/definitions/Cache"
Expand Down Expand Up @@ -151,6 +151,24 @@
}
]
},
{
"description": "Can't be predicted yet: a step that runs before it would write files first, by running or by restoring a hit's outputs. `changes` shows the check of the files as they are now.",
"type": "object",
"properties": {
"status": {
"type": "string",
"const": "unknown"
}
},
"required": [
"status"
],
"allOf": [
{
"$ref": "#/definitions/Changed"
}
]
},
{
"description": "Would run: there is no cached run to compare with, as on a first run or after `vp cache clean`. Arguments given after the task name count, so a new combination of arguments is also `noBaseline`.",
"type": "object",
Expand Down Expand Up @@ -393,7 +411,7 @@
]
},
"changes": {
"description": "What differs from the cached run, sorted by kind in the order listed here, then by name or path. Undoing all of them would make the step a hit, unless `changesPartial` is set.",
"description": "What differs from the cached run, sorted by kind in the order listed here, then by name or path. Undoing all of them would make the step a hit, unless `changesPartial` is set. Present for `changed`, and for `unknown` when today's check found differences.",
"type": "array",
"items": {
"$ref": "#/definitions/Change"
Expand All @@ -404,10 +422,7 @@
"type": "boolean",
"const": true
}
},
"required": [
"changes"
]
}
},
"NestedStep": {
"description": "A nested `vp run`, which runs a plan of its own.",
Expand Down
7 changes: 5 additions & 2 deletions crates/vt/src/session/cache/display.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,11 @@ mod tests {

#[test]
fn inline_cache_hit_names_remote_source() {
let hit =
|source| CacheStatus::Hit { replayed_duration: std::time::Duration::ZERO, source };
let hit = |source| CacheStatus::Hit {
replayed_duration: std::time::Duration::ZERO,
source,
restores_outputs: false,
};

assert_eq!(
format_cache_status_inline(&hit(CacheHitSource::Local), RunKind::Real).as_deref(),
Expand Down
7 changes: 6 additions & 1 deletion crates/vt/src/session/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,12 @@ pub enum RunKind {
pub enum CacheStatus {
Disabled(CacheDisabledReason),
Miss(CacheMiss),
Hit { replayed_duration: Duration, source: CacheHitSource },
Hit {
replayed_duration: Duration,
source: CacheHitSource,
/// The hit has output files, which a real run restores.
restores_outputs: bool,
},
}

/// Convert `ExitStatus` to an i32 exit code.
Expand Down
1 change: 1 addition & 0 deletions crates/vt/src/session/execute/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,6 +454,7 @@ async fn run(
let mut stdio_config = reporter.start(CacheStatus::Hit {
replayed_duration: hit.value.duration,
source: hit.source,
restores_outputs: hit.value.output_archive.is_some(),
});
if run_kind == RunKind::Dry {
return Ok(Report::CacheHit);
Expand Down
16 changes: 15 additions & 1 deletion crates/vt/src/session/reporter/json/document.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ pub struct CommandStep {

/// Whether the cache would be used. Absent for a built-in such as `echo`.
///
/// The check looks at the files as they are now.
/// The check looks at the files as they are now. `unknown` marks a step whose files an
/// earlier step may change before it starts.
#[serde(skip_serializing_if = "Option::is_none")]
#[cfg_attr(test, schemars(with = "Cache"))]
pub cache: Option<Cache>,
Expand All @@ -128,6 +129,10 @@ pub enum Cache {
/// Differs from `baseline`, so it would run. `changes` lists the differences.
Changed(Changed),

/// Can't be predicted yet: a step that runs before it would write files first, by running
/// or by restoring a hit's outputs. `changes` shows the check of the files as they are now.
Unknown(Changed),

/// Would run: there is no cached run to compare with, as on a first run or after
/// `vp cache clean`. Arguments given after the task name count, so a new combination of
/// arguments is also `noBaseline`.
Expand All @@ -151,6 +156,13 @@ pub enum Cache {
pub struct Hit {
/// The cached run the step matches.
pub baseline: Baseline,

/// The cached run has output files, which a real run restores. Not printed: it only
/// decides which later steps are `unknown`. It comes from the cache entry, not from a list
/// of the files: a remote hit's archive isn't downloaded, so its files can't be listed, and
/// listing a local one decompresses the whole archive.
#[serde(skip)]
pub restores_outputs: bool,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should/can we make use of the outputs field in -vv? We can always generate outputs internally and only show it under -vv.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd keep the flag, for two reasons:

  • Remote hits. A remote hit restores outputs too, but a dry run doesn't download its archive, so its files can't be listed, and its outputs is absent even with -vv. The flag comes from the cache entry itself (output_archive.is_some()), which a remote entry has. The fetch returns the entry's value, and the archive is a separate blob that a dry run never downloads. The new e2e case dry_run_json::unknown_after_remote_hit shows both outcomes: after vp cache clean, after-emit is unknown because emit's entry has a blob, while after-build stays a hit because build's entry has none. Neither dry run makes a GET /blob request.
  • Cost. Listing a local hit's outputs means reading its whole zstd-compressed tar, since tar has no index. Doing that on every dry run would decompress every hit's archive just to learn a yes or no that the entry already records.

The unknown rule only needs to know whether a hit writes files, not which ones. The list would help only a finer rule that marks as unknown just the steps whose inputs overlap the restored outputs, and that rule would still need the flag for remote hits. The doc comment now explains why the flag isn't derived from outputs. Changed in bf3248b.

🤖 Generated with Claude Code

}

/// A step that differs from its cached run.
Expand All @@ -166,6 +178,8 @@ pub struct Changed {

/// What differs from the cached run, sorted by kind in the order listed here, then by name
/// or path. Undoing all of them would make the step a hit, unless `changesPartial` is set.
/// Present for `changed`, and for `unknown` when today's check found differences.
#[serde(skip_serializing_if = "Vec::is_empty")]
pub changes: Vec<Change>,

/// Other files may have changed too: without `-v`, the file check stops at the first
Expand Down
183 changes: 177 additions & 6 deletions crates/vt/src/session/reporter/json/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use document::{
Baseline, Cache, Change, Changed, CommandStep, Hit, HowChanged, NestedStep, PlanTask, RunPlan,
Setting, Step,
};
use rustc_hash::FxHashMap;
use rustc_hash::{FxHashMap, FxHashSet};
use vt_materialized_artifact::{Artifact, artifact};
use vt_path::AbsolutePath;
use vt_plan::{
Expand Down Expand Up @@ -208,16 +208,18 @@ impl GraphExecutionReporter for JsonGraphReporter {
}

/// A run stopped by an error, or cancelled by Ctrl-C, leaves some steps
/// unreported, and prints no plan. Its errors go to stderr.
/// unreported, and prints no plan. Its errors go to stderr. Otherwise,
/// the steps that earlier steps may change are marked `unknown` first.
fn finish(self: Box<Self>) -> Result<(), ExitStatus> {
let state = self.state.borrow();
let mut state = self.state.borrow_mut();
if state.unreported > 0 {
#[expect(clippy::print_stderr, reason = "errors go to stderr, and stdout gets no plan")]
for message in &state.errors {
eprintln!("{message}");
}
return Err(ExitStatus::FAILURE);
}
mark_unknown(&mut state.document.plan, false);
let mut writer = self.writer;
json::write_document(&mut writer, &state.document, self.pretty)
.map_err(|_| ExitStatus::FAILURE)
Expand Down Expand Up @@ -292,12 +294,99 @@ impl LeafExecutionReporter for JsonLeafReporter {
}
}

/// Mark as `unknown` each `hit` or `changed` step of `plan` that waits on a
/// step that writes files. `before_writes` says whether something the plan
/// waits on writes: the steps before the nested `vp run` step it comes from.
///
/// A step waits on the earlier steps of its task and on every step of the
/// tasks in its `dependsOn`, transitively. Whether a step writes is decided
/// from today's check, before any step is marked.
fn mark_unknown(plan: &mut BTreeMap<Str, PlanTask>, before_writes: bool) {
let task_writes: FxHashMap<Str, bool> =
plan.iter().map(|(id, task)| (id.clone(), task.steps.iter().any(step_writes))).collect();
let ids: Vec<Str> = plan.keys().cloned().collect();
for id in ids {
let mut writes = before_writes || depends_on_writes(plan, &task_writes, &id);
let Some(task) = plan.get_mut(&id) else { continue };
for step in &mut task.steps {
let step_writes = step_writes(step);
match step {
Step::Command(CommandStep { cache: Some(cache), .. }) if writes => {
mark_step_unknown(cache);
}
Step::Command(_) => {}
Step::Nested(nested) => mark_unknown(&mut nested.plan, writes),
}
writes |= step_writes;
}
}
}

/// Whether a task in the transitive `dependsOn` of the task `id` writes files.
fn depends_on_writes(
plan: &BTreeMap<Str, PlanTask>,
task_writes: &FxHashMap<Str, bool>,
id: &Str,
) -> bool {
let mut seen = FxHashSet::default();
let mut pending: Vec<&Str> =
plan.get(id).map(|task| task.depends_on.iter().collect()).unwrap_or_default();
while let Some(dependency) = pending.pop() {
if !seen.insert(dependency) {
continue;
}
if task_writes.get(dependency).copied().unwrap_or(false) {
return true;
}
if let Some(task) = plan.get(dependency) {
pending.extend(&task.depends_on);
}
}
false
}

/// Whether a step would write files before the steps that wait on it start:
/// it would run, or it's a hit that restores outputs. A nested plan writes if
/// any of its steps does. A built-in writes nothing.
fn step_writes(step: &Step) -> bool {
match step {
Step::Command(CommandStep { cache, .. }) => match cache {
None | Some(Cache::Hit(Hit { restores_outputs: false, .. })) => false,
Some(
Cache::Hit(_)
| Cache::Changed(_)
| Cache::Unknown(_)
| Cache::NoBaseline
| Cache::Error { .. }
| Cache::Disabled,
) => true,
},
Step::Nested(nested) => nested.plan.values().any(|task| task.steps.iter().any(step_writes)),
}
}

/// Turn a `hit` or `changed` status into `unknown`, keeping today's check.
fn mark_step_unknown(cache: &mut Cache) {
let unknown = match std::mem::replace(cache, Cache::Disabled) {
Cache::Hit(Hit { baseline, .. }) => {
Changed { baseline: Some(baseline), changes: Vec::new(), changes_partial: false }
}
Cache::Changed(changed) => changed,
other => {
*cache = other;
return;
}
};
*cache = Cache::Unknown(unknown);
}

/// How the plan shows a cache status. `None` for a built-in, which isn't cached.
fn cache_of(status: &CacheStatus) -> Option<Cache> {
Some(match status {
CacheStatus::Hit { replayed_duration, .. } => {
Cache::Hit(Hit { baseline: baseline(*replayed_duration) })
}
CacheStatus::Hit { replayed_duration, restores_outputs, .. } => Cache::Hit(Hit {
baseline: baseline(*replayed_duration),
restores_outputs: *restores_outputs,
}),
CacheStatus::Miss(CacheMiss::NotFound) => Cache::NoBaseline,
CacheStatus::Miss(CacheMiss::Changed(comparison)) => changed(comparison),
CacheStatus::Miss(CacheMiss::RemoteReadFailed(error)) => {
Expand Down Expand Up @@ -442,6 +531,88 @@ mod tests {
);
}

fn command(cache: Option<Cache>) -> Step {
Step::Command(CommandStep {
program: Str::from("vtt"),
args: Vec::new(),
cwd: None,
hook: None,
cache,
})
}

fn hit(restores_outputs: bool) -> Cache {
Cache::Hit(Hit { baseline: baseline(Duration::ZERO), restores_outputs })
}

fn task(depends_on: &[&str], steps: Vec<Step>) -> PlanTask {
PlanTask {
package: Str::from("."),
name: Str::from("t"),
depends_on: depends_on.iter().copied().map(Str::from).collect(),
steps,
}
}

fn statuses(plan: &BTreeMap<Str, PlanTask>) -> serde_json::Value {
serde_json::Value::Object(
plan.iter()
.map(|(id, task)| {
let steps = task.steps.iter().map(|step| match step {
Step::Command(step) => {
serde_json::to_value(&step.cache).unwrap()["status"].clone()
}
Step::Nested(nested) => statuses(&nested.plan),
});
(id.to_string(), serde_json::Value::Array(steps.collect()))
})
.collect(),
)
}

/// A step is `unknown` when a step it waits on writes files: an earlier
/// step of its task, a step of a task in its `dependsOn` (transitively),
/// or a step before the nested run it's in. A hit without outputs and a
/// built-in write nothing.
#[test]
fn steps_after_a_write_are_unknown() {
let nested_plan =
BTreeMap::from([(Str::from("n"), task(&[], vec![command(Some(hit(false)))]))]);
let mut plan = BTreeMap::from([
(Str::from("restores"), task(&[], vec![command(Some(hit(true)))])),
(Str::from("after-restore"), task(&["through"], vec![command(Some(hit(false)))])),
(Str::from("through"), task(&["restores"], vec![command(None)])),
(Str::from("plain"), task(&[], vec![command(Some(hit(false))), command(None)])),
(Str::from("after-plain"), task(&["plain"], vec![command(Some(hit(false)))])),
(
Str::from("steps"),
task(
&[],
vec![
command(Some(hit(false))),
command(Some(Cache::NoBaseline)),
Step::Nested(NestedStep {
expanded_from: Str::from("vt run n"),
plan: nested_plan,
}),
],
),
),
]);
mark_unknown(&mut plan, false);
assert_eq!(
statuses(&plan),
serde_json::json!({
"after-plain": ["hit"],
"after-restore": ["unknown"],
"plain": ["hit", null],
"restores": ["hit"],
"steps": ["hit", "noBaseline", { "n": ["unknown"] }],
"through": [null],
})
);
}

/// After a config change, only the previous run's key is compared: the
/// step is changed, with no `baseline`, and other files may have changed.
#[test]
Expand Down
2 changes: 1 addition & 1 deletion crates/vt/src/session/reporter/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ impl TaskResult {

match cache_status {
// The only error a cache hit can have is a failed restore.
CacheStatus::Hit { replayed_duration, source } => saved_error.map_or_else(
CacheStatus::Hit { replayed_duration, source, .. } => saved_error.map_or_else(
|| Self::CacheHit {
saved_duration_ms: duration_to_ms(*replayed_duration),
source: *source,
Expand Down
Loading
Loading