diff options
| -rw-r--r-- | CHANGELOG.md | 3 | ||||
| -rw-r--r-- | cmd/krino/explain.go | 3 | ||||
| -rw-r--r-- | cmd/krino/sort_test.go | 6 | ||||
| -rw-r--r-- | internal/engine/exclude_test.go | 22 | ||||
| -rw-r--r-- | internal/engine/match.go | 23 |
5 files changed, 54 insertions, 3 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c4c0bd..392d933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ could change its answer; `explain` shows `?`. A document read only in part answers the keywords found in what was read, and leaves the others unknown. +- `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. - `{1}` … `{9}` keep the name's diacritics: a folded `name` test still matches `Lodz`, but `Łódź-faktura.pdf` now files under `Łódź`, not `Lodz`. - A tool's error message is shown even when it writes more than the diff --git a/cmd/krino/explain.go b/cmd/krino/explain.go index 5f9f6de..f65df42 100644 --- a/cmd/krino/explain.go +++ b/cmd/krino/explain.go @@ -78,6 +78,9 @@ func cmdExplain(g *globals, args []string, stdout, stderr io.Writer) int { fmt.Fprintf(stdout, "rule %s: %s\n", rt.Rule.Name, status) printTrace(stdout, rt.Trace) } + if x.NoDelete != "" { + fmt.Fprintf(stdout, "\nkrino would skip deleting this file: %s\n", display(x.NoDelete)) + } return 0 } diff --git a/cmd/krino/sort_test.go b/cmd/krino/sort_test.go index 321c9e8..12edb24 100644 --- a/cmd/krino/sort_test.go +++ b/cmd/krino/sort_test.go @@ -225,4 +225,10 @@ func TestDryRunShowsNeverDeletedSkip(t *testing.T) { if !strings.Contains(out, "skipped: a duplicate is never deleted") { t.Errorf("plan lacks the skipped delete:\n%s", out) } + // explain shows it too, for the copy that is the duplicate (triage 30a). + _, outA, _ := runCLI(t, "explain", filepath.Join(dl, "a.pdf")) + _, outB, _ := runCLI(t, "explain", filepath.Join(dl, "b.pdf")) + if strings.Count(outA+outB, "a duplicate is never deleted") != 1 { + t.Errorf("explain should name the skipped delete for exactly one copy:\n%s\n%s", outA, outB) + } } diff --git a/internal/engine/exclude_test.go b/internal/engine/exclude_test.go index ceb8442..cd1cc4b 100644 --- a/internal/engine/exclude_test.go +++ b/internal/engine/exclude_test.go @@ -426,3 +426,25 @@ func TestPartlyReadableDocument(t *testing.T) { t.Errorf("no warning for the keyword the unreadable part might hold") } } + +// TestExplainLeavesTheCacheAlone: explain never writes the keyword cache - +// not even to remove one a directory without content tests no longer uses +// (triage 28e: explain runs without the directory's lock). +func TestExplainLeavesTheCacheAlone(t *testing.T) { + h, dl := excludeTree(t, map[string]string{"a.txt": "x"}) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": "(path \"~/dl\")\n(rule \"all\" (move \"Out\"))\n"}) + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + e.CacheDir = filepath.Join(h, "cache") + os.MkdirAll(e.CacheDir, 0o700) + cache := filepath.Join(e.CacheDir, "dl.cache") + os.WriteFile(cache, []byte("old"), 0o600) + if _, err := e.Explain(context.Background(), filepath.Join(dl, "a.txt")); err != nil { + t.Fatal(err) + } + if _, err := os.Stat(cache); err != nil { + t.Errorf("explain removed the cache: %v", err) + } +} diff --git a/internal/engine/match.go b/internal/engine/match.go index 162730b..0e8b3d5 100644 --- a/internal/engine/match.go +++ b/internal/engine/match.go @@ -227,6 +227,7 @@ type Explanation struct { Excludes []ExcludeTrace Excluded string // the first exclude that matches, which sets the file aside; "" when none does Rules []RuleTrace + NoDelete string // why a delete from the matching rules would be skipped (spec §5.5); "" when it would not } // Explain reports, for one file, whether krino's ordinary scan would ever @@ -266,8 +267,18 @@ func (e *Engine) Explain(ctx context.Context, path string) (*Explanation, error) excl := e.excludeDirs(d) skip := explainSkip(d, sf, excl, now) - run := newMatchRun(e, d, ctx, now, e.filesForExplain(d, sf, excl, now)) - e.openCache(run) // read only: Explain never writes the cache + // The directory is walked only for a duplicate test, the one thing that + // needs its other files (triage 21). + files := []scan.File{sf} + if len(d.DupScopes) > 0 { + files = e.filesForExplain(d, sf, excl, now) + } + run := newMatchRun(e, d, ctx, now, files) + if len(d.ContentKeywords) > 0 { + // Loaded only: openCache would remove the cache of a directory with + // no content tests, and Explain holds no lock (triage 28e). + e.openCache(run) + } f := newFacts(run, sf) var excludes []ExcludeTrace @@ -282,6 +293,7 @@ func (e *Engine) Explain(ctx context.Context, path string) (*Explanation, error) } var rules []RuleTrace + var matched []RuleMatch stoppedBy := "" for _, r := range d.Rules { if stoppedBy != "" { @@ -292,14 +304,19 @@ func (e *Engine) Explain(ctx context.Context, path string) (*Explanation, error) match := trace.Value if match { f.matched = true + matched = append(matched, RuleMatch{Rule: r}) } rules = append(rules, RuleTrace{Rule: r, Match: match, Trace: trace}) if match && r.Conf.Stop { stoppedBy = r.Name } } + noDel := "" + if len(d.DupScopes) > 0 && deletes(matched) { + noDel = noDelete(f, d.DupScopes) + } - return &Explanation{Dir: d, File: sf, Skip: skip, Excludes: excludes, Excluded: excluded, Rules: rules}, nil + return &Explanation{Dir: d, File: sf, Skip: skip, Excludes: excludes, Excluded: excluded, Rules: rules, NoDelete: noDel}, nil } // cacheFingerprint identifies what a cached keyword answer of d depends on |