diff --git a/.claude/rules/cleanup.md b/.claude/rules/cleanup.md index befc6e0..5836c1c 100644 --- a/.claude/rules/cleanup.md +++ b/.claude/rules/cleanup.md @@ -2,4 +2,4 @@ paths: ["internal/cleaner/**", "internal/fstree/**", "internal/classifier/**"] --- Changing what gets deleted, protected or traversed: read docs/SPEC.md § Classification and docs/ARCHITECTURE.md § Deletion Strategy first. -Traversal rules: docs/decisions/symlink-no-follow.md, docs/decisions/one-filesystem.md. +Traversal rules: docs/decisions/symlink-no-follow.md, docs/decisions/one-filesystem.md, docs/decisions/skip-dataless.md. diff --git a/docs/ARCHITECTURE.md b/docs/ARCHITECTURE.md index 2ccfb92..6f35d05 100644 --- a/docs/ARCHITECTURE.md +++ b/docs/ARCHITECTURE.md @@ -38,7 +38,6 @@ Three invariants keep the figures honest: - **Hard links** (`Nlink>1`) are counted once per artifact, keyed by `(dev, ino)`, so shared blocks net out across artifacts. - **APFS clones** in a `CloneAware` artifact count once; clone groups split across artifacts go to `CloneShares` so `DedupedTotal` counts them once. - **Hugging Face shared blobs** referenced by several models go to `BlobShares` so `DedupedTotal` counts them once. -- **Shared traversal**: sizing walks with `fstree`, so it counts what finding reports and deletion removes. Neither scanner family sizes inline. Each collects its artifacts first — the walk during its single pass, stat scanners via `stat` — then sizes them through one shared bounded worker pool (`sizePending`, `min(NumCPU, 8)`) so the tree-walk I/O overlaps. @@ -49,6 +48,7 @@ Finding, sizing and the pre-delete check walk through one package, so they cover - **Read each directory once.** - **Never follow symlinks.** Only the root is read through, and only for `--path` — [decision](decisions/symlink-no-follow.md) - **Stay on the root's filesystem** (like `du -x`); a directory on another device, of unknown device, or unreadable is returned as skipped — [decision](decisions/one-filesystem.md) +- **Never read a dataless directory** (cloud placeholder whose content is not on disk); it is returned as skipped — [decision](decisions/skip-dataless.md) ### Walk engine @@ -89,7 +89,7 @@ Use this pattern when a single ecosystem produces directories whose names alone Reclaiming is not always `os.RemoveAll` on a path — Docker images or simulator devices are reclaimed through vendor commands or API calls. Each `ScanResult` carries an optional `Delete *model.DeleteMethod` (kind path/command/api + display + `Run`); nil means path removal. -The cleaner applies its policy gates (protected refusal, dry-run) uniformly, then executes: `Delete.Run(ctx)` when a method is attached, otherwise path removal (trash or permanent). A method with a nil `Run` is refused rather than falling back to path removal — a misconfigured item must never delete a path its method didn't intend. Trash/permanent choice only applies to path removal. Path removal (including dry-run) is refused when `fstree` skips any directory in the item: a recursive delete does not stop at mounts. Example: the global scanner's Homebrew item (`brew cleanup`). +The cleaner applies its policy gates (protected refusal, dry-run) uniformly, then executes: `Delete.Run(ctx)` when a method is attached (example: the global scanner's Homebrew item, `brew cleanup`), otherwise path removal (trash or permanent). A method with a nil `Run` is refused rather than falling back to path removal — a misconfigured item must never delete a path its method didn't intend. Trash/permanent choice only applies to path removal. Path removal (including dry-run) is refused when `fstree` skipped any directory in the item: a mount ([decision](decisions/one-filesystem.md)), a cloud placeholder ([decision](decisions/skip-dataless.md)) or an unreadable directory. After path removal the cleaner runs the item's `Release`, if any, with the same trash/permanent removal; dry-run skips it. diff --git a/docs/SPEC.md b/docs/SPEC.md index 35c4eda..f423ed4 100644 --- a/docs/SPEC.md +++ b/docs/SPEC.md @@ -218,6 +218,15 @@ List supported ecosystems, categories, activity statuses, and safety levels. devclean list ``` +## Traversal + +Finding, sizing and the pre-delete check of `clean` read the same directories: + +- Symlinks are never followed; only the `--path` root is read through ([decision](decisions/symlink-no-follow.md)). +- A mount under the scan root is not entered; pass it as `--path` to scan it ([decision](decisions/one-filesystem.md)). +- A cloud placeholder directory (iCloud Drive, Dropbox or another File Provider folder whose content is not on disk) is not read, so a scan never downloads cloud content; a placeholder file adds nothing to `size`, only its logical size to `apparent_size` ([decision](decisions/skip-dataless.md)). +- `clean`, including `--dry-run`, refuses an artifact containing a directory it did not read (a mount, a cloud placeholder, or an unreadable directory); a dry run marks it "would fail". + ## Classification ### Categories diff --git a/docs/decisions/skip-dataless.md b/docs/decisions/skip-dataless.md new file mode 100644 index 0000000..06cc67c --- /dev/null +++ b/docs/decisions/skip-dataless.md @@ -0,0 +1,18 @@ +# Do not read cloud placeholders + +## Decision + +`fstree` does not read a directory whose `st_flags` has `SF_DATALESS` (a File Provider placeholder: iCloud Drive, Dropbox, …). It returns the directory as skipped, so the pre-delete check treats it like a directory on another filesystem ([one-filesystem.md](one-filesystem.md)). + +## Rationale + +- Reading a dataless directory makes macOS download its content and blocks the read until the download ends. One scan sat 8 minutes in a single read under `~/Library/Mobile Documents`, and scans downloaded evicted iCloud items, using the disk space devclean is meant to free. +- A placeholder takes no space on this disk, so skipping it hides nothing reclaimable. +- The flag is in the `lstat` result the walk already has, so the check adds no system call. +- Dataless files need no flag check: they have 0 allocated blocks (measured), and sizing never opens a file without blocks. + +## Rejected alternatives + +- **Excluding cloud folders by path** (`~/Library/Mobile Documents`, `~/Library/CloudStorage`): this misses Desktop and Documents synced to iCloud, and providers that sync other folders. +- **Asking Foundation whether an item is in iCloud**: this needs cgo or an external command. An iCloud item that is already downloaded reads without blocking, so the flag is the exact condition. +- **Turning materialization off for the process** (`setiopolicy_np` with `IOPOL_MATERIALIZE_DATALESS_FILES_OFF`): measured on macOS 27, an iCloud directory read still downloaded for 17 s and a file open still downloaded the file. diff --git a/internal/fstree/dataless_darwin.go b/internal/fstree/dataless_darwin.go new file mode 100644 index 0000000..a9a6fac --- /dev/null +++ b/internal/fstree/dataless_darwin.go @@ -0,0 +1,15 @@ +package fstree + +import ( + "io/fs" + "syscall" + + "golang.org/x/sys/unix" +) + +// isDataless reports whether info is a File Provider placeholder (iCloud +// Drive, Dropbox, ...) whose content is not on this disk. +func isDataless(info fs.FileInfo) bool { + sys, ok := info.Sys().(*syscall.Stat_t) + return ok && sys.Flags&unix.SF_DATALESS != 0 +} diff --git a/internal/fstree/dataless_other.go b/internal/fstree/dataless_other.go new file mode 100644 index 0000000..c1499b9 --- /dev/null +++ b/internal/fstree/dataless_other.go @@ -0,0 +1,7 @@ +//go:build !darwin + +package fstree + +import "io/fs" + +func isDataless(fs.FileInfo) bool { return false } diff --git a/internal/fstree/fstree.go b/internal/fstree/fstree.go index 51f2f6f..d57ce51 100644 --- a/internal/fstree/fstree.go +++ b/internal/fstree/fstree.go @@ -1,7 +1,8 @@ // Package fstree is the one tree traversal shared by finding artifacts, // sizing them and checking them before deletion, so all three cover the same // files. It never follows symlinks (only the root, when asked) and stays on -// the root's filesystem, like du -x. +// the root's filesystem, like du -x. It never reads a dataless directory, whose +// content is in a cloud provider and would be downloaded by the read. package fstree import ( @@ -28,6 +29,10 @@ var deviceOf = func(_ string, info fs.FileInfo) (uint64, bool) { return uint64(sys.Dev), true //nolint:unconvert // Dev is int32 on darwin, uint64 on linux } +// dataless is indirected so a test can mark a directory dataless without a +// cloud provider. +var dataless = isDataless + // Walker walks a tree depth-first, reading each directory once. Every hook is // optional. type Walker struct { @@ -46,8 +51,9 @@ type Walker struct { } // Walk walks the tree under root and returns the directories it could not -// check: on another filesystem, of unknown filesystem, or unreadable. A root -// that does not exist is an empty tree. The only error is ctx's. +// check: on another filesystem, of unknown filesystem, dataless, or +// unreadable. A root that does not exist is an empty tree. The only error is +// ctx's. func (w Walker) Walk(ctx context.Context, root string) ([]string, error) { statRoot := os.Lstat if w.FollowRoot { @@ -57,7 +63,7 @@ func (w Walker) Walk(ctx context.Context, root string) ([]string, error) { if errors.Is(err, fs.ErrNotExist) { return nil, nil } - if err != nil { + if err != nil || (info.IsDir() && dataless(info)) { return []string{root}, nil } rootDev, rootDevOK := deviceOf(root, info) @@ -95,6 +101,10 @@ func (w Walker) Walk(ctx context.Context, root string) ([]string, error) { skipped = append(skipped, child) continue } + if dataless(childInfo) { + skipped = append(skipped, child) + continue + } if rootDevOK { if dev, ok := deviceOf(child, childInfo); !ok || dev != rootDev { skipped = append(skipped, child) @@ -120,13 +130,13 @@ type UncheckedError struct { } func (e *UncheckedError) Error() string { - return fmt.Sprintf("refusing to delete %s: contains directories on another filesystem or unreadable (%s)", + return fmt.Sprintf("refusing to delete %s: contains directories on another filesystem, not downloaded from a cloud provider, or unreadable (%s)", e.Path, strings.Join(e.Dirs, ", ")) } // CheckOneFilesystem returns an *UncheckedError when the tree at path has // directories Walk could not check. A recursive delete does not stop at -// mounts, so it would remove files that were never sized or shown. +// them, so it would remove files that were never sized or shown. func CheckOneFilesystem(ctx context.Context, path string) error { skipped, err := Walker{}.Walk(ctx, path) if err != nil { diff --git a/internal/fstree/fstree_test.go b/internal/fstree/fstree_test.go index 4391fc7..0f86cf8 100644 --- a/internal/fstree/fstree_test.go +++ b/internal/fstree/fstree_test.go @@ -80,6 +80,46 @@ func TestWalk_DoesNotEnterDirWithoutDevice(t *testing.T) { } } +// fakeDataless marks directories with the given base name dataless. +func fakeDataless(t *testing.T, name string) { + t.Helper() + orig := dataless + dataless = func(info fs.FileInfo) bool { return info.Name() == name } + t.Cleanup(func() { dataless = orig }) +} + +func TestWalk_DoesNotReadDatalessDir(t *testing.T) { + root := t.TempDir() + cloud := filepath.Join(root, "cloud") + mkdirAll(t, filepath.Join(cloud, "inner")) + mkdirAll(t, filepath.Join(root, "local")) + fakeDataless(t, "cloud") + + got, skipped := entered(t, Walker{}, root) + + if want := []string{root, filepath.Join(root, "local")}; !slices.Equal(got, want) { + t.Errorf("entered = %v, want %v", got, want) + } + if want := []string{cloud}; !slices.Equal(skipped, want) { + t.Errorf("skipped = %v, want %v", skipped, want) + } +} + +func TestWalk_DatalessRootIsUnchecked(t *testing.T) { + root := filepath.Join(t.TempDir(), "cloud") + mkdirAll(t, filepath.Join(root, "inner")) + fakeDataless(t, "cloud") + + got, skipped := entered(t, Walker{}, root) + + if len(got) != 0 { + t.Errorf("entered = %v, want none", got) + } + if want := []string{root}; !slices.Equal(skipped, want) { + t.Errorf("skipped = %v, want %v", skipped, want) + } +} + func TestWalk_ReadsEachDirOnce(t *testing.T) { root := t.TempDir() mkdirAll(t, filepath.Join(root, "a", "b")) diff --git a/internal/scanner/clone.go b/internal/scanner/clone.go index cc52ad9..3b7e0a2 100644 --- a/internal/scanner/clone.go +++ b/internal/scanner/clone.go @@ -30,9 +30,9 @@ type cloneInfo struct { } type cloneGroup struct { - model.CloneShare // Blocks: st_blocks of one member - path string // one member, for reading its extents - size int64 + model.CloneShare // Blocks: st_blocks of one member + path string // one member, for reading its extents + info fs.FileInfo // of path } // extent is a physical byte range [start, end) on a device. @@ -129,7 +129,7 @@ func (c *cloneSources) get(ctx context.Context, path string) (extents []extent, continue } if info, err := e.Info(); err == nil { - if fx, ok := fileExtents(filepath.Join(dir, e.Name()), info.Size()); ok { + if fx, ok := fileExtents(filepath.Join(dir, e.Name()), info); ok { xs = append(xs, fx...) } } diff --git a/internal/scanner/clone_darwin.go b/internal/scanner/clone_darwin.go index e96b95e..6cfb3cf 100644 --- a/internal/scanner/clone_darwin.go +++ b/internal/scanner/clone_darwin.go @@ -2,6 +2,8 @@ package scanner import ( "encoding/binary" + "io/fs" + "syscall" "unsafe" "golang.org/x/sys/unix" @@ -13,9 +15,12 @@ const ( ) // fileExtents returns a file's physical extents via fcntl(F_LOG2PHYS_EXT). ok -// is false when a lookup fails (e.g. a compressed file). -func fileExtents(path string, size int64) (xs []extent, ok bool) { - if size == 0 { +// is false when a lookup fails (e.g. a compressed file). A file with no +// allocated blocks has none and is not opened: opening a cloud placeholder +// would download it. +func fileExtents(path string, info fs.FileInfo) (xs []extent, ok bool) { + size := info.Size() + if sys, isStat := info.Sys().(*syscall.Stat_t); size == 0 || (isStat && sys.Blocks == 0) { return nil, true } fd, err := unix.Open(path, unix.O_RDONLY|unix.O_CLOEXEC, 0) diff --git a/internal/scanner/clone_other.go b/internal/scanner/clone_other.go index bfafd21..552502b 100644 --- a/internal/scanner/clone_other.go +++ b/internal/scanner/clone_other.go @@ -2,10 +2,12 @@ package scanner +import "io/fs" + // cloneSizingSupported is false off macOS: APFS clones are macOS-only, and // other filesystems' shared extents (btrfs, XFS reflinks) are not read. const cloneSizingSupported = false -func fileExtents(string, int64) ([]extent, bool) { return nil, false } +func fileExtents(string, fs.FileInfo) ([]extent, bool) { return nil, false } func fileCloneInfo(string) (cloneInfo, bool) { return cloneInfo{}, false } diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 882c90b..14ec725 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -237,13 +237,13 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri if groups == nil { groups = make(map[uint64]*cloneGroup) } - g = &cloneGroup{CloneShare: model.CloneShare{Refcnt: ci.refcnt, Blocks: blocks}, path: full, size: info.Size()} + g = &cloneGroup{CloneShare: model.CloneShare{Refcnt: ci.refcnt, Blocks: blocks}, path: full, info: info} groups[ci.id] = g } g.Seen++ groupBlocks += blocks case !ok || ci.mayShare: - if xs, ok := fileExtents(full, info.Size()); ok { + if xs, ok := fileExtents(full, info); ok { extents = append(extents, xs...) extentBlocks += blocks } @@ -288,7 +288,7 @@ func measure(ctx context.Context, path string, clones *cloneSources, source stri } // A partly rewritten copy may overlap the group: count by extent. if len(extents) > 0 { - if xs, ok := fileExtents(g.path, g.size); ok { + if xs, ok := fileExtents(g.path, g.info); ok { extents = append(extents, xs...) continue }