diff --git a/CHANGELOG.md b/CHANGELOG.md index ac98a0e..0b9f14e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,11 +15,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Global Caches**: Browser Temp detection (macOS) — zombie Chromium-family code-sign clones under `/private/var/folders/*/*/X/*.code_sign_clone` left behind by force-killed browsers (headless automation like lighthouse/puppeteer), labeled with browser name and copy count. `safe` when the browser is not running; `caution` while it runs (newest copy may be in use) or for unrecognized bundle IDs. - **LLM Model Stores** scanner (`llm`) covering local model weights at fixed home paths: LM Studio (`~/.lmstudio/models`, per model) and Hugging Face hub (`~/.cache/huggingface/hub`, per model, `models--org--name` decoded to `org/name`), plus the Ollama (`~/.ollama/models`) and llamafile (`~/.llamafile`) stores as a whole. All `safe` with re-download notes; the Ollama note points to `ollama rm ` for removing individual models. Results carry a new `last_used_at` JSON field (model directory mtime; omitted when unknown), shown in the table as a dim "last used …" hint. +#### Sizing +- Sparse-aware sizing: a sparse artifact shows its real on-disk size next to the size it reports — `8.6 GB (appears as 494.4 GB)` for a `Docker.raw` image. JSON gains an `apparent_size` field alongside the disk-based `size`. + ### Changed - Project scanners (Node, Rust, Ruby, Python, Go) are now declarative rule tables executed by a single shared filesystem walk instead of five independent traversals — one pass over the scan root regardless of how many project ecosystems are active. Stat-based scanners (xcode, global, llm) are unchanged. - The scan spinner shows a single "Scanning projects..." stage for all project scanners (previously "Scanning node...", "Scanning rust...", … in sequence). Stat scanners still report under their own names. -- The project walk now sizes matched artifacts concurrently (bounded worker pool) instead of one `du` call at a time — up to ~4× faster warm-cache sizing on a many-artifact tree, with cold-scan and few-large-artifact gains varying; disk-usage numbers are unchanged. +- Artifact sizing runs in-process (`scanner.Measure`) instead of forking `du` per artifact, collecting disk (allocated blocks) and apparent (logical) size in one pass. Disk stays the primary figure for sorting, filtering and totals. +- Totals count hard-linked blocks shared across artifacts once (e.g. a pnpm store blob linked into `node_modules`), so `total_size` no longer double-counts them. Per-artifact sizes are unchanged. - The walk engine now reads each directory once and reuses those entries for both project-marker detection and recursion, instead of reading every directory twice (a `filepath.WalkDir` read plus a second `os.ReadDir`). Directory traversal — the dominant cost of scanning a large tree — is roughly 1.8× faster (~29% faster end-to-end on a workspace with tens of thousands of directories); scan results are unchanged. - Git classification now forks `git` across a bounded worker pool (`min(NumCPU, 8)`) instead of one repo at a time — root resolution, `status`/`log`, and `check-ignore` all run concurrently across repos. On a workspace spanning dozens of repos this cut the classify phase roughly in half (~1.24s → ~0.54s warm cache, ~19% faster end-to-end); protection and activity results are unchanged. - The status-badge, safety-icon, and relative-time display formatters, previously duplicated between the table output and the interactive tree selector, are now single shared functions in `internal/ui` (`StatusBadge`, `SafetyIcon`, `RelativeTime`). Table and selector rendering are unchanged; the formatters gained direct unit tests. diff --git a/README.md b/README.md index 915b18d..c452b72 100644 --- a/README.md +++ b/README.md @@ -38,10 +38,11 @@ See [Ecosystems](docs/ecosystems.md) for full detection and safety details. - Activity classification — active, recent, stale, dormant (based on git + filesystem) - Gitignore-aware protection — only git-tracked artifacts are protected - `--min-size` filter to suppress small artifacts and focus on real targets +- Accurate sizing — sparse-aware (a 460 GB `Docker.raw` using 8.6 GB shows as such) and hard-link-aware (shared blocks counted once) - Interactive tree selector for clean — select by project or individual artifact - Soft delete (Trash) by default, with force delete option - Vendor-native cleanup hooks (`xcrun simctl delete unavailable`, etc.) via `--vendor-cleanup` -- JSON output for scripting and AI agent integration +- JSON output (`--json`) for scripting and AI agents, with `apparent_size` and a deduped `total_size` - Colored terminal output with ecosystem grouping ## Install diff --git a/docs/architecture.md b/docs/architecture.md index 96f9251..5f66f53 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -45,7 +45,7 @@ type Scanner interface { } ``` -Scanners report progress via context-attached callbacks for real-time UI updates; the walk batch reports under a single "projects" label, stat scanners under their own names. Size is calculated with `du -sk` for accurate disk usage. The walk engine does not size artifacts inline — it collects every matched artifact during the single pass, then sizes them across a bounded worker pool (`min(NumCPU, 8)`) so the per-artifact `du` calls and their I/O overlap. `du` is faster per artifact than an in-process traversal (macOS uses bulk attribute syscalls), so the win comes from concurrency, not from replacing `du`. +Scanners report progress via context-attached callbacks for real-time UI updates; the walk batch reports under a single "projects" label, stat scanners under their own names. Sizing collects two figures per artifact via an in-process walk (`scanner.Measure`): **disk** (allocated blocks, `st_blocks×512` — sparse-accurate and matching `du`) and **apparent** (sum of logical file sizes). Directories contribute their own blocks to disk (real on ext4, ~0 on APFS); symlinks are never followed. Hard-linked inodes (`Nlink>1`) are counted once per artifact and recorded (keyed by `(dev, ino)`) so shared blocks can be netted out across artifacts. The walk engine does not size inline — it collects every matched artifact during the single pass, then sizes them across a bounded worker pool (`min(NumCPU, 8)`) so the traversals and their I/O overlap. Disk is the primary figure (sorting, `--min-size`, totals); apparent surfaces only when a file is materially sparse. ### Walk engine @@ -72,7 +72,9 @@ Results are sorted by (table order, path) before returning, keeping output order ### Display Units -`model.HumanSize` formats sizes with **decimal SI units** (1 KB = 1000 B). The CLI's `--min-size` flag uses the same convention by default (humanize.ParseBytes), so the threshold a user types and the size they see in output agree on the same arithmetic. Internally, `du -sk` returns binary kilobytes, but the formatting layer is decimal — so a 1 GiB directory renders as `1.1 GB` and `--min-size 1GB` will include it. Binary suffixes (`KiB`, `MiB`, `GiB`) are still accepted by `--min-size` for users who want explicit binary thresholds. +`model.HumanSize` formats sizes with **decimal SI units** (1 KB = 1000 B). The CLI's `--min-size` flag uses the same convention by default (humanize.ParseBytes), so the threshold a user types and the size they see in output agree on the same arithmetic. Internally sizes come from `st_blocks×512` (binary 512-byte units), but the formatting layer is decimal — so a 1 GiB directory renders as `1.1 GB` and `--min-size 1GB` will include it. Binary suffixes (`KiB`, `MiB`, `GiB`) are still accepted by `--min-size` for users who want explicit binary thresholds. + +**Sparse-aware display**: the table shows an artifact's real on-disk size, annotating it with the larger size the file nominally reports when that apparent size exceeds double the disk figure by more than 1 GiB — e.g. a `Docker.raw` image renders `24.0 GB (appears as 460.0 GB)`, making clear it only uses 24 GB on disk though it presents as 460 GB. The JSON output always carries `apparent_size` (`omitzero`, so dropped when zero) so agents can detect sparse files. Ordinary directories, where block-rounding leaves apparent ≤ disk, are never annotated. ### Metadata Enrichment diff --git a/docs/commands.md b/docs/commands.md index 35324ce..af35417 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -47,6 +47,36 @@ Legend: ✔ safe ⚠ caution ✖ protected ● Active ● Recent ● Stale Run 'devclean list' for details ``` +A sparse artifact is shown as `8.6 GB (appears as 494.4 GB)` — real disk size, then the size it reports. When hard-linked blocks are shared across artifacts, the total counts them once and says so. + +### JSON (`--json`) + +```json +{ + "total_size": 5583457484, + "total_count": 5, + "results": [ + { + "path": "/Users/you/workspace/my-app/node_modules", + "ecosystem": "node", + "category": "deps", + "size": 1782579200, + "apparent_size": 1690123456, + "last_modified": "2026-07-14T09:12:00Z", + "activity": "active", + "safety": "safe", + "protected": false + } + ] +} +``` + +- `size` — disk usage (allocated blocks); sparse-aware, used for sorting and `--min-size`. +- `apparent_size` — logical size; omitted when zero. Much larger than `size` for sparse files. +- `total_size` — sum of `size` with hard-linked blocks counted once. + +Also present when known: `reason`, `project_root`, `label`, `recommendation`, `last_used_at`. + ## clean Clean reclaimable disk space. See `devclean clean --help` for all flags. diff --git a/internal/integration_test.go b/internal/integration_test.go index 6060b66..efee531 100644 --- a/internal/integration_test.go +++ b/internal/integration_test.go @@ -258,7 +258,9 @@ func TestGolden_JSONOutput(t *testing.T) { // Normalize for golden file: zero out volatile fields for i := range results { results[i].Path = filepath.Base(results[i].Path) - results[i].Size = 0 // du returns block-aligned sizes + results[i].Size = 0 // block-aligned, machine-specific + results[i].ApparentSize = 0 // block-rounding slack varies by filesystem + results[i].Links = nil // inode-keyed; also keeps DedupedTotal at 0 with Size zeroed results[i].LastMod = time.Time{} // zero out to make golden file stable results[i].ProjectRoot = "" // holds the absolute temp path — machine-specific } diff --git a/internal/model/types.go b/internal/model/types.go index 3fc2f4f..4946537 100644 --- a/internal/model/types.go +++ b/internal/model/types.go @@ -74,12 +74,22 @@ type ArtifactDef struct { AlwaysSafe bool `json:"always_safe"` } +// InodeKey identifies a physical inode uniquely. Inode numbers are only unique +// per device, so any cross-artifact dedup key must include Dev — a home scan can +// span multiple filesystems (external volumes, network mounts, Docker/APFS +// containers), where two unrelated files may share an inode number. +type InodeKey struct { + Dev uint64 + Ino uint64 +} + // ScanResult represents a single scannable item on disk. type ScanResult struct { Path string `json:"path"` Ecosystem Ecosystem `json:"ecosystem"` Category Category `json:"category"` - Size int64 `json:"size"` + Size int64 `json:"size"` // disk usage: allocated blocks (st_blocks×512), sparse-aware + ApparentSize int64 `json:"apparent_size,omitzero"` // sum of logical file sizes; exceeds Size for sparse files LastMod time.Time `json:"last_modified"` Activity ActivityStatus `json:"activity"` Safety SafetyLevel `json:"safety"` @@ -89,6 +99,11 @@ type ScanResult struct { Label string `json:"label,omitempty"` // human-readable display name (e.g. "iPhone 17 Pro · iOS 26.3") Recommendation string `json:"recommendation,omitempty"` // hint for the user (e.g. "old build", "unavailable runtime") LastUsedAt time.Time `json:"last_used_at,omitzero"` // when the item itself was last used, if the scanner can tell (omitzero: zero time is dropped from JSON) + + // Links maps each hard-linked inode (Nlink>1) found in this artifact to its + // disk blocks, so a caller can dedup blocks shared across artifacts (e.g. + // pnpm store ↔ node_modules) when computing a grand total. Not serialized. + Links map[InodeKey]int64 `json:"-"` } // HumanSize returns a human-readable size string. @@ -99,9 +114,10 @@ func (r ScanResult) HumanSize() string { // HumanSize formats bytes into a human-readable string using decimal SI // units (1 KB = 1000 B). This matches the macOS Finder convention and keeps // the display aligned with the `--min-size` parser, which treats "MB" as -// 10^6 per humanize/SI convention. Internally we still derive sizes from -// `du -sk` (binary kilobytes), but the formatting layer is decimal so the -// number a user types and the number they see agree on the same threshold. +// 10^6 per humanize/SI convention. Sizes are derived internally from +// st_blocks×512 (binary 512-byte units), but the formatting layer is decimal +// so the number a user types and the number they see agree on the same +// threshold. func HumanSize(size int64) string { const ( KB = 1000 @@ -124,6 +140,30 @@ func HumanSize(size int64) string { } } +// DedupedTotal returns the total disk usage across results with blocks shared +// via hard links counted once. Each result's Size already counts its own +// hard-linked inodes once (intra-artifact); this nets out inodes that recur +// across artifacts — e.g. a pnpm store blob also hard-linked into a project's +// node_modules — so the total reflects the space actually freed by deleting +// everything shown, not an inflated sum. +func DedupedTotal(results []ScanResult) int64 { + var total int64 + seen := make(map[InodeKey]struct{}) + for _, r := range results { + total += r.Size + for key, blocks := range r.Links { + // First artifact to hold this inode keeps it (already in r.Size); + // every later artifact double-counted it, so subtract it back out. + if _, dup := seen[key]; dup { + total -= blocks + continue + } + seen[key] = struct{}{} + } + } + return total +} + // ProtectionResult holds the result of a protection analysis. type ProtectionResult struct { IsProtected bool `json:"is_protected"` diff --git a/internal/output/json.go b/internal/output/json.go index dfeccbf..511b6fb 100644 --- a/internal/output/json.go +++ b/internal/output/json.go @@ -16,13 +16,8 @@ type ScanOutput struct { // WriteJSON writes scan results as formatted JSON. func WriteJSON(w io.Writer, results []model.ScanResult) error { - var totalSize int64 - for _, r := range results { - totalSize += r.Size - } - out := ScanOutput{ - TotalSize: totalSize, + TotalSize: model.DedupedTotal(results), // nets out blocks shared via hard links TotalCount: len(results), Results: results, } diff --git a/internal/output/output_test.go b/internal/output/output_test.go index 6adbcdf..602f421 100644 --- a/internal/output/output_test.go +++ b/internal/output/output_test.go @@ -35,6 +35,86 @@ func sampleResults() []model.ScanResult { } } +// TestWriteTableSparseAnnotation verifies a materially sparse artifact renders +// its apparent size alongside disk — the point of A1. Full render-path test: +// it pins that the artifact size cell actually routes through sizeCell. +func TestWriteTableSparseAnnotation(t *testing.T) { + sparse := model.ScanResult{ + Path: "/Users/dev/proj/node_modules", + Ecosystem: model.EcoNode, + Category: model.CatDeps, + ProjectRoot: "/Users/dev/proj", + Size: 4096, // disk: sparse, almost nothing allocated + ApparentSize: 2147483649, // apparent: ~2 GiB logical + Activity: model.StatusDormant, + Safety: model.SafetySafe, + } + var buf bytes.Buffer + output.WriteTableWithOptions(&buf, []model.ScanResult{sparse}, output.TableOptions{Verbose: true}) + if out := buf.String(); !strings.Contains(out, "appears as") { + t.Errorf("sparse artifact must show its nominal size; got:\n%s", out) + } +} + +// TestWriteTableNoSparseAnnotationForDense verifies a normal artifact (apparent +// below disk from block slack) is not annotated — the threshold must not fire +// on ordinary trees. +func TestWriteTableNoSparseAnnotationForDense(t *testing.T) { + dense := model.ScanResult{ + Path: "/Users/dev/proj/node_modules", + Ecosystem: model.EcoNode, + Category: model.CatDeps, + ProjectRoot: "/Users/dev/proj", + Size: 150 * 1000 * 1000, // 150 MB disk + ApparentSize: 122 * 1000 * 1000, // apparent below disk (rounding slack) + Activity: model.StatusDormant, + Safety: model.SafetySafe, + } + var buf bytes.Buffer + output.WriteTableWithOptions(&buf, []model.ScanResult{dense}, output.TableOptions{Verbose: true}) + if out := buf.String(); strings.Contains(out, "appears as") { + t.Errorf("dense artifact must not be annotated; got:\n%s", out) + } +} + +// TestWriteJSONDedupsHardlinkedTotal pins that the JSON total_size nets out +// blocks shared across artifacts via hard links (the pnpm case), rather than +// summing the overlapping per-artifact sizes. +func TestWriteJSONDedupsHardlinkedTotal(t *testing.T) { + shared := map[model.InodeKey]int64{{Dev: 1, Ino: 7}: 400} + results := []model.ScanResult{ + {Path: "/a", Ecosystem: model.EcoNode, Category: model.CatDeps, Size: 500, Links: shared}, + {Path: "/b", Ecosystem: model.EcoNode, Category: model.CatDeps, Size: 500, Links: shared}, + } + var buf bytes.Buffer + if err := output.WriteJSON(&buf, results); err != nil { + t.Fatalf("WriteJSON error: %v", err) + } + var out output.ScanOutput + if err := json.Unmarshal(buf.Bytes(), &out); err != nil { + t.Fatalf("JSON parse error: %v", err) + } + // 500 + 500 − 400 (shared inode counted once) = 600, not 1000. + if out.TotalSize != 600 { + t.Errorf("TotalSize = %d, want 600 (deduped)", out.TotalSize) + } +} + +// TestWriteTableDedupNote verifies the table annotates its grand total when +// hard-link dedup made it smaller than the naive sum. +func TestWriteTableDedupNote(t *testing.T) { + shared := map[model.InodeKey]int64{{Dev: 1, Ino: 7}: 400} + results := []model.ScanResult{ + {Path: "/a/node_modules", Ecosystem: model.EcoNode, Category: model.CatDeps, ProjectRoot: "/a", Size: 500, Links: shared, Safety: model.SafetySafe, Activity: model.StatusDormant}, + {Path: "/b/node_modules", Ecosystem: model.EcoNode, Category: model.CatDeps, ProjectRoot: "/b", Size: 500, Links: shared, Safety: model.SafetySafe, Activity: model.StatusDormant}, + } + var buf bytes.Buffer + output.WriteTableWithOptions(&buf, results, output.TableOptions{Verbose: true}) + if out := buf.String(); !strings.Contains(out, "excludes hard-linked") { + t.Errorf("expected hard-link dedup note in total; got:\n%s", out) + } +} + func TestWriteJSON(t *testing.T) { results := sampleResults() var buf bytes.Buffer diff --git a/internal/output/table.go b/internal/output/table.go index 271dbe1..1fd9994 100644 --- a/internal/output/table.go +++ b/internal/output/table.go @@ -39,13 +39,14 @@ func WriteTableWithOptions(w io.Writer, results []model.ScanResult, opts TableOp ecoGroups = applyTopN(ecoGroups, opts.TopN) } - var grandTotal int64 + var naiveTotal int64 var grandCount int - var safeTotal int64 + var allItems []model.ScanResult for _, eg := range ecoGroups { - grandTotal += eg.totalSize + naiveTotal += eg.totalSize grandCount += len(eg.items) + allItems = append(allItems, eg.items...) projects := model.GroupByProject(eg.items) @@ -74,23 +75,26 @@ func WriteTableWithOptions(w io.Writer, results []model.ScanResult, opts TableOp // Project path fmt.Fprintf(w, " %s\n", ui.DimStyle.Render(pathutil.ShortenHome(p.Path))) - // Count safe items - for _, r := range p.Items { - if r.Safety == model.SafetySafe { - safeTotal += r.Size - } - } - // Group artifacts by sub-package subPkgs := groupBySubPackage(p.Items, p.Path) renderSubPackages(w, subPkgs, opts) } } - fmt.Fprintf( - w, "\n%s\n", - ui.TotalStyle.Render(fmt.Sprintf("Total: %s (%d items)", model.HumanSize(grandTotal), grandCount)), - ) + // Totals dedup blocks shared across artifacts via hard links (e.g. a pnpm + // store blob also linked into node_modules), so the figures reflect space + // actually freed rather than an inflated sum of overlapping artifacts. + grandTotal := model.DedupedTotal(allItems) + safeItems := model.FilterResults(allItems, func(r model.ScanResult) bool { + return r.Safety == model.SafetySafe + }) + safeTotal := model.DedupedTotal(safeItems) + + fmt.Fprintf(w, "\n%s", ui.TotalStyle.Render(fmt.Sprintf("Total: %s (%d items)", model.HumanSize(grandTotal), grandCount))) + if grandTotal < naiveTotal { + fmt.Fprintf(w, " %s", ui.DimStyle.Render("(excludes hard-linked blocks shared across items)")) + } + fmt.Fprintln(w) if safeTotal > 0 { fmt.Fprintf( w, "%s\n", @@ -259,7 +263,7 @@ func renderSubPackages(w io.Writer, subPkgs []subPackage, opts TableOptions) { w, " %s %-24s %10s%s%s\n", icon, name+" "+cat, - ui.InfoStyle.Render(model.HumanSize(r.Size)), + ui.InfoStyle.Render(sizeCell(r)), lastUsedTag(r), rec, ) @@ -274,6 +278,23 @@ func renderSubPackages(w io.Writer, subPkgs []subPackage, opts TableOptions) { } } +// sparseMinDiff is the apparent−disk gap above which a size is worth annotating +// as sparse. Below it, ordinary block-rounding slack (apparent can even fall +// under disk) would produce noise. +const sparseMinDiff = 1 << 30 // 1 GiB + +// sizeCell renders an artifact's real on-disk size, annotating it with the +// larger size the file nominally reports when it is materially sparse — +// nominal more than double disk and over sparseMinDiff larger. Example: a +// Docker.raw image shows "24.0 GB (appears as 460.0 GB)", making clear it only +// uses 24 GB on disk though it presents itself as 460 GB. +func sizeCell(r model.ScanResult) string { + if r.ApparentSize > r.Size*2 && r.ApparentSize-r.Size > sparseMinDiff { + return fmt.Sprintf("%s (appears as %s)", model.HumanSize(r.Size), model.HumanSize(r.ApparentSize)) + } + return model.HumanSize(r.Size) +} + func renderArtifactsFlat(w io.Writer, items []model.ScanResult, projectRoot string, opts TableOptions) { var collapsed int var collapsedSize int64 @@ -293,7 +314,7 @@ func renderArtifactsFlat(w io.Writer, items []model.ScanResult, projectRoot stri w, " %s %-30s %10s%s%s\n", icon, name+" "+cat, - ui.InfoStyle.Render(model.HumanSize(r.Size)), + ui.InfoStyle.Render(sizeCell(r)), lastUsedTag(r), rec, ) diff --git a/internal/scanner/global.go b/internal/scanner/global.go index ee9e025..0d08fab 100644 --- a/internal/scanner/global.go +++ b/internal/scanner/global.go @@ -176,17 +176,16 @@ func (s *GlobalScanner) Scan(ctx context.Context, root string) ([]model.ScanResu continue } - results = append(results, model.ScanResult{ + results = append(results, sized(model.ScanResult{ Path: full, Ecosystem: model.EcoGlobal, Category: c.category, - Size: DirSize(full), LastMod: ModTime(full), Safety: c.safety, ProjectRoot: filepath.Dir(full), Label: c.description, Recommendation: c.rec, - }) + })) ReportProgress(ctx, len(results)) } @@ -248,9 +247,11 @@ var codeSignCloneBrowsers = map[string]codeSignCloneBrowser{ // safe zombies; while the browser runs (checked once per browser via // ProcessRunning) or when the bundle ID is unrecognized, they are caution. // -// Sizes come from du (DirSize) and may overstate real disk usage when the -// copies are APFS clones of the installed app bundle; clone-aware measurement -// is a planned follow-up. +// Sizes come from allocated blocks (Measure) and still overstate real disk +// usage when the copies are APFS clones of the installed app bundle: clones +// have distinct inodes and each reports full blocks, so neither block counting +// nor inode dedup catches the sharing. Clone-aware measurement is a planned +// follow-up (needs APFS extent-level accounting). func (s *GlobalScanner) scanCodeSignClones(ctx context.Context, found int) []model.ScanResult { matches, err := filepath.Glob(filepath.Join(s.TmpRoot, "*", "*", "X", "*.code_sign_clone")) if err != nil { @@ -298,17 +299,16 @@ func (s *GlobalScanner) scanCodeSignClones(ctx context.Context, found int) []mod } } - out = append(out, model.ScanResult{ + out = append(out, sized(model.ScanResult{ Path: m, Ecosystem: model.EcoGlobal, Category: model.CatCache, - Size: DirSize(m), LastMod: ModTime(m), Safety: safety, ProjectRoot: filepath.Dir(m), Label: codeSignCloneLabel(m, name), Recommendation: rec, - }) + })) ReportProgress(ctx, found+len(out)) } return out diff --git a/internal/scanner/llm.go b/internal/scanner/llm.go index 0da6e3e..ce285e7 100644 --- a/internal/scanner/llm.go +++ b/internal/scanner/llm.go @@ -53,18 +53,17 @@ func (s *LLMScanner) Scan(ctx context.Context, root string) ([]model.ScanResult, return false default: } - results = append(results, model.ScanResult{ + results = append(results, sized(model.ScanResult{ Path: path, Ecosystem: model.EcoLLM, Category: model.CatCache, - Size: DirSize(path), LastMod: ModTime(path), Safety: model.SafetySafe, ProjectRoot: filepath.Dir(path), Label: label, Recommendation: rec, LastUsedAt: ModTime(path), - }) + })) ReportProgress(ctx, len(results)) return true } diff --git a/internal/scanner/scanner.go b/internal/scanner/scanner.go index 1f8ab93..37b1fbb 100644 --- a/internal/scanner/scanner.go +++ b/internal/scanner/scanner.go @@ -3,10 +3,8 @@ package scanner import ( "context" "io/fs" - "os" - "os/exec" - "strconv" - "strings" + "path/filepath" + "syscall" "time" "github.com/ohing504/devclean/internal/model" @@ -157,36 +155,87 @@ func partitionScanners(scanners []Scanner) ([]walkEcosystem, []Scanner) { return tables, rest } -// DirSize calculates the disk usage of a directory using `du -sk`. -// Falls back to walking the filesystem if du is not available. +// SizeStat holds the apparent (sum of logical file sizes) and disk (allocated +// blocks) bytes for a path, plus the hard-linked inodes it counted so a caller +// can dedup blocks shared across artifacts (e.g. pnpm store ↔ node_modules). +type SizeStat struct { + Apparent int64 + Disk int64 + Links map[model.InodeKey]int64 // Nlink>1 inode → disk blocks, keyed by (dev, ino) +} + +// Measure walks path in-process and returns its apparent and disk sizes. // -// du is faster than an in-process traversal (macOS uses bulk attribute -// syscalls), so the per-artifact fork is kept; the walk engine amortizes it by -// sizing artifacts concurrently (see sizePending). -func DirSize(path string) int64 { - cmd := exec.Command("du", "-sk", path) - out, err := cmd.Output() - if err == nil { - parts := strings.SplitN(strings.TrimSpace(string(out)), "\t", 2) - if kb, err := strconv.ParseInt(parts[0], 10, 64); err == nil { - return kb * 1024 // KB to bytes +// Disk uses st_blocks×512 (allocated blocks), so it stays correct for sparse +// files where the logical size vastly exceeds what is on disk, and matches +// `du`. Apparent sums logical file sizes. Directories contribute their own +// blocks to Disk (ext4 dirs use real blocks; APFS reports ~0) but not to +// Apparent. Symlinks are not followed — neither the target nor the link's own +// blocks are counted (consistent with the walk engine's no-follow policy). +// Files hard-linked more than once are counted once within this artifact and +// recorded in Links so a caller can net out blocks shared across artifacts. +func Measure(path string) SizeStat { + var st SizeStat + seen := make(map[model.InodeKey]struct{}) + _ = filepath.WalkDir(path, func(_ string, d fs.DirEntry, err error) error { + if err != nil { + return nil // skip unreadable / racing entries, keep summing the rest } - } - - // Fallback: walk filesystem - var size int64 - _ = fs.WalkDir(os.DirFS(path), ".", func(_ string, d fs.DirEntry, err error) error { - if err != nil || d.IsDir() { + info, ierr := d.Info() + if ierr != nil { return nil } - info, err := d.Info() - if err != nil { + sys, ok := info.Sys().(*syscall.Stat_t) + if !ok { + if !d.IsDir() && d.Type()&fs.ModeSymlink == 0 { + st.Apparent += info.Size() // non-unix fallback: apparent only + } return nil } - size += info.Size() + blocks := int64(sys.Blocks) * 512 + if d.IsDir() { + st.Disk += blocks + return nil + } + if d.Type()&fs.ModeSymlink != 0 { + return nil + } + if sys.Nlink > 1 { + // Count a multiply-linked inode once per artifact for both apparent + // and disk (matches `du`/`du -A` within-call dedup), and record it + // so DedupedTotal can net it out across artifacts. + key := model.InodeKey{Dev: uint64(sys.Dev), Ino: uint64(sys.Ino)} + if _, dup := seen[key]; dup { + return nil + } + seen[key] = struct{}{} + if st.Links == nil { + st.Links = make(map[model.InodeKey]int64) + } + st.Links[key] = blocks + } + st.Apparent += info.Size() + st.Disk += blocks return nil }) - return size + return st +} + +// DirSize returns the disk usage (allocated blocks) of a path. Thin wrapper over +// Measure for callers that only need the disk figure. +func DirSize(path string) int64 { + return Measure(path).Disk +} + +// sized fills Size, ApparentSize and Links on r by measuring r.Path. Convenience +// for the stat-based scanners (xcode/global/llm) that build results one at a +// time, mirroring what the walk engine's sizePending does in bulk. +func sized(r model.ScanResult) model.ScanResult { + st := Measure(r.Path) + r.Size = st.Disk + r.ApparentSize = st.Apparent + r.Links = st.Links + return r } // ModTime returns the modification time of a path, or zero time on error. diff --git a/internal/scanner/sizing_test.go b/internal/scanner/sizing_test.go index 0a2ddb4..792746c 100644 --- a/internal/scanner/sizing_test.go +++ b/internal/scanner/sizing_test.go @@ -114,3 +114,147 @@ func TestDirSizeMissingPath(t *testing.T) { t.Errorf("DirSize(missing) = %d, want 0", got) } } + +// TestMeasureSparseFile is the core A1 case: a sparse file's disk usage +// (allocated blocks) must stay far below its apparent (logical) size. This is +// the Docker.raw scenario in miniature — the old du-only path already got this +// right, and the in-process Measure must not regress it. +func TestMeasureSparseFile(t *testing.T) { + dir := t.TempDir() + f, err := os.Create(filepath.Join(dir, "sparse.img")) + if err != nil { + t.Fatal(err) + } + const apparent = 100 << 20 // 100 MiB hole, no data written + if err := f.Truncate(apparent); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + + st := Measure(dir) + if st.Apparent < apparent { + t.Errorf("Apparent = %d, want >= %d (logical size of the hole)", st.Apparent, int64(apparent)) + } + if st.Disk > 1<<20 { + t.Errorf("Disk = %d, want < 1MiB (sparse file allocates almost no blocks)", st.Disk) + } + if st.Disk >= st.Apparent { + t.Errorf("Disk (%d) should be far below Apparent (%d) for a sparse file", st.Disk, st.Apparent) + } +} + +// TestMeasureNormalFileBothPositive: a fully-written file reports both apparent +// and disk > 0, and disk covers the logical content (block rounding makes it a +// lower bound, never below). +func TestMeasureNormalFileBothPositive(t *testing.T) { + dir := t.TempDir() + const size = 200_000 + if err := os.WriteFile(filepath.Join(dir, "data.bin"), make([]byte, size), 0o644); err != nil { + t.Fatal(err) + } + st := Measure(dir) + if st.Apparent < size { + t.Errorf("Apparent = %d, want >= %d", st.Apparent, int64(size)) + } + if st.Disk < size { + t.Errorf("Disk = %d, want >= %d (allocated blocks cover the content)", st.Disk, int64(size)) + } +} + +// TestMeasureHardlinkIntraDedup: two directory entries pointing at the same +// inode within one artifact must be counted once for both apparent and disk, +// and the shared inode recorded in Links (so DedupedTotal can net it across +// artifacts). +func TestMeasureHardlinkIntraDedup(t *testing.T) { + dir := t.TempDir() + const size = 300_000 + orig := filepath.Join(dir, "orig") + if err := os.WriteFile(orig, make([]byte, size), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Link(orig, filepath.Join(dir, "hardlink")); err != nil { + t.Fatal(err) + } + + st := Measure(dir) + // Apparent counts the inode once, not once per link (matches du -A). + if st.Apparent >= 2*size { + t.Errorf("Apparent = %d, want ~%d (hard link counted once, not doubled)", st.Apparent, int64(size)) + } + if st.Apparent < size { + t.Errorf("Apparent = %d, want >= %d", st.Apparent, int64(size)) + } + if len(st.Links) != 1 { + t.Fatalf("Links = %v, want exactly 1 shared inode", st.Links) + } + for _, blocks := range st.Links { + if blocks <= 0 { + t.Errorf("recorded shared inode blocks = %d, want > 0", blocks) + } + } +} + +// TestDedupedTotalHardlinkAcrossArtifacts is the pnpm case in miniature: one +// file hard-linked into two separate artifacts (store ↔ consumer) must be +// counted once in the grand total, not once per artifact. +func TestDedupedTotalHardlinkAcrossArtifacts(t *testing.T) { + base := t.TempDir() + store := filepath.Join(base, "store") + consumer := filepath.Join(base, "consumer") + for _, d := range []string{store, consumer} { + if err := os.MkdirAll(d, 0o755); err != nil { + t.Fatal(err) + } + } + const size = 500_000 + blob := filepath.Join(store, "blob") + if err := os.WriteFile(blob, make([]byte, size), 0o644); err != nil { + t.Fatal(err) + } + if err := os.Link(blob, filepath.Join(consumer, "blob")); err != nil { + t.Fatal(err) + } + + a := sized(model.ScanResult{Path: store}) + b := sized(model.ScanResult{Path: consumer}) + results := []model.ScanResult{a, b} + + naive := a.Size + b.Size + total := model.DedupedTotal(results) + if total >= naive { + t.Errorf("DedupedTotal = %d, want < naive sum %d (shared blob counted once)", total, naive) + } + // Deduped total keeps one copy of the shared blob, so it cannot drop below + // a single artifact's standalone size. + if total < a.Size { + t.Errorf("DedupedTotal = %d, want >= one artifact's size %d", total, a.Size) + } +} + +// TestDedupedTotalDistinctDevNotDeduped pins the (dev, ino) key: the same inode +// number on two different devices (external volume, container fs) is a genuine +// collision and must NOT be deduped, or the total would silently under-count. +func TestDedupedTotalDistinctDevNotDeduped(t *testing.T) { + const ino, blocks = 42, int64(1_000_000) + a := model.ScanResult{Size: blocks, Links: map[model.InodeKey]int64{{Dev: 1, Ino: ino}: blocks}} + b := model.ScanResult{Size: blocks, Links: map[model.InodeKey]int64{{Dev: 2, Ino: ino}: blocks}} + + if got := model.DedupedTotal([]model.ScanResult{a, b}); got != 2*blocks { + t.Errorf("DedupedTotal (distinct devices) = %d, want %d (no dedup)", got, 2*blocks) + } + // Same dev+ino is a real hard link and MUST dedup. + b.Links = map[model.InodeKey]int64{{Dev: 1, Ino: ino}: blocks} + if got := model.DedupedTotal([]model.ScanResult{a, b}); got != blocks { + t.Errorf("DedupedTotal (same dev+ino) = %d, want %d (deduped once)", got, blocks) + } +} + +// TestDedupedTotalNoLinks: with no hard links, the total is just the sum. +func TestDedupedTotalNoLinks(t *testing.T) { + results := []model.ScanResult{{Size: 100}, {Size: 250}, {Size: 0}} + if got := model.DedupedTotal(results); got != 350 { + t.Errorf("DedupedTotal = %d, want 350", got) + } +} diff --git a/internal/scanner/walk.go b/internal/scanner/walk.go index 6b7790b..54b98ed 100644 --- a/internal/scanner/walk.go +++ b/internal/scanner/walk.go @@ -265,16 +265,15 @@ func runWalk(ctx context.Context, root string, tables []walkEcosystem) ([]model. return results, nil } -// sizeWorkerCap bounds concurrent du forks so a large scan does not spawn a -// subprocess per artifact all at once. +// sizeWorkerCap bounds concurrent sizing walks so a large scan does not +// traverse every artifact tree at once. const sizeWorkerCap = 8 -// sizePending fills in Size for every result by running DirSize concurrently -// across a bounded worker pool. The walk finds artifacts serially but defers -// their sizing (one du fork each) to here so the forks and their I/O overlap. -// du is faster per artifact than an in-process traversal, so the speedup comes -// from overlapping the sizing, not from replacing du. Returns ctx.Err() if the -// scan is cancelled mid-sizing. +// sizePending fills in Size, ApparentSize and Links for every result by running +// Measure concurrently across a bounded worker pool. The walk finds artifacts +// serially but defers their sizing (one in-process tree walk each) to here so +// the traversals and their I/O overlap. Returns ctx.Err() if the scan is +// cancelled mid-sizing. func sizePending(ctx context.Context, results []model.ScanResult) error { workers := runtime.NumCPU() if workers > sizeWorkerCap { @@ -303,7 +302,10 @@ func sizePendingWorkers(ctx context.Context, results []model.ScanResult, workers go func() { defer wg.Done() for i := range idx { - results[i].Size = DirSize(results[i].Path) + st := Measure(results[i].Path) + results[i].Size = st.Disk + results[i].ApparentSize = st.Apparent + results[i].Links = st.Links } }() } diff --git a/internal/scanner/xcode.go b/internal/scanner/xcode.go index ec4f2dc..bd23ea5 100644 --- a/internal/scanner/xcode.go +++ b/internal/scanner/xcode.go @@ -120,16 +120,14 @@ func (s *XcodeScanner) Scan(ctx context.Context, root string) ([]model.ScanResul continue } - size := DirSize(full) - results = append(results, model.ScanResult{ + results = append(results, sized(model.ScanResult{ Path: full, Ecosystem: model.EcoXcode, Category: a.category, - Size: size, LastMod: ModTime(full), Safety: a.safety, ProjectRoot: filepath.Dir(full), - }) + })) ReportProgress(ctx, len(results)) } return results, nil @@ -153,15 +151,14 @@ func expandChildren(ctx context.Context, parent string, a xcodeArtifact) []model default: } child := filepath.Join(parent, e.Name()) - out = append(out, model.ScanResult{ + out = append(out, sized(model.ScanResult{ Path: child, Ecosystem: model.EcoXcode, Category: a.category, - Size: DirSize(child), LastMod: ModTime(child), Safety: a.safety, ProjectRoot: parent, - }) + })) ReportProgress(ctx, len(out)) } return out @@ -208,7 +205,10 @@ func enrichDeviceSupport(results []model.ScanResult) { parsedAll[i] = parsed{matched: false} continue } - key := m[1] // " " + // Scope the key to the parent dir so builds only supersede within the + // same DeviceSupport root; otherwise model-less names like "16.4 (…)" + // could collide across platforms if slices are ever batched together. + key := filepath.Dir(r.Path) + "\x00" + m[1] // parent + " " parsedAll[i] = parsed{key: key, matched: true} mt := r.LastMod.Unix() diff --git a/internal/scanner/xcode_test.go b/internal/scanner/xcode_test.go index 3a9a50c..1438619 100644 --- a/internal/scanner/xcode_test.go +++ b/internal/scanner/xcode_test.go @@ -166,6 +166,57 @@ func TestXcodeScanner_FlagsOldDeviceSupportBuilds(t *testing.T) { } } +// Older Xcode names DeviceSupport folders without a model ("16.4 (build)"). +// Such a version-only key must group builds within its own platform but never +// collide across platforms. +func TestXcodeScanner_DeviceSupportKeyIsPlatformScoped(t *testing.T) { + home := t.TempDir() + t.Setenv("HOME", home) + + type entry struct { + platform string + dir string + mtime time.Time + } + entries := []entry{ + // iOS: two builds of the same version — older is superseded. + {"iOS DeviceSupport", "16.4 (20E247)", time.Now().Add(-20 * 24 * time.Hour)}, + {"iOS DeviceSupport", "16.4 (20E252)", time.Now().Add(-1 * 24 * time.Hour)}, // newest + // tvOS: same version-only name, different platform — must not be flagged. + {"tvOS DeviceSupport", "16.4 (20K672)", time.Now().Add(-40 * 24 * time.Hour)}, + } + for _, e := range entries { + dir := filepath.Join(home, "Library/Developer/Xcode", e.platform, e.dir) + mustMkdir(t, dir) + mustWriteFile(t, filepath.Join(dir, "Symbols.bin"), make([]byte, 4096)) + if err := os.Chtimes(dir, e.mtime, e.mtime); err != nil { + t.Fatalf("chtimes %s: %v", dir, err) + } + } + + s := scanner.NewXcodeScanner() + results, err := s.Scan(context.Background(), home) + if err != nil { + t.Fatalf("Scan error: %v", err) + } + + recs := make(map[string]string) + for _, r := range results { + recs[r.Path] = r.Recommendation + } + base := filepath.Join(home, "Library/Developer/Xcode") + + if got := recs[filepath.Join(base, "iOS DeviceSupport", "16.4 (20E247)")]; !strings.Contains(got, "superseded") { + t.Errorf("older iOS build should be superseded, got %q", got) + } + if got := recs[filepath.Join(base, "iOS DeviceSupport", "16.4 (20E252)")]; got != "" { + t.Errorf("newest iOS build should not be flagged, got %q", got) + } + if got := recs[filepath.Join(base, "tvOS DeviceSupport", "16.4 (20K672)")]; got != "" { + t.Errorf("tvOS build must not collide with iOS key, got %q", got) + } +} + func TestXcodeScanner_ExpandsSimulatorDevices(t *testing.T) { home := t.TempDir() t.Setenv("HOME", home) diff --git a/internal/ui/spinner.go b/internal/ui/spinner.go index 1a1294b..72d7545 100644 --- a/internal/ui/spinner.go +++ b/internal/ui/spinner.go @@ -13,6 +13,7 @@ type Spinner struct { mu sync.Mutex message string done chan struct{} + wg sync.WaitGroup stopped bool } @@ -22,6 +23,7 @@ func NewSpinner(message string) *Spinner { message: message, done: make(chan struct{}), } + s.wg.Add(1) go s.run() return s } @@ -43,11 +45,14 @@ func (s *Spinner) Stop() { s.stopped = true s.mu.Unlock() close(s.done) - // Clear the spinner line + // Wait for run() to return before clearing, so no late frame reprints + // over the cleared line and leaves a stale spinner artifact. + s.wg.Wait() fmt.Print("\r\033[K") } func (s *Spinner) run() { + defer s.wg.Done() ticker := time.NewTicker(80 * time.Millisecond) defer ticker.Stop() diff --git a/internal/ui/spinner_test.go b/internal/ui/spinner_test.go new file mode 100644 index 0000000..08f201d --- /dev/null +++ b/internal/ui/spinner_test.go @@ -0,0 +1,93 @@ +package ui_test + +import ( + "bytes" + "os" + "sync" + "testing" + "time" + + "github.com/ohing504/devclean/internal/ui" +) + +// Stop must join the animation goroutine before returning, so no late frame +// reprints over the cleared line and leaves a stale spinner artifact. +func TestSpinner_StopJoinsGoroutine(t *testing.T) { + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("pipe: %v", err) + } + orig := os.Stdout + os.Stdout = w + defer func() { os.Stdout = orig }() + + var mu sync.Mutex + var buf bytes.Buffer + drained := make(chan struct{}) + go func() { + defer close(drained) + b := make([]byte, 256) + for { + n, readErr := r.Read(b) + if n > 0 { + mu.Lock() + buf.Write(b[:n]) + mu.Unlock() + } + if readErr != nil { + return + } + } + }() + + s := ui.NewSpinner("scanning") + time.Sleep(200 * time.Millisecond) // let it tick a few frames + s.Stop() + + // Stop joined the goroutine, but the pipe→buf copy is async; let it drain + // before sampling the baseline. + time.Sleep(100 * time.Millisecond) + mu.Lock() + afterStop := buf.Len() + mu.Unlock() + + // Well past two tick intervals (80ms) — a leaked goroutine would write here. + time.Sleep(200 * time.Millisecond) + mu.Lock() + final := buf.Len() + mu.Unlock() + + if final != afterStop { + t.Errorf("spinner wrote %d bytes after Stop; goroutine not joined", final-afterStop) + } + + os.Stdout = orig + w.Close() + <-drained + r.Close() + + // The last write should be the clear sequence, not a frame. + if !bytes.HasSuffix(buf.Bytes(), []byte("\r\033[K")) { + t.Errorf("output should end with the clear sequence, got %q", tail(buf.Bytes())) + } +} + +// Stop is idempotent — a second call must not panic on the closed channel. +func TestSpinner_StopIdempotent(t *testing.T) { + orig := os.Stdout + _, w, _ := os.Pipe() + os.Stdout = w + defer func() { os.Stdout = orig; w.Close() }() + + s := ui.NewSpinner("x") + s.Stop() + s.Stop() +} + +func tail(b []byte) []byte { + const n = 8 + if len(b) <= n { + return b + } + return b[len(b)-n:] +}