diff options
| -rw-r--r-- | CHANGELOG.md | 5 | ||||
| -rw-r--r-- | internal/engine/facts.go | 38 | ||||
| -rw-r--r-- | internal/engine/match_test.go | 44 |
3 files changed, 77 insertions, 10 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c34962c..af4124c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,8 +43,9 @@ name's continuation lines - in the plan, and in the per-file headings of review and undo - start past the label column, so a name cannot pass for a step line. -- Duplicate warnings share one form, `duplicate: PATH: cause`, with the - path shortened to `~` and not repeated inside the cause. +- Duplicate warnings, for the directory and on a rule, name the path + shortened to `~` and do not repeat it inside the cause + (`duplicate: ~/dl/a.txt: open: permission denied`). - `--json` says which exclude set a file aside, carries the warnings raised while matching, and lists unmatched files that raised one. - A literal `{{` or `}}` in a destination is text, not a placeholder: the diff --git a/internal/engine/facts.go b/internal/engine/facts.go index 89bd71b..641f1ef 100644 --- a/internal/engine/facts.go +++ b/internal/engine/facts.go @@ -5,6 +5,7 @@ package engine import ( "context" "errors" + "fmt" "io/fs" "path/filepath" "sort" @@ -83,18 +84,39 @@ func (run *matchRun) drainDupErrors() { // PATH: cause", the path abbreviated like every other one shown, and an OS // error's cause without the raw path it would repeat. func dupWarning(err error) string { + return "duplicate: " + dupCause(err).Error() +} + +// dupCause is a duplicate check's error as "PATH: cause", the path +// shortened with xdg.Abbrev and not repeated inside the cause - a candidate +// that could not be hashed, or an OS error on the file itself (plan 11 +// review L7). It still unwraps to err. +func dupCause(err error) error { + path, cause := "", err var ce dup.CandidateError - if !errors.As(err, &ce) { - return "duplicate: " + err.Error() - } - cause := ce.Err.Error() var pe *fs.PathError - if errors.As(ce.Err, &pe) { - cause = pe.Op + ": " + pe.Err.Error() + switch { + case errors.As(err, &ce): + path, cause = ce.Path, ce.Err + if errors.As(ce.Err, &pe) { + cause = fmt.Errorf("%s: %w", pe.Op, pe.Err) + } + case errors.As(err, &pe): + path, cause = pe.Path, fmt.Errorf("%s: %w", pe.Op, pe.Err) + default: + return err } - return "duplicate: " + xdg.Abbrev(ce.Path) + ": " + cause + return shortenedErr{msg: xdg.Abbrev(path) + ": " + cause.Error(), err: err} +} + +type shortenedErr struct { + msg string + err error } +func (e shortenedErr) Error() string { return e.msg } +func (e shortenedErr) Unwrap() error { return e.err } + // dupIndex returns the shared *dup.Index for the resolved, sorted extra // directories named by key, building it exactly once across every // concurrent caller that asks for the same key. @@ -263,7 +285,7 @@ func (f *facts) Duplicate(dirs []string) (string, bool, error) { idx := f.run.dupIndex(key, sorted) orig, isDup, err := idx.Lookup(f.file.Path) if err != nil { - return "", false, err + return "", false, dupCause(err) } if !isDup { return "", false, nil diff --git a/internal/engine/match_test.go b/internal/engine/match_test.go index 93311df..63bb54a 100644 --- a/internal/engine/match_test.go +++ b/internal/engine/match_test.go @@ -416,3 +416,47 @@ func TestDuplicateWarningsShareOneFormat(t *testing.T) { } } } + +// TestRuleDuplicateWarningShortensThePath: a duplicate check that fails on +// the file itself is reported on the rule with the path shortened and not +// repeated raw inside the cause, like the directory-level warnings (plan 11 +// review L7). +func TestRuleDuplicateWarningShortensThePath(t *testing.T) { + if os.Getuid() == 0 { + t.Skip("root reads a chmod 000 file") + } + h := sandbox(t) + dl := filepath.Join(h, "dl") + os.MkdirAll(dl, 0o755) + old := time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC) + for _, n := range []string{"a.txt", "b.txt"} { + p := filepath.Join(dl, n) + os.WriteFile(p, []byte("same size"), 0o644) + os.Chtimes(p, old, old) + } + os.Chmod(filepath.Join(dl, "a.txt"), 0) + t.Cleanup(func() { os.Chmod(filepath.Join(dl, "a.txt"), 0o644) }) + main := writeConfig(t, h, `(include "dl")`, map[string]string{"dl": "(path \"~/dl\")\n(rule \"dups\" (when (duplicate)) (move \"Dupes\"))\n"}) + e, errs := Load(main) + if len(errs) > 0 { + t.Fatal(errs) + } + r, err := e.Match(context.Background(), e.Dirs[0]) + if err != nil { + t.Fatal(err) + } + found := false + for _, fm := range append(append([]FileMatch{}, r.Matched...), r.Unmatched...) { + for _, w := range fm.Warnings { + if strings.Contains(w, h) { + t.Errorf("%s: warning repeats the raw path: %q", fm.File.Rel, w) + } + if w == "dups: duplicate check failed: ~/dl/a.txt: open: permission denied" { + found = true + } + } + } + if !found { + t.Errorf("no shortened rule warning for a.txt: %+v %+v", r.Matched, r.Unmatched) + } +} |