From c03a72f1d7b598c1fe8fd01bb1f5bbfbd0256313 Mon Sep 17 00:00:00 2001 From: Lukasz Kasprzak Date: Mon, 14 Sep 2026 22:29:06 +0200 Subject: plan 10: an interrupt stops between steps; nohup keeps ignoring hangups --- cmd/krino/sort.go | 8 +++++++- docs/design.md | 7 +++++-- internal/apply/apply.go | 12 ++++++++++-- internal/apply/swap_test.go | 34 +++++++++++++++++++++++++++++++++- internal/engine/apply.go | 6 +++--- man/krino.1 | 8 ++++++-- 6 files changed, 64 insertions(+), 11 deletions(-) diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index 901b19e..52875ed 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -359,7 +359,13 @@ func installSignalHandler(cancel context.CancelFunc) func() { } sig := make(chan os.Signal, 1) - signal.Notify(sig, os.Interrupt, syscall.SIGTERM, syscall.SIGHUP) + signal.Notify(sig, os.Interrupt, syscall.SIGTERM) + // A hangup stops krino like SIGTERM - unless it was started ignoring + // hangups (nohup), which must go on meaning "keep running" (re-review + // pa F3): asking for SIGHUP would re-enable it. + if !signal.Ignored(syscall.SIGHUP) { + signal.Notify(sig, syscall.SIGHUP) + } done := make(chan struct{}) go func() { signals := 0 diff --git a/docs/design.md b/docs/design.md index 99e5b55..07728fc 100644 --- a/docs/design.md +++ b/docs/design.md @@ -701,8 +701,11 @@ Exit status: 0 success, including nothing to do and everything declined; 1 one or more steps failed; 2 usage or config error; 130 interrupted. A config error anywhere stops the whole run before any scanning: krino never -acts on a config it only partly understood. Ctrl-C during apply finishes the -current step, logs it, and stops. A second krino on the same directory waits +acts on a config it only partly understood. Ctrl-C (SIGTERM, or SIGHUP unless +started under `nohup`) during apply finishes the current step, logs it, skips +the rest of that file's chain, and stops. A second interrupt exits at once: +the step in flight is not logged, and a copy in progress can leave a +temporary `.krino-*` file and an empty directory behind. A second krino on the same directory waits for the lock, or fails immediately with `-y` (so a cron job never piles up). ## 12. Architecture diff --git a/internal/apply/apply.go b/internal/apply/apply.go index 684641e..a93aad4 100644 --- a/internal/apply/apply.go +++ b/internal/apply/apply.go @@ -7,6 +7,7 @@ package apply import ( + "context" "errors" "fmt" "os" @@ -44,7 +45,7 @@ type StepResult struct { // marking the rest skipped. It never touches a file whose size or mtime no // longer matches what the plan recorded. It is ChainLogged with no done. func Chain(c plan.Chain) []StepResult { - results, _ := ChainLogged(c, nil) + results, _ := ChainLogged(context.Background(), c, nil) return results } @@ -58,11 +59,18 @@ func Chain(c plan.Chain) []StepResult { // 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) { +// +// Once ctx is cancelled (an interrupt), the step already under way finishes +// and every later step is skipped as "interrupted": an interrupt stops after +// the current step, not after the file's whole chain (spec ยง11). +func ChainLogged(ctx context.Context, c plan.Chain, done func(i int, sr StepResult) error) ([]StepResult, error) { results := make([]StepResult, len(c.Steps)) stopWhy := "" for i, step := range c.Steps { + if stopWhy == "" && ctx.Err() != nil { + stopWhy = "interrupted" + } switch { case stopWhy != "": results[i] = StepResult{Step: step, Status: "skipped", Detail: stopWhy} diff --git a/internal/apply/swap_test.go b/internal/apply/swap_test.go index c185686..ccac945 100644 --- a/internal/apply/swap_test.go +++ b/internal/apply/swap_test.go @@ -3,6 +3,7 @@ package apply import ( + "context" "os" "path/filepath" "strings" @@ -144,7 +145,7 @@ func TestChainLoggedReportsEachStepBeforeTheNext(t *testing.T) { plan.Step{Kind: plan.Rename, Src: moved, Dst: renamed}, ) var calls []int - res, err := ChainLogged(c, func(i int, sr StepResult) error { + res, err := ChainLogged(context.Background(), c, func(i int, sr StepResult) error { calls = append(calls, i) if i == 0 { if _, err := os.Lstat(moved); err != nil { @@ -185,3 +186,34 @@ func TestChainRefusesToDisplaceANonRegularTarget(t *testing.T) { t.Errorf("the directory was displaced: %v", err) } } + +// TestChainLoggedStopsBetweenStepsWhenInterrupted: once the context is +// cancelled, the chain finishes the step it is on and skips the rest, so an +// interrupt stops after the current step, not after the file's whole chain +// (re-review pa F7). +func TestChainLoggedStopsBetweenStepsWhenInterrupted(t *testing.T) { + root := t.TempDir() + p := filepath.Join(root, "a.pdf") + moved := filepath.Join(root, "W", "a.pdf") + c := planned(t, root, "a.pdf", "body", + plan.Step{Kind: plan.Move, Src: p, Dst: moved}, + plan.Step{Kind: plan.Rename, Src: moved, Dst: filepath.Join(root, "W", "b.pdf")}, + ) + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + res, err := ChainLogged(ctx, c, func(i int, sr StepResult) error { + if i == 0 { + cancel() + } + return nil + }) + if err != nil { + t.Fatal(err) + } + if res[0].Status != "ok" || res[1].Status != "skipped" || res[1].Detail != "interrupted" { + t.Errorf("steps = %s, %s %q; want ok, then skipped as interrupted", res[0].Status, res[1].Status, res[1].Detail) + } + if _, err := os.Lstat(moved); err != nil { + t.Errorf("the finished step was undone or never ran: %v", err) + } +} diff --git a/internal/engine/apply.go b/internal/engine/apply.go index 75248fb..f9cb9ef 100644 --- a/internal/engine/apply.go +++ b/internal/engine/apply.go @@ -80,7 +80,7 @@ func (e *Engine) Apply(ctx context.Context, dp *DirPlan, approved map[string]boo if err := ctx.Err(); err != nil { return result, err } - fr, err := e.applyFile(dp.Dir.Name, c, approved[c.File.Rel], j, run) + fr, err := e.applyFile(ctx, dp.Dir.Name, c, approved[c.File.Rel], j, run) if err != nil { return result, fmt.Errorf("engine: apply: %w", err) } @@ -95,7 +95,7 @@ func (e *Engine) Apply(ctx context.Context, dp *DirPlan, approved map[string]boo } // applyFile carries out (or declines) one file's chain and logs it. -func (e *Engine) applyFile(dirName string, c plan.Chain, approved bool, j *journal.Writer, run string) (FileResult, error) { +func (e *Engine) applyFile(ctx context.Context, dirName string, c plan.Chain, approved bool, j *journal.Writer, run string) (FileResult, error) { rel := c.File.Rel if !approved { @@ -112,7 +112,7 @@ func (e *Engine) applyFile(dirName string, c plan.Chain, approved bool, j *journ // Each step is logged the moment it has run (review M9), not after the // whole chain: a run killed mid-chain must leave what it did undoable. - results, err := apply.ChainLogged(c, func(i int, sr apply.StepResult) error { + results, err := apply.ChainLogged(ctx, c, func(i int, sr apply.StepResult) error { if err := e.logStep(j, run, dirName, rel, i+1, c.Steps[i], sr); err != nil { return unloggedStep(rel, c.Steps[i], sr, err) } diff --git a/man/krino.1 b/man/krino.1 index 246184b..e9d41da 100644 --- a/man/krino.1 +++ b/man/krino.1 @@ -431,7 +431,8 @@ found after a directory name is a usage error rather than a guess. .It 130 Interrupted .Pq Ic Ctrl-C -or terminated; the current step, if any, is finished and logged first. +or terminated; the current step, if any, is finished and logged first, and +the rest of that file's steps are skipped. .El .Sh KNOWN LIMITATIONS .Ic krino explain @@ -454,7 +455,10 @@ Wrapping counts characters, not terminal columns: a name in a wide script (CJK) or with many combining marks can run past the terminal's width. .Pp A second interrupt exits at once: every step completed so far is logged -and can be undone, but the step in flight when it arrived is not logged. +and can be undone, but the step in flight when it arrived is not logged, +and a copy in progress can leave a temporary +.Pa .krino-* +file and an empty directory behind. .Pp Undo identifies a file by its directory and its path within it. A file one directory's rules move into another included directory, which -- cgit v1.3