From 3bfafbc8664a2a1ba8efc3f64376ff63c3dc11b9 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Mon, 14 Sep 2026 21:26:12 +0200 Subject: plan 9: apply logs each step as it completes and stops a chain that landed elsewhere --- internal/apply/apply.go | 64 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 45 insertions(+), 19 deletions(-) (limited to 'internal/apply/apply.go') diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 713913e..5624b27 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -42,38 +42,59 @@ type StepResult struct { // Chain runs one file's steps in order and stops at the first failure, // marking the rest skipped. It never touches a file whose size or mtime no -// longer matches what the plan recorded. +// longer matches what the plan recorded. It is ChainLogged with no done. func Chain(c plan.Chain) []StepResult { + results, _ := ChainLogged(c, nil) + return results +} + +// ChainLogged is Chain, calling done with each step's result as soon as +// that step has run or been skipped, before the next one starts - so a +// caller that logs from done never has a completed step missing from the +// log when the process dies mid-chain (review M9). An error from done stops +// the chain at once and is returned with the results so far. +// +// A move or rename that had to take a free name at apply time, because its +// planned destination was taken since planning, stops the chain too: every +// later step was planned against the name the file did not get, and must +// not act on whatever is at that path (review M3). +func ChainLogged(c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) { results := make([]StepResult, len(c.Steps)) - stopped := false + stopWhy := "" for i, step := range c.Steps { - if stopped { - results[i] = StepResult{Step: step, Status: "skipped", Detail: "an earlier step in this chain failed"} - continue - } - if step.Skip != "" { + switch { + case stopWhy != "": + results[i] = StepResult{Step: step, Status: "skipped", Detail: stopWhy} + case step.Skip != "": // Planning already decided this step will not run; it must not // be attempted, so no pre-step check, no directory creation, no // touching the file (spec: a step already marked Skip is // reported, not attempted). results[i] = StepResult{Step: step, Status: "skipped", Detail: step.Skip} - continue + default: + if err := checkUnchanged(step.Src, c.File); err != nil { + results[i] = StepResult{Step: step, Status: "failed", Detail: err.Error()} + stopWhy = "an earlier step in this chain failed" + break + } + res := runStep(step) + results[i] = res + switch { + case res.Status == "failed": + stopWhy = "an earlier step in this chain failed" + case (step.Kind == plan.Move || step.Kind == plan.Rename) && res.Dst != step.Dst: + stopWhy = fmt.Sprintf("an earlier step put the file at %s, not the planned %s", res.Dst, step.Dst) + } } - if err := checkUnchanged(step.Src, c.File); err != nil { - results[i] = StepResult{Step: step, Status: "failed", Detail: err.Error()} - stopped = true - continue - } - - res := runStep(step) - results[i] = res - if res.Status == "failed" { - stopped = true + if done != nil { + if err := done(i, results[i]); err != nil { + return results[:i+1], err + } } } - return results + return results, nil } // checkUnchanged is the guard that matters most: before every step, the @@ -131,6 +152,11 @@ func runFileStep(step plan.Step) StepResult { var displacedEntry string if step.Displaces != "" { + // Re-checked at apply time (review M4): only a regular file may be + // trashed to make room, never a directory or link put there since. + if fi, err := os.Lstat(step.Displaces); err != nil || !fi.Mode().IsRegular() { + return StepResult{Step: step, Status: "failed", Detail: "the file to replace is gone or no longer a regular file"} + } // overwrite policy: the file already at dst must be trashed before // this step's own destination name is used, so no free-name search // applies here — the whole point of displacing was to clear this -- cgit v1.3