summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-09-14 10:56:21 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-09-14 10:56:21 +0200
commit1506c7dcd6032c04c1df6f5785b6dd3dfe4511cc (patch)
treefb6d573ca8726c1b1d91ba273a284ba666cde3f6
parentebdd7bb254a0f19f815a644d26ce232de7be0adb (diff)
downloadkrino-1506c7dcd6032c04c1df6f5785b6dd3dfe4511cc.tar.gz
krino-1506c7dcd6032c04c1df6f5785b6dd3dfe4511cc.zip
krino: duplicates are found, never deleted
A rule combining (duplicate) with a delete action is refused at load, and a file that is a duplicate under any duplicate scope its directory uses gets no delete step from any rule: the plan shows it skipped and the chain continues. A failed duplicate check blocks the delete too. Tests cover (matched), another rule's own condition, a test evaluation skipped, two scopes and a failed check, each checking every copy is still on disk.
-rw-r--r--CHANGELOG.md7
-rw-r--r--README.md4
-rw-r--r--cmd/krino/matching_test.go4
-rw-r--r--cmd/krino/sort_test.go101
-rw-r--r--docs/design.md4
-rw-r--r--internal/engine/engine.go50
-rw-r--r--internal/engine/engine_test.go44
-rw-r--r--internal/engine/match.go42
-rw-r--r--internal/engine/match_test.go2
-rw-r--r--internal/engine/nodelete_test.go212
-rw-r--r--internal/engine/plan.go2
-rw-r--r--internal/plan/chain.go8
-rw-r--r--internal/plan/chain_test.go28
-rw-r--r--man/krino.conf.5123
14 files changed, 551 insertions, 80 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 5bd80c8..36b50e7 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -2,6 +2,13 @@
## Unreleased
+- Duplicates are found, never deleted. A rule combining `(duplicate)` with
+ `(delete)` or `(delete permanent)` is refused by `krino check` and every
+ run, and a file that is a duplicate under any duplicate scope its
+ directory's rules use gets no delete step from any rule. This replaces
+ 0.0.1's known limitation: duplicate conditions with different scopes can no
+ longer delete every copy of a group, only move copies aside.
+
## 0.0.1 — 2026-09-13
- Per-directory rule files, a main file listing which directories run, and a
diff --git a/README.md b/README.md
index 4b7d4c3..34d3a78 100644
--- a/README.md
+++ b/README.md
@@ -189,6 +189,10 @@ rule content only when a private pattern list is configured with
- `(delete)` moves a file to the freedesktop.org Trash by default, not
`unlink(2)` — recoverable, by hand or with `krino undo`. `(delete
permanent)` is the explicit opt-out, and undo can never reverse it.
+- Duplicates are found, never deleted. A rule can combine `(duplicate)` with
+ `move`, never with `delete`, and no other rule can delete a file krino has
+ found to be a duplicate. Move duplicates aside, for example to `~/.dupes/`,
+ and delete them later yourself or with a tool such as jdupes.
- Every step of every run is appended to `$XDG_STATE_HOME/krino/krino.log`
(default `~/.local/state/krino/krino.log`), tab-separated and
`grep`-able.
diff --git a/cmd/krino/matching_test.go b/cmd/krino/matching_test.go
index 69c0eca..25a2993 100644
--- a/cmd/krino/matching_test.go
+++ b/cmd/krino/matching_test.go
@@ -18,7 +18,7 @@ const dlRules = `
(recursive yes)
(min-age 0s)
(ignore "*.part")
-(rule "dups" (when (duplicate)) (delete) (stop))
+(rule "dups" (when (duplicate)) (move "Dupes") (stop))
(rule "acme" (when (type document) (content "acme ltd")) (move "Work/Acme") (stop))
(rule "images" (when (type image)) (move "Pictures"))
(rule "rest" (when (not (matched)) (type text)) (move "Other"))
@@ -66,7 +66,7 @@ func TestDryRun(t *testing.T) {
"krino: dl ~/dl\n8 scanned · 4 to act on · 2 warnings · ",
"\n 1 inv1.txt move → Work/Acme/ acme type txt, content \"acme ltd\"\n",
"\n 2 notes.txt move → Other/ rest not matched, type txt\n",
- "\n 4 report (1).pdf trash dups duplicate of report.pdf\n",
+ "\n 4 report (1).pdf move → Dupes/ dups duplicate of report.pdf\n",
"\nwarnings\n brochure.doc acme: content unreadable: needs antiword or catdoc, not installed\n",
"\nnot acted on: 1 ignored · 1 busy · 2 unmatched (-v lists them)\n",
} {
diff --git a/cmd/krino/sort_test.go b/cmd/krino/sort_test.go
index fa9a7ca..321c9e8 100644
--- a/cmd/krino/sort_test.go
+++ b/cmd/krino/sort_test.go
@@ -111,14 +111,14 @@ func TestAllSkippedDirectoryReportsZeroAndLogsNothing(t *testing.T) {
}
}
-// TestPermanentDeleteOfDuplicatesUnderOverlappingDirKeepsACopy drives the
-// whole run end to end on the shape where a (duplicate "DIR") overlaps the
-// scanned tree: a recursive root holding Archive/x.pdf and a loose, older
-// x-copy.pdf with the same bytes, and a rule that permanently deletes
-// duplicates of anything under Archive. Whatever the verdicts, applying
-// the plan must leave the content on disk; the expected outcome is that
-// the archived copy stays and only the loose one goes.
-func TestPermanentDeleteOfDuplicatesUnderOverlappingDirKeepsACopy(t *testing.T) {
+// TestDuplicatesUnderOverlappingDirMoveOnlyTheLooseCopy drives the whole run
+// end to end on the shape where a (duplicate "DIR") overlaps the scanned
+// tree: a recursive root holding Archive/x.pdf and a loose, older
+// x-copy.pdf with the same bytes, and a rule that moves duplicates of
+// anything under Archive aside. Every lookup must elect the same original,
+// so the archived copy stays where it is and only the loose one is moved;
+// an inconsistent election would move both.
+func TestDuplicatesUnderOverlappingDirMoveOnlyTheLooseCopy(t *testing.T) {
h := home(t)
dl := filepath.Join(h, "dl")
content := []byte("%PDF acme statement")
@@ -143,33 +143,86 @@ func TestPermanentDeleteOfDuplicatesUnderOverlappingDirKeepsACopy(t *testing.T)
t.Fatal(errOut)
}
rules := "(path \"~/dl\")\n(recursive yes)\n(min-age 0s)\n" +
- "(rule \"dups\" (when (duplicate \"Archive\")) (delete permanent))\n"
+ "(rule \"dups\" (when (duplicate \"Archive\")) (move \"~/dupes\") (stop))\n"
if err := os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(rules), 0o644); err != nil {
t.Fatal(err)
}
code, out, errOut := runCLI(t, "-y")
+ if code != 0 {
+ t.Fatalf("run: exit %d\nstdout:\n%s\nstderr:\n%s", code, out, errOut)
+ }
+ moved := filepath.Join(h, "dupes", "x-copy.pdf")
+ for _, p := range []string{archived, moved} {
+ b, err := os.ReadFile(p)
+ if err != nil || string(b) != string(content) {
+ t.Errorf("%s: want the content there, got err %v", p, err)
+ }
+ }
+ if _, err := os.Stat(loose); !os.IsNotExist(err) {
+ t.Errorf("%s is still in place; want it moved to %s", loose, moved)
+ }
+}
- var copies []string
- err := filepath.WalkDir(dl, func(p string, d os.DirEntry, err error) error {
- if err != nil || !d.Type().IsRegular() {
- return err
+// TestCheckRefusesDuplicateWithDelete: krino check reports the §4.5
+// refusal on stderr and fails, before any run could act on the rule.
+func TestCheckRefusesDuplicateWithDelete(t *testing.T) {
+ h := home(t)
+ dl := filepath.Join(h, "dl")
+ if err := os.MkdirAll(dl, 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if code, _, errOut := runCLI(t, "init"); code != 0 {
+ t.Fatal(errOut)
+ }
+ if code, _, errOut := runCLI(t, "new", "dl", dl); code != 0 {
+ t.Fatal(errOut)
+ }
+ rules := "(path \"~/dl\")\n(rule \"d\" (when (duplicate)) (delete))\n"
+ if err := os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(rules), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ code, _, errOut := runCLI(t, "check")
+ want := `rule "d": (duplicate) cannot be combined with (delete)`
+ if code == 0 || !strings.Contains(errOut, want) {
+ t.Errorf("check: exit %d, stderr %q; want non-zero and %q", code, errOut, want)
+ }
+}
+
+// TestDryRunShowsNeverDeletedSkip pins the plan line spec §5.5 names.
+func TestDryRunShowsNeverDeletedSkip(t *testing.T) {
+ h := home(t)
+ dl := filepath.Join(h, "dl")
+ old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+ for name, mt := range map[string]time.Time{"a.pdf": old, "b.pdf": old.Add(time.Hour)} {
+ p := filepath.Join(dl, name)
+ if err := os.MkdirAll(dl, 0o755); err != nil {
+ t.Fatal(err)
}
- if b, err := os.ReadFile(p); err == nil && string(b) == string(content) {
- copies = append(copies, p)
+ if err := os.WriteFile(p, []byte("%PDF same"), 0o644); err != nil {
+ t.Fatal(err)
}
- return nil
- })
- if err != nil {
- t.Fatal(err)
+ if err := os.Chtimes(p, mt, mt); err != nil {
+ t.Fatal(err)
+ }
+ }
+ if code, _, errOut := runCLI(t, "init"); code != 0 {
+ t.Fatal(errOut)
+ }
+ if code, _, errOut := runCLI(t, "new", "dl", dl); code != 0 {
+ t.Fatal(errOut)
}
- if len(copies) == 0 {
- t.Fatalf("every copy was deleted (exit %d)\nstdout:\n%s\nstderr:\n%s", code, out, errOut)
+ rules := "(path \"~/dl\")\n(min-age 0s)\n" +
+ "(rule \"dupes\" (when (duplicate)) (move \"Dupes\"))\n" +
+ "(rule \"cleanup\" (when (matched)) (delete))\n"
+ if err := os.WriteFile(filepath.Join(h, ".config", "krino", "dirs", "dl.conf"), []byte(rules), 0o644); err != nil {
+ t.Fatal(err)
}
+ code, out, errOut := runCLI(t, "-n")
if code != 0 {
- t.Fatalf("run: %d %s", code, errOut)
+ t.Fatalf("exit %d: %s", code, errOut)
}
- if len(copies) != 1 || copies[0] != archived {
- t.Errorf("copies left = %v, want only %s", copies, archived)
+ if !strings.Contains(out, "skipped: a duplicate is never deleted") {
+ t.Errorf("plan lacks the skipped delete:\n%s", out)
}
}
diff --git a/docs/design.md b/docs/design.md
index 150e135..153bab0 100644
--- a/docs/design.md
+++ b/docs/design.md
@@ -243,7 +243,9 @@ jdupes. Two rules enforce this:
plan shows the step as "skipped: a duplicate is never deleted" and the
rest of the chain continues from the file's current path. This covers what
rule 1 cannot see: a later rule deleting through `(matched)` or through a
- condition of its own.
+ condition of its own. If that lookup fails, the delete is skipped too, as
+ "duplicate check failed, so not deleted: REASON": krino cannot show the
+ file is not a duplicate, so it keeps it.
Together they mean no rule can delete every copy of content that a duplicate
test in that directory can see. Under each scope every scanned file in a
diff --git a/internal/engine/engine.go b/internal/engine/engine.go
index ea61e8c..b5f2106 100644
--- a/internal/engine/engine.go
+++ b/internal/engine/engine.go
@@ -8,6 +8,7 @@ package engine
import (
"fmt"
"os"
+ "strings"
"time"
"krino/internal/cond"
@@ -44,6 +45,12 @@ type Dir struct {
// other variant will ever be asked for; with more than one, both must
// stay memoised, as before.
ContentVariants []cond.Options
+
+ // DupScopes is every distinct directory list the duplicate tests of
+ // Rules use, in first-seen order, the plain (duplicate) as an empty
+ // list. Spec §5.5 rule 2 looks a file up under each of them before any
+ // rule may delete it.
+ DupScopes [][]string
}
// Rule is one directory's rule, with its condition compiled.
@@ -90,9 +97,14 @@ func Load(mainFile string, names ...string) (*Engine, []*config.Diag) {
errs = append(errs, diag)
continue
}
+ if diag := checkDuplicateDelete(d.File, r, c); diag != nil {
+ errs = append(errs, diag)
+ continue
+ }
dir.Rules = append(dir.Rules, &Rule{Name: r.Name, Conf: r, Settings: rs, Cond: c})
}
dir.ContentVariants = contentVariants(dir.Rules)
+ dir.DupScopes = dupScopes(dir.Rules)
dirs = append(dirs, dir)
}
@@ -132,6 +144,23 @@ func checkCaptures(file string, r *config.Rule, c *cond.Cond) *config.Diag {
return nil
}
+// checkDuplicateDelete refuses a rule that combines a duplicate test with a
+// delete action (spec §4.5, §5.5): duplicates are found, never deleted.
+// Cond.DupDirs records every duplicate test compiled, inside or and not
+// too, so a test anywhere in the condition counts. It reports only the
+// first delete action, so one config mistake yields one diagnostic.
+func checkDuplicateDelete(file string, r *config.Rule, c *cond.Cond) *config.Diag {
+ if len(c.DupDirs) == 0 {
+ return nil
+ }
+ for _, a := range r.Actions {
+ if a.Kind == config.Delete || a.Kind == config.DeletePermanent {
+ return &config.Diag{File: file, Pos: a.Pos, Msg: fmt.Sprintf("rule %q: (duplicate) cannot be combined with (%s): duplicates are never deleted, move them aside instead", r.Name, a.Kind)}
+ }
+ }
+ return nil
+}
+
// captureGroups renders a capture-group count with correct singular/plural.
func captureGroups(n int) string {
if n == 1 {
@@ -177,6 +206,27 @@ func contentVariants(rules []*Rule) []cond.Options {
return out
}
+// dupScopes returns the distinct Cond.DupDirs lists of rules, in first-seen
+// order. Lists are compared as written, with their length in the key so
+// (duplicate) and (duplicate "") stay apart; two spellings of one directory
+// stay two entries, which costs a second lookup but never a wrong answer,
+// since facts.Duplicate resolves and shares the index itself.
+func dupScopes(rules []*Rule) [][]string {
+ var out [][]string
+ seen := map[string]bool{}
+ for _, r := range rules {
+ for _, dirs := range r.Cond.DupDirs {
+ key := fmt.Sprintf("%d\x00%s", len(dirs), strings.Join(dirs, "\x00"))
+ if seen[key] {
+ continue
+ }
+ seen[key] = true
+ out = append(out, dirs)
+ }
+ }
+ return out
+}
+
// Report is what Check reports: the files involved and each directory's
// state.
type Report struct {
diff --git a/internal/engine/engine_test.go b/internal/engine/engine_test.go
index 8dcfcb3..18b35e7 100644
--- a/internal/engine/engine_test.go
+++ b/internal/engine/engine_test.go
@@ -213,3 +213,47 @@ func TestContentVariantsComputedAtLoad(t *testing.T) {
t.Fatalf("ContentVariants = %+v, want %+v", got, want)
}
}
+
+// TestLoadRefusesDuplicateWithDelete is spec §4.5: a (duplicate) test
+// anywhere in a rule's condition, and a delete action in the same rule, is
+// a load error, however the test is nested.
+func TestLoadRefusesDuplicateWithDelete(t *testing.T) {
+ tests := []struct{ rule, want string }{
+ {`(rule "a" (when (duplicate)) (delete))`,
+ `rule "a": (duplicate) cannot be combined with (delete): duplicates are never deleted, move them aside instead`},
+ {`(rule "a" (when (duplicate "Archive")) (delete permanent))`,
+ `rule "a": (duplicate) cannot be combined with (delete permanent)`},
+ {`(rule "a" (when (or (type pdf) (duplicate))) (move "Keep") (delete))`,
+ `rule "a": (duplicate) cannot be combined with (delete)`},
+ {`(rule "a" (when (not (duplicate))) (delete))`,
+ `rule "a": (duplicate) cannot be combined with (delete)`},
+ }
+ for _, tt := range tests {
+ h := sandbox(t)
+ main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": `(path "/tmp")
+` + tt.rule})
+ _, errs := Load(main, "dl")
+ joined := ""
+ for _, d := range errs {
+ joined += d.Error() + "\n"
+ }
+ if len(errs) != 1 || !strings.Contains(joined, tt.want) {
+ t.Errorf("rule %s: errs %v, want %q", tt.rule, errs, tt.want)
+ }
+ }
+}
+
+// TestLoadAcceptsDuplicateWithMove: the way §5.5 recommends dealing with
+// duplicates loads clean, and so does a delete rule with no duplicate test.
+func TestLoadAcceptsDuplicateWithMove(t *testing.T) {
+ for _, rule := range []string{
+ `(rule "dupes" (when (duplicate "Archive")) (move "~/.dupes/") (stop))`,
+ `(rule "old" (when (type iso) (age > 90d)) (delete))`,
+ } {
+ h := sandbox(t)
+ main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": "(path \"/tmp\")\n" + rule})
+ if _, errs := Load(main, "dl"); len(errs) != 0 {
+ t.Errorf("rule %s: errs %v, want none", rule, errs)
+ }
+ }
+}
diff --git a/internal/engine/match.go b/internal/engine/match.go
index e68a6e8..ffee7f0 100644
--- a/internal/engine/match.go
+++ b/internal/engine/match.go
@@ -33,6 +33,12 @@ type FileMatch struct {
File scan.File
Rules []RuleMatch // matching rules in order, ending at the first with (stop)
Warnings []string // "<rule>: <warning>", e.g. "acme: content unreadable: needs pdftotext, not installed"
+
+ // NoDelete is non-empty when no delete step may run for this file (spec
+ // §5.5 rule 2), and says why: the file is a duplicate under a scope its
+ // directory's rules use, or that check failed. Only set for a file some
+ // matching rule would delete.
+ NoDelete string
}
// Result is everything Match found in one directory.
@@ -127,9 +133,45 @@ func evalFile(run *matchRun, file scan.File) FileMatch {
break
}
}
+ if len(run.d.DupScopes) > 0 && deletes(fm.Rules) {
+ fm.NoDelete = noDelete(f, run.d.DupScopes)
+ }
return fm
}
+// NeverDeleted is the reason a duplicate's delete step is skipped (spec
+// §5.5).
+const NeverDeleted = "a duplicate is never deleted"
+
+// deletes reports whether any of rules has a delete action.
+func deletes(rules []RuleMatch) bool {
+ for _, rm := range rules {
+ for _, a := range rm.Rule.Conf.Actions {
+ if a.Kind == config.Delete || a.Kind == config.DeletePermanent {
+ return true
+ }
+ }
+ }
+ return false
+}
+
+// noDelete looks f up under every scope, whether or not evaluating the
+// rules reached that test (spec §5.5 rule 2), and returns why f must not be
+// deleted, or "" when it may be. A failed lookup blocks the delete too:
+// krino cannot show the file is not a duplicate, so it keeps it.
+func noDelete(f *facts, scopes [][]string) string {
+ for _, dirs := range scopes {
+ _, dup, err := f.Duplicate(dirs)
+ if err != nil {
+ return "duplicate check failed, so not deleted: " + err.Error()
+ }
+ if dup {
+ return NeverDeleted
+ }
+ }
+ return ""
+}
+
// RuleTrace is one rule's outcome in an Explain call.
type RuleTrace struct {
Rule *Rule
diff --git a/internal/engine/match_test.go b/internal/engine/match_test.go
index 1356074..477e06f 100644
--- a/internal/engine/match_test.go
+++ b/internal/engine/match_test.go
@@ -18,7 +18,7 @@ const dlConf = `
(recursive yes)
(min-age 0s)
(ignore "*.part")
-(rule "dups" (when (duplicate)) (delete) (stop))
+(rule "dups" (when (duplicate)) (move "Dupes") (stop))
(rule "acme" (when (type document) (content "acme ltd")) (move "Work/Acme") (stop))
(rule "images" (when (type image)) (move "Pictures"))
(rule "rest" (when (not (matched)) (type text)) (move "Other"))
diff --git a/internal/engine/nodelete_test.go b/internal/engine/nodelete_test.go
new file mode 100644
index 0000000..80d9ed9
--- /dev/null
+++ b/internal/engine/nodelete_test.go
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package engine
+
+import (
+ "context"
+ "io/fs"
+ "os"
+ "path/filepath"
+ "sort"
+ "strings"
+ "testing"
+ "time"
+
+ "krino/internal/journal"
+ "krino/internal/plan"
+)
+
+// sameBytes is the content every file in these tests shares.
+const sameBytes = "%PDF identical bytes"
+
+// dlTree creates ~/dl in a sandbox. Each file holds body and is modified
+// the given number of hours after a fixed old time, so the smallest number
+// is the oldest file. PATH is emptied so no extraction tool runs.
+func dlTree(t *testing.T, files map[string]int, body string) (home, dl string) {
+ t.Helper()
+ home = sandbox(t)
+ t.Setenv("PATH", t.TempDir())
+ dl = filepath.Join(home, "dl")
+ old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)
+ for rel, hours := range files {
+ p := filepath.Join(dl, rel)
+ if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil {
+ t.Fatal(err)
+ }
+ if err := os.WriteFile(p, []byte(body), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ mt := old.Add(time.Duration(hours) * time.Hour)
+ if err := os.Chtimes(p, mt, mt); err != nil {
+ t.Fatal(err)
+ }
+ }
+ return home, dl
+}
+
+// planAndApply configures ~/dl (recursive, min-age 0s) with rules, plans
+// it, approves and applies every chain, and returns the plan.
+func planAndApply(t *testing.T, home, rules string) *DirPlan {
+ t.Helper()
+ conf := "(path \"~/dl\")\n(recursive yes)\n(min-age 0s)\n" + rules
+ main := writeConfig(t, home, `(include "dl")`, map[string]string{"dl": conf})
+ e, errs := Load(main)
+ if len(errs) > 0 {
+ t.Fatal(errs)
+ }
+ dp, err := e.Plan(context.Background(), e.Dirs[0], plan.NewClaims())
+ if err != nil {
+ t.Fatal(err)
+ }
+ approved := map[string]bool{}
+ for _, c := range dp.Chains {
+ approved[c.File.Rel] = true
+ }
+ j, err := journal.Open(filepath.Join(home, ".local", "state", "krino", "krino.log"))
+ if err != nil {
+ t.Fatal(err)
+ }
+ defer j.Close()
+ res, err := e.Apply(context.Background(), dp, approved, j, journal.NewRunID(time.Now()))
+ if err != nil {
+ t.Fatal(err)
+ }
+ if res.Failed != 0 {
+ t.Fatalf("%d files failed: %+v", res.Failed, res)
+ }
+ return dp
+}
+
+// assertCopies checks that exactly the files want (paths relative to dl,
+// in any order) hold body after the run.
+func assertCopies(t *testing.T, dl, body string, want []string) {
+ t.Helper()
+ var got []string
+ err := filepath.WalkDir(dl, func(p string, d fs.DirEntry, err error) error {
+ if err != nil || !d.Type().IsRegular() {
+ return err
+ }
+ if b, err := os.ReadFile(p); err == nil && string(b) == body {
+ rel, _ := filepath.Rel(dl, p)
+ got = append(got, filepath.ToSlash(rel))
+ }
+ return nil
+ })
+ if err != nil {
+ t.Fatal(err)
+ }
+ sort.Strings(got)
+ w := append([]string(nil), want...)
+ sort.Strings(w)
+ if strings.Join(got, "\n") != strings.Join(w, "\n") {
+ t.Errorf("copies on disk = %q, want %q", got, w)
+ }
+}
+
+// assertSkip checks that rel's chain has a step from rule skipped with a
+// reason starting with want.
+func assertSkip(t *testing.T, dp *DirPlan, rel, rule, want string) {
+ t.Helper()
+ for _, c := range dp.Chains {
+ if c.File.Rel != rel {
+ continue
+ }
+ for _, s := range c.Steps {
+ if s.Rule == rule && strings.HasPrefix(s.Skip, want) {
+ return
+ }
+ }
+ t.Errorf("%s: no step of rule %q skipped with %q; steps %+v", rel, rule, want, c.Steps)
+ return
+ }
+ t.Errorf("%s: no chain in the plan", rel)
+}
+
+// TestNoDeleteThroughMatched: a later rule deleting through (matched)
+// cannot delete what an earlier duplicate rule found.
+func TestNoDeleteThroughMatched(t *testing.T) {
+ h, dl := dlTree(t, map[string]int{"a.pdf": 0, "b.pdf": 1}, sameBytes)
+ dp := planAndApply(t, h, `
+(rule "dupes" (when (duplicate)) (move "Dupes"))
+(rule "cleanup" (when (matched)) (delete permanent))
+`)
+ assertCopies(t, dl, sameBytes, []string{"a.pdf", "Dupes/b.pdf"})
+ assertSkip(t, dp, "b.pdf", "cleanup", NeverDeleted)
+}
+
+// TestNoDeleteThroughAnotherRulesCondition: a rule with no duplicate test
+// of its own deletes the original, but not the duplicate.
+func TestNoDeleteThroughAnotherRulesCondition(t *testing.T) {
+ h, dl := dlTree(t, map[string]int{"a.pdf": 0, "b.pdf": 1}, sameBytes)
+ dp := planAndApply(t, h, `
+(rule "pdfs" (when (type pdf)) (delete permanent))
+(rule "dupes" (when (duplicate)) (move "Dupes"))
+`)
+ assertCopies(t, dl, sameBytes, []string{"Dupes/b.pdf"})
+ assertSkip(t, dp, "b.pdf", "pdfs", NeverDeleted)
+}
+
+// TestNoDeleteWhenEvaluationSkippedTheDuplicateTest: "dupes" tests size
+// first, which is false, so its duplicate test is never evaluated; b.pdf
+// is still a duplicate under that scope, so "pdfs" cannot delete it.
+func TestNoDeleteWhenEvaluationSkippedTheDuplicateTest(t *testing.T) {
+ h, dl := dlTree(t, map[string]int{"a.pdf": 0, "b.pdf": 1}, sameBytes)
+ dp := planAndApply(t, h, `
+(rule "dupes" (when (size > 1G) (duplicate)) (move "Dupes"))
+(rule "pdfs" (when (type pdf)) (delete permanent))
+`)
+ assertCopies(t, dl, sameBytes, []string{"b.pdf"})
+ assertSkip(t, dp, "b.pdf", "pdfs", NeverDeleted)
+}
+
+// TestNoDeleteWithTwoScopesWhoseOriginalsDiffer: under "Archive" the
+// original is Archive/x.pdf, under the plain scope it is the older
+// x-copy.pdf, so each file is a duplicate somewhere. Neither is deleted;
+// both are moved aside.
+func TestNoDeleteWithTwoScopesWhoseOriginalsDiffer(t *testing.T) {
+ h, dl := dlTree(t, map[string]int{"Archive/x.pdf": 1, "x-copy.pdf": 0}, sameBytes)
+ dp := planAndApply(t, h, `
+(rule "pdfs" (when (type pdf)) (delete permanent))
+(rule "archive-dupes" (when (duplicate "Archive")) (move "Dupes") (stop))
+(rule "local-dupes" (when (duplicate)) (move "Dupes") (stop))
+`)
+ assertCopies(t, dl, sameBytes, []string{"Dupes/x.pdf", "Dupes/x-copy.pdf"})
+ assertSkip(t, dp, "Archive/x.pdf", "pdfs", NeverDeleted)
+ assertSkip(t, dp, "x-copy.pdf", "pdfs", NeverDeleted)
+}
+
+// TestDeleteWithoutDuplicateTestsStillDeletes: the guarantee applies only
+// where a directory's rules use (duplicate); a plain delete rule still does
+// what it says.
+func TestDeleteWithoutDuplicateTestsStillDeletes(t *testing.T) {
+ h, dl := dlTree(t, map[string]int{"a.pdf": 0, "b.pdf": 1}, sameBytes)
+ planAndApply(t, h, `(rule "pdfs" (when (type pdf)) (delete permanent))`)
+ assertCopies(t, dl, sameBytes, nil)
+}
+
+// TestNoDeleteWhenTheDuplicateCheckFails: b.pdf cannot be read, so krino
+// cannot show it is not a duplicate; its delete is skipped rather than
+// guessed. a.pdf, whose only candidate could not be hashed, is not a
+// duplicate and is deleted.
+func TestNoDeleteWhenTheDuplicateCheckFails(t *testing.T) {
+ if os.Geteuid() == 0 {
+ t.Skip("root reads files regardless of their mode")
+ }
+ h, dl := dlTree(t, map[string]int{"a.pdf": 0, "b.pdf": 1}, sameBytes)
+ b := filepath.Join(dl, "b.pdf")
+ if err := os.Chmod(b, 0); err != nil {
+ t.Fatal(err)
+ }
+ t.Cleanup(func() { os.Chmod(b, 0o644) })
+ dp := planAndApply(t, h, `
+(rule "dupes" (when (size > 1G) (duplicate)) (move "Dupes"))
+(rule "pdfs" (when (type pdf)) (delete))
+`)
+ assertSkip(t, dp, "b.pdf", "pdfs", "duplicate check failed, so not deleted: ")
+ if _, err := os.Lstat(b); err != nil {
+ t.Errorf("b.pdf is gone: %v", err)
+ }
+ if _, err := os.Lstat(filepath.Join(dl, "a.pdf")); !os.IsNotExist(err) {
+ t.Errorf("a.pdf was not deleted: %v", err)
+ }
+}
diff --git a/internal/engine/plan.go b/internal/engine/plan.go
index 979bec1..d9ffeb3 100644
--- a/internal/engine/plan.go
+++ b/internal/engine/plan.go
@@ -52,7 +52,7 @@ func (e *Engine) Plan(ctx context.Context, d *Dir, claims *plan.Claims) (*DirPla
Reasons: rm.Reasons,
}
}
- inputs[i] = plan.Input{File: fm.File, Rules: rules}
+ inputs[i] = plan.Input{File: fm.File, Rules: rules, NoDelete: fm.NoDelete}
}
chains := plan.Build(d.Root, inputs, e.Now(), plan.OS{}, claims)
diff --git a/internal/plan/chain.go b/internal/plan/chain.go
index bfa2484..e504504 100644
--- a/internal/plan/chain.go
+++ b/internal/plan/chain.go
@@ -32,6 +32,10 @@ func NewClaims() *Claims {
type Input struct {
File scan.File
Rules []RuleMatch
+ // NoDelete, when non-empty, skips every delete step of this file with
+ // this text as the step's Skip, without ending the chain (spec §5.5
+ // rule 2, §7.1).
+ NoDelete string
}
// Build turns each file's matching rules into a chain. root is the
@@ -166,6 +170,10 @@ func buildOne(root string, in Input, now time.Time, d Disk, claim claimed) Chain
}
case config.Delete, config.DeletePermanent:
+ if in.NoDelete != "" {
+ step.Skip = in.NoDelete
+ break
+ }
deletedBy = rule.Name
}
diff --git a/internal/plan/chain_test.go b/internal/plan/chain_test.go
index b6e3dfe..28c208f 100644
--- a/internal/plan/chain_test.go
+++ b/internal/plan/chain_test.go
@@ -141,3 +141,31 @@ func TestBuildKeepsSteplessChains(t *testing.T) {
}
}
}
+
+// TestBuildNoDeleteSkipsDeletesAndContinues is spec §5.5 rule 2 and §7.1:
+// with Input.NoDelete set, every delete step is skipped with that reason,
+// and the chain carries on from the file's current path instead of ending.
+func TestBuildNoDeleteSkipsDeletesAndContinues(t *testing.T) {
+ in := []Input{{
+ File: file("/r", "b.pdf"),
+ NoDelete: "a duplicate is never deleted",
+ Rules: []RuleMatch{
+ {Name: "old", Actions: []config.Action{act(config.DeletePermanent, "")}},
+ {Name: "dupes", Actions: []config.Action{act(config.Move, "Dupes")}},
+ {Name: "cleanup", Actions: []config.Action{act(config.Delete, "")}},
+ },
+ }}
+ steps := Build("/r", in, time.Now(), NoDisk{}, NewClaims())[0].Steps
+ if len(steps) != 3 {
+ t.Fatalf("steps = %+v", steps)
+ }
+ if steps[0].Kind != DeletePermanent || steps[0].Skip != "a duplicate is never deleted" {
+ t.Errorf("step 0 = %+v, want a skipped permanent delete", steps[0])
+ }
+ if steps[1].Kind != Move || steps[1].Skip != "" || steps[1].Dst != "/r/Dupes/b.pdf" {
+ t.Errorf("step 1 = %+v, want the move to run", steps[1])
+ }
+ if steps[2].Kind != Trash || steps[2].Skip != "a duplicate is never deleted" || steps[2].Src != "/r/Dupes/b.pdf" {
+ t.Errorf("step 2 = %+v, want a skipped trash from the moved path", steps[2])
+ }
+}
diff --git a/man/krino.conf.5 b/man/krino.conf.5
index fbd1c06..0778779 100644
--- a/man/krino.conf.5
+++ b/man/krino.conf.5
@@ -254,6 +254,20 @@ A rule with only
.Ic (stop)
is an exclusion: files it matches receive no actions from later rules.
.Pp
+A rule whose condition contains
+.Ic (duplicate)
+anywhere, including inside
+.Ic or
+or
+.Ic not ,
+cannot contain
+.Ic (delete)
+or
+.Ic (delete permanent) ;
+.Ic krino check
+and every run refuse it
+.Pq Sx DUPLICATES .
+.Pp
.Ar dest
is a directory: a relative path is relative to the root,
.Ql ~
@@ -462,6 +476,11 @@ its directory, and later steps use the new path;
.Ic delete
ends the chain, and steps after it are shown in the plan as
.Dq skipped: deleted by rule Ar x .
+A
+.Ic delete
+of a duplicate is itself skipped
+.Pq Sx DUPLICATES
+and does not end the chain.
.Pp
The plan warns when a chain moves a file more than once; that is usually
a missing
@@ -609,54 +628,62 @@ between
and
.Dq this should not exist twice
exists only in the user's intent, not in the files.
-Pairing
-.Ic (duplicate)
-with
-.Ic (delete)
-over a directory that may hold intentional copies
+A
+.Ic move
+rule over a directory that holds intentional copies
.Pq a mirror, a staging queue, anything another tool manages
-will delete things the user meant to keep.
-Keep such a rule on a directory scoped narrowly enough that every
-byte-identical pair in it really is an accident.
+will move things the user meant to keep there; keep such a rule on a
+directory scoped narrowly enough that every byte-identical pair in it really
+is an accident.
.Pp
-.Sy Warning:
-each
+.Sy Duplicates are found, never deleted.
+Deciding which copy to remove is left to the user, or to a tool built for it
+such as
+.Xr jdupes 1 .
+A rule combining
.Ic (duplicate)
-or
-.Ic (duplicate Ar dir )
-condition elects its own original from its own candidates; two conditions
-with different scopes can therefore elect
-.Em different
-originals for the same content.
-Within one directory's rules
-.Pq one rule combining conditions with Ic or , or two separate rules ,
-this can make every copy in a group selected by some condition, and with
-.Ic (delete permanent)
-every copy selected is gone for good.
-.Ic krino
-plans and applies one directory at a time, so a directory already applied
-in the run stays applied while a later directory is planned; a
-.Fl n
-run only ever plans, so it shows every directory's plan as if none of the
-others had been applied.
-Two files in different directories that a
-.Fl n
-plan each lists as a duplicate of the other are therefore not necessarily
-both deleted when the same run is applied with
-.Fl y :
-deleting the first can remove the very original the second file was a
-duplicate of, leaving the second no longer a duplicate by the time its own
-directory is planned.
-Use one duplicate scope for rules that delete within a directory; prefer
-.Ic (delete)
-to
-.Ic (delete permanent)
-with
-.Ic (duplicate) ;
-and within one directory's rules, two files each listed as a duplicate of
-the other in the
-.Fl n
-plan means both would be deleted.
+with a delete action is refused
+.Pq Sx RULES .
+And in a directory whose rules use
+.Ic (duplicate) ,
+a file that is a duplicate under any of the duplicate scopes those rules use
+\(em each distinct set of
+.Ar dir
+arguments, and the plain
+.Ic (duplicate) ,
+looked up for the file whether or not evaluation reached that test \(em gets
+no delete step from any rule.
+The plan shows the step as
+.Dq skipped: a duplicate is never deleted ,
+or, when that lookup fails,
+.Dq skipped: duplicate check failed, so not deleted: Ar reason ;
+the rest of the chain continues from the file's current path.
+This covers what the refusal cannot see: a later rule deleting through
+.Ic (matched)
+or through a condition of its own.
+.Pp
+Together they mean no rule can delete every copy of content a duplicate test
+in that directory can see.
+A file displaced by
+.Ic (on-conflict overwrite)
+is not covered; it goes to the trash, and
+.Ic krino undo
+restores it.
+.Pp
+The way to deal with duplicates is to move them aside and decide later:
+.Bd -literal -offset indent
+(rule "dupes"
+ (when (duplicate "~/docs/Archive"))
+ (move "~/.dupes/")
+ (stop))
+.Ed
+.Pp
+Every move is logged, so
+.Ic krino undo
+puts them back.
+Duplicate conditions with different scopes do not share an original, so two
+such rules can between them move every copy of a group aside; nothing is
+deleted.
.Sh KNOWN LIMITATIONS
A
.Ar dest
@@ -682,12 +709,6 @@ Give such a rule its own narrow
pattern, or a
.Ar dest
whose first path component is a literal string, when that matters.
-.Pp
-.Ic (duplicate)
-conditions with different scopes do not share an original: two conditions
-electing from different candidate sets can each treat a different file as
-the original of the same content
-.Pq Sx DUPLICATES .
.Sh SEE ALSO
.Xr krino 1
.Pp