From 24b0fc486dceab8025bc0b3c2ec8339699e68261 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 6 Nov 2026 19:56:33 +0100 Subject: [PATCH] Resolve scan operands to their real paths (closes #144) scan now resolves each operand to its real path, every symlink followed, before the walk, as sneak ruled. Files are recorded and warnings printed under that path. With /srv/link a symlink to a directory outside /srv, scan /srv /srv/link/sub now walks sub on its own and keeps its records, where before it dropped sub and deleted them. A symlink operand is scanned at its target. A dangling symlink is a fatal error, exit 1. A .. after a symlink counts from the symlink's target, as realpath does. The .zfs rule applies to the real path. The walk still follows no symlink it finds. Judgement call: records an earlier scan stored under a path through a symlink are not migrated; there is no installed base. Model: opus-5-5 --- README.md | 69 ++++++----- TODO.md | 6 + db.go | 43 ++++--- exclude_test.go | 2 +- main_test.go | 318 +++++++++++++++++++++++++++++++++++++++--------- scan.go | 57 ++++----- scan_test.go | 65 +++++----- 7 files changed, 396 insertions(+), 164 deletions(-) diff --git a/README.md b/README.md index 6b7e6cc..121afcd 100644 --- a/README.md +++ b/README.md @@ -348,17 +348,26 @@ default. An operand may be a directory or a regular file. - `scan` with no operand is a usage error. The usage text goes to stderr, and the exit code is 2. -- An operand that does not exist is a fatal error. `scan` then stops before it - walks anything, and exits 1. -- Each operand is made absolute before the walk. A relative operand is joined to - the working directory as `pwd` prints it, with its symlinks kept. A `.` or - `..` in it is resolved as text, and symlinks are not followed. Each record is - stored under that absolute path. +- An operand that does not exist is a fatal error, and so is a symlink whose + target does not exist. `scan` then stops before it walks anything, and exits + with code 1. +- Each operand is resolved to its real path before the walk, as `realpath` + prints it. A relative operand is joined to the working directory as `pwd -P` + prints it. Every symlink in the path is then followed. A `..` after a symlink + counts from the directory the symlink leads to. From then on the operand is + that real path. Its files are recorded under it, and warnings name it. +- An operand that is a symlink is scanned at its target's real path. After a + `cd` through a symlink, `scan .` records its files under the real path of the + working directory. - All operands belong to one scan, and one walk covers them all at once (see "Concurrency"). -- An operand that repeats another is dropped before the walk. +- An operand that repeats another is dropped before the walk. Two operands + repeat each other when their real paths are the same. - An operand under another operand is dropped the same way, unless one of the - cases below applies. No path is then walked twice. + cases below applies. No path is then walked twice. Whether one operand is + under another is judged on their real paths. With `/srv/link` a symlink to a + directory outside `/srv`, `scan /srv /srv/link/sub` walks `sub` on its own, + because the real path of `sub` is not under `/srv`. - An operand under another is kept, and walked on its own, when the inner operand or a directory between the two is one of these: - a default exclude (see "Rules for the walk"); @@ -369,25 +378,19 @@ default. An operand may be a directory or a regular file. memory (see "Rules for the walk"). The outer operand's walk does not enter such a directory, so the inner operand is walked on its own. -- No other case keeps an operand under another. An operand reached through a - symlinked directory below the outer operand is dropped too. The outer - operand's walk does not follow the symlink, so no file under the inner operand - is scanned. The records that an earlier scan left under it are deleted. The - `-x` case above also covers an operand reached that way. -- An operand that is neither a regular file nor a directory is not scanned. - `scan` prints a warning naming the path and what it is. It counts the operand - as skipped and drops it. An operand beneath a symlink operand is scanned all - the same. The scan keeps the records beneath a dropped operand, unless another - operand covers them. After a `cd` through a symlink, `scan .` names the - symlink and is skipped as a symlink operand. `scan sub` in that directory - records its files under the symlink's path. `report` can then list each as a - duplicate of the same file recorded under its real path. +- No other case keeps an operand under another. +- An operand that is neither a regular file nor a directory is not scanned. Such + an operand is, for example, a symlink to a device node. `scan` prints a + warning naming the real path and what it is. It counts the operand as skipped + and drops it. The scan keeps the records beneath a dropped operand, unless + another operand covers them. - An operand at or inside a directory named `.zfs` is never scanned, whether - given alone or under another operand. The check looks at the path as given and - at the path with its symlinks resolved. `scan` prints a warning naming the - path, counts the operand as skipped and drops it. The records beneath it are - kept, unless it lies under another operand. Then they are deleted, because - that operand's walk never enters `.zfs` (see "Rules for the walk"). + given alone or under another operand. The check looks at the real path, so a + symlink into a `.zfs` directory is not scanned either. `scan` prints a warning + naming the real path, counts the operand as skipped and drops it. The records + beneath it are kept, unless it lies under another operand. Then they are + deleted, because that operand's walk never enters `.zfs` (see "Rules for the + walk"). - An operand on a filesystem that is not persistent is dropped too. `scan` prints a line naming it and its filesystem type, and does not count it as skipped. @@ -484,12 +487,12 @@ walk practical on large or busy storage pools. The walk knows them by device ID and inode number. The device ID is the number by which the kernel tells filesystems apart. The inode number tells the files of one filesystem apart. So the walk passes over them whatever - path leads to them. The walk follows no symlink it finds. It does open each - operand at the path given, even when that path runs through a symlinked - directory. Such an operand can lead to the database files, and the walk - passes over them there as well. The walk prints no line for the database - files and does not count them as skipped. A record that an earlier scan left - for one of them is deleted. + path leads to them. The walk follows no symlink it finds. It opens each + operand at its real path, not the path given. An operand given through a + symlink can lead to the database files, and the walk passes over them there + as well. The walk prints no line for the database files and does not count + them as skipped. A record that an earlier scan left for one of them is + deleted. - By default the walk enters directories on other filesystems than its operand's. With `-x`, it never enters a directory on a different filesystem @@ -1138,7 +1141,7 @@ showing the count it stopped at. - `0`: success, even when files were skipped with warnings. - `1`: a fatal error, or a `scan` stopped by `SIGINT` or `SIGTERM`. A fatal error is one of these: - - an operand that does not exist, or whose details `scan` cannot read; + - an operand that does not exist, or whose real path `scan` cannot find; - a list of mounted filesystems that `scan` cannot read; - another `scan` holding the lock (see "The scan lock"); - a database, its directory or its lock file that cannot be created or diff --git a/TODO.md b/TODO.md index 52806ed..603a468 100644 --- a/TODO.md +++ b/TODO.md @@ -29,6 +29,12 @@ Newest first. An entry ends with its date and its issue or branch, where those are known. +- `scan` resolves each operand to its real path, symlinks followed, before the + walk. A symlink operand is scanned at its target instead of skipped. With + `/srv/link` a symlink to a directory outside `/srv`, `scan /srv /srv/link/sub` + walks `sub` on its own instead of dropping it and deleting its records + (2026-11-06, https://git.eeqj.de/sneak/sfdupes/issues/144) + - `README.md` and a comment in `scan.go` no longer say ZFS counts a new file's blocks only seconds after the write. The count can land in under a second. Both now say up to seconds after the write, as the comment on `diskSizeOf` in diff --git a/db.go b/db.go index e92f6db..a27d163 100644 --- a/db.go +++ b/db.go @@ -146,33 +146,44 @@ var errHashSize = errors.New("hash is not 32 bytes") // databasePath resolves the database location: SFDUPES_DATABASE when // set and non-empty, the compiled-in default otherwise. It returns the -// path, with the working directory, its symlinks resolved, put in front -// of it when it is relative, and where the path came from, "default" or -// "from SFDUPES_DATABASE", for the line each subcommand prints first on -// stderr. +// path, made absolute by absolutePath, and where the path came from, +// "default" or "from SFDUPES_DATABASE", for the line each subcommand +// prints first on stderr. func databasePath() (string, string, error) { path, source := defaultDatabasePath, "default" if p := os.Getenv(databaseEnv); p != "" { path, source = p, "from "+databaseEnv } - if filepath.IsAbs(path) { - return path, source, nil - } - - // The working directory as the kernel reports it, with no symlink - // in it, as README "Database" says. os.Getwd may return PWD - // instead, which can name it through a symlink. - wd, err := unix.Getwd() + abs, err := absolutePath(path) if err != nil { return "", "", fmt.Errorf("resolve database path %s: %w", path, err) } - // Not filepath.Abs or filepath.Join: they clean the path, which - // drops each ".." together with the directory name before it. When - // that directory is a symlink, the cleaned path names another file. + return abs, source, nil +} + +// absolutePath returns path unchanged when it is absolute, and +// otherwise the working directory, a "/" and path. The working +// directory is the one the kernel reports, with no symlink in it; +// os.Getwd may return PWD instead, which can name it through a symlink. +// The result is not cleaned, as filepath.Abs and filepath.Join would +// clean it. Cleaning drops each ".." together with the directory name +// before it. When that name is a symlink, the cleaned path names +// another file than the one the system reaches by following the symlink +// first. +func absolutePath(path string) (string, error) { + if filepath.IsAbs(path) { + return path, nil + } + + wd, err := unix.Getwd() + if err != nil { + return "", err + } + // The working directory ends in "/" only when it is the root. - return strings.TrimSuffix(wd, "/") + "/" + path, source, nil + return strings.TrimSuffix(wd, "/") + "/" + path, nil } // createDatabaseDirectory creates the directory the database at path diff --git a/exclude_test.go b/exclude_test.go index fb8aa67..25b593e 100644 --- a/exclude_test.go +++ b/exclude_test.go @@ -286,7 +286,7 @@ func TestRunScanTempDirOperand(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - dir := t.TempDir() + dir := realTempDir(t) if !underAnyRoot(dir, defaultExcludes(runtime.GOOS)) { t.Skip(dir + " is not inside a default exclude") } diff --git a/main_test.go b/main_test.go index 3d494ab..e00e617 100644 --- a/main_test.go +++ b/main_test.go @@ -14,6 +14,8 @@ import ( "strconv" "strings" "testing" + + "golang.org/x/sys/unix" ) // usageMarker is the first line of cobra's usage text, which a usage @@ -797,7 +799,7 @@ func TestRunVersionWriteFailureIsFatal(t *testing.T) { func scanFixture(t *testing.T) []string { t.Helper() - dir := t.TempDir() + dir := realTempDir(t) dupes := []string{ writeFile(t, dir, "one/a.bin", pattern(1, 300)), @@ -867,7 +869,7 @@ func TestRunScanSucceedsDespiteWarnings(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - dir := t.TempDir() + dir := realTempDir(t) writeFile(t, dir, "a.bin", pattern(1, 300)) // Same size as a.bin, so the scan reads it, and the read fails. @@ -895,58 +897,278 @@ func TestRunScanSucceedsDespiteWarnings(t *testing.T) { assertNoSidecars(t, path) } -func TestRunScanSkipsSymlinkOperand(t *testing.T) { - path := testDBPath(t) - t.Setenv(databaseEnv, path) +// realTempDir returns a new temporary directory by its real path, so +// tests hold where the temporary directory is reached through a +// symlink. +func realTempDir(t *testing.T) string { + t.Helper() - dir := t.TempDir() - writeFile(t, dir, "target/sub/f", pattern(1, 10)) - - link := filepath.Join(dir, "link") - - err := os.Symlink(filepath.Join(dir, "target"), link) + dir, err := filepath.EvalSymlinks(t.TempDir()) if err != nil { t.Fatal(err) } - // Scanning a directory through the symlink stores a record beneath - // the symlink's own path for a file beneath its target. - scanOK(t, filepath.Join(link, "sub")) - - assertOperandSkipped(t, path, link, "symlink", - filepath.Join(link, "sub", "f")) + return dir } +// symlink creates link pointing at target. +func symlink(t *testing.T, target, link string) { + t.Helper() + + err := os.Symlink(target, link) + if err != nil { + t.Fatal(err) + } +} + +// TestRunScanFollowsSymlinkOperand scans link, a symlink to target. +// The operand is scanned at its real path, and its file is recorded +// there. +func TestRunScanFollowsSymlinkOperand(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + f := writeFile(t, dir, "target/sub/f", pattern(1, 10)) + + link := filepath.Join(dir, "link") + symlink(t, filepath.Join(dir, "target"), link) + + stderr := scanOK(t, link) + + summary := "scan: 1 files seen (1 added, 0 updated, 0 removed, " + + "0 unchanged), 0 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + + if got := storedPaths(t, path); !slices.Equal(got, []string{f}) { + t.Errorf("records = %q, want %q", got, []string{f}) + } +} + +// TestRunScanWalksOperandUnderSymlinkOperand scans link and link/sub, +// with link a symlink to target. Both resolve under target, so +// target/sub is walked once, as part of target. func TestRunScanWalksOperandUnderSymlinkOperand(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - dir := t.TempDir() - writeFile(t, dir, "target/sub/f", pattern(1, 10)) + dir := realTempDir(t) + f := writeFile(t, dir, "target/sub/f", pattern(1, 10)) link := filepath.Join(dir, "link") + symlink(t, filepath.Join(dir, "target"), link) - err := os.Symlink(filepath.Join(dir, "target"), link) + stderr := scanOK(t, link, filepath.Join(link, "sub")) + + summary := "scan: 1 files seen (1 added, 0 updated, 0 removed, " + + "0 unchanged), 0 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + + if got := storedPaths(t, path); !slices.Equal(got, []string{f}) { + t.Errorf("records = %q, want %q", got, []string{f}) + } +} + +// TestRunScanWalksSymlinkedOperandBelowOperand is the case of +// https://git.eeqj.de/sneak/sfdupes/issues/144: scan dir dir/link/sub, +// with dir/link a symlink to a directory outside dir. The walk of dir +// does not follow link, so dir/link/sub is walked on its own at its +// real path, and the record an earlier scan stored there survives. +func TestRunScanWalksSymlinkedOperandBelowOperand(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + a := writeFile(t, dir, "a", pattern(1, 10)) + + outside := realTempDir(t) + f := writeFile(t, outside, "sub/f", pattern(2, 10)) + + symlink(t, outside, filepath.Join(dir, "link")) + + scanOK(t, filepath.Join(outside, "sub")) + + stderr := scanOK(t, dir, filepath.Join(dir, "link", "sub")) + + summary := "scan: 2 files seen (1 added, 0 updated, 0 removed, " + + "1 unchanged), 0 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + + want := []string{a, f} + slices.Sort(want) + + if got := storedPaths(t, path); !slices.Equal(got, want) { + t.Errorf("records = %q, want %q", got, want) + } +} + +// TestRunScanDropsSymlinkOperandInsideOperand scans dir and link, with +// link, outside dir, a symlink to dir/real. link resolves inside dir, +// so it is dropped as covered by dir, and its file is recorded once. +func TestRunScanDropsSymlinkOperandInsideOperand(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + f := writeFile(t, dir, "real/f", pattern(1, 10)) + + link := filepath.Join(realTempDir(t), "link") + symlink(t, filepath.Join(dir, "real"), link) + + stderr := scanOK(t, dir, link) + + summary := "scan: 1 files seen (1 added, 0 updated, 0 removed, " + + "0 unchanged), 0 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } + + if got := storedPaths(t, path); !slices.Equal(got, []string{f}) { + t.Errorf("records = %q, want %q", got, []string{f}) + } +} + +// TestRunScanDanglingSymlinkOperandIsFatal scans a valid directory and +// a symlink whose target does not exist. The scan fails with exit 1 and +// an error naming the symlink as given, before it walks anything, so no +// database is created. +func TestRunScanDanglingSymlinkOperandIsFatal(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + writeFile(t, dir, "f", pattern(1, 10)) + + dangling := filepath.Join(realTempDir(t), "dangling") + symlink(t, filepath.Join(dir, "missing"), dangling) + + stdout := captureStdout(t) + + var stderr bytes.Buffer + + code := run([]string{cmdScan, dir, dangling}, os.Stdout, &stderr) + if code != exitFatal { + t.Errorf("run(scan %s %s) = %d, want %d", dir, dangling, code, + exitFatal) + } + + assertFatalOutput(t, stderr.String(), stdout()) + + want := "sfdupes: resolve " + dangling + ": " + if !strings.Contains(stderr.String(), want) { + t.Errorf("stderr = %q, want %q", stderr.String(), want) + } + + _, err := os.Stat(path) + if !errors.Is(err, fs.ErrNotExist) { + t.Errorf("database: stat error %v, want it missing", err) + } +} + +// TestRunScanFromSymlinkedWorkingDirectoryRecordsRealPath runs scan . +// after a cd through home/proj, a symlink to data/proj. The file is +// recorded under data/proj, its real path. +func TestRunScanFromSymlinkedWorkingDirectoryRecordsRealPath(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + f := writeFile(t, dir, "data/proj/f", pattern(1, 10)) + + err := os.Mkdir(filepath.Join(dir, "home"), 0o750) if err != nil { t.Fatal(err) } - // link is dropped as a symlink, but link/sub must still be scanned, - // not dropped as lying under link. - scanOK(t, link, filepath.Join(link, "sub")) + symlink(t, filepath.Join(dir, "data", "proj"), + filepath.Join(dir, "home", "proj")) - db, err := openDB(path, reportParams) + // t.Chdir also sets PWD to the path through the symlink, as a + // shell does after cd. + t.Chdir(filepath.Join(dir, "home", "proj")) + + scanOK(t, ".") + + if got := storedPaths(t, path); !slices.Equal(got, []string{f}) { + t.Errorf("records = %q, want %q", got, []string{f}) + } +} + +// TestRunScanFollowsSymlinkBeforeDotDot scans a path with a ".." after +// home/proj, a symlink to data/proj: home/proj/.. from dir, and .. after +// a cd through home/proj. As with realpath, the ".." counts from +// data/proj, so data is scanned and home is not. +func TestRunScanFollowsSymlinkBeforeDotDot(t *testing.T) { + for _, tc := range []struct{ wd, operand string }{ + {"", "home/proj/.."}, + {"home/proj", ".."}, + } { + t.Run(tc.operand, func(t *testing.T) { + path := testDBPath(t) + t.Setenv(databaseEnv, path) + + dir := realTempDir(t) + f := writeFile(t, dir, "data/proj/f", pattern(1, 10)) + g := writeFile(t, dir, "data/g", pattern(2, 10)) + writeFile(t, dir, "home/h", pattern(3, 10)) + symlink(t, filepath.Join(dir, "data", "proj"), + filepath.Join(dir, "home", "proj")) + + // t.Chdir also sets PWD to the path given, as a shell does + // after cd. + t.Chdir(filepath.Join(dir, tc.wd)) + + scanOK(t, tc.operand) + + want := []string{f, g} + slices.Sort(want) + + if got := storedPaths(t, path); !slices.Equal(got, want) { + t.Errorf("records = %q, want %q", got, want) + } + }) + } +} + +// TestRunScanSkipsSymlinkToFIFOOperand scans link, a symlink to a FIFO. +// The scan warns about the FIFO by its real path, counts it as skipped, +// and exits 0. +func TestRunScanSkipsSymlinkToFIFOOperand(t *testing.T) { + t.Setenv(databaseEnv, testDBPath(t)) + + dir := realTempDir(t) + fifo := filepath.Join(dir, "fifo") + + err := unix.Mkfifo(fifo, 0o600) if err != nil { t.Fatal(err) } - t.Cleanup(func() { _ = db.Close() }) + link := filepath.Join(dir, "link") + symlink(t, fifo, link) - recordByPath(t, dbRecords(t, db), filepath.Join(link, "sub", "f")) + stderr := scanOK(t, link) + + warning := "walk " + fifo + ": skipping FIFO operand\n" + if !strings.Contains(stderr, warning) { + t.Errorf("stderr = %q, want %q", stderr, warning) + } + + summary := "scan: 0 files seen (0 added, 0 updated, 0 removed, " + + "0 unchanged), 1 skipped\n" + if !strings.Contains(stderr, summary) { + t.Errorf("stderr = %q, want %q", stderr, summary) + } } func TestRunScanSkipsZFSOperand(t *testing.T) { - zfs := filepath.Join(t.TempDir(), ".zfs") + zfs := filepath.Join(realTempDir(t), ".zfs") f := writeFile(t, zfs, "snapshot/hourly/f", pattern(1, 10)) // No scan records a file inside a .zfs directory, so the record @@ -997,7 +1219,7 @@ func TestRunScanSkipsOperandInsideZFS(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - dir := t.TempDir() + dir := realTempDir(t) hourly := filepath.Join(dir, ".zfs", "snapshot", "hourly") writeFile(t, hourly, "f", pattern(1, 10)) f := writeFile(t, dir, "f", pattern(2, 10)) @@ -1041,23 +1263,19 @@ func TestRunScanSkipsOperandInsideZFS(t *testing.T) { // TestRunScanSkipsOperandThroughSymlinkIntoZFS scans snaps/daily, where // snaps is a symlink to pool/.zfs/snapshot. The operand is warned about -// by the path given, counted as skipped, and not walked. +// by its real path, counted as skipped, and not walked. func TestRunScanSkipsOperandThroughSymlinkIntoZFS(t *testing.T) { t.Setenv(databaseEnv, testDBPath(t)) - dir := t.TempDir() + dir := realTempDir(t) writeFile(t, dir, "pool/.zfs/snapshot/daily/f", pattern(1, 10)) snaps := filepath.Join(dir, "snaps") + symlink(t, filepath.Join(dir, "pool", ".zfs", "snapshot"), snaps) - err := os.Symlink(filepath.Join(dir, "pool", ".zfs", "snapshot"), snaps) - if err != nil { - t.Fatal(err) - } - - daily := filepath.Join(snaps, "daily") - stderr := scanOK(t, daily) + stderr := scanOK(t, filepath.Join(snaps, "daily")) + daily := filepath.Join(dir, "pool", ".zfs", "snapshot", "daily") warning := "walk " + daily + ": skipping operand inside a .zfs directory\n" summary := "scan: 0 files seen (0 added, 0 updated, 0 removed, " + "0 unchanged), 1 skipped\n" @@ -1069,24 +1287,19 @@ func TestRunScanSkipsOperandThroughSymlinkIntoZFS(t *testing.T) { } // TestRunScanWalksOperandThroughSymlink scans data/daily, where data is -// a symlink to a directory outside any .zfs directory. The operand is -// scanned as usual, and its file is recorded by the path given. +// a symlink to pool/data. The operand is scanned as usual, and its file +// is recorded by its real path. func TestRunScanWalksOperandThroughSymlink(t *testing.T) { path := testDBPath(t) t.Setenv(databaseEnv, path) - dir := t.TempDir() - writeFile(t, dir, "pool/data/daily/f", pattern(1, 10)) + dir := realTempDir(t) + f := writeFile(t, dir, "pool/data/daily/f", pattern(1, 10)) data := filepath.Join(dir, "data") + symlink(t, filepath.Join(dir, "pool", "data"), data) - err := os.Symlink(filepath.Join(dir, "pool", "data"), data) - if err != nil { - t.Fatal(err) - } - - daily := filepath.Join(data, "daily") - stderr := scanOK(t, daily) + stderr := scanOK(t, filepath.Join(data, "daily")) summary := "scan: 1 files seen (1 added, 0 updated, 0 removed, " + "0 unchanged), 0 skipped\n" @@ -1094,14 +1307,9 @@ func TestRunScanWalksOperandThroughSymlink(t *testing.T) { t.Errorf("stderr = %q, want %q", stderr, summary) } - db, err := openDB(path, reportParams) - if err != nil { - t.Fatal(err) + if got := storedPaths(t, path); !slices.Equal(got, []string{f}) { + t.Errorf("records = %q, want %q", got, []string{f}) } - - t.Cleanup(func() { _ = db.Close() }) - - recordByPath(t, dbRecords(t, db), filepath.Join(daily, "f")) } func TestRunReportSucceeds(t *testing.T) { diff --git a/scan.go b/scan.go index a823094..3c2ec57 100644 --- a/scan.go +++ b/scan.go @@ -209,29 +209,30 @@ func interrupted(walked int) error { return errInterrupted } -// resolveRoots converts each PATH operand to an absolute, lexically -// cleaned path (symlinks are not resolved) and verifies that it -// exists. Database records are keyed by absolute path, so scan results -// must not depend on the working directory. +// resolveRoots converts each PATH operand to its real path: absolute, +// with every symlink in it followed. Database records are keyed by +// that path, so scan results depend neither on the working directory +// nor on the symlinks an operand was given through. An operand that +// cannot be resolved, because it or a symlink's target does not exist +// or cannot be read, is a fatal error before any scanning. func resolveRoots(roots []string) ([]string, error) { - abs := make([]string, 0, len(roots)) + resolved := make([]string, 0, len(roots)) for _, root := range roots { - a, err := filepath.Abs(root) + a, err := absolutePath(root) if err != nil { return nil, fmt.Errorf("resolve %s: %w", root, err) } - // A nonexistent operand is a fatal error before any scanning. - _, err = os.Lstat(a) + r, err := filepath.EvalSymlinks(a) if err != nil { - return nil, err + return nil, fmt.Errorf("resolve %s: %w", root, err) } - abs = append(abs, a) + resolved = append(resolved, r) } - return abs, nil + return resolved, nil } // pruneRoots drops operands already covered by another operand: @@ -417,14 +418,16 @@ func (s *scanState) runPhases(ctx context.Context, roots []string, // walkableRoots returns the operands the walk can start from: regular // files, and directories not named .zfs, none of them inside a -// directory named .zfs. Every other operand is warned about, counted as -// skipped, and dropped. A dropped operand is no longer a root, so the -// records stored beneath it are treated as if it had not been given: -// those under no other root count as outside the roots and are not -// deleted as unverified. An operand on a filesystem in rules that is not -// persistent is dropped the same way, with a line saying so that is not -// a warning, and is not counted as skipped. An operand that fails lstat -// here is kept, and the walk warns about it unless it no longer exists. +// directory named .zfs. The operands are real paths from resolveRoots, +// so these checks apply to what a symlink operand points at. Every +// other operand is warned about, counted as skipped, and dropped. A +// dropped operand is no longer a root, so the records stored beneath it +// are treated as if it had not been given: those under no other root +// count as outside the roots and are not deleted as unverified. An +// operand on a filesystem in rules that is not persistent is dropped +// the same way, with a line saying so that is not a warning, and is not +// counted as skipped. An operand that fails lstat here is kept, and the +// walk warns about it unless it no longer exists. func (s *scanState) walkableRoots(roots []string, rules walkRules) []string { kept := make([]string, 0, len(roots)) @@ -894,19 +897,13 @@ func sendEvent(ctx context.Context, events chan<- walkEvent, // operandWarning returns the one-line warning for an operand the walk // does not start from, naming the path and what it is, or "" for one it // does: a regular file, or a directory not named .zfs, that is not -// inside a directory named .zfs, neither as given nor with its symlinks -// resolved. Symlinks are never followed, including as operands. +// inside a directory named .zfs. resolveRoots has already followed the +// operand's symlinks, so a symlink here is an operand replaced by one +// after it was resolved, and is not followed. func operandWarning(root string, fi fs.FileInfo) string { // The walk never enters a .zfs directory, so it starts from nothing - // inside one either, not even through a symlink. An operand whose - // symlinks cannot be resolved is checked as given. - resolved, err := filepath.EvalSymlinks(root) - if err != nil { - resolved = root - } - - if slices.Contains(strings.Split(filepath.Dir(root), "/"), ".zfs") || - slices.Contains(strings.Split(filepath.Dir(resolved), "/"), ".zfs") { + // inside one either. + if slices.Contains(strings.Split(filepath.Dir(root), "/"), ".zfs") { return fmt.Sprintf("walk %s: skipping operand inside a .zfs directory", root) } diff --git a/scan_test.go b/scan_test.go index 54bda15..298b248 100644 --- a/scan_test.go +++ b/scan_test.go @@ -1094,7 +1094,7 @@ func TestScanOneFilesystemWalksNestedFileOperand(t *testing.T) { func buildSmokeTree(t *testing.T) string { t.Helper() - dir := t.TempDir() + dir := realTempDir(t) one := pattern(10, 2000) f1 := pattern(30, 3000) f2 := pattern(40, 100) @@ -2310,33 +2310,24 @@ func TestUnderRoot(t *testing.T) { } // TestRunScanPassesOverDatabase scans a tree that holds the scan's own -// database, reached directly and through a symlinked directory. The -// walk passes over the database file, its -wal and -shm files and its -// .lock file with no line and no count, and the record an earlier scan -// left for the .lock file is deleted. A rescan of the unchanged tree -// updates nothing, and report lists none of the database's files. +// database, and in one case also a hard link to the database file. The +// walk knows the database's files by device ID and inode number, so it +// passes over the database file, its -wal and -shm files, its .lock +// file and the hard link with no line and no count. The record an +// earlier scan left for the .lock file is deleted. A rescan of the +// unchanged tree updates nothing, and report lists none of the +// database's files. func TestRunScanPassesOverDatabase(t *testing.T) { - for _, viaSymlink := range []bool{false, true} { - t.Run(fmt.Sprintf("via symlink %v", viaSymlink), func(t *testing.T) { - base := t.TempDir() - tree := filepath.Join(base, "real", "tree") - operand := tree + for _, viaHardLink := range []bool{false, true} { + t.Run(fmt.Sprintf("via hard link %v", viaHardLink), func(t *testing.T) { + tree := realTempDir(t) + hardLink := filepath.Join(tree, "hardlink") writeFile(t, tree, "a", pattern(1, 100)) writeFile(t, tree, "b", pattern(1, 100)) writeFile(t, tree, "empty1", nil) writeFile(t, tree, "empty2", nil) - if viaSymlink { - err := os.Symlink(filepath.Join(base, "real"), - filepath.Join(base, "link")) - if err != nil { - t.Fatal(err) - } - - operand = filepath.Join(base, "link", "tree") - } - dbPath := filepath.Join(tree, "db.sqlite") t.Setenv(databaseEnv, dbPath) @@ -2346,7 +2337,7 @@ func TestRunScanPassesOverDatabase(t *testing.T) { } err = applyChanges(t.Context(), db, []scanRec{ - {path: filepath.Join(operand, "db.sqlite.lock")}, + {path: filepath.Join(tree, "db.sqlite.lock")}, }, nil, nil) if err != nil { t.Fatal(err) @@ -2354,16 +2345,29 @@ func TestRunScanPassesOverDatabase(t *testing.T) { closeScanDatabase(t.Context(), db, dbPath) - assertScanSummary(t, + if viaHardLink { + err = os.Link(dbPath, hardLink) + if err != nil { + t.Fatal(err) + } + } + + stderr := assertScanSummary(t, "4 files seen (4 added, 0 updated, 1 removed, 0 unchanged)", - operand) - assertScanSummary(t, + tree) + stderr += assertScanSummary(t, "4 files seen (0 added, 0 updated, 0 removed, 4 unchanged)", - operand) + tree) + + if strings.Contains(stderr, hardLink) { + t.Errorf("scan stderr = %q, want no line naming %s", + stderr, hardLink) + } report := runStdout(t, cmdReport) if strings.Contains(report, "db.sqlite") || - !strings.Contains(report, filepath.Join(operand, "empty2")) { + strings.Contains(report, hardLink) || + !strings.Contains(report, filepath.Join(tree, "empty2")) { t.Errorf("report = %q, want the empty files and none of "+ "the database's files", report) } @@ -2388,8 +2392,9 @@ func TestRunScanPassesOverDatabaseOperands(t *testing.T) { // assertScanSummary scans operands and fails the test unless the // summary line gives counts with nothing skipped, and the line naming -// the database is the only one that names one of its files. -func assertScanSummary(t *testing.T, counts string, operands ...string) { +// the database is the only one that names one of its files. It returns +// everything the scan printed to stderr. +func assertScanSummary(t *testing.T, counts string, operands ...string) string { t.Helper() stderr := scanOK(t, operands...) @@ -2403,4 +2408,6 @@ func assertScanSummary(t *testing.T, counts string, operands ...string) { t.Errorf("scan %q: stderr = %q, want only the line naming the "+ "database to name one of its files", operands, stderr) } + + return stderr }