diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index a9ca557..a97c00c 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,13 +12,17 @@ jobs: fail-fast: false matrix: os: [ubuntu-latest, macos-latest, windows-latest] + # 3.9 is the declared floor (a stock macOS `python3` and RHEL 9 both + # ship it) and is tested so it stays real; 3.13 catches deprecations + # early. The middle of the range is left to those two to bracket. + python-version: ["3.9", "3.13"] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v4 - uses: actions/setup-python@v5 with: - python-version: "3.11" + python-version: ${{ matrix.python-version }} - name: Install gdu (Linux/macOS only -- the preferred scan backend, du is the fallback) if: runner.os != 'Windows' @@ -34,3 +38,66 @@ jobs: - name: Run unit + integration tests run: pytest tests/unit tests/integration -q + + # The backend a real Mac actually uses. gdu is a `brew install` away, so + # the job above never exercises BSD `du` -- which is where macOS' own + # quirks live (its -a and -d are mutually exclusive, so an unguarded + # file-level scan is unbounded in depth; a `storops scan ~` cost ~2.6GB + # peak RSS before that was caught). Without this job those regressions + # are invisible on every runner. + test-macos-without-gdu: + runs-on: macos-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.13" + + - name: Confirm gdu is absent, so the du backend is the one under test + run: | + ! command -v gdu || { echo "gdu unexpectedly on PATH"; exit 1; } + + - name: Install storops (editable) + dev dependencies + run: pip install -e ".[dev]" + + - name: Run unit + integration tests + run: pytest tests/unit tests/integration -q + + - name: The du backend really is what got selected + run: | + python - <<'PY' + from storops import platform + backend = platform.get_scan_backend() + assert backend.name == "Du", backend.name + print("scan backend:", backend.name) + PY + + # `pip install -e .` leaves the repo-root rules/ directory reachable, so it + # cannot tell whether the built distribution actually ships the rule files. + # It did not: every command in an installed storops failed with "could not + # locate the rules/ directory" until this was caught. + test-installed-wheel: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.9" + + - name: Build the wheel + run: | + pip install build + python -m build --wheel + + - name: Install it somewhere the source tree cannot be reached + run: | + python -m venv /tmp/wheel-venv + /tmp/wheel-venv/bin/pip install dist/*.whl + + - name: The installed package can identify a path (i.e. it found its rules) + run: | + cd /tmp + /tmp/wheel-venv/bin/storops identify /tmp --json + /tmp/wheel-venv/bin/storops scan /tmp --json > /dev/null diff --git a/README.md b/README.md index 9ca8ea2..60c3b84 100644 --- a/README.md +++ b/README.md @@ -51,8 +51,10 @@ coverage. ## Requirements -- **Python 3.11+** — the only implementation (`src/storops/`); `python3`/ - `python` needs to be on `PATH`. No `pip install` is required for the +- **Python 3.9+** — the only implementation (`src/storops/`); `python3`/ + `python` needs to be on `PATH`. 3.9 is the floor deliberately: it is what + a stock macOS ships as `python3`, and StorOps is most useful on a machine + nobody has set a modern toolchain up on yet. No `pip install` is required for the common "cloned into a skills directory" install path — `python -m storops` works straight out of the checkout. - **Windows**: NTFS volumes. [WizTree](https://diskanalyzer.com/) is @@ -79,7 +81,7 @@ StorOps is a plain agent skill: a directory with a `SKILL.md` at its root, discovered by name and description rather than invoked as a slash command. No build step and no `pip install` required — the agent reads `SKILL.md` to decide when to use the skill, then invokes `python -m storops ` -directly. The only runtime requirements are Python 3.11+ and, on Windows, +directly. The only runtime requirements are Python 3.9+ and, on Windows, WizTree — see [Requirements](#requirements) above. ### Ask your agent to install it (recommended) diff --git a/README.zh-CN.md b/README.zh-CN.md index 42f223c..9c0948b 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -45,8 +45,9 @@ Windows token;只有关键系统路径规则(`rules/windows.yaml`/`linux.yaml`/ ## 环境要求 -- **Python 3.11+**——唯一的实现(`src/storops/`);`python3`/`python` - 需要在 `PATH` 上。最常见的"克隆进 skills 目录"安装方式不需要 +- **Python 3.9+**——唯一的实现(`src/storops/`);`python3`/`python` + 需要在 `PATH` 上。下限定在 3.9 是刻意的:macOS 自带的 `python3` 就是 + 3.9,而 StorOps 最该派上用场的,恰恰是还没配好现代工具链的机器。最常见的"克隆进 skills 目录"安装方式不需要 `pip install`——从 checkout 目录直接运行 `python -m storops` 即可。 - **Windows**:NTFS 卷。[WizTree](https://diskanalyzer.com/) 是可选的—— StorOps 自带的原生扫描(`os.scandir`,以工作队列在整棵扫描树内并行; @@ -67,7 +68,7 @@ Windows token;只有关键系统路径规则(`rules/windows.yaml`/`linux.yaml`/ StorOps 是一个标准的 agent skill:一个根目录带有 `SKILL.md` 的目录,agent 依据 其 name/description 自动发现并调用,而非以 slash command 的形式手动触发。无需 构建步骤,也无需 `pip install`——agent 会读取 `SKILL.md` 来判断何时使用该 -skill,然后直接调用 `python -m storops `。运行时依赖是 Python 3.11+, +skill,然后直接调用 `python -m storops `。运行时依赖是 Python 3.9+, 以及在 Windows 上的 WizTree,详见上方[环境要求](#环境要求)。 ### 直接让 Agent 帮你安装(推荐) diff --git a/SKILL.md b/SKILL.md index 756cd6b..d8b4ba6 100644 --- a/SKILL.md +++ b/SKILL.md @@ -79,11 +79,24 @@ commands ad hoc; the user should not need to know command names. (e.g. "by the way, installing gdu would make these scans noticeably faster") -- don't repeat it on every single command, and don't mention it at all on Windows or when it's `null`. +14. On macOS, a scan's per-directory sizes will not add up to the volume's + used space, and you must not present them as if they should. Two + separate reasons: StorOps already prunes APFS firmlinks (`/Users` and + `/System/Volumes/Data/Users` are literally the same directory, and an + unpruned `du /` counts the user's whole home twice), but APFS *block + sharing* -- file clones, and Time Machine local snapshots -- remains, + and no per-path size can attribute shared blocks to one path. Report + the ranking and the individual sizes, which are sound; do not compute + "everything else" by subtracting the total from the drive's used + figure, and do not tell the user their disk is lying to them. ## Workflow: "why is my drive full?" 1. `storops scan C:\` (or the drive the user mentioned) for top-level - consumers and free space. + consumers and free space. On macOS scan `/` -- not `/System/Volumes/Data`: + StorOps prunes the Data volume's firmlinked duplicates when it is handed + the real root, and scanning the Data volume directly reports the same + content under its uglier internal paths. 2. To see what's inside several large entries at once, prefer one `storops search --folders --max-depth 2` (bump to `3` if two levels isn't enough) over `storops inspect`-ing each one individually. `inspect` diff --git a/pyproject.toml b/pyproject.toml index 6fdbbd6..892f031 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -7,11 +7,18 @@ name = "storops" version = "2.0.0a0" description = "Storage Operations for AI Agents -- cross-platform disk/storage diagnosis, cleanup, and migration CLI." readme = "README.md" -requires-python = ">=3.11" +# 3.9 is the floor because that is what a stock macOS ships as +# `python3` (Xcode CLT) and what RHEL 9 ships -- StorOps is most useful on +# a machine the user has NOT already set up a modern toolchain on, so +# demanding 3.11 there turned `pip install storops` into a hard stop for +# no reason: the whole test suite passes unmodified on 3.9, and CI now +# pins the floor so it stays that way. +requires-python = ">=3.9" license = { text = "MIT" } authors = [{ name = "tzzs" }] classifiers = [ "Programming Language :: Python :: 3", + "Programming Language :: Python :: 3.9", "Operating System :: OS Independent", "Environment :: Console", ] @@ -23,11 +30,30 @@ dev = ["pytest>=8"] [project.scripts] storops = "storops.cli:main" -[tool.setuptools.packages.find] -where = ["src"] +# `rules/` deliberately lives at the repo root, not under src/storops/: it +# is agent-facing content that SKILL.md/README.md point at by that path, +# and rules/README.md documents it as the place to add rules. Mapping it +# in as the `storops.rules` package is what gets those YAML files into the +# wheel -- without this, `pip install storops` produced a package whose +# every command died with "could not locate the rules/ directory", since +# package-data patterns can only ever match files that are already inside +# a package directory. An explicit `packages` list is required because +# `packages.find` cannot discover a package that lives outside `where`; +# tests/unit/test_packaging.py keeps the list from drifting. +[tool.setuptools] +package-dir = { "" = "src", "storops.rules" = "rules" } +packages = [ + "storops", + "storops.core", + "storops.output", + "storops.platform", + "storops.platform.backends", + "storops.platform.windows", + "storops.rules", +] [tool.setuptools.package-data] -storops = ["rules/*.yaml"] +"storops.rules" = ["*.yaml"] [tool.pytest.ini_options] testpaths = ["tests"] diff --git a/rules/README.md b/rules/README.md index 4ba925d..a85da8c 100644 --- a/rules/README.md +++ b/rules/README.md @@ -8,9 +8,9 @@ path is from its name alone (see [`docs/DESIGN.md`](../docs/DESIGN.md) §3.3). | File | Covers | |---|---| -| `ai-models.yaml` | AI/ML model weights and inference-tool caches (LM Studio, Ollama, Hugging Face, ComfyUI/Stable Diffusion, PyTorch/CUDA) -- `path_patterns` are currently Windows-token-only | -| `applications.yaml` | Dev tooling (npm, pnpm, pip, uv, conda, Git, VS Code, JetBrains, Visual Studio, Docker, WSL) and general consumer apps (Steam, Chrome, Edge, Discord, Adobe) -- `path_patterns` are currently Windows-token-only | -| `caches.yaml` | Generic OS/browser/temp caches not owned by one specific application above -- `path_patterns` are currently Windows-token-only | +| `ai-models.yaml` | AI/ML model weights and inference-tool caches (LM Studio, Ollama, Hugging Face, ComfyUI/Stable Diffusion, PyTorch/CUDA) | +| `applications.yaml` | Dev tooling (npm, pnpm, pip, uv, conda, Git, VS Code, JetBrains, Visual Studio, Docker, WSL, Gradle, Cargo, Homebrew, the Xcode/CoreSimulator toolchain) and general consumer apps (Steam, Chrome, Edge, Discord, Adobe) | +| `caches.yaml` | Generic OS/browser/temp caches not owned by one specific application above | | `windows.yaml` | Windows system paths StorOps must never classify as safe to touch | | `linux.yaml` | Linux system paths StorOps must never classify as safe to touch | | `macos.yaml` | macOS system paths StorOps must never classify as safe to touch | diff --git a/rules/applications.yaml b/rules/applications.yaml index 3fbae5f..9d7901f 100644 --- a/rules/applications.yaml +++ b/rules/applications.yaml @@ -357,3 +357,167 @@ notes: > No Linux pattern here by design -- the Adobe Creative Cloud desktop suite does not ship a Linux build, so there is no real path to match. + +# --- Apple developer toolchain (macOS) -------------------------------------- +# The largest reclaimable consumers on a Mac used for development, and all +# previously "unknown"/critical -- which meant StorOps could see tens of GB +# in a scan and had nothing to say about any of it. + +- id: xcode-derived-data + application: Xcode + category: ide-cache + path_patterns: + - "%HOME%/Library/Developer/Xcode/DerivedData/*" + confidence: 0.95 + owner: user + purpose: > + Per-project build intermediates, module caches, and indexes that Xcode + regenerates from the project source on the next build. + deletable: true + migratable: false + cleanup_risk: low + cleanup_consequence: > + The next build of each affected project is a full rebuild (slower + once); nothing that is not reproducible from source is lost. + notes: > + Routinely the single largest directory under ~/Library/Developer on a + machine with more than a couple of Xcode projects. + +- id: xcode-device-support + application: Xcode + category: ide-cache + path_patterns: + - "%HOME%/Library/Developer/Xcode/iOS DeviceSupport/*" + - "%HOME%/Library/Developer/Xcode/watchOS DeviceSupport/*" + - "%HOME%/Library/Developer/Xcode/tvOS DeviceSupport/*" + confidence: 0.95 + owner: user + purpose: > + Debug symbols copied off each physical device the first time it is + attached, kept per OS build -- so one entry accumulates per iOS version + ever connected, including long-obsolete ones. + deletable: true + migratable: false + cleanup_risk: medium + cleanup_consequence: > + Re-copied off the device (a few minutes) the next time that exact OS + build is attached for debugging; symbolication of already-captured + crash logs from that build is lost until then. + +- id: xcode-archives + application: Xcode + category: build-output + path_patterns: + - "%HOME%/Library/Developer/Xcode/Archives/*" + confidence: 0.95 + owner: user + purpose: > + Archived builds with their dSYMs -- what App Store submissions were cut + from, and the only way to symbolicate crash reports from a shipped build. + deletable: false + migratable: true + migration_method: manual + migration_config_hint: > + Move the Archives directory to external storage by hand; Xcode's + Organizer reads whatever is present at this path. + cleanup_risk: high + cleanup_consequence: > + Not reproducible: deleting an archive permanently loses the ability to + symbolicate crash reports from the build it came from. + notes: > + Deliberately never offered for deletion even though it sits beside + DerivedData and looks similar -- these are shipping artifacts. + +- id: coresimulator-caches + application: Xcode Simulator + category: ide-cache + path_patterns: + - "%HOME%/Library/Developer/CoreSimulator/Caches/*" + confidence: 0.95 + owner: user + purpose: Downloaded simulator runtime disk images and their extraction scratch space. + deletable: true + migratable: false + cleanup_risk: medium + cleanup_consequence: > + Simulator runtimes are re-downloaded from Apple (several GB each) the + next time one is needed. + +- id: coresimulator-devices + application: Xcode Simulator + category: application-data + path_patterns: + - "%HOME%/Library/Developer/CoreSimulator/Devices/*" + confidence: 0.95 + owner: user + purpose: > + One full disk image per created simulator device, including every app + installed into it and its data. + deletable: true + migratable: false + cleanup_risk: high + cleanup_consequence: > + Deleting these directories by hand leaves CoreSimulator's own device + index pointing at devices that no longer exist. + notes: > + Reclaim this through Xcode itself -- `xcrun simctl delete unavailable` + removes devices for uninstalled runtimes and keeps the index + consistent. Classified high on purpose so `cleanup plan` identifies it + but never proposes deleting it. + +- id: homebrew-cache + application: Homebrew + category: package-manager-cache + path_patterns: + - "%CACHES%/Homebrew/*" + - "%HOME%/Library/Caches/Homebrew/*" + - "%XDG_CACHE_HOME%/Homebrew/*" + confidence: 0.95 + owner: user + purpose: Downloaded bottles, source tarballs, and their partial downloads. + deletable: true + migratable: false + cleanup_risk: low + cleanup_consequence: > + Re-downloaded on the next install or upgrade of the affected formula; + installed software is unaffected. + notes: "`brew cleanup -s` does the same thing through Homebrew itself." + +# --- Cross-platform build caches missing from the rule base entirely -------- + +- id: gradle-cache + application: Gradle + category: package-manager-cache + path_patterns: + - "%HOME%/.gradle/caches/*" + - "%USERPROFILE%\\.gradle\\caches\\*" + confidence: 0.9 + owner: user + purpose: Resolved dependency jars, build-script compilation output, and transformed artifacts. + deletable: true + migratable: true + migration_method: app-config + migration_config_hint: > + Set GRADLE_USER_HOME to the new location (Gradle puts caches/ beneath it). + cleanup_risk: medium + cleanup_consequence: > + Every dependency is re-downloaded and every build script recompiled on + the next build; offline builds stop working until then. + +- id: cargo-registry + application: Cargo (Rust) + category: package-manager-cache + path_patterns: + - "%HOME%/.cargo/registry/*" + - "%USERPROFILE%\\.cargo\\registry\\*" + confidence: 0.9 + owner: user + purpose: Downloaded crate sources and the registry index. + deletable: true + migratable: true + migration_method: app-config + migration_config_hint: "Set CARGO_HOME to the new location." + cleanup_risk: medium + cleanup_consequence: > + Every dependency is re-downloaded from crates.io on the next build; + offline builds stop working until then. diff --git a/rules/caches.yaml b/rules/caches.yaml index 0b170ea..2ab7b41 100644 --- a/rules/caches.yaml +++ b/rules/caches.yaml @@ -142,3 +142,23 @@ this as a large-consumer candidate for the user to review, never auto-classifies individual files as safe to delete. notes: Report largest/oldest files here for the user's own review rather than proposing bulk deletion. + +- id: macos-user-logs + application: macOS + category: os-temp + path_patterns: + - "%HOME%/Library/Logs/*" + confidence: 0.85 + owner: user + purpose: > + Per-user application and crash logs, written by apps and by macOS' + diagnostic reporting. + deletable: true + migratable: false + cleanup_risk: low + cleanup_consequence: > + Only historical diagnostics are lost; nothing here is needed for any + application to run. + notes: > + The system-wide counterpart under /Library/Logs is covered by + macos.yaml's critical /Library rule and is never offered. diff --git a/rules/macos.yaml b/rules/macos.yaml index ad684d4..0c5b343 100644 --- a/rules/macos.yaml +++ b/rules/macos.yaml @@ -8,12 +8,11 @@ # migratable: false. StorOps must never offer these for cleanup or migration, # regardless of what the user asks for. # -# NOTE: this is a starter set covering the same *kind* of paths windows.yaml -# covers (OS itself, installed apps, user profile/library, user documents) -- -# it does not yet attempt parity with ai-models.yaml/applications.yaml/ -# caches.yaml, whose path_patterns are still Windows-token-only. Extending -# those to macOS equivalents (LM Studio, Ollama, Docker Desktop, npm/pip -# caches under %CACHES%/%XDG_CACHE_HOME%, etc.) is separate follow-up work. +# NOTE: this file covers only the macOS paths StorOps must never classify as +# safe to touch. The macOS *reclaimable* paths live where their application +# does: applications.yaml carries the Xcode/CoreSimulator/Homebrew rules and +# the %CACHES%/%APP_SUPPORT% patterns for the cross-platform tools, and +# caches.yaml carries ~/Library/Logs. - id: macos-system application: macOS diff --git a/src/storops/core/cleanup.py b/src/storops/core/cleanup.py index aeb1733..2b12843 100644 --- a/src/storops/core/cleanup.py +++ b/src/storops/core/cleanup.py @@ -13,6 +13,7 @@ import json import os import shutil +from concurrent.futures import ThreadPoolExecutor from datetime import datetime, timezone from pathlib import Path @@ -44,6 +45,32 @@ def _probe_path(pattern: str) -> str | None: return rules.expand_pattern_tokens(stripped) +# Sizing one probe path means walking its whole subtree, so a plan over a +# handful of caches is almost entirely time spent waiting on filesystem +# metadata -- measured at 6.3s for nine probes on a real Mac, all of it +# serialized for no reason. Kept modest: past this the backends' own +# per-call subprocesses start competing for the same disk. +_PROBE_CONCURRENCY = 8 + + +def _sized_probes(backend, probe_paths: list[str], *, admin: bool) -> list: + """backend.path_size() for each path, concurrently where the backend + says that is safe. + + Opt-in per backend rather than assumed: the Windows native backend + resets per-call state on `self` (its warnings list) and already splits + its own walk across threads internally, so sizing several paths at once + through it would both clobber that state and oversubscribe the disk. + Backends that are safe declare `path_size_is_concurrent = True`. + """ + if len(probe_paths) < 2 or not getattr(backend, "path_size_is_concurrent", False): + return [backend.path_size(p, admin=admin) for p in probe_paths] + + workers = min(_PROBE_CONCURRENCY, len(probe_paths)) + with ThreadPoolExecutor(max_workers=workers) as pool: + return list(pool.map(lambda p: backend.path_size(p, admin=admin), probe_paths)) + + def plan(max_risk: str = "low", *, out_file: str | None = None, admin: bool = False) -> CleanupPlan: backend = platform_pkg.get_scan_backend() deletable_rules = [r for r in rules.load_rules() if r.deletable] @@ -60,11 +87,11 @@ def plan(max_risk: str = "low", *, out_file: str | None = None, admin: bool = Fa # double-counted it when keyed on the raw expansion. probes.setdefault(resolve_path(probe), rule) + existing = [(p, r) for p, r in probes.items() if os.path.exists(p)] + sizes = _sized_probes(backend, [p for p, _ in existing], admin=admin) + items: list[CleanupItem] = [] - for probe_path, rule in probes.items(): - if not os.path.exists(probe_path): - continue - sized = backend.path_size(probe_path, admin=admin) + for (probe_path, rule), sized in zip(existing, sizes): if not sized or sized.size_bytes <= 0: continue diff --git a/src/storops/platform/backends/du.py b/src/storops/platform/backends/du.py index f4a17ec..1dda3de 100644 --- a/src/storops/platform/backends/du.py +++ b/src/storops/platform/backends/du.py @@ -13,13 +13,26 @@ from __future__ import annotations import os +import platform as _platform import subprocess +from collections import deque +from concurrent.futures import ThreadPoolExecutor from fnmatch import fnmatch +from typing import Iterator from storops.core.errors import InvalidPathError, PermissionDeniedError from storops.core.models import Entry, ScanWarning from storops.core.paths import resolve_path +try: + import fcntl +except ImportError: # pragma: no cover - Windows has no fcntl + # This module is POSIX-only at runtime (platform/base.py never selects + # it on Windows), but the test suite imports it everywhere in order to + # collect its skip markers -- a hard import here would turn every + # Windows CI run into a collection error rather than a set of skips. + fcntl = None # type: ignore[assignment] + _DU_FALLBACK_ADVICE = ( "Install gdu for noticeably faster scans on large directory trees: " "https://github.com/dundee/gdu#installation" @@ -30,11 +43,148 @@ def _split_segments(path: str) -> list[str]: return [p for p in path.replace("\\", "/").split("/") if p] +# --- Depth-1 child sizing, with volume-alias pruning ------------------------ + +# Beyond this many immediate children, sizing each one with its own `du -s` +# costs more in process spawns than the parallelism buys back, so +# top_entries() falls back to a single `du -d 1` over the whole target. +_MAX_PARALLEL_CHILDREN = 512 + +# Measured sweet spot for concurrent `du -s` processes on a real macOS home +# directory: 8 workers took 99.9s down to 66.0s, 16 gave nothing back (70.7s) +# -- the walk is bound by filesystem metadata I/O, not by CPU. +_DU_CONCURRENCY = 8 + +# macOS' firmlinked directories all live under this one mount, so the alias +# pre-pass only ever walks toward and inside it. Everything big gets pruned +# at its entry point (a firmlink is recognized before it is descended into), +# which is what keeps the pre-pass to well under a second on a full disk. +_DARWIN_DATA_VOLUME = "/System/Volumes" + +# Guards against a pathological tree, not tuning knobs. +_ALIAS_SCAN_MAX_DEPTH = 12 +_ALIAS_SCAN_BUDGET = 50_000 + +# Darwin fcntl(2) command: write the file descriptor's canonical path into +# the supplied buffer. Stable since OS X 10.5 (sys/fcntl.h), and the only +# thing that reports a firmlink's real identity -- os.path.realpath() does +# not resolve them, and neither st_dev (identical across a volume group) +# nor a symlink check can see them at all. +_F_GETPATH = 50 +_F_GETPATH_BUFSIZE = 1024 + + +def _canonical_path(path: str) -> str | None: + """The kernel's own canonical path for `path`, or None if it cannot be + determined (no permission, not Darwin, path vanished mid-scan). + """ + if fcntl is None: + return None + try: + fd = os.open(path, os.O_RDONLY | getattr(os, "O_DIRECTORY", 0)) + except OSError: + return None + try: + raw = fcntl.fcntl(fd, _F_GETPATH, b"\0" * _F_GETPATH_BUFSIZE) + except (OSError, ValueError): + return None + finally: + os.close(fd) + return raw.split(b"\0", 1)[0].decode(errors="replace") or None + + +def _alias_scan_is_warranted(target: str) -> bool: + """True where the filesystem is known to expose the same directory + under more than one path below `target`. + + This is macOS' APFS volume-group topology, and only it: `/` is the + sealed, read-only System volume, and the writable Data volume is + mounted at /System/Volumes/Data and *also* projected into `/` by + firmlinks. So `/Users` and `/System/Volumes/Data/Users` are literally + the same directory -- same st_dev, same st_ino, neither one a symlink + -- and a plain `du /` walks the user's entire home twice and reports + a total to match. `du -x` cannot help: a volume group reports one + st_dev throughout, so there is no device boundary to stop at. + + Restricted to the volume roots rather than run on every scan: a scan + of e.g. a home directory has nothing for the pre-pass to find. + """ + if _platform.system() != "Darwin": + return False + head = target.rstrip("/") or "/" + return head == "/" or os.path.dirname(head) in ("/Volumes", _DARWIN_DATA_VOLUME) + + +def _alias_pre_pass_descends_into(path: str) -> bool: + """Keep the pre-pass on the one branch that can contain firmlinks: + /System/Volumes and everything under it (plus the ancestors needed to + reach it). Without this the walk would wander into the user's home + looking for aliases that, by construction, are never there. + """ + head = path.rstrip("/") or "/" + # "/" is its own separator, so the usual head + "/" would build "//". + prefix = head if head == "/" else head + "/" + return ( + head == _DARWIN_DATA_VOLUME + or head.startswith(_DARWIN_DATA_VOLUME + "/") + or _DARWIN_DATA_VOLUME.startswith(prefix) + ) + + +def _alias_paths(target: str) -> set[str]: + """Directories under `target` that the kernel reports as living at a + different canonical path -- i.e. a second route to content already + counted under that canonical path, which `du` would otherwise walk and + add in twice. + + An alias is never descended into, so the expensive subtrees + (/System/Volumes/Data/Users and friends) are recognized and dropped at + their entry point rather than walked. + """ + aliases: set[str] = set() + frontier = deque([(target.rstrip("/") or "/", 0)]) + budget = _ALIAS_SCAN_BUDGET + + while frontier: + current, depth = frontier.popleft() + if depth >= _ALIAS_SCAN_MAX_DEPTH: + continue + try: + with os.scandir(current) as it: + for entry in it: + if budget <= 0: + return aliases + try: + if not entry.is_dir(follow_symlinks=False): + continue + except OSError: + continue + budget -= 1 + canonical = _canonical_path(entry.path) + if canonical is not None and canonical != entry.path: + aliases.add(entry.path) + continue + if _alias_pre_pass_descends_into(entry.path): + frontier.append((entry.path, depth + 1)) + except OSError: + continue + + return aliases + + class DuBackend: """ScanBackend implementation shelling out to the system `du`.""" name = "Du" + # core/cleanup.py sizes every probe path it found; each one is an + # independent `du -s` subprocess here, so they are safe to run + # concurrently (no shared mutable state on this instance -- _flavor is + # write-once-with-the-same-value, and take_warnings() is a constant). + # Backends that keep per-call state (e.g. the Windows native backend's + # self._warnings) must NOT set this. + path_size_is_concurrent = True + def __init__(self) -> None: self._flavor: str | None = None # "gnu" | "bsd", cached per instance @@ -50,6 +200,78 @@ def _du_flavor(self) -> str: self._flavor = "bsd" return self._flavor + def _du_args(self, target: str, *, dirs_only: bool, max_depth: int) -> list[str]: + """Build the `du` command line for this flavor. + + `dirs_only` (the caller wants directories and no file-level rows -- + i.e. every scan()/inspect() call, since both go through + top_entries(include_files=False)) is the case worth special-casing: + without `-a`, BOTH flavors report directories only AND accept a + native depth limit, so du prints tens of rows instead of millions + and StorOps never has to classify or even look at a single file. + On a real macOS home directory that is the difference between + 2.4M output rows / ~2.6GB peak RSS and ~75 rows / a few MB, for + byte-identical results. + + With `-a` the two flavors diverge. GNU takes `-a` and + `--max-depth` together. BSD's are mutually exclusive + (`usage: du [-a | -s | -d depth]`) -- combining them always fails + with a usage error (exit 64), and `-d depth` alone reports only + directory totals, never individual files (confirmed against a real + macOS runner), so a caller that needs files at a limited depth + gets `-a` at unbounded depth here and the depth filter in scan() + does the real depth-limiting instead. + """ + if self._du_flavor() == "gnu": + # -b = --apparent-size --block-size=1: logical/apparent size in + # bytes, comparable to WizTree's "Size" column (not its "Allocated"). + args = ["du", "-b"] + if not dirs_only: + args.append("-a") + if max_depth > 0: + args.append(f"--max-depth={max_depth}") + else: + # BSD/macOS du has no portable apparent-size-in-bytes flag; + # report 1024-byte blocks (-k) and scale in scan(). This is + # disk-usage, not apparent size, on this flavor -- a known, + # documented approximation. + args = ["du", "-k"] + if dirs_only: + if max_depth > 0: + args.extend(["-d", str(max_depth)]) + else: + args.append("-a") + args.extend(["--", target]) + return args + + @staticmethod + def _du_lines(args: list[str]) -> Iterator[str]: + """Stream `du`'s stdout line by line. + + Streaming rather than subprocess.run(capture_output=True) is the + point: a `du -a` over a large home directory emits hundreds of MB + of text, and capturing it whole (plus the list splitlines() builds + from it) was measured at ~2.6GB peak RSS for a scan that returns + 15 rows. StorOps is most likely to be run on a machine that is + already out of disk and under memory pressure, so holding the + whole listing in memory is exactly the wrong trade. + + stderr is discarded, matching Du.psm1's `2>$null`: a + permission-denied subtree during `du` is silently skipped rather + than surfaced as a structured warning -- see take_warnings(). + """ + proc = subprocess.Popen( + args, stdout=subprocess.PIPE, stderr=subprocess.DEVNULL, text=True + ) + try: + assert proc.stdout is not None + for line in proc.stdout: + yield line + finally: + if proc.stdout is not None: + proc.stdout.close() + proc.wait() + def scan( self, path: str, @@ -69,89 +291,67 @@ def scan( raise InvalidPathError(f"StorOps: '{target}' does not exist.") flavor = self._du_flavor() - if flavor == "gnu": - # -b = --apparent-size --block-size=1: logical/apparent size in - # bytes, comparable to WizTree's "Size" column (not its "Allocated"). - args: list[str] = ["du", "-a", "-b"] - if max_depth > 0: - args.append(f"--max-depth={max_depth}") - else: - # BSD/macOS du has no portable apparent-size-in-bytes flag; - # report 1024-byte blocks (-k) and scale below. This is - # disk-usage, not apparent size, on this flavor -- a known, - # documented approximation. - # - # BSD du's -a, -s, and -d depth are mutually exclusive - # (`usage: du [-a | -s | -d depth]`) -- combining -a with -d - # always fails with a usage error (exit 64), which used to be - # misreported below as "permission denied". Unlike GNU, BSD's - # -d depth alone reports only directory totals, never - # individual files (confirmed against a real macOS runner -- - # dropping -a silently loses every file-level entry, which - # broke callers that need files at a limited depth, e.g. - # top_entries/path_size). There is no single BSD du invocation - # that gives both file-level entries and a native depth limit, - # so on this flavor we always run -a (every file, unbounded - # depth) and let the depth filter below -- originally just a - # belt-and-braces guard for GNU -- do the real depth-limiting - # for BSD instead. - args = ["du", "-k", "-a"] - args.extend(["--", target]) + dirs_only = export_folders and not export_files + args = self._du_args(target, dirs_only=dirs_only, max_depth=max_depth) - # stderr is discarded, matching Du.psm1's `2>$null`: a - # permission-denied subtree during `du` is silently skipped rather - # than surfaced as a structured warning. This is an acceptable, - # documented v1 limitation -- see take_warnings() below. - proc = subprocess.run(args, capture_output=True, text=True) - raw = proc.stdout - if proc.returncode != 0 and not raw: - raise PermissionDeniedError( - f"StorOps: du exited with code {proc.returncode} scanning '{target}' " - "(permission denied on a subtree? re-run the whole command under sudo " - "-- StorOps never self-elevates)." - ) + # Depth is limited natively except on BSD-with-`-a` (see _du_args); + # computing a row's depth means splitting its whole path into + # segments, which is real per-row work at scale, so it is skipped + # whenever du already did the limiting. + filter_depth = max_depth if (max_depth > 0 and not dirs_only and flavor != "gnu") else 0 + root_segments = len(_split_segments(target)) if filter_depth else 0 + scale = 1 if flavor == "gnu" else 1024 - root_segments = len(_split_segments(target)) + # In `-a` mode du emits a directory only AFTER everything inside + # it, so by the time a directory's own row arrives it has already + # been recorded here as some child's parent -- that makes the + # os.path.isdir() stat below unnecessary for every non-empty + # directory. (An *empty* directory is never any row's parent and + # is indistinguishable from a file in du's output, so those still + # get stat'ed; they are a rounding error next to the file rows.) + known_dirs: set[str] = set() entries: list[Entry] = [] + saw_any_row = False - for line in raw.splitlines(): - if not line: - continue - parts = line.split("\t", 1) - if len(parts) < 2: + for line in self._du_lines(args): + size_text, tab, entry_path = line.partition("\t") + if not tab: continue + saw_any_row = True + entry_path = entry_path.rstrip("\n") + + if not dirs_only: + parent = entry_path.rpartition("/")[0] + if parent: + known_dirs.add(parent) - size = int(parts[0]) - if flavor != "gnu": - size *= 1024 - entry_path = parts[1] if entry_path == target: continue - # Belt-and-braces: du was already asked to stop at max_depth, - # this just guards against any flavor quirk that returns deeper - # rows. Computing a row's depth is itself real per-row work - # (splitting the whole path into segments) that matters at - # scale, so it's skipped entirely for the common max_depth=0 - # (unlimited) case, where the check below is a no-op anyway. - if max_depth != 0: + if filter_depth: depth = len(_split_segments(entry_path)) - root_segments - if depth > max_depth: + if depth > filter_depth: continue - is_folder = os.path.isdir(entry_path) - if (is_folder and not export_folders) or ((not is_folder) and not export_files): - continue - - # Likewise: basename() is only ever needed for these two - # filters, so skip it when neither was given. + # Cheapest discriminators first: the name filters are pure + # string work, while is_folder can still cost a stat(). if name_filter or name_exclude: - name = os.path.basename(entry_path) + name = entry_path.rpartition("/")[2] if name_filter and not fnmatch(name, name_filter): continue if name_exclude and fnmatch(name, name_exclude): continue + if dirs_only: + is_folder = True + elif entry_path in known_dirs: + is_folder = True + else: + is_folder = os.path.isdir(entry_path) + if (is_folder and not export_folders) or ((not is_folder) and not export_files): + continue + + size = int(size_text) * scale entries.append( Entry( full_name=entry_path, @@ -164,6 +364,145 @@ def scan( ) ) + if not saw_any_row: + raise PermissionDeniedError( + f"StorOps: du produced no output scanning '{target}' " + "(permission denied on a subtree? re-run the whole command under sudo " + "-- StorOps never self-elevates)." + ) + + return entries + + def _entry_size(self, dir_entry: os.DirEntry) -> int: + """Size of one already-scandir'd file, in the same units this + flavor's `du` reports: apparent bytes for GNU `-b`, allocated + bytes (st_blocks is always 512-byte units, regardless of -k) for + BSD. Mixing the two would make a directory listing's file rows + incomparable with its folder rows. + """ + try: + stat_result = dir_entry.stat(follow_symlinks=False) + except OSError: + return 0 + if self._du_flavor() == "gnu": + return stat_result.st_size + return stat_result.st_blocks * 512 + + def _total_size(self, path: str, aliases: set[str]) -> int: + """`du -s path`, except where a pruned alias lives inside `path`: + then sum `path`'s non-alias children instead, so `du` is never + handed the duplicate subtree in the first place. Correcting the + total by subtracting the alias afterwards would give the same + number but none of the speed -- du would still have walked it. + + The recursion only expands along branches that actually contain an + alias (nine directories under `/` on macOS, all within four levels), + so in practice this is one `du -s` per child plus a shallow detour. + """ + prefix = path.rstrip("/") + "/" + if not any(alias.startswith(prefix) for alias in aliases): + sized = self.path_size(path) + return sized.size_bytes if sized else 0 + + total = 0 + try: + with os.scandir(path) as it: + for child in it: + if child.path in aliases: + continue + try: + is_dir = child.is_dir(follow_symlinks=False) + except OSError: + continue + total += self._total_size(child.path, aliases) if is_dir else self._entry_size(child) + except OSError: + return 0 + return total + + def _depth_one_entries(self, target: str, *, include_files: bool) -> list[Entry] | None: + """Size each of `target`'s immediate children independently, or + None to tell top_entries() to fall back to a single `du` run. + + Two things this buys over `du -d 1 `, which is otherwise + exactly equivalent: + + * Concurrency. Each child is its own `du -s` process, so the walk + is no longer serialized through one single-threaded `du`; + measured 99.9s -> 66s on a real macOS home directory. + * A place to prune volume aliases. `du` has no way to be told "you + have already counted that directory under another name", and on + macOS it has to be told -- see _alias_scan_is_warranted(). + + Falls back (returns None) when the target is not a directory or has + so many children that per-child process spawns would cost more than + the concurrency returns. + + Known trade-off: one `du` run counts a hardlinked file once no + matter how many of its links it meets, and N independent runs + cannot. Two top-level children sharing hardlinked content therefore + each count it in full here. Checked against `du -d 1` on a real + macOS home directory, the top-15 ordering was identical and every + size matched to within the churn of the live caches being written + during the runs -- cross-child hardlinks are rare enough in + practice to be worth the ~1.5x, and are the reason this is a + depth-1-only path rather than the default everywhere. + """ + if not os.path.isdir(target): + return None + try: + with os.scandir(target) as it: + children = list(it) + except OSError: + return None + + directories: list[str] = [] + files: list[Entry] = [] + for child in children: + try: + is_dir = child.is_dir(follow_symlinks=False) + except OSError: + continue + if is_dir: + directories.append(child.path) + elif include_files: + size = self._entry_size(child) + files.append( + Entry( + full_name=child.path, + is_folder=False, + size_bytes=size, + allocated_bytes=size, + modified=None, + file_count=None, + folder_count=None, + ) + ) + + if len(directories) > _MAX_PARALLEL_CHILDREN: + return None + + aliases = _alias_paths(target) if _alias_scan_is_warranted(target) else set() + directories = [d for d in directories if d not in aliases] + + sizes: list[int] = [] + if directories: + workers = min(_DU_CONCURRENCY, len(directories)) + with ThreadPoolExecutor(max_workers=workers) as pool: + sizes = list(pool.map(lambda d: self._total_size(d, aliases), directories)) + + entries = files + for directory, size in zip(directories, sizes): + entries.append( + Entry( + full_name=directory, + is_folder=True, + size_bytes=size, + allocated_bytes=size, + modified=None, + file_count=None, + folder_count=None, + ) + ) return entries def top_entries( @@ -175,13 +514,17 @@ def top_entries( admin: bool = False, include_files: bool = False, ) -> list[Entry]: - entries = self.scan( - path, - export_folders=True, - export_files=include_files, - max_depth=max_depth, - admin=admin, - ) + entries: list[Entry] | None = None + if max_depth == 1: + entries = self._depth_one_entries(resolve_path(path), include_files=include_files) + if entries is None: + entries = self.scan( + path, + export_folders=True, + export_files=include_files, + max_depth=max_depth, + admin=admin, + ) entries.sort(key=lambda e: e.size_bytes, reverse=True) return entries[:top] diff --git a/src/storops/platform/backends/gdu.py b/src/storops/platform/backends/gdu.py index ec69b5f..e7fc39d 100644 --- a/src/storops/platform/backends/gdu.py +++ b/src/storops/platform/backends/gdu.py @@ -222,6 +222,12 @@ class GduBackend: name = "Gdu" + # Each path_size() call is its own `gdu` subprocess over its own + # target, with no state kept on this instance between calls -- see + # core/cleanup.py's _sized_probes(), which sizes every probe path + # concurrently when a backend declares this. + path_size_is_concurrent = True + def scan( self, path: str, diff --git a/tests/unit/test_cleanup_probe_concurrency.py b/tests/unit/test_cleanup_probe_concurrency.py new file mode 100644 index 0000000..39e8850 --- /dev/null +++ b/tests/unit/test_cleanup_probe_concurrency.py @@ -0,0 +1,72 @@ +"""Unit tests for core/cleanup.py's _sized_probes(). + +Every probe path in a cleanup plan is sized by walking its whole subtree, +so a plan is almost entirely time spent waiting on filesystem metadata -- +measured at 6.3s for nine probes on a real Mac, down to 2.8s once they run +concurrently. Concurrency is opt-in per backend because the Windows native +backend keeps per-call state on `self` and already parallelizes its own +walk internally. +""" +from __future__ import annotations + +import threading + +from storops.core.cleanup import _sized_probes +from storops.core.models import Entry + + +class _Backend: + def __init__(self, *, concurrent: bool): + if concurrent: + self.path_size_is_concurrent = True + self.calls: list[str] = [] + self.max_in_flight = 0 + self._in_flight = 0 + self._lock = threading.Lock() + self._gate = threading.Barrier(2, timeout=5) + + def path_size(self, path, *, admin=False): + with self._lock: + self.calls.append(path) + self._in_flight += 1 + self.max_in_flight = max(self.max_in_flight, self._in_flight) + try: + # Blocks until a second call joins -- which can only ever + # happen if the caller really is running them concurrently. + try: + self._gate.wait() + except threading.BrokenBarrierError: + pass + finally: + with self._lock: + self._in_flight -= 1 + return Entry(full_name=path, is_folder=True, size_bytes=1, allocated_bytes=1) + + +def test_probes_are_sized_concurrently_when_the_backend_allows_it(): + backend = _Backend(concurrent=True) + + results = _sized_probes(backend, ["/a", "/b"], admin=False) + + assert [r.full_name for r in results] == ["/a", "/b"] # order preserved + assert backend.max_in_flight == 2 + + +def test_probes_stay_sequential_for_a_backend_that_does_not_opt_in(): + backend = _Backend(concurrent=False) + backend._gate.abort() # a sequential caller would otherwise deadlock + + results = _sized_probes(backend, ["/a", "/b"], admin=False) + + assert [r.full_name for r in results] == ["/a", "/b"] + assert backend.max_in_flight == 1 + + +def test_a_single_probe_never_spins_up_a_pool(): + backend = _Backend(concurrent=True) + backend._gate.abort() + + results = _sized_probes(backend, ["/only"], admin=False) + + assert [r.full_name for r in results] == ["/only"] + assert backend.max_in_flight == 1 diff --git a/tests/unit/test_du_backend.py b/tests/unit/test_du_backend.py index f6b4cab..dda73fa 100644 --- a/tests/unit/test_du_backend.py +++ b/tests/unit/test_du_backend.py @@ -9,6 +9,7 @@ """ from __future__ import annotations +import io import subprocess import sys @@ -207,6 +208,27 @@ def fake_run(args, **kwargs): assert backend._du_flavor() == "bsd" +class _FakePopen: + """Minimal stand-in for subprocess.Popen as scan() uses it: an + iterable text-mode stdout plus wait()/close().""" + + def __init__(self, stdout: str): + self.stdout = io.StringIO(stdout) + self.returncode = 0 + + def wait(self): + return self.returncode + + +def _fake_popen_returning(stdout, *, recorder=None): + def factory(args, **kwargs): + if recorder is not None: + recorder.append(args) + return _FakePopen(stdout) + + return factory + + def test_scan_parses_bsd_style_kilobyte_output(monkeypatch, tmp_path): """Force the BSD parsing branch (size in 1024-byte blocks, scaled up) regardless of the real `du` installed on the test runner, using real @@ -216,28 +238,97 @@ def test_scan_parses_bsd_style_kilobyte_output(monkeypatch, tmp_path): backend = DuBackend() backend._flavor = "bsd" # skip flavor probing - def fake_run(args, **kwargs): - assert "-k" in args - # BSD `du -a -k` output: \t - stdout = f"4\t{tmp_path / 'sub'}\n8\t{tmp_path}\n" - return _FakeCompleted(returncode=0, stdout=stdout) - - monkeypatch.setattr(subprocess, "run", fake_run) + args_seen: list[list[str]] = [] + # BSD `du -a -k` output: \t + stdout = f"4\t{tmp_path / 'sub'}\n8\t{tmp_path}\n" + monkeypatch.setattr(subprocess, "Popen", _fake_popen_returning(stdout, recorder=args_seen)) entries = backend.scan(str(tmp_path), export_folders=True, export_files=True, max_depth=1) + assert "-k" in args_seen[0] assert len(entries) == 1 assert entries[0].full_name == str(tmp_path / "sub") assert entries[0].size_bytes == 4 * 1024 # scaled from 1024-byte blocks -def test_scan_raises_permission_denied_when_du_fails_with_no_output(monkeypatch, tmp_path): +def test_scan_raises_permission_denied_when_du_produces_no_output(monkeypatch, tmp_path): backend = DuBackend() backend._flavor = "gnu" - def fake_run(args, **kwargs): - return _FakeCompleted(returncode=1, stdout="", stderr="du: Permission denied") - - monkeypatch.setattr(subprocess, "run", fake_run) + monkeypatch.setattr(subprocess, "Popen", _fake_popen_returning("")) with pytest.raises(PermissionDeniedError): backend.scan(str(tmp_path)) + + +# --- macOS/BSD flag selection (the expensive `-a` is opt-in) ---------------- + + +def _args_for(monkeypatch, backend, tmp_path, **scan_kwargs): + args_seen: list[list[str]] = [] + stdout = f"8\t{tmp_path}\n" + monkeypatch.setattr(subprocess, "Popen", _fake_popen_returning(stdout, recorder=args_seen)) + backend.scan(str(tmp_path), **scan_kwargs) + return args_seen[0] + + +def test_bsd_directory_only_scan_skips_dash_a_and_limits_depth_natively(monkeypatch, tmp_path): + """The scan/inspect hot path (top_entries(include_files=False)) must + not ask BSD du for file-level rows: `-a` there is unbounded-depth by + necessity (BSD's -a and -d are mutually exclusive), which on a real + macOS home directory meant millions of rows and ~2.6GB peak RSS to + produce ~15 rows of output. + """ + backend = DuBackend() + backend._flavor = "bsd" + + args = _args_for( + monkeypatch, backend, tmp_path, export_folders=True, export_files=False, max_depth=1 + ) + + assert "-a" not in args + assert args[args.index("-d") + 1] == "1" + + +def test_bsd_file_level_scan_still_uses_dash_a_without_depth_flag(monkeypatch, tmp_path): + backend = DuBackend() + backend._flavor = "bsd" + + args = _args_for( + monkeypatch, backend, tmp_path, export_folders=True, export_files=True, max_depth=2 + ) + + assert "-a" in args + assert "-d" not in args # mutually exclusive with -a on BSD; filtered in Python + + +def test_gnu_directory_only_scan_skips_dash_a(monkeypatch, tmp_path): + backend = DuBackend() + backend._flavor = "gnu" + + args = _args_for( + monkeypatch, backend, tmp_path, export_folders=True, export_files=False, max_depth=1 + ) + + assert "-a" not in args + assert "--max-depth=1" in args + + +def test_non_empty_directories_are_classified_without_stat(monkeypatch, tmp_path): + """In `-a` mode du emits a directory after its contents, so a + directory is already known to be one by the time its own row arrives + -- no os.path.isdir() call needed. Proven by pointing the backend at + paths that do not exist on disk: isdir() would report False for all + of them. + """ + backend = DuBackend() + backend._flavor = "gnu" + ghost = tmp_path / "ghost" + stdout = f"10\t{ghost / 'inner' / 'f.bin'}\n20\t{ghost / 'inner'}\n30\t{ghost}\n40\t{tmp_path}\n" + monkeypatch.setattr(subprocess, "Popen", _fake_popen_returning(stdout)) + + entries = backend.scan(str(tmp_path), export_folders=True, export_files=True) + + by_path = {e.full_name: e.is_folder for e in entries} + assert by_path[str(ghost)] is True + assert by_path[str(ghost / "inner")] is True + assert by_path[str(ghost / "inner" / "f.bin")] is False diff --git a/tests/unit/test_du_depth_one.py b/tests/unit/test_du_depth_one.py new file mode 100644 index 0000000..7ceee0c --- /dev/null +++ b/tests/unit/test_du_depth_one.py @@ -0,0 +1,192 @@ +"""Unit tests for DuBackend's depth-1 fast path and its volume-alias +pruning (src/storops/platform/backends/du.py). + +The macOS topology these exist for: `/` is the sealed System volume, the +writable Data volume is mounted at /System/Volumes/Data, and firmlinks +project the Data volume's directories into `/`. `/Users` and +`/System/Volumes/Data/Users` are therefore the same directory -- same +st_dev, same st_ino, neither a symlink -- so `du /` walks the user's home +twice. Measured on a real Mac: `storops scan /` reported /System at 95.0G +before pruning and 72.9G after, with the Data volume's own contribution +dropping from du's 309G to its actual 3.5G. +""" +from __future__ import annotations + +import sys + +import pytest + +pytestmark = pytest.mark.skipif(sys.platform == "win32", reason="POSIX-only backend") + +from storops.platform.backends import du as du_module +from storops.platform.backends.du import ( + DuBackend, + _alias_pre_pass_descends_into, + _alias_scan_is_warranted, +) + + +# --- Where the pre-pass runs at all ---------------------------------------- + + +@pytest.mark.parametrize( + "target,expected", + [ + ("/", True), + ("/System/Volumes/Data", True), + ("/Volumes/External", True), + ("/Users/someone", False), + ("/Users/someone/Library/Caches", False), + ("/tmp", False), + ], +) +def test_alias_scan_only_runs_at_volume_roots_on_darwin(monkeypatch, target, expected): + monkeypatch.setattr(du_module._platform, "system", lambda: "Darwin") + assert _alias_scan_is_warranted(target) is expected + + +def test_alias_scan_never_runs_off_darwin(monkeypatch): + for system in ("Linux", "Windows"): + monkeypatch.setattr(du_module._platform, "system", lambda: system) + assert _alias_scan_is_warranted("/") is False + + +@pytest.mark.parametrize( + "path,expected", + [ + ("/", True), # ancestor of /System/Volumes -- must be walked through + ("/System", True), + ("/System/Volumes", True), # the boundary itself, not just below it + ("/System/Volumes/Data", True), + ("/System/Volumes/Data/System/Library", True), + ("/Users", False), # by construction never holds a firmlink + ("/System/Applications", False), + ], +) +def test_pre_pass_stays_on_the_branch_that_can_hold_firmlinks(path, expected): + assert _alias_pre_pass_descends_into(path) is expected + + +# --- Pruning arithmetic ----------------------------------------------------- + + +class _RecordingBackend(DuBackend): + """DuBackend whose `du -s` is replaced by a directory-size lookup, so + the pruning logic can be tested without depending on real firmlinks + (which only exist on a real macOS volume group).""" + + def __init__(self, sizes: dict[str, int]): + super().__init__() + self._sizes = sizes + self.sized: list[str] = [] + + def path_size(self, path, *, admin=False): + from storops.core.models import Entry + + self.sized.append(path) + size = self._sizes.get(path, 0) + return Entry( + full_name=path, is_folder=True, size_bytes=size, allocated_bytes=size, + modified=None, file_count=None, folder_count=None, + ) + + +def test_total_size_without_aliases_is_a_single_du_call(tmp_path): + branch = tmp_path / "branch" + branch.mkdir() + backend = _RecordingBackend({str(branch): 4096}) + + assert backend._total_size(str(branch), set()) == 4096 + assert backend.sized == [str(branch)] + + +def test_total_size_never_hands_du_a_pruned_alias(tmp_path): + """The whole point of pruning rather than subtracting afterwards: the + duplicate subtree must never be walked, or the correction costs the + same time it was meant to save.""" + branch = tmp_path / "branch" + keep = branch / "keep" + alias = branch / "alias" + keep.mkdir(parents=True) + alias.mkdir() + backend = _RecordingBackend({str(keep): 1000, str(alias): 999_999}) + + total = backend._total_size(str(branch), {str(alias)}) + + assert total == 1000 + assert str(alias) not in backend.sized + + +def test_total_size_recurses_only_along_branches_holding_an_alias(tmp_path): + branch = tmp_path / "branch" + clean = branch / "clean" + dirty = branch / "dirty" + alias = dirty / "alias" + clean.mkdir(parents=True) + alias.mkdir(parents=True) + backend = _RecordingBackend({str(clean): 10, str(dirty): 20, str(alias): 30}) + + total = backend._total_size(str(branch), {str(alias)}) + + # `clean` was sized in one `du -s` without being descended into; only + # `dirty` (which contains the alias) was opened up. + assert str(clean) in backend.sized + assert str(dirty) not in backend.sized + assert total == 10 + + +# --- Fast path vs. the plain `du` path must agree --------------------------- + + +def _tree(tmp_path): + (tmp_path / "big").mkdir() + (tmp_path / "big" / "a.bin").write_bytes(b"x" * 20_000) + (tmp_path / "small").mkdir() + (tmp_path / "small" / "b.bin").write_bytes(b"y" * 1_000) + (tmp_path / "loose.bin").write_bytes(b"z" * 5_000) + return tmp_path + + +def test_depth_one_fast_path_matches_the_du_scan_it_replaces(tmp_path): + root = _tree(tmp_path) + backend = DuBackend() + + fast = backend.top_entries(str(root), top=10, max_depth=1, include_files=False) + slow = backend.scan(str(root), export_folders=True, export_files=False, max_depth=1) + slow.sort(key=lambda e: e.size_bytes, reverse=True) + + assert [e.full_name for e in fast] == [e.full_name for e in slow] + assert all(e.is_folder for e in fast) + + +def test_depth_one_fast_path_includes_files_when_asked(tmp_path): + root = _tree(tmp_path) + backend = DuBackend() + + entries = backend.top_entries(str(root), top=10, max_depth=1, include_files=True) + + by_path = {e.full_name: e for e in entries} + assert by_path[str(root / "loose.bin")].is_folder is False + assert by_path[str(root / "big")].is_folder is True + + +def test_wide_directories_fall_back_to_a_single_du_run(tmp_path, monkeypatch): + """One `du -s` per child stops paying for itself once the process + spawns outnumber the work; past the threshold top_entries() must go + back to a single `du` over the whole target.""" + for index in range(5): + (tmp_path / f"d{index}").mkdir() + monkeypatch.setattr(du_module, "_MAX_PARALLEL_CHILDREN", 2) + backend = DuBackend() + + assert backend._depth_one_entries(str(tmp_path), include_files=False) is None + # ...and top_entries still returns the same answer via the fallback. + assert len(backend.top_entries(str(tmp_path), top=10, max_depth=1)) == 5 + + +def test_depth_one_fast_path_declines_a_non_directory(tmp_path): + target = tmp_path / "f.bin" + target.write_bytes(b"x" * 10) + backend = DuBackend() + + assert backend._depth_one_entries(str(target), include_files=True) is None diff --git a/tests/unit/test_packaging.py b/tests/unit/test_packaging.py new file mode 100644 index 0000000..5cc27ca --- /dev/null +++ b/tests/unit/test_packaging.py @@ -0,0 +1,69 @@ +"""Guards on pyproject.toml's distribution metadata. + +These exist because the packaging bugs they cover are invisible to every +other test in this suite: CI installs the project with `pip install -e .`, +which leaves the repo-root `rules/` directory reachable via +core/rules.py's development-layout candidate. A real `pip install storops` +does not -- and shipped a wheel with no rule files at all, so every single +command failed with "could not locate the rules/ directory". +""" +from __future__ import annotations + +import sys +from pathlib import Path + +if sys.version_info >= (3, 11): + import tomllib +else: # pragma: no cover - 3.9/3.10 runners + import pytest + + tomllib = pytest.importorskip("tomli", reason="needs tomllib (3.11+) or tomli") + +REPO_ROOT = Path(__file__).resolve().parents[2] +SRC = REPO_ROOT / "src" + + +def _pyproject() -> dict: + with (REPO_ROOT / "pyproject.toml").open("rb") as handle: + return tomllib.load(handle) + + +def _declared_packages() -> list[str]: + return _pyproject()["tool"]["setuptools"]["packages"] + + +def test_rules_directory_is_mapped_into_the_wheel(): + setuptools_config = _pyproject()["tool"]["setuptools"] + assert setuptools_config["package-dir"]["storops.rules"] == "rules" + assert "storops.rules" in setuptools_config["packages"] + assert setuptools_config["package-data"]["storops.rules"] == ["*.yaml"] + + +def test_every_rule_file_the_engine_loads_is_shipped(): + from storops.core.rules import _RULE_FILE_ORDER + + for filename in _RULE_FILE_ORDER: + assert (REPO_ROOT / "rules" / filename).is_file(), filename + + +def test_declared_packages_cover_every_package_under_src(): + """`packages.find` had to be replaced by an explicit list to map + `storops.rules` in from outside `src/` (see pyproject.toml's comment). + That trades discovery for a list that can silently go stale, so a new + subpackage that nobody remembers to add here -- and would therefore be + missing from the wheel -- fails right here instead. + """ + on_disk = { + ".".join(path.parent.relative_to(SRC).parts) + for path in SRC.rglob("__init__.py") + } + missing = on_disk - set(_declared_packages()) + assert not missing, f"not listed in pyproject.toml [tool.setuptools].packages: {sorted(missing)}" + + +def test_requires_python_floor_matches_the_running_interpreter_support(): + """StorOps must stay installable on a stock macOS `python3` (3.9) and + RHEL 9 (3.9); raising this floor is a deliberate decision, not a + drive-by. + """ + assert _pyproject()["project"]["requires-python"] == ">=3.9"