diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | cmd/krino/sort.go | 20 | ||||
| -rw-r--r-- | cmd/krino/sort_test.go | 33 | ||||
| -rw-r--r-- | docs/design.md | 4 | ||||
| -rw-r--r-- | internal/plan/chain.go | 6 |
5 files changed, 63 insertions, 4 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 572ecef..80355e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,9 @@ directory a placeholder may not leave, and the directory left out of the walk, now include it. - In a real run, a later directory is no longer blocked by paths an earlier - one moved files away from, or planned and did not apply ("target exists"). + one moved files away from, or planned and did not apply ("target exists"); + what an earlier directory did put somewhere stays protected from a later + `overwrite`. - `krino explain` says when a duplicate's delete would be skipped, no longer removes a directory's unused keyword cache (it holds no lock), and walks the directory only when a duplicate test needs it. diff --git a/cmd/krino/sort.go b/cmd/krino/sort.go index 5a175a4..b516629 100644 --- a/cmd/krino/sort.go +++ b/cmd/krino/sort.go @@ -151,6 +151,7 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { continue } + var applied []string // where this directory's applied steps put files quit := func() bool { defer func() { if rerr := l.Release(); rerr != nil { @@ -262,6 +263,15 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { notReviewed = len(actionable) - len(reviewedChains(actionable, approved)) } res, aerr := e.Apply(ctx, toApply, approved, j, run) + if res != nil { + for _, fr := range res.Files { + for _, sr := range fr.Steps { + if sr.Status == "ok" && sr.Dst != "" { + applied = append(applied, sr.Dst) + } + } + } + } if aerr != nil { // Interrupted mid-apply (fix round 2026-09-12/item 2): // treated exactly like the cancelled lock wait above - not @@ -290,9 +300,15 @@ func cmdSort(g *globals, names []string, stdout, stderr io.Writer) int { // This directory is applied (or skipped) now, so the disk is the // truth for the next one: its claims - sources it moved away, // destinations it planned but declined or failed - must not - // block a later directory (triage 28i). A dry run applies - // nothing, so there they carry over. + // block a later directory (triage 28i). What it did put + // somewhere stays claimed, so a later (on-conflict overwrite) + // takes a free name rather than trash this run's own result + // (plan 11 review M1). A dry run applies nothing, so there every + // claim carries over. claims = plan.NewClaims() + for _, p := range applied { + claims.Claim(p) + } } } diff --git a/cmd/krino/sort_test.go b/cmd/krino/sort_test.go index 22e00b4..071a64d 100644 --- a/cmd/krino/sort_test.go +++ b/cmd/krino/sort_test.go @@ -333,3 +333,36 @@ func TestWriteStopsKrinoWhenApplyFails(t *testing.T) { t.Errorf("d2 was planned after [w]:\n%s", out) } } + +// TestLaterDirectoryNeverOverwritesAnEarlierOnesResult: what an earlier +// directory of the run put somewhere stays claimed, so a later directory's +// (on-conflict overwrite) takes a free name instead of trashing it - as a +// dry run of the same two directories shows (plan 11 review M1). +func TestLaterDirectoryNeverOverwritesAnEarlierOnesResult(t *testing.T) { + h := home(t) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + if code, _, errOut := runCLI(t, "init"); code != 0 { + t.Fatal(errOut) + } + for _, n := range []string{"a", "b"} { + p := filepath.Join(h, n, "x.pdf") + os.MkdirAll(filepath.Dir(p), 0o755) + os.WriteFile(p, []byte("from "+n), 0o644) + os.Chtimes(p, old, old) + if code, _, errOut := runCLI(t, "new", n, filepath.Join(h, n)); code != 0 { + t.Fatal(errOut) + } + os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", n+".conf"), []byte("(path \"~/"+n+"\")\n(on-conflict overwrite)\n(rule \"out\" (move \"~/Out\"))\n"), 0o644) + } + if code, out, errOut := runCLI(t, "-y", "a", "b"); code != 0 { + t.Fatalf("exit %d\n%s\n%s", code, out, errOut) + } + for rel, want := range map[string]string{"Out/x.pdf": "from a", "Out/x_1.pdf": "from b"} { + if b, err := os.ReadFile(filepath.Join(h, rel)); err != nil || string(b) != want { + t.Errorf("%s: %q, %v; want %q", rel, b, err, want) + } + } + if entries, _ := os.ReadDir(filepath.Join(h, ".local", "share", "Trash", "files")); len(entries) != 0 { + t.Errorf("the Trash holds %d entries; nothing should have been displaced", len(entries)) + } +} diff --git a/docs/design.md b/docs/design.md index ca8d550..43a6975 100644 --- a/docs/design.md +++ b/docs/design.md @@ -475,7 +475,9 @@ finds a target another step of the same plan has already claimed, it takes the next free name instead, because that path will hold the other step's output by the time either runs, and displacing it would destroy that step's result. A real run applies each directory before planning the next, so a later -directory sees the earlier one's result on disk and its claims end with it. +directory sees the earlier one's result on disk; of its claims only the paths +it actually put files at carry over, so a later `overwrite` never displaces +this run's own result. A dry run (`-n`) applies nothing: its claim set spans every directory, so two configured directories cannot show the same final name, but each directory is otherwise planned as if no earlier one had been applied (a file the first diff --git a/internal/plan/chain.go b/internal/plan/chain.go index b32cc7d..bb4876b 100644 --- a/internal/plan/chain.go +++ b/internal/plan/chain.go @@ -28,6 +28,12 @@ func NewClaims() *Claims { return &Claims{taken: claimed{}} } +// Claim marks path as spoken for: a later Build call of the run treats it +// as another step's result, never displacing it (spec ยง7.4). +func (c *Claims) Claim(path string) { + c.taken[path] = true +} + // Input is one file and the rules that matched it, in match order. type Input struct { File scan.File |