diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c5c30f41..ddf03ef3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,6 @@ # Changelog +- **Fixed** Automatic tracking now classifies files that a task both reads and writes, so tasks that publish an output directory by renaming a staging directory into place, or that read their own generated files, cache correctly without hand-written `input`/`output` patterns ([#571](https://github.com/voidzero-dev/vite-task/pull/571)). - **Added** Tasks now run with `VP_RUN=1` set, so tools can tell they are running under `vp run` instead of being invoked directly ([#570](https://github.com/voidzero-dev/vite-task/pull/570)). - **Fixed** The task cache now supports much larger automatically tracked input sets without hitting wincode's default 4 MiB sequence preallocation limit ([#554](https://github.com/voidzero-dev/vite-task/pull/554)). - **Fixed** npm workspace patterns beginning with `./` now discover matching packages correctly ([vite-plus#2201](https://github.com/voidzero-dev/vite-plus/issues/2201), [#547](https://github.com/voidzero-dev/vite-task/pull/547)). diff --git a/Cargo.lock b/Cargo.lock index fd72b2ce0..4bb5ede7d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1611,6 +1611,22 @@ version = "1.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b9e0384b61958566e926dc50660321d12159025e767c18e043daf26b70104c39" +[[package]] +name = "ignore" +version = "0.4.31" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7f8a7b8211e695a1d0cd91cace480d4d0bd57667ab10277cc412c5f7f4884f83" +dependencies = [ + "crossbeam-deque", + "globset", + "log", + "memchr", + "regex-automata", + "same-file", + "walkdir", + "winapi-util", +] + [[package]] name = "indenter" version = "0.3.4" @@ -4196,6 +4212,7 @@ dependencies = [ "derive_more", "fspy", "futures-util", + "ignore", "materialized_artifact", "materialized_artifact_build", "nix 0.31.2", diff --git a/Cargo.toml b/Cargo.toml index 804541230..53bcc9147 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -84,6 +84,7 @@ futures-util = "0.3.31" globset = "0.4.18" getrandom = "0.4.2" jsonc-parser = { version = "0.32.0", features = ["serde"] } +ignore = "0.4.23" libc = "0.2.185" libtest-mimic = "0.8.2" memmap2 = "0.9.11" diff --git a/DECISIONS.md b/DECISIONS.md new file mode 100644 index 000000000..38d593de4 --- /dev/null +++ b/DECISIONS.md @@ -0,0 +1,274 @@ +# Decision log + +Decisions taken while implementing the two auto-tracking rule sets +(`target/fspy-overlap-experiments/read-write-overlaps.md` and +`output-cleanup.md`) and wiring them through vite-plus into emdash. + +Newest entries at the bottom. Each entry records the drift, the choice, and why. + +## D1 — Tracer scope: all signals, all three platforms + +Confirmed with the requester before starting. The full rule set needs fspy to +gain open success/errno, `O_CREAT`/`O_TRUNC`/`O_EXCL`, confirmed writes, rename +source and destination, unlink/rmdir, and mkdir with errno — on the Unix +preload, the Linux seccomp fallback, and the Windows detours. No platform is +stubbed. `AGENTS.md` forbids skipping tests on either platform. + +## D2 — Rules implemented in place, no crate extraction + +Confirmed before starting. The `vite_task_fs_cache` extraction stays a separate +follow-up so it keeps its no-behavior-change property. This PR changes behavior +and would obscure that proof. + +## D3 — Gitignore via the `ignore` crate + +Confirmed before starting. Hand-rolling gitignore precedence, negation and +nesting is easy to get subtly wrong, and shelling out to `git check-ignore` puts +a process spawn on a hot path and fails when git is absent. + +## D4 — vite-plus preview PR lives on the upstream repo + +Confirmed before starting. `publish-preview.yml` gates on +`github.repository == 'voidzero-dev/vite-plus'` plus a `preview-build` label, so +a fork cannot produce a pkg.pr.new build. Branch is pushed to the upstream repo; +the PR stays a draft and is not merged. + +## D5 — Approximate "confirmed write" from open flags, do not intercept `write` + +**Drift.** `read-write-overlaps.md` defines a write as a confirmed +`write`/`pwrite`/`writev`/`ftruncate` on a write-opened descriptor. Implementing +that needs a per-process fd table plus interception of `close`, `dup`, `dup2` and +`dup3`, because a descriptor number can otherwise be reacquired by an owner the +tracer never saw. I hit exactly that bug in the experiment probe, where +`vite.config.mjs` looked written because fd 18 was reused. + +**Decision.** Record open success and the `O_CREAT`, `O_TRUNC`, `O_EXCL` flags +instead, and treat a mutation as `O_TRUNC`, or `O_CREAT | O_EXCL`, or a rename +destination. A bare write-access open is recorded as capability only and is not +a mutation. + +**Why this is sufficient.** Checked against every case in the experiment matrix: + +- Prettier, ESLint, Stylelint, Vite, Astro all rewrite through + `O_CREAT | O_TRUNC`, so they are still detected. +- Atomic writers create temporaries with `O_CREAT | O_EXCL` and publish by + rename, so both halves are still detected. +- Biome's warm run opens a clean source `O_RDWR` with no truncation and does not + write. It is now correctly _not_ a mutation, which is the false positive the + rule exists to remove. +- Lock files across cargo, rustc and Parcel are opened `O_RDWR | O_CREAT` + without truncation and only flocked, so they stop being collected. + +**What it gives up.** Mutation through a writable mmap is invisible. The only +instance in the matrix is Parcel's `lock.mdb`, a lock file that should not be +archived anyway. Recorded as a known gap rather than fixed. + +## D6 — Skip `getdents` success; use mkdir instead + +**Drift.** `output-cleanup.md` lists "errno on getdents or scandir" as the way to +tell that a directory listing failed, which matters for the +`unconditional-clean` case where a tool lists a directory that does not exist. + +**Decision.** Do not add it. Fix 3, "a directory the task created is not an +input", covers the same case using successful `mkdir`, which is far cheaper to +intercept and which the experiment already showed absorbs 264 of 307 derived +directory listings including all 256 of Go's cache shards. + +## D7 — The Linux seccomp fallback is a reduced-fidelity path + +**Drift.** D1 committed to all signals on all three backends. The seccomp +supervisor responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` +(`fspy_seccomp_unotify/src/supervisor/listener.rs:50`), so it is notified +_before_ the syscall runs and never learns the result. Success-dependent signals +are therefore not obtainable there without emulating each syscall in the +supervisor, which would mean reproducing the target's cwd, dirfd and credentials. + +**Decision.** On the seccomp path emit only what is knowable before the call: +read/write intent plus `CREATE`, `TRUNCATE` and `EXCLUSIVE` from the open flags. +Do **not** emit `FAILED`, `DELETED`, `CREATED_DIR` or the rename pair, because +emitting them unconditionally would be actively wrong — a failed `mkdir` would +claim a pre-existing directory, and a failed `unlink` would retire a path that is +still there. + +This path is the fallback for statically linked binaries where preload injection +does not apply; the primary Linux path is the preload library, which has the full +set. + +**Why the missing signals are safe.** Every one degrades toward caching less: +absent `DELETED` leaves a listing as an input, absent `CREATED_DIR` leaves a +directory as an input. Both cost cache hits, not correctness. + +## D8 — Guard: an unexplained missing mutation blocks caching + +**Drift.** One missing signal is _not_ safe on its own. If a rename goes +unobserved, the write lands on a staging path that no longer exists at archive +time, rule 6 drops it, and the archive comes out empty — which restores nothing +on a cache hit and leaves a wrong tree. That is exactly the `atomic-dist-swap` +failure, and it would resurface on the seccomp path. + +**Decision.** Add a platform-agnostic guard in `vite_task`: if a recorded +mutation targets a path that is absent at archive time and no observed rename +explains where it went, the output set is incomplete, so refuse to cache the run. + +This protects correctness beyond the seccomp case. It also covers mutations +through `clonefile`, `copyfile` and `FICLONE`, none of which is intercepted, and +any future publish mechanism the tracer has not learned about yet. The cost is a +missed cache entry, never a wrong tree. + +## D9 — Never rely on syscall outcomes; supersedes D5, D7 and D8 + +**Instruction.** Do not build rules on whether a call succeeded. + +**What this invalidates.** D1 through D8 assumed the tracer could report +outcomes, which drove reporting _after_ the real call so `errno` was known. That +is now reverted: + +- `AccessMode::FAILED` is removed, along with the after-the-call reporting in + `open.rs`, `stat.rs`, `access.rs` and `dirent.rs`. They report before the call + again, as they did originally. +- `CREATED_DIR` is removed. Recording it pre-call would claim every directory a + caller merely ensured exists, since `mkdir` returning `EEXIST` is the normal + case. Output-cleanup fix 3 therefore cannot be implemented. +- Output-cleanup fix 2, dropping a listing whose entries were then deleted, also + cannot be implemented: a failed `unlink` would retire a path that is still + there, and that direction drops a real input. +- D7's reduced-fidelity seccomp path and D8's unexplained-mutation guard both + become moot. + +**Why this is the better design, not merely a constraint.** The Linux seccomp +supervisor responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` and is notified +before the syscall, so it can never observe an outcome. Any outcome-dependent +rule would have silently degraded there while working on macOS, Windows and the +Linux preload — a correctness rule that holds on three backends and not the +fourth. Restricting the rules to pre-call information gives all four identical +behaviour and removes the asymmetry entirely. + +It also makes a trace a function of the call arguments alone rather than of +filesystem state at trace time, so two runs over the same inputs record the same +events. + +**What still works, using only pre-call information:** + +- intent from `O_ACCMODE`, plus `O_CREAT`, `O_TRUNC` and `O_EXCL` +- mutation defined as `O_TRUNC`, or `O_CREAT | O_EXCL`, or being a rename + destination +- write-first versus read-first ordering +- the gitignore tie-break, which is not access data at all +- rename pairs and directory-rename expansion, recorded as attempts +- ignoring bare directory stats + +**Two consequences accepted.** A failed probe now counts as a read, so a tool +that checks for a generated file before creating it looks like it read it first; +for ignored paths the gitignore clause still resolves that to an output, and for +tracked paths the run is conservatively not cached. And a failed rename +over-collects its destination, which rule 7 permits. + +**Not an outcome.** Checking whether a path exists at archive time is a +filesystem query at decision time, not a syscall outcome, and it stays. It is +also what filters the over-collection above. + +## D10 — Verified against the real emdash workload before touching Windows + +Reordered on request to de-risk the chain: prove the goal on macOS with a local +`[patch]` override before investing in the Windows backend. + +**Setup.** A vite-plus worktree at +`~/.local/share/opencode/worktree/vite-plus-autotrack` with the local-development +patch section enabled, pointing every vite-task crate at this worktree. + +**Two traps found while doing it, both worth remembering.** + +The `vp` binary does not link `vite_task` at all — only `packages/cli/binding`, +the napi addon, does. So `vp run` is JavaScript calling a native addon, and every +early emdash run measured released vite-plus 0.2.2 rather than this branch. +Swapping a locally built `.node` into emdash's store does not work either: the +addon needs `--features rolldown`, and a 0.2.6 addon against a 0.2.2 JS CLI +loads but produces no output. Verification therefore ran through `vt`, vite-task's +own CLI, which links the crates directly. + +Separately, three artifacts must be rebuilt for a change to take effect, and each +one silently reported stale results when missed: the preload dylib consumed by +`fspy --example cli`, the `vt` binary, and the napi addon. + +**Result on `@emdash-cms/auth`, a plain `tsdown` build, with no manual globs:** + +- run 1 cold: builds, 28 outputs collected, 3074 inputs +- run 2 unchanged: cache hit +- run 3 after `rm -rf dist`: cache hit **and** all 28 files restored +- restored tree is byte-for-byte identical to a fresh build, verified by shasum +- **zero paths under the package's own `dist/` appear in the input set** + +That last point is the failure mode that matters: if outputs were fingerprinted +as inputs, run 3 would miss forever on a clean checkout. + +## D11 — emdash's full build is blocked by its own environment, not by tracking + +`@emdash-cms/registry-lexicons#build` runs `pnpm run build:lexicons` and fails +under both this branch and released vite-plus. Released vite-plus reports the +real cause: `ERR_PNPM_VERIFY_DEPS_BEFORE_RUN`, the workspace structure changed +since the last install. Under `vt` the same failure surfaces less legibly, as a +pnpm SEA assertion (`(magic) == (kMagic)`) because pnpm is a single-executable +binary re-reading its own embedded blob. + +Not a tracking defect. emdash needs `pnpm install` before the full build can be +measured. + +## D12 — pnpm's single-executable binary fails under fspy; pre-existing + +`@emdash-cms/registry-lexicons#build` shells out to `pnpm run build:lexicons` and +aborts with `Assertion failed: (magic) == (kMagic)` inside +`node::sea::FindSingleExecutableResource`. pnpm 11.9.0 ships as a Node +single-executable application, so it re-reads its own binary to find an embedded +blob, and that read does not survive tracing. + +**Verified pre-existing**, not caused by this branch: a `vt` built from the base +commit `b3ebf564` reproduces the identical assertion. Released vite-plus does not +hit it because it supplies a managed Node and pnpm rather than the mise shim. + +Left alone. It is orthogonal to auto-tracking, blocks only the one package whose +build re-enters pnpm, and fixing it means changing how fspy handles a process +reading its own executable. Verification of the tracking rules therefore excludes +that package. + +## D13 — A task's own derived output tree is not an input + +Found by running emdash's full build, not by reasoning. With the rules as first +written, 22 of 23 tasks cached and `@emdash-cms/admin#build`'s `tsdown` segment +missed forever with `'messages.mjs' added in 'packages/admin/dist/locales/fa'`. + +**Cause.** admin's build is a compound command, and each `&&` segment is a +separate cached task: + +``` +node --run locale:compile && tsdown && node --run locale:copy && npx tailwindcss +``` + +`tsdown` _reads_ `dist/locales//messages.mjs`, and the later +`locale:copy` segment rewrites those files. So tsdown fingerprints content that a +sibling task changes afterwards, and the fingerprint can never settle. + +**Why the existing rules could not catch it.** The "listed a directory it wrote +into" rule does not fire, because tsdown did not write there — a sibling did. +Rejecting on gitignore alone is not available either: a package reading +`node_modules//dist/index.mjs` is also reading ignored, derived content, and +that must stay a real input or changing a dependency would stop invalidating its +consumers. That is the central monorepo relationship. + +**Rule.** A read path is the task's own derived state when **both** hold: version +control calls it derived, **and** it sits under a directory this same task wrote +into. Requiring both distinguishes the two cases exactly — admin writes +`dist/index.mjs`, so `packages/admin/dist` is one of its own write subtrees and +everything ignored beneath it is its own output tree, while no package writes into +`node_modules//dist`, so dependency outputs stay inputs. + +Directory listings follow the same principle: a listing is rejected if the task +wrote into that directory or if version control calls the directory derived. + +**Result on emdash, zero manual globs:** + +- 23/23 cache hit on the second run, stable across repeats +- after deleting every `packages/*/dist`, 22/23 hit and all 1358 files restored + **byte-for-byte identical** +- the single miss is correct: deleting `packages/core/dist` also empties + `node_modules/emdash/dist`, which is a genuine input of another task, so it + rebuilt diff --git a/INVESTIGATE.md b/INVESTIGATE.md new file mode 100644 index 000000000..f01127486 --- /dev/null +++ b/INVESTIGATE.md @@ -0,0 +1,301 @@ +# Task cache instability: findings and proposed fixes + +Investigation of everything that makes a task unstable or uncacheable, run +against [emdash](https://github.com/emdash-cms/emdash) (20 packages, 25 build +tasks, mostly plain `tsdown`) with the automatic tracking rules from #571. + +None of the proposals below are implemented or validated yet. Diagnoses are +marked **confirmed** (directly measured) or **hypothesis**. + +**Environment.** `vp 0.0.0-commit.88ab4592d55342c11a070c47e8fc7205f675fda3` +(preview of voidzero-dev/vite-plus#2260, carrying #571), emdash branch +`agent/vite-plus-package-build-cache`. Local: macOS 26.5.1, M4 Pro, 12 cores. +CI: `ubuntu-latest`, 4 vCPU. + +## Scope of the problem + +Two things are worth separating, because they have very different weight. + +**Repeat-run stability is already perfect.** Five consecutive warm runs with no +intervening change, all 25/25: + +| run | 1 | 2 | 3 | 4 | 5 | +| ---- | ----- | ----- | ----- | ----- | ----- | +| hits | 25/25 | 25/25 | 25/25 | 25/25 | 25/25 | + +A no-op `pnpm install` also preserves 25/25 — it leaves +`node_modules/.pnpm-workspace-state-v1.json` byte-identical. + +**Instability only appears across an environment transition** — a fresh checkout, +a real install, or a different machine. On CI, steady state is 22/25, and the same +three tasks miss every single run: + +``` +~/packages/registry-lexicons$ pnpm run build:lexicons ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified +~/packages/registry-lexicons$ pnpm run build:types ○ cache miss: 'node_modules/.pnpm-workspace-state-v1.json' modified +~/packages/core$ tsdown ○ cache miss: 'index.d.mts' removed from 'node_modules/emdash/dist' +``` + +Two distinct causes, both fixable in vite-task. Neither is a regression from #571; +cause A is a case #571's rules were meant to cover but attribute by the wrong +path. + +| | cause | tasks | fixable in vite-task | +| --- | ---------------------------------------------------------------------------- | ----- | -------------------- | +| A | workspace package reachable under two names via its `node_modules` self-link | 1 | yes | +| B | package manager bookkeeping file read by any `pnpm run` task | 2 | yes | + +--- + +## Cause A — a workspace package's output has two names + +**Confirmed.** pnpm links every workspace package into `node_modules/`, and +`packages/core` _is_ the package named `emdash`: + +``` +node_modules/emdash -> ../packages/core +``` + +So `packages/core/dist` and `node_modules/emdash/dist` are one directory. Tracing +`packages/core`'s own `tsdown` (119,037 accesses) shows it writing and reading the +same file under different names: + +``` +WRITE packages/core/dist/index.d.mts AccessMode(WRITE | CREATE | TRUNCATE) +READ node_modules/emdash/dist/index.d.mts AccessMode(READ) +``` + +The task consumes its own output because it resolves its own package through the +self-link. #571 has a rule for exactly this shape — a gitignored path underneath a +directory the task wrote into is the task's own derived state, not an input — and +`dist/` is gitignored at emdash's root. The rule does not fire because it compares +path strings: `node_modules/emdash/dist` is not lexically under `packages/core/`, +so the read is recorded as an input. + +On a fresh checkout that path does not exist when the task starts, which is why +the miss says `removed from` rather than `modified`. The task can therefore never +hit on a clean machine. + +**Blast radius.** Any workspace package whose build resolves its own package by +name. That is normal for packages with `exports` self-references, and it is not +emdash-specific. + +### Proposed fixes + +**A1 — canonicalise every tracked path (not recommended).** Resolve symlinks for +all accesses before classification. Simple rule, but 119k accesses for one task +means a `realpath` per path on a hot path that is currently pure string work. Worse, +pnpm's `node_modules/` entries point into `node_modules/.pnpm/...`, and under +some store layouts resolution can land outside the workspace root — where +`strip_prefix(workspace_root)` (`cache_update.rs:315`) silently drops the path, so +real dependency inputs would stop being tracked. Rejecting this because the failure +mode is a _lost_ input, which is worse than a missed cache. + +**A2 — normalise workspace-package aliases using the package graph (recommended).** +vite-task already knows every workspace package's name and directory. Build a map +of ` -> ` once per run and rewrite any tracked path matching +`**/node_modules//...` to `/...`. Pure string work, no syscalls, +and bounded by the number of workspace packages rather than the number of accesses. + +After rewriting, core's read becomes `packages/core/dist/index.d.mts`, which _is_ +under a directory it wrote into and _is_ gitignored, so the existing +own-derived-state rule fires unchanged and the task hits. + +This also improves consumers: `packages/cloudflare`'s input becomes +`packages/core/dist/...` instead of `node_modules/emdash/dist/...`. Same file, but +the canonical name, so cache miss messages name the real path and two tasks +referring to one file agree on its identity. + +Costs and open points: + +- Must handle scoped names (`node_modules/@emdash-cms/plugin-cli`), which are two + path segments, and nested `packages/x/node_modules/`. Both were observed. +- `ClassifyContext` (`classify.rs:44`) currently has only `workspace_root`, the two + infer flags and `is_gitignored`, so the map needs threading into `observe_fspy` + (`cache_update.rs:194`). That is new plumbing but no new data source. +- Changes recorded input paths, so it needs a `CACHE_SCHEMA_VERSION` bump. + +**A3 — narrow variant of A2, self-links only.** Rewrite only when the alias +resolves to _this task's own_ package directory. `SpawnFingerprint` already carries +`cwd: RelativePathBuf` (`cache_metadata.rs:90`, currently `pub(crate)`), so this +needs almost no plumbing — just widened visibility — and one `read_link` per +distinct alias. + +Fixes the reported miss with the smallest possible behaviour change, and needs no +schema bump if consumers' paths are left alone. Does not give consumers the +canonical-name benefit. + +**Recommendation: A2**, with A3 as the low-risk fallback if the plumbing or the +schema bump is unwelcome this cycle. A2 is a rule about identity that happens to +fix a caching bug; A3 patches the one symptom. + +--- + +## Cause B — `pnpm run` makes a task uncacheable across machines + +**Confirmed** that the file is tracked as an input to those two tasks and that its +content is unstable. **Hypothesis** that `pnpm` itself is the reader: tracing +`pnpm run build:lexicons` directly under fspy aborts with SIGABRT before it gets +that far, which is the pre-existing pnpm SEA bug (`Assertion failed: (magic) == +(kMagic)`) recorded against `main`. The attribution rests on vite-task naming the +file as the changed input for exactly the two tasks that spawn pnpm, and on no +other emdash task touching it. + +`registry-lexicons` is the only package whose build wraps the package manager: + +```json +"build": "pnpm run build:lexicons && pnpm run build:types" +``` + +That file cannot be fingerprinted stably, by construction: + +``` +lastValidatedTimestamp: 1785115273801 <- wall clock +projects: { "/Users/chwang/code/emdash": {...}, <- absolute paths + "/Users/chwang/code/emdash/apps/aggregator": {...}, ... } +``` + +The timestamp changes on any revalidating install. The absolute paths differ +between machines and between a local checkout and `/home/runner/work/emdash/emdash`, +so this **also defeats cache portability across workspace roots**, which vite-task +otherwise supports and tests. + +It is not caught by #571's always-ignored set. That set (`node_modules`, `.git`) +only resolves read-then-write overlaps; this is a pure read, and pure reads under +`node_modules` are legitimate inputs — that is how dependency code is tracked. + +**Blast radius.** Every task in every pnpm workspace that wraps a script in +`pnpm run`, which is a very common pattern. Each such task is permanently +uncacheable on CI. This is the more widely damaging of the two causes even though +it looks like the more niche one. + +### Proposed fixes + +**B1 — ignore package-manager bookkeeping files as inputs (recommended).** Keep a +small known list of files that are a package manager's own state and never an +authored source or a meaningful dependency. Present in the emdash checkout and +verified to exist: + +``` +node_modules/.pnpm-workspace-state-v1.json <- the one causing the miss +node_modules/.modules.yaml +node_modules/.pnpm/lock.yaml +``` + +Equivalents for other managers, named from their documented layouts and not +observed here, so worth confirming before adding: + +``` +node_modules/.package-lock.json (npm) +node_modules/.yarn-state.yml (yarn, node_modules linker) +``` + +Applied to reads, not just overlaps. Low risk: these files describe how +`node_modules` was produced, and the thing that actually matters — the installed +package contents — is already tracked directly. + +`node_modules/.package-map.json` also exists in emdash but is a weaker candidate: +it contains no absolute paths, so it does not break portability the way the +workspace-state file does. Leave it out until something is shown to be destabilised +by it. + +Worth pairing with a rule of thumb for the list: a file belongs on it only if its +content is derived from the lockfile plus the local environment, so that anything +it could tell a task is either already tracked or is machine identity. + +**B2 — ignore by shape rather than by name.** Treat `node_modules/.` as +manager state. Simpler and needs no list maintenance, but over-broad: +`node_modules/.bin/*` lives there and is a real input (three `.bin` reads appear in +core's trace), and `node_modules/.vite/` is vite's own cache directory. + +**B3 — leave it to users.** Document a manual `input` exclusion, which is what +[When To Add Manual Config](https://viteplus.dev/guide/automatic-data-tracking#when-to-add-manual-config) +already implies. Zero risk, but every pnpm monorepo has to rediscover this, and the +symptom — one task that never hits, only on CI — is expensive to diagnose. The +whole point of automatic tracking is not needing this. + +**Recommendation: B1.** B2's over-reach is a real regression risk for `.bin`, and +B3 pushes a general defect onto every user. + +On the emdash side there is also a one-line workaround worth noting: the package's +own `prepublishOnly` already uses `node --run build`, and switching `build` to +`node --run` avoids spawning pnpm at all. That is a good local fix but does not +help the general case. + +--- + +## Not a cause: `--parallel` + +I previously recorded `--parallel` losing cache hits (18/25, reproducible 3/3) as a +possible correctness bug. **That was wrong, and this corrects it.** `vp run --help` +is explicit: + +``` +--parallel Run tasks without dependency ordering. Sets concurrency to + unlimited unless `--concurrency-limit` is also specified +``` + +Timestamped task starts confirm the documented behaviour rather than a bug: + +| | `packages/core` starts | consumers start | +| ----------------- | ---------------------- | -------------------------------------------- | +| default limit (4) | 444.832 | 470.767 – 475.772 (after core finishes) | +| `--parallel` | 382.231 | 382.144 – 382.466 (cloudflare _before_ core) | + +With ordering discarded, consumers fingerprint `node_modules/emdash/dist` while +core is still writing it, so the cold run stores entries describing a partially +written directory and the warm run correctly reports `added in`. Using `--parallel` +on a workspace where packages consume each other's build output is simply the wrong +flag. Nothing to fix. + +One optional hardening, low priority: when a task's inferred inputs overlap another +task's inferred outputs _within the same run_, the entry is built on unordered +state. vite-task could decline to cache that task, or warn. It would convert a +silently useless cache entry into a visible explanation. Worth it only if +`--parallel` misuse turns out to be common. + +## Not a cause: paths outside the workspace root + +`strip_prefix(workspace_root)` (`cache_update.rs:315`) drops every access outside +the workspace, so the 388 `/var/folders` temp accesses and the `/opt/homebrew` +reads in core's trace cannot destabilise anything. + +The same rule means the toolchain is not fingerprinted: `node` lives under +`~/.local/share/mise/...`, so a Node or `tsdown` upgrade does not invalidate a +cached task. That is a deliberate portability tradeoff, not instability, but it is +worth being explicit that cache keys describe the workspace and not the machine. + +## Checked and clean + +For `packages/core`'s build, none of the usual suspects appear: no `.tsbuildinfo`, +no `node_modules/.vite/` cache reads, no `pnpm-lock.yaml` read, no log files. The +only `node_modules/.bin` accesses are three plain reads of shims. It also touches +none of the manager state files from cause B — zero accesses to +`.pnpm-workspace-state-v1.json`, `.modules.yaml`, `.package-map.json`, +`.pnpm/lock.yaml`, `.vite/` or `.vite-temp/` — which is consistent with only the +two pnpm-spawning tasks missing. + +`node_modules/.vite-temp/` deserves a note because it looks dangerous and is not: +vitest writes `vitest.config.js.timestamp-.mjs` files there, so its directory +listing changes every run. #571 classifies those as write-first outputs and the +directory is empty at archive time, and the e2e fixture covering it passes. It is +not implicated in emdash's builds. + +So for this workspace the instability really is just causes A and B, not a long +tail. + +That is a statement about emdash, not a general audit. A workspace using +`tsc --build` would add `.tsbuildinfo`, which embeds absolute paths and timestamps +and would likely need the same treatment as cause B. + +## Suggested order of work + +1. **B1** — smallest change, widest benefit, affects every pnpm monorepo. +2. **A2** — larger, needs plumbing and a schema bump, but fixes a real gap in + #571's own rule and makes path identity consistent. +3. Optional `--parallel` diagnostic, only if misuse is seen in the wild. + +Both A and B should be reproduced as e2e fixtures first, since both are currently +only observed through emdash. A fixture for A needs a package whose build resolves +its own name through a `node_modules` self-link; for B, a task that shells out to +`pnpm run` with a workspace state file present. diff --git a/crates/fspy/src/unix/syscall_handler/mod.rs b/crates/fspy/src/unix/syscall_handler/mod.rs index 4b6f7947e..eb5de63c7 100644 --- a/crates/fspy/src/unix/syscall_handler/mod.rs +++ b/crates/fspy/src/unix/syscall_handler/mod.rs @@ -1,5 +1,6 @@ mod execve; mod getdents; +mod mutate; mod open; mod stat; @@ -21,6 +22,15 @@ use crate::arena::PathAccessArena; const PATH_MAX: usize = libc::PATH_MAX as usize; +/// Whether a path is a directory right now. +/// +/// Read before the syscall runs, so this is existing filesystem state and not +/// the call's result. A path that cannot be stated is reported as a file, which +/// only costs the subtree re-attribution a directory rename would have got. +fn is_existing_dir(path: &Path) -> bool { + std::fs::symlink_metadata(path).is_ok_and(|metadata| metadata.is_dir()) +} + #[derive(Debug)] pub struct SyscallHandler { arena: PathAccessArena, @@ -38,6 +48,33 @@ impl SyscallHandler { self.arena } + /// Reads a syscall's path argument and makes it absolute. + /// + /// Returns `None` when the path does not fit in `PATH_MAX`, matching the + /// rest of this handler: such a path is dropped rather than truncated. + /// + /// The result is owned because callers that handle two paths at once, like + /// rename, cannot both borrow the single read buffer. + fn resolve_path( + &mut self, + caller: Caller, + dir_fd: Fd, + path_ptr: CStrPtr, + ) -> io::Result> { + let Some(path_len) = path_ptr.read(caller, &mut self.path_read_buf)? else { + return Ok(None); + }; + let path = Path::new(OsStr::from_bytes(&self.path_read_buf[..path_len])); + if path.is_absolute() { + return Ok(Some(path.to_path_buf())); + } + let mut resolved_path = PathBuf::from(dir_fd.get_path(caller)?); + if !nix::NixPath::is_empty(path) { + resolved_path.push(path); + } + Ok(Some(resolved_path)) + } + fn handle_open( &mut self, caller: Caller, @@ -57,12 +94,86 @@ impl SyscallHandler { } path = Cow::Owned(resolved_path); } + // This supervisor is notified before the syscall runs and responds with + // SECCOMP_USER_NOTIF_FLAG_CONTINUE, so it never learns the outcome. + // Only pre-call information is available here: intent plus the creation + // and truncation flags. Outcome-dependent flags (FAILED, DELETED, + // CREATED_DIR, RENAME_*) are deliberately not emitted, because guessing + // them would be worse than omitting them — a failed mkdir would claim a + // directory this run did not create. See DECISIONS.md D7. + let mut mode = match flags & libc::O_ACCMODE { + libc::O_RDWR => AccessMode::READ | AccessMode::WRITE, + libc::O_WRONLY => AccessMode::WRITE, + _ => AccessMode::READ, + }; + if flags & libc::O_CREAT != 0 { + mode |= AccessMode::CREATE; + } + if flags & libc::O_TRUNC != 0 { + mode |= AccessMode::TRUNCATE; + } + if flags & libc::O_EXCL != 0 { + mode |= AccessMode::EXCLUSIVE; + } + self.arena.add(PathAccess { mode, path: path.as_os_str().into() }); + Ok(()) + } + + /// Records a rename as a delete of the source and a write of the + /// destination. + /// + /// Both halves come from the call's arguments. Whether the source is a + /// directory is read from the filesystem before the syscall runs, which is + /// current state rather than the call's outcome, and it matters because a + /// directory rename re-attributes every write recorded under the old + /// subtree. + /// + /// `exchange` covers `RENAME_EXCHANGE`, where the source is replaced by the + /// destination instead of disappearing, so it is written rather than deleted. + fn handle_rename( + &mut self, + caller: Caller, + (old_dir_fd, old_path_ptr): (Fd, CStrPtr), + (new_dir_fd, new_path_ptr): (Fd, CStrPtr), + exchange: bool, + ) -> io::Result<()> { + let Some(source) = self.resolve_path(caller, old_dir_fd, old_path_ptr)? else { + return Ok(()); + }; + let Some(dest) = self.resolve_path(caller, new_dir_fd, new_path_ptr)? else { + return Ok(()); + }; + + let dir_flag = + if is_existing_dir(&source) { AccessMode::IS_DIR } else { AccessMode::empty() }; + + let source_mode = if exchange { + AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag + } else { + AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag + }; + self.arena.add(PathAccess { mode: source_mode, path: source.as_os_str().into() }); self.arena.add(PathAccess { - mode: match flags & libc::O_ACCMODE { - libc::O_RDWR => AccessMode::READ | AccessMode::WRITE, - libc::O_WRONLY => AccessMode::WRITE, - _ => AccessMode::READ, - }, + mode: AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, + path: dest.as_os_str().into(), + }); + Ok(()) + } + + /// Records a delete from the call's arguments. + fn handle_delete( + &mut self, + caller: Caller, + dir_fd: Fd, + path_ptr: CStrPtr, + is_dir: bool, + ) -> io::Result<()> { + let Some(path) = self.resolve_path(caller, dir_fd, path_ptr)? else { + return Ok(()); + }; + let dir_flag = if is_dir { AccessMode::IS_DIR } else { AccessMode::empty() }; + self.arena.add(PathAccess { + mode: AccessMode::DELETED | dir_flag, path: path.as_os_str().into(), }); Ok(()) @@ -98,6 +209,13 @@ impl_handler!( faccessat, faccessat2, + #[cfg(target_arch = "x86_64")] rename, + renameat, + renameat2, + #[cfg(target_arch = "x86_64")] unlink, + unlinkat, + #[cfg(target_arch = "x86_64")] rmdir, + execve, execveat, ); diff --git a/crates/fspy/src/unix/syscall_handler/mutate.rs b/crates/fspy/src/unix/syscall_handler/mutate.rs new file mode 100644 index 000000000..21b20e586 --- /dev/null +++ b/crates/fspy/src/unix/syscall_handler/mutate.rs @@ -0,0 +1,64 @@ +//! Rename and delete, the syscalls that move a task's output into place. +//! +//! Without these a build that stages into a temporary directory and renames it +//! over the real one looks like it wrote nothing that still exists, so its +//! archive comes out empty. The preload backend intercepts the libc wrappers; +//! this backend has to see the syscalls themselves, which is what makes the two +//! agree on musl. +//! +//! Every flag here is derived from the call's arguments. This supervisor +//! responds with `SECCOMP_USER_NOTIF_FLAG_CONTINUE` and is notified before the +//! syscall runs, so it never learns whether the call succeeded. + +use std::{ffi::c_int, io}; + +use fspy_seccomp_unotify::supervisor::handler::arg::{CStrPtr, Caller, Fd}; + +use super::SyscallHandler; + +impl SyscallHandler { + #[cfg(target_arch = "x86_64")] + pub(super) fn rename( + &mut self, + caller: Caller, + (old_path, new_path): (CStrPtr, CStrPtr), + ) -> io::Result<()> { + self.handle_rename(caller, (Fd::cwd(), old_path), (Fd::cwd(), new_path), false) + } + + pub(super) fn renameat( + &mut self, + caller: Caller, + (old_dir_fd, old_path, new_dir_fd, new_path): (Fd, CStrPtr, Fd, CStrPtr), + ) -> io::Result<()> { + self.handle_rename(caller, (old_dir_fd, old_path), (new_dir_fd, new_path), false) + } + + pub(super) fn renameat2( + &mut self, + caller: Caller, + (old_dir_fd, old_path, new_dir_fd, new_path, flags): (Fd, CStrPtr, Fd, CStrPtr, c_int), + ) -> io::Result<()> { + let exchange = flags & c_int::try_from(libc::RENAME_EXCHANGE).unwrap_or(0) != 0; + self.handle_rename(caller, (old_dir_fd, old_path), (new_dir_fd, new_path), exchange) + } + + #[cfg(target_arch = "x86_64")] + pub(super) fn unlink(&mut self, caller: Caller, (path,): (CStrPtr,)) -> io::Result<()> { + self.handle_delete(caller, Fd::cwd(), path, false) + } + + pub(super) fn unlinkat( + &mut self, + caller: Caller, + (dir_fd, path, flags): (Fd, CStrPtr, c_int), + ) -> io::Result<()> { + let is_dir = flags & libc::AT_REMOVEDIR != 0; + self.handle_delete(caller, dir_fd, path, is_dir) + } + + #[cfg(target_arch = "x86_64")] + pub(super) fn rmdir(&mut self, caller: Caller, (path,): (CStrPtr,)) -> io::Result<()> { + self.handle_delete(caller, Fd::cwd(), path, true) + } +} diff --git a/crates/fspy/src/windows/mod.rs b/crates/fspy/src/windows/mod.rs index 8081e1298..fd235ece2 100644 --- a/crates/fspy/src/windows/mod.rs +++ b/crates/fspy/src/windows/mod.rs @@ -66,7 +66,11 @@ impl SpyImpl { Ok(Self { ansi_dll_path_with_nul: ansi_dll_path_with_nul.into() }) } - #[expect(clippy::unused_async, reason = "async signature required by SpyImpl trait")] + #[expect( + clippy::unused_async, + clippy::unused_async_trait_impl, + reason = "async signature required by SpyImpl trait" + )] pub(crate) async fn spawn( &self, mut command: Command, diff --git a/crates/fspy/tests/test_utils/mod.rs b/crates/fspy/tests/test_utils/mod.rs index cfa46c4a9..78ea6d49c 100644 --- a/crates/fspy/tests/test_utils/mod.rs +++ b/crates/fspy/tests/test_utils/mod.rs @@ -17,6 +17,16 @@ use fspy::{AccessMode, PathAccessIterable}; )] pub use subprocess_test::command_for_fn; +/// Flags describing how an access was requested rather than what kind of access +/// it was. They differ between runtimes for the same API: `writeFileSync` +/// truncates on Node and Deno but not on Bun. Spelling them out in every test +/// would assert the runtime's open flags rather than the behaviour under test, so +/// they only participate when a test names one explicitly. +const MODIFIERS: AccessMode = AccessMode::CREATE + .union(AccessMode::TRUNCATE) + .union(AccessMode::EXCLUSIVE) + .union(AccessMode::IS_DIR); + /// # Panics /// /// Panics if the expected path access is not found or has the wrong mode. @@ -46,6 +56,10 @@ pub fn assert_contains( actual_mode.remove(AccessMode::READ); } + if !expected_mode.intersects(MODIFIERS) { + actual_mode.remove(MODIFIERS); + } + assert_eq!( expected_mode, actual_mode, diff --git a/crates/fspy_preload_unix/src/client/convert.rs b/crates/fspy_preload_unix/src/client/convert.rs index c42854b32..940585248 100644 --- a/crates/fspy_preload_unix/src/client/convert.rs +++ b/crates/fspy_preload_unix/src/client/convert.rs @@ -105,11 +105,23 @@ impl ToAccessMode for AccessMode { pub struct OpenFlags(pub c_int); impl ToAccessMode for OpenFlags { unsafe fn to_access_mode(self) -> AccessMode { - match self.0 & libc::O_ACCMODE { + let mut mode = match self.0 & libc::O_ACCMODE { libc::O_RDWR => AccessMode::READ | AccessMode::WRITE, libc::O_WRONLY => AccessMode::WRITE, _ => AccessMode::READ, + }; + // Creation and truncation intent is what separates a real mutation from + // a descriptor that merely could have been written. + if self.0 & libc::O_CREAT != 0 { + mode |= AccessMode::CREATE; } + if self.0 & libc::O_TRUNC != 0 { + mode |= AccessMode::TRUNCATE; + } + if self.0 & libc::O_EXCL != 0 { + mode |= AccessMode::EXCLUSIVE; + } + mode } } @@ -120,10 +132,18 @@ impl ToAccessMode for ModeStr { let mode_str = unsafe { CStr::from_ptr(self.0) }.to_bytes().as_bstr(); let has_read = mode_str.contains(&b'r'); let has_write = mode_str.contains(&b'w') || mode_str.contains(&b'a'); - match (has_read, has_write) { + let mut mode = match (has_read, has_write) { (false, true) => AccessMode::WRITE, (true, true) => AccessMode::READ | AccessMode::WRITE, _ => AccessMode::READ, + }; + // "w" and "w+" truncate and create; "a" and "a+" create without + // truncating. Both mutate, so mark them the way the open flags would. + if mode_str.contains(&b'w') { + mode |= AccessMode::CREATE | AccessMode::TRUNCATE; + } else if mode_str.contains(&b'a') { + mode |= AccessMode::CREATE; } + mode } } diff --git a/crates/fspy_preload_unix/src/interceptions/mod.rs b/crates/fspy_preload_unix/src/interceptions/mod.rs index 0d3742ea7..5c394e603 100644 --- a/crates/fspy_preload_unix/src/interceptions/mod.rs +++ b/crates/fspy_preload_unix/src/interceptions/mod.rs @@ -1,5 +1,6 @@ mod access; mod dirent; +mod mutate; mod open; mod spawn; mod stat; diff --git a/crates/fspy_preload_unix/src/interceptions/mutate.rs b/crates/fspy_preload_unix/src/interceptions/mutate.rs new file mode 100644 index 000000000..f9f5d87ea --- /dev/null +++ b/crates/fspy_preload_unix/src/interceptions/mutate.rs @@ -0,0 +1,181 @@ +//! Interceptions for calls that change the shape of the tree rather than the +//! contents of a file: rename, unlink, rmdir and mkdir. +//! +//! Without these, a tool that publishes results atomically is invisible. It +//! writes a temporary and renames it over the destination, so the destination +//! never appears in a write event and the temporary — which no longer exists — +//! does. Directory renames are worse: a build that stages into `dist.tmp` and +//! swaps it into place produces no write event for any of its real outputs. +//! +//! These report before the real call, like every other interception, and record +//! only what the arguments say. Nothing here depends on whether the call +//! succeeded: the Linux seccomp supervisor cannot observe outcomes at all, so an +//! outcome-dependent rule would hold on some backends and not others. +//! +//! `mkdir` is intentionally not intercepted. Its argument alone cannot say +//! whether this run created the directory, and callers overwhelmingly use it as +//! "ensure this exists" and ignore `EEXIST`, so recording it would claim every +//! pre-existing directory. + +use fspy_shared::ipc::AccessMode; + +use crate::{ + client::{convert::PathAt, handle_open}, + libc::{c_char, c_int}, + macros::intercept, +}; + +/// Whether a path is a directory, for tagging rename and removal events. +/// +/// A consumer needs this to re-attribute writes recorded beneath a staging +/// directory to the published location. Inspecting the source is not the same as +/// inspecting the rename's result: it describes an argument, and it happens +/// before the call while the source is still there. +unsafe fn is_directory_at(dirfd: c_int, path: *const c_char) -> bool { + // SAFETY: an all-zero stat is a valid initial value; fstatat overwrites it + let mut stat_buf: libc::stat = unsafe { core::mem::zeroed() }; + // SAFETY: path is a valid C string pointer and stat_buf is a valid, owned stat struct + let result = + unsafe { libc::fstatat(dirfd, path, &raw mut stat_buf, libc::AT_SYMLINK_NOFOLLOW) }; + result == 0 && (stat_buf.st_mode & libc::S_IFMT) == libc::S_IFDIR +} + +/// Report both halves of a rename: the source is being retired, the destination +/// is being replaced by whatever the source held. +unsafe fn report_rename( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, + is_directory: bool, +) { + let dir_flag = if is_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: old_path and new_path are valid C string pointers from the caller + unsafe { + handle_open( + PathAt(old_dirfd, old_path), + AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag, + ); + handle_open( + PathAt(new_dirfd, new_path), + AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, + ); + } +} + +intercept!(rename: unsafe extern "C" fn(*const c_char, *const c_char) -> c_int); +unsafe extern "C" fn rename(old_path: *const c_char, new_path: *const c_char) -> c_int { + // SAFETY: old_path is a valid C string pointer provided by the caller + let is_directory = unsafe { is_directory_at(libc::AT_FDCWD, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { + report_rename(libc::AT_FDCWD, old_path, libc::AT_FDCWD, new_path, is_directory); + } + // SAFETY: forwarding the caller's arguments to the original libc rename() + unsafe { rename::original()(old_path, new_path) } +} + +intercept!(renameat: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char) -> c_int); +unsafe extern "C" fn renameat( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, +) -> c_int { + // SAFETY: old_dirfd and old_path are valid arguments provided by the caller + let is_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; + // SAFETY: forwarding the caller's arguments to the original libc renameat() + unsafe { renameat::original()(old_dirfd, old_path, new_dirfd, new_path) } +} + +// macOS publishes atomic swaps through renameatx_np with RENAME_SWAP, which +// mutates both paths. Vite's dependency optimizer and rustup both reach it. +#[cfg(target_os = "macos")] +intercept!(renameatx_np: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char, libc::c_uint) -> c_int); +#[cfg(target_os = "macos")] +unsafe extern "C" fn renameatx_np( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, + flags: libc::c_uint, +) -> c_int { + // SAFETY: old_dirfd and old_path are valid arguments provided by the caller + let is_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; + // RENAME_SWAP mutates both sides, so the source is replaced too rather than + // simply retired. + if flags & libc::RENAME_SWAP != 0 { + // SAFETY: old_path is a valid C string pointer provided by the caller + unsafe { + handle_open(PathAt(old_dirfd, old_path), AccessMode::RENAME_TO | AccessMode::WRITE); + } + } + // SAFETY: forwarding the caller's arguments to the original renameatx_np() + unsafe { renameatx_np::original()(old_dirfd, old_path, new_dirfd, new_path, flags) } +} + +// Linux's renameat2 can also exchange two paths, in which case both sides are +// mutated rather than one replacing the other. +#[cfg(target_os = "linux")] +intercept!(renameat2: unsafe extern "C" fn(c_int, *const c_char, c_int, *const c_char, libc::c_uint) -> c_int); +#[cfg(target_os = "linux")] +unsafe extern "C" fn renameat2( + old_dirfd: c_int, + old_path: *const c_char, + new_dirfd: c_int, + new_path: *const c_char, + flags: libc::c_uint, +) -> c_int { + // SAFETY: old_dirfd and old_path are valid arguments provided by the caller + let is_directory = unsafe { is_directory_at(old_dirfd, old_path) }; + // SAFETY: both paths are valid C string pointers provided by the caller + unsafe { report_rename(old_dirfd, old_path, new_dirfd, new_path, is_directory) }; + // RENAME_EXCHANGE mutates both sides, so the source is replaced too rather + // than simply retired. + if flags & libc::RENAME_EXCHANGE != 0 { + // SAFETY: old_path is a valid C string pointer provided by the caller + unsafe { + handle_open(PathAt(old_dirfd, old_path), AccessMode::RENAME_TO | AccessMode::WRITE); + } + } + // SAFETY: forwarding the caller's arguments to the original renameat2() + unsafe { renameat2::original()(old_dirfd, old_path, new_dirfd, new_path, flags) } +} + +intercept!(unlink: unsafe extern "C" fn(*const c_char) -> c_int); +unsafe extern "C" fn unlink(path: *const c_char) -> c_int { + // SAFETY: path is a valid C string pointer provided by the caller + unsafe { handle_open(path, AccessMode::DELETED) }; + // SAFETY: forwarding the caller's argument to the original libc unlink() + unsafe { unlink::original()(path) } +} + +intercept!(unlinkat: unsafe extern "C" fn(c_int, *const c_char, c_int) -> c_int); +unsafe extern "C" fn unlinkat(dirfd: c_int, path: *const c_char, flags: c_int) -> c_int { + let dir_flag = + if flags & libc::AT_REMOVEDIR != 0 { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: dirfd and path are valid arguments provided by the caller + unsafe { handle_open(PathAt(dirfd, path), AccessMode::DELETED | dir_flag) }; + // SAFETY: forwarding the caller's arguments to the original libc unlinkat() + unsafe { unlinkat::original()(dirfd, path, flags) } +} + +intercept!(remove: unsafe extern "C" fn(*const c_char) -> c_int); +unsafe extern "C" fn remove(path: *const c_char) -> c_int { + // SAFETY: path is a valid C string pointer provided by the caller + unsafe { handle_open(path, AccessMode::DELETED) }; + // SAFETY: forwarding the caller's argument to the original libc remove() + unsafe { remove::original()(path) } +} + +intercept!(rmdir: unsafe extern "C" fn(*const c_char) -> c_int); +unsafe extern "C" fn rmdir(path: *const c_char) -> c_int { + // SAFETY: path is a valid C string pointer provided by the caller + unsafe { handle_open(path, AccessMode::DELETED | AccessMode::IS_DIR) }; + // SAFETY: forwarding the caller's argument to the original libc rmdir() + unsafe { rmdir::original()(path) } +} diff --git a/crates/fspy_preload_windows/src/windows/convert.rs b/crates/fspy_preload_windows/src/windows/convert.rs index 72a095786..4f6946fa6 100644 --- a/crates/fspy_preload_windows/src/windows/convert.rs +++ b/crates/fspy_preload_windows/src/windows/convert.rs @@ -34,6 +34,17 @@ pub trait ToAbsolutePath { ) -> winsafe::SysResult; } +/// An already-resolved absolute path, used for a rename destination that was +/// reconstructed from `FILE_RENAME_INFORMATION` rather than taken from a handle. +impl ToAbsolutePath for &U16Str { + unsafe fn to_absolute_path) -> winsafe::SysResult>( + self, + f: F, + ) -> winsafe::SysResult { + f(Some(self)) + } +} + impl ToAbsolutePath for HANDLE { unsafe fn to_absolute_path) -> winsafe::SysResult>( self, diff --git a/crates/fspy_preload_windows/src/windows/detours/nt.rs b/crates/fspy_preload_windows/src/windows/detours/nt.rs index 12fb8c050..2b49c3d28 100644 --- a/crates/fspy_preload_windows/src/windows/detours/nt.rs +++ b/crates/fspy_preload_windows/src/windows/detours/nt.rs @@ -27,6 +27,7 @@ use crate::windows::{ client::global_client, convert::{ToAbsolutePath, ToAccessMode}, detour::{Detour, DetourAny}, + winapi_utils::{create_disposition_to_mode, handle_is_directory, rename_target_path}, }; // CreateProcess ultimately asks NtCreateUserProcess to open the executable image. Some Windows @@ -191,8 +192,17 @@ static DETOUR_NT_CREATE_FILE: Detour< ea_buffer: PVOID, ea_length: ULONG, ) -> HFILE { + // CreateDisposition is Windows' O_CREAT/O_TRUNC/O_EXCL. Without + // it a handle opened for writing is indistinguishable from one + // that actually replaces content, and only the latter is a + // mutation. + let mode = { + // SAFETY: converting the access mask through the FFI-aware trait + let access = unsafe { desired_access.to_access_mode() }; + access | create_disposition_to_mode(create_disposition) + }; // SAFETY: intercepting file open to record access before forwarding to real function - unsafe { handle_open(desired_access, object_attributes) }; + unsafe { handle_open(mode, object_attributes) }; // SAFETY: calling the original NtCreateFile with all original arguments unsafe { @@ -528,4 +538,97 @@ pub const DETOURS: &[DetourAny] = &[ DETOUR_NT_QUERY_INFORMATION_BY_NAME.as_any(), DETOUR_NT_QUERY_DIRECTORY_FILE.as_any(), DETOUR_NT_QUERY_DIRECTORY_FILE_EX.as_any(), + DETOUR_NT_SET_INFORMATION_FILE.as_any(), ]; + +/// Rename and delete both travel through `NtSetInformationFile` on Windows +/// rather than through dedicated syscalls, so this one detour covers what +/// `rename`, `unlink` and `rmdir` cover on Unix. +/// +/// Without it, a tool that publishes atomically is invisible: the write lands on +/// a temporary path that no longer exists, and the destination is never seen to +/// change. Directory renames are worse, because a build that stages into +/// `dist.tmp` and swaps it into place reports no write for any real output. +/// +/// Like every other interception here, this records the call's arguments and +/// never its result. The Linux seccomp backend cannot observe outcomes at all, so +/// an outcome-dependent rule would hold on some platforms and not others. +pub static DETOUR_NT_SET_INFORMATION_FILE: Detour< + unsafe extern "system" fn( + file_handle: HANDLE, + io_status_block: PIO_STATUS_BLOCK, + file_information: PVOID, + length: ULONG, + file_information_class: FILE_INFORMATION_CLASS, + ) -> NTSTATUS, +> = + // SAFETY: initializing Detour with the real NtSetInformationFile and our replacement + unsafe { + Detour::new(c"NtSetInformationFile", ntapi::ntioapi::NtSetInformationFile, { + unsafe extern "system" fn new_nt_set_information_file( + file_handle: HANDLE, + io_status_block: PIO_STATUS_BLOCK, + file_information: PVOID, + length: ULONG, + file_information_class: FILE_INFORMATION_CLASS, + ) -> NTSTATUS { + // Values from the NtSetInformationFile contract. + const FILE_RENAME_INFORMATION_CLASS: FILE_INFORMATION_CLASS = 10; + const FILE_RENAME_INFORMATION_EX_CLASS: FILE_INFORMATION_CLASS = 65; + const FILE_DISPOSITION_INFORMATION_CLASS: FILE_INFORMATION_CLASS = 13; + const FILE_DISPOSITION_INFORMATION_EX_CLASS: FILE_INFORMATION_CLASS = 64; + + match file_information_class { + FILE_RENAME_INFORMATION_CLASS | FILE_RENAME_INFORMATION_EX_CLASS => { + // SAFETY: file_handle is supplied by the caller + let is_directory = unsafe { handle_is_directory(file_handle) }; + let dir_flag = + if is_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: file_handle refers to the rename source, still + // valid before the call + unsafe { + handle_open( + AccessMode::RENAME_FROM | AccessMode::DELETED | dir_flag, + file_handle, + ); + } + // SAFETY: file_information holds `length` bytes of + // FILE_RENAME_INFORMATION per the caller's contract, and + // the helper bounds-checks against `length` + let target = + unsafe { rename_target_path(file_information.cast::(), length) }; + if let Some(target) = target { + // SAFETY: target is an owned null-terminated wide string + unsafe { + handle_open( + AccessMode::RENAME_TO | AccessMode::WRITE | dir_flag, + target.as_ucstr().as_ustr(), + ); + } + } + } + FILE_DISPOSITION_INFORMATION_CLASS | FILE_DISPOSITION_INFORMATION_EX_CLASS => { + // SAFETY: file_handle is supplied by the caller + let is_directory = unsafe { handle_is_directory(file_handle) }; + let dir_flag = + if is_directory { AccessMode::IS_DIR } else { AccessMode::empty() }; + // SAFETY: file_handle is still valid before the call + unsafe { handle_open(AccessMode::DELETED | dir_flag, file_handle) }; + } + _ => {} + } + + // SAFETY: calling the original NtSetInformationFile with all original arguments + unsafe { + (DETOUR_NT_SET_INFORMATION_FILE.real())( + file_handle, + io_status_block, + file_information, + length, + file_information_class, + ) + } + } + new_nt_set_information_file + }) + }; diff --git a/crates/fspy_preload_windows/src/windows/winapi_utils.rs b/crates/fspy_preload_windows/src/windows/winapi_utils.rs index 38eba1c19..9451fa29d 100644 --- a/crates/fspy_preload_windows/src/windows/winapi_utils.rs +++ b/crates/fspy_preload_windows/src/windows/winapi_utils.rs @@ -1,8 +1,11 @@ -use std::slice; +use std::{ + mem::{offset_of, size_of}, + slice, +}; use fspy_shared::ipc::AccessMode; use smallvec::SmallVec; -use widestring::{U16CStr, U16Str}; +use widestring::{U16CStr, U16CString, U16Str}; use winapi::{ ctypes::c_long, shared::{ @@ -103,6 +106,35 @@ pub const fn access_mask_to_mode(desired_access: ACCESS_MASK) -> AccessMode { } } +/// Translate `NtCreateFile`'s `CreateDisposition` into creation and truncation +/// intent. +/// +/// This is Windows' equivalent of `O_CREAT`, `O_TRUNC` and `O_EXCL`, and the +/// consumer needs it for the same reason: a handle opened for writing is only +/// capability, while replacing or newly creating a file is a mutation. +pub const fn create_disposition_to_mode(create_disposition: u32) -> AccessMode { + // Values from the NtCreateFile contract. Named locally because ntapi + // exposes them as plain constants of varying integer types. + const FILE_SUPERSEDE: u32 = 0; + const FILE_CREATE: u32 = 2; + const FILE_OPEN_IF: u32 = 3; + const FILE_OVERWRITE: u32 = 4; + const FILE_OVERWRITE_IF: u32 = 5; + + match create_disposition { + // Replaces the file if it exists, creates it otherwise. + FILE_SUPERSEDE | FILE_OVERWRITE_IF => AccessMode::CREATE.union(AccessMode::TRUNCATE), + // Fails if the file already exists, so the handle refers to new content. + FILE_CREATE => AccessMode::CREATE.union(AccessMode::EXCLUSIVE), + // Creates only when absent, leaving existing content in place. + FILE_OPEN_IF => AccessMode::CREATE, + // Requires the file to exist, then discards its contents. + FILE_OVERWRITE => AccessMode::TRUNCATE, + // FILE_OPEN and anything unrecognized: no creation, no truncation. + _ => AccessMode::empty(), + } +} + pub struct HeapPath(PWSTR); impl HeapPath { #[must_use] @@ -136,6 +168,77 @@ pub fn combine_paths(path1: &U16CStr, path2: &U16CStr) -> winsafe::SysResult bool { + use winapi::um::{ + fileapi::{BY_HANDLE_FILE_INFORMATION, GetFileInformationByHandle}, + winnt::FILE_ATTRIBUTE_DIRECTORY, + }; + + // SAFETY: an all-zero BY_HANDLE_FILE_INFORMATION is a valid initial value + let mut info: BY_HANDLE_FILE_INFORMATION = unsafe { core::mem::zeroed() }; + // SAFETY: handle comes from the interposed caller and info is a valid owned struct + if unsafe { GetFileInformationByHandle(handle, &raw mut info) } == 0 { + return false; + } + info.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY != 0 +} + +/// The destination path carried by a `FILE_RENAME_INFORMATION` buffer. +/// +/// The struct is variable-length: a fixed header followed by the target name. +/// When `RootDirectory` is set the name is relative to that directory, matching +/// `renameat` on Unix. +/// +/// # Safety +/// +/// `info` must point to at least `length` readable bytes holding a +/// `FILE_RENAME_INFORMATION`, as supplied by the `NtSetInformationFile` caller. +#[expect( + clippy::cast_ptr_alignment, + reason = "the kernel requires callers to pass a correctly aligned FILE_RENAME_INFORMATION" +)] +pub unsafe fn rename_target_path(info: *const u8, length: u32) -> Option { + use ntapi::ntioapi::FILE_RENAME_INFORMATION; + + if info.is_null() || (length as usize) < size_of::() { + return None; + } + // SAFETY: the caller guarantees `info` holds a FILE_RENAME_INFORMATION + let rename = unsafe { &*info.cast::() }; + let name_bytes = rename.FileNameLength as usize; + if name_bytes == 0 || !name_bytes.is_multiple_of(2) { + return None; + } + let name_offset = offset_of!(FILE_RENAME_INFORMATION, FileName); + if name_offset.saturating_add(name_bytes) > length as usize { + return None; + } + // SAFETY: bounds checked against `length` above; FileName is a wide string + let name = + unsafe { std::slice::from_raw_parts(info.add(name_offset).cast::(), name_bytes / 2) }; + let target = U16Str::from_slice(name); + + let is_absolute = + name.first() == Some(&u16::from(b'\\')) || name.get(1) == Some(&u16::from(b':')); + if is_absolute || rename.RootDirectory.is_null() { + return Some(U16CString::from_ustr_truncate(target)); + } + // Relative to RootDirectory, so resolve it the same way an open would. + // SAFETY: RootDirectory is a directory handle supplied by the caller + let mut root = unsafe { get_path_name(rename.RootDirectory) }.ok()?; + root.push(0); + // SAFETY: a null terminator was just pushed + let root = unsafe { U16CStr::from_ptr_str(root.as_ptr()) }; + let combined = combine_paths(root, U16CString::from_ustr_truncate(target).as_ucstr()).ok()?; + Some(U16CString::from_ustr_truncate(combined.to_u16_str())) +} + #[cfg(test)] mod tests { use std::{ diff --git a/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs b/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs index fa9dc305d..85baab3bd 100644 --- a/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs +++ b/crates/fspy_seccomp_unotify/src/supervisor/handler/arg.rs @@ -227,3 +227,22 @@ impl FromNotify for (T1, T2, T3, T4, T5) +{ + fn from_notify(notif: &seccomp_notif) -> io::Result { + Ok(( + T1::from_syscall_arg(notif.data.args[0])?, + T2::from_syscall_arg(notif.data.args[1])?, + T3::from_syscall_arg(notif.data.args[2])?, + T4::from_syscall_arg(notif.data.args[3])?, + T5::from_syscall_arg(notif.data.args[4])?, + )) + } +} diff --git a/crates/fspy_shared/src/ipc/mod.rs b/crates/fspy_shared/src/ipc/mod.rs index c7236e5d6..92ecb4420 100644 --- a/crates/fspy_shared/src/ipc/mod.rs +++ b/crates/fspy_shared/src/ipc/mod.rs @@ -9,13 +9,79 @@ pub use native_str::NativeStr; use wincode::{SchemaRead, SchemaWrite}; #[derive(SchemaWrite, SchemaRead, PartialEq, Eq, PartialOrd, Ord, Hash, Clone, Copy)] -pub struct AccessMode(u8); +pub struct AccessMode(u16); bitflags! { - impl AccessMode: u8 { + /// What a process asked the kernel to do with one path. + /// + /// Every flag here is derived from the call's *arguments*, never from its + /// result. A tracer that reported outcomes could not behave the same on all + /// backends: the Linux seccomp supervisor is notified before the syscall + /// runs and never learns whether it worked. Restricting the vocabulary to + /// pre-call information keeps every backend equivalent, and makes a trace a + /// function of the arguments alone rather than of filesystem state at trace + /// time. + /// + /// `READ`, `WRITE` and `READ_DIR` are intent. The rest qualify it, and + /// consumers need them because `WRITE` on its own is only capability: + /// truncating, exclusively creating, or being a rename destination is what + /// makes an access a mutation. + impl AccessMode: u16 { + /// Opened for reading, or its metadata was inspected. const READ = 1; + /// Opened for writing. On its own this is capability, not mutation. const WRITE = 1 << 1; + /// Directory entries were listed. const READ_DIR = 1 << 2; + /// The open carried `O_CREAT`. + const CREATE = 1 << 3; + /// The open carried `O_TRUNC`, so any previous contents are discarded. + const TRUNCATE = 1 << 4; + /// The open carried `O_EXCL`, so the path is new by construction and had + /// no previous contents to read. + const EXCLUSIVE = 1 << 5; + /// The path was named as the source of a rename. + const RENAME_FROM = 1 << 6; + /// The path was named as the destination of a rename, which replaces + /// whatever is there. + const RENAME_TO = 1 << 7; + /// Removal was requested through `unlink`, `remove` or `rmdir`. + const DELETED = 1 << 8; + /// The path is a directory. Set on rename and removal events so a + /// consumer can re-attribute writes recorded beneath a renamed + /// directory. + const IS_DIR = 1 << 9; + } +} + +impl AccessMode { + /// Whether this access changes the path's contents. + /// + /// Write capability alone does not qualify. A descriptor opened `O_RDWR` and + /// never written leaves the file untouched, which is what Biome does to a + /// clean source file on a warm run, and what every `flock`-only lock file + /// looks like. + #[must_use] + pub const fn is_mutation(self) -> bool { + if self.contains(Self::RENAME_TO) { + return true; + } + self.contains(Self::WRITE) + && self.intersects(Self::TRUNCATE.union(Self::CREATE.union(Self::EXCLUSIVE))) + } + + /// Whether this access can observe existing content, making the path a + /// genuine dependency. + /// + /// An exclusive create observes nothing, because the path is new by + /// definition. That is how Go's `os.CreateTemp` opens atomic-write + /// temporaries, which would otherwise look read-first. + #[must_use] + pub const fn is_content_read(self) -> bool { + if self.contains(Self::EXCLUSIVE) { + return false; + } + self.intersects(Self::READ.union(Self::READ_DIR)) } } diff --git a/crates/vite_powershell/src/lib.rs b/crates/vite_powershell/src/lib.rs index 2819cdf35..52e1b7a2d 100644 --- a/crates/vite_powershell/src/lib.rs +++ b/crates/vite_powershell/src/lib.rs @@ -27,9 +27,10 @@ use vite_path::{AbsolutePath, AbsolutePathBuf}; pub const POWERSHELL_PREFIX: &[&str] = &["-NoProfile", "-NoLogo", "-ExecutionPolicy", "Bypass", "-File"]; -/// Cached location of the `PowerShell` host. Prefers cross-platform -/// `pwsh.exe` when present, falling back to the Windows built-in -/// `powershell.exe`. Returns `None` on non-Windows or when neither host +/// Cached location of the `PowerShell` host. +/// +/// Prefers cross-platform `pwsh.exe` when present, falling back to the Windows +/// built-in `powershell.exe`. Returns `None` on non-Windows or when neither host /// is on `PATH`. /// /// Cached as `Arc` so callers that want shared ownership diff --git a/crates/vite_task/Cargo.toml b/crates/vite_task/Cargo.toml index 77a9771ef..70756f3f4 100644 --- a/crates/vite_task/Cargo.toml +++ b/crates/vite_task/Cargo.toml @@ -20,6 +20,7 @@ clap = { workspace = true, features = ["derive"] } ctrlc = { workspace = true } derive_more = { workspace = true, features = ["debug", "from"] } futures-util = { workspace = true } +ignore = { workspace = true } once_cell = { workspace = true } owo-colors = { workspace = true } petgraph = { workspace = true } diff --git a/crates/vite_task/src/session/cache/mod.rs b/crates/vite_task/src/session/cache/mod.rs index b18bccaca..0b0b793fa 100644 --- a/crates/vite_task/src/session/cache/mod.rs +++ b/crates/vite_task/src/session/cache/mod.rs @@ -274,7 +274,7 @@ pub fn split_path(path: &str) -> (Option<&str>, &str) { /// its own cache warm across branch switches, and a cache from a different /// version is simply ignored (it lives in a directory this build never looks /// at) rather than aborting the run. Bumping the version starts a fresh cache. -const CACHE_SCHEMA_VERSION: u32 = 18; +const CACHE_SCHEMA_VERSION: u32 = 19; /// Name of the per-version subdirectory (e.g. `v14`) under the task-cache /// directory that holds the database and output archives for the current diff --git a/crates/vite_task/src/session/execute/cache_update.rs b/crates/vite_task/src/session/execute/cache_update.rs index 0cf4df1c5..b07ff8b9b 100644 --- a/crates/vite_task/src/session/execute/cache_update.rs +++ b/crates/vite_task/src/session/execute/cache_update.rs @@ -11,28 +11,26 @@ use vite_task_server::Reports; use super::{ CacheState, - fingerprint::{PathRead, PostRunFingerprint, TrackedEnvQuery}, + fingerprint::{PostRunFingerprint, TrackedEnvQuery}, glob, spawn::ChildOutcome, }; -use crate::{ - collections::HashMap, - session::{ - cache::{CacheEntryValue, ExecutionCache, archive}, - event::{CacheErrorKind, CacheNotUpdatedReason, CacheUpdateStatus, ExecutionError}, - }, +use crate::session::{ + cache::{CacheEntryValue, ExecutionCache, archive}, + event::{CacheErrorKind, CacheNotUpdatedReason, CacheUpdateStatus, ExecutionError}, }; /// Post-execution summary of what fspy observed for a single task. Fields are /// cfg-agnostic so the decision logic below doesn't need `cfg(fspy)` — the /// value is only ever `Some` when tracking happened (see [`observe_fspy`]). struct TrackingOutcome { - path_reads: HashMap, + /// Paths to fingerprint after the run. + path_reads: FxHashSet, /// Auto-output writes after output exclusions are applied. Empty when /// `output_config.includes_auto` is false. path_writes: FxHashSet, - /// First path that was both read and written during execution, if any. - /// A non-empty value means caching this task is unsound. + /// A tracked path this run read and then rewrote. The prerun fingerprint no + /// longer describes it, so caching this task is unsound. read_write_overlap: Option, } @@ -135,7 +133,7 @@ pub(super) async fn update_cache( // Paths already in globbed_inputs are skipped: the overlap check above // guarantees no input modification, so the prerun hash is the correct // post-exec hash. - let empty_path_reads = HashMap::default(); + let empty_path_reads = FxHashSet::default(); let path_reads = fspy_outcome.as_ref().map_or(&empty_path_reads, |o| &o.path_reads); let post_run_fingerprint = match PostRunFingerprint::create( path_reads, @@ -203,53 +201,85 @@ fn observe_fspy( ) -> Option { #[cfg(fspy)] { - use super::tracked_accesses::TrackedPathAccesses; + use super::{ + classify::{ClassifyContext, classify}, + tracked_accesses::TrackedPathAccesses, + }; outcome.path_accesses.as_ref().map(|raw| { let tracked = TrackedPathAccesses::from_raw(raw, workspace_root); - let filtered_path_reads: HashMap = - // fspy can be attached for auto-output-only tasks. In that - // mode reads must not become inferred inputs. - if metadata.input_config.includes_auto - && let Some(fspy) = fspy - { - tracked - .path_reads - .iter() - .filter(|(path, _)| { - !fspy.input_negative_globs.is_match(path.as_str()) - && !is_ignored(path, ignored_input_rels) - }) - .map(|(path, read)| (path.clone(), *read)) - .collect() - } else { - HashMap::default() - }; - let filtered_path_writes: FxHashSet = - // fspy can also be attached for auto-input-only tasks. In that - // mode writes must not become auto outputs or overlap candidates. - if metadata.output_config.includes_auto - && let Some(fspy) = fspy + + // fspy can be attached for auto-output-only or auto-input-only + // tasks. In those modes the other side must stay empty. + let infer_inputs = metadata.input_config.includes_auto && fspy.is_some(); + let infer_outputs = metadata.output_config.includes_auto && fspy.is_some(); + + let gitignore = super::gitignore::WorkspaceGitignore::open(workspace_root); + let is_gitignored = |path: &RelativePathBuf| gitignore.is_ignored(path); + + let context = ClassifyContext { + workspace_root, + infer_inputs, + infer_outputs, + is_gitignored: &is_gitignored, + }; + + // Negative globs and tool-reported ignores are applied to the + // classified result rather than to raw events, because a path + // excluded from inputs may still be a legitimate output and vice + // versa. + let classification = classify(&tracked.events, &context, &|_| false); + + let excluded_from_inputs = |path: &RelativePathBuf| { + fspy.is_some_and(|fspy| fspy.input_negative_globs.is_match(path.as_str())) + || is_ignored(path, ignored_input_rels) + }; + let excluded_from_outputs = |path: &RelativePathBuf| { + fspy.is_some_and(|fspy| fspy.output_negative_globs.is_match(path.as_str())) + || is_ignored(path, ignored_output_rels) + }; + + let path_reads: FxHashSet = classification + .inputs + .iter() + .filter(|path| !excluded_from_inputs(path)) + .cloned() + .collect(); + + let path_writes: FxHashSet = classification + .outputs + .iter() + .filter(|path| !excluded_from_outputs(path)) + .cloned() + .collect(); + + let modified_for_debug = classification.modified_input.clone(); + let unexplained_for_debug = classification.unexplained_mutation.clone(); + + // A modified input only blocks caching if it is still tracked as an + // input after exclusions; excluding it is the user saying they know. + let read_write_overlap = classification + .modified_input + .filter(|path| !excluded_from_inputs(path)) + .or_else(|| { + classification.unexplained_mutation.filter(|path| !excluded_from_outputs(path)) + }); + + if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { + #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] { - tracked - .path_writes - .iter() - .filter(|path| { - !fspy.output_negative_globs.is_match(path.as_str()) - && !is_ignored(path, ignored_output_rels) - }) - .cloned() - .collect() - } else { - FxHashSet::default() - }; - let read_write_overlap = - filtered_path_reads.keys().find(|p| filtered_path_writes.contains(*p)).cloned(); - TrackingOutcome { - path_reads: filtered_path_reads, - path_writes: filtered_path_writes, - read_write_overlap, + eprintln!( + "[tracking] result inputs={} outputs={} modified={:?} unexplained={:?} \ + overlap={:?} infer_in={infer_inputs} infer_out={infer_outputs}", + classification.inputs.len(), + classification.outputs.len(), + modified_for_debug, + unexplained_for_debug, + read_write_overlap, + ); + } } + TrackingOutcome { path_reads, path_writes, read_write_overlap } }) } #[cfg(not(fspy))] diff --git a/crates/vite_task/src/session/execute/classify.rs b/crates/vite_task/src/session/execute/classify.rs new file mode 100644 index 000000000..0ec0b5b62 --- /dev/null +++ b/crates/vite_task/src/session/execute/classify.rs @@ -0,0 +1,425 @@ +//! Decide which observed paths a task consumed and which it produced. +//! +//! Two problems are solved here. +//! +//! **Read-write overlaps.** A path a task both read and wrote is either a source +//! it rewrote or state it manages, and the two need opposite treatment. Access +//! mechanics cannot tell them apart: a formatter rewriting a source and a linter +//! rewriting its own cache both read, truncate and write. So order decides +//! first — a task that wrote before reading is observing its own output — and +//! where order does not settle it, version control does. A tracked file the task +//! rewrote is a modified input; an ignored one is derived state. +//! +//! **Output directory cleanup.** Build tools stat and list their output +//! directory before writing to it, which would make the directory an input whose +//! content is the task's own product. Such a fingerprint can never match on a +//! clean checkout, because the directory is not there. Two rules drop those +//! reads: a bare directory stat carries only existence and is ignored, and a +//! directory whose subtree this task wrote into is the task's own product rather +//! than something it consumed. +//! +//! Nothing here consults whether a call succeeded. Only the call's arguments are +//! available, because the Linux seccomp supervisor is notified before the +//! syscall runs and can never report an outcome; a rule built on outcomes would +//! hold on some backends and silently not on others. +//! +//! Explicit input and output globs are applied by the caller and always win; +//! this module only classifies what the user did not declare. + +#![cfg(fspy)] + +use fspy::AccessMode; +use rustc_hash::{FxHashMap, FxHashSet}; +use vite_path::{AbsolutePath, RelativePathBuf}; + +/// One normalized, workspace-relative access, in the order it was observed. +#[derive(Debug)] +pub struct TrackedEvent { + pub path: RelativePathBuf, + pub mode: AccessMode, +} + +/// What the caller must supply for the parts of the decision that do not come +/// from access data. +pub struct ClassifyContext<'a> { + pub workspace_root: &'a AbsolutePath, + /// Whether reads may become inferred inputs. + pub infer_inputs: bool, + /// Whether writes may become inferred outputs. + pub infer_outputs: bool, + /// Version control's answer for a path, used only to break a read-first + /// overlap. `None` means not ignored, or no repository, which resolves to + /// input. + pub is_gitignored: &'a dyn Fn(&RelativePathBuf) -> bool, +} + +/// The classification result. +#[derive(Default)] +pub struct Classification { + /// Paths to fingerprint. A path present here is a dependency of the task. + pub inputs: FxHashSet, + /// Of those inputs, the ones whose directory entries were listed and so + /// must be fingerprinted as a listing rather than as content. + pub listed_directories: FxHashSet, + /// Paths to archive as this task's outputs. + pub outputs: FxHashSet, + /// A tracked path the task rewrote after reading it, so the prerun + /// fingerprint is stale and this run must not be cached. + pub modified_input: Option, + /// A mutation whose target is gone at archive time with no rename to explain + /// it. The output set is incomplete, so this run must not be cached. + pub unexplained_mutation: Option, +} + +/// Everything observed about one path, folded in observation order. +#[derive(Default)] +struct PathHistory { + /// Index of the first access that observed existing content. + first_content_read: Option, + /// Index of the first access that changed the path. + first_mutation: Option, + /// Index of the first successful directory listing. + first_listing: Option, + /// Index of the first successful removal. + first_deletion: Option, + /// The path was named as a rename source, so its absence is explained. + renamed_away: bool, + /// The path was replaced by a rename. + rename_destination: bool, + /// Any access described the path as a directory. + known_directory: bool, +} + +impl PathHistory { + const fn observe(&mut self, index: usize, mode: AccessMode) { + if mode.contains(AccessMode::IS_DIR) { + self.known_directory = true; + } + if mode.contains(AccessMode::RENAME_FROM) { + self.renamed_away = true; + } + if mode.contains(AccessMode::RENAME_TO) { + self.rename_destination = true; + } + if mode.contains(AccessMode::DELETED) && self.first_deletion.is_none() { + self.first_deletion = Some(index); + } + if mode.contains(AccessMode::READ_DIR) && self.first_listing.is_none() { + self.first_listing = Some(index); + } + if mode.is_content_read() && self.first_content_read.is_none() { + self.first_content_read = Some(index); + } + if mode.is_mutation() && self.first_mutation.is_none() { + self.first_mutation = Some(index); + } + } +} + +/// Apply the rules to an ordered event stream. +/// +/// `already_fingerprinted` reports paths the caller hashed before the run, which +/// do not need fingerprinting again. +/// Fold the event stream into per-path histories, and collect the directory +/// renames that need writes re-attributed. +fn fold_events( + events: &[TrackedEvent], +) -> (FxHashMap<&RelativePathBuf, PathHistory>, Vec<(&RelativePathBuf, &RelativePathBuf)>) { + let mut histories: FxHashMap<&RelativePathBuf, PathHistory> = FxHashMap::default(); + // Directory renames, newest last. A build that stages into `dist.tmp` and + // swaps it into place records every write against the staging path, so those + // writes have to be re-attributed or the real outputs are lost entirely. + let mut directory_renames: Vec<(&RelativePathBuf, &RelativePathBuf)> = Vec::new(); + let mut last_rename_source: Option<&RelativePathBuf> = None; + + for (index, event) in events.iter().enumerate() { + histories.entry(&event.path).or_default().observe(index, event.mode); + + // A rename is reported as source then destination, adjacent. + if event.mode.contains(AccessMode::RENAME_FROM) { + last_rename_source = Some(&event.path); + } else if event.mode.contains(AccessMode::RENAME_TO) { + if event.mode.contains(AccessMode::IS_DIR) + && let Some(source) = last_rename_source + { + directory_renames.push((source, &event.path)); + } + last_rename_source = None; + } + } + (histories, directory_renames) +} + +/// Every directory the task wrote into, including ancestors. +/// +/// A listing of one of these enumerated the task's own product, so treating it +/// as a dependency would make the cache key depend on the output and could never +/// match on a clean checkout where the directory does not exist yet. +fn mutated_subtrees<'a>( + candidates: &[(&'a RelativePathBuf, &'a PathHistory)], +) -> FxHashSet<&'a str> { + let mut subtrees: FxHashSet<&str> = FxHashSet::default(); + for (path, history) in candidates { + if history.first_mutation.is_none() { + continue; + } + let mut remaining = path.as_str(); + while let Some(separator) = remaining.rfind('/') { + remaining = &remaining[..separator]; + if !subtrees.insert(remaining) { + // An ancestor already recorded, so the rest are too. + break; + } + } + } + subtrees +} + +/// Whether a mutated path that is now gone can be accounted for. +/// +/// Without an explanation the thing that produced it is missing from the archive +/// and a cache hit would restore an incomplete tree. Being renamed or removed +/// explains it, including by an ancestor: renaming a staging directory takes +/// everything beneath it along, and only the directory itself carries the rename. +fn absence_is_explained( + path: &RelativePathBuf, + history: &PathHistory, + retired_ancestors: &[&str], +) -> bool { + history.renamed_away + || history.first_deletion.is_some() + || retired_ancestors.iter().any(|retired| { + path.as_str().strip_prefix(*retired).is_some_and(|tail| tail.starts_with('/')) + }) +} + +/// Re-attribute mutations recorded beneath a renamed directory to the published +/// location. +/// +/// A build that stages into `dist.tmp` and swaps it into place records every +/// write against the staging path. Only the directory carries the rename, so +/// without this the real outputs are never collected and a cache hit restores an +/// empty tree. +fn relocate_directory_renames( + histories: &FxHashMap<&RelativePathBuf, PathHistory>, + directory_renames: &[(&RelativePathBuf, &RelativePathBuf)], +) -> FxHashMap { + let mut relocated: FxHashMap = FxHashMap::default(); + for (source, destination) in directory_renames { + let source_prefix = source.as_str(); + for (path, history) in histories { + if history.first_mutation.is_none() { + continue; + } + let Some(tail) = + path.as_str().strip_prefix(source_prefix).and_then(|tail| tail.strip_prefix('/')) + else { + continue; + }; + let Ok(moved) = + RelativePathBuf::new(vite_str::format!("{destination}/{tail}").as_str()) + else { + continue; + }; + relocated.entry(moved).or_default().first_mutation = history.first_mutation; + } + } + relocated +} + +pub fn classify( + events: &[TrackedEvent], + context: &ClassifyContext<'_>, + already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, +) -> Classification { + let (histories, directory_renames) = fold_events(events); + let mut classification = Classification::default(); + + let relocated = relocate_directory_renames(&histories, &directory_renames); + + let mut candidates: Vec<(&RelativePathBuf, &PathHistory)> = + histories.iter().map(|(path, history)| (*path, history)).collect(); + let relocated_entries: Vec<(&RelativePathBuf, &PathHistory)> = relocated.iter().collect(); + candidates.extend(relocated_entries); + + // Directories that were renamed away or removed. Anything that used to live + // beneath one of these is accounted for, so its absence needs no further + // explanation. + let retired_ancestors: Vec<&str> = histories + .iter() + .filter(|(_, history)| history.renamed_away || history.first_deletion.is_some()) + .map(|(path, _)| path.as_str()) + .collect(); + + let mutated_subtrees = mutated_subtrees(&candidates); + + for (path, history) in candidates { + let absolute = context.workspace_root.join(path); + let metadata = std::fs::metadata(absolute.as_path()).ok(); + let exists = metadata.is_some(); + let is_directory = metadata.as_ref().is_some_and(std::fs::Metadata::is_dir) + || (history.known_directory && !exists); + + if history.first_mutation.is_some() + && !exists + && !absence_is_explained(path, history, &retired_ancestors) + { + { + if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { + #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] + { + eprintln!( + "[tracking] unexplained {path} renamed_away={} deleted={:?} \ + mutation={:?} read={:?}", + history.renamed_away, + history.first_deletion, + history.first_mutation, + history.first_content_read, + ); + } + } + classification.unexplained_mutation.get_or_insert_with(|| path.clone()); + } + } + + let wrote_into_subtree = is_directory && mutated_subtrees.contains(path.as_str()); + let own_derived_state = is_own_derived_state( + path, + is_directory, + wrote_into_subtree, + context, + &mutated_subtrees, + ); + let mutated = history.first_mutation.is_some() && context.infer_outputs; + let consumed = + is_input_candidate(history, is_directory, own_derived_state) && context.infer_inputs; + + match (mutated, consumed) { + (true, false) => { + if exists { + classification.outputs.insert(path.clone()); + } + } + (true, true) => { + let write_first = history.first_mutation < history.first_content_read; + if std::env::var_os("VITE_TASK_DEBUG_TRACKING").is_some() { + #[expect(clippy::print_stderr, reason = "opt-in diagnostic")] + { + eprintln!( + "[tracking] overlap {path} write_first={write_first} \ + gitignored={}", + (context.is_gitignored)(path), + ); + } + } + if write_first || (context.is_gitignored)(path) { + if exists { + classification.outputs.insert(path.clone()); + } + } else { + // A tracked file this run read and then rewrote. The prerun + // fingerprint no longer describes it. + classification.modified_input.get_or_insert_with(|| path.clone()); + record_input(&mut classification, path, history, already_fingerprinted); + } + } + (false, true) => { + record_input(&mut classification, path, history, already_fingerprinted); + } + (false, false) => {} + } + } + + classification +} + +/// Whether a path's reads make it a dependency. +/// +/// Directories are the interesting case, and three kinds of read are rejected. +/// +/// A bare stat carries only existence and type. It cannot detect a content +/// change and is routinely a probe of the task's own output directory, where a +/// stored fingerprint could never match on a clean checkout. +/// +/// A listing of a directory the task wrote into enumerated the task's own +/// product. +/// +/// A listing of a *gitignored* directory enumerated derived state. Version +/// control already says this content is not source, so depending on the set of +/// names in it is not a source dependency — and that set moves for reasons this +/// task does not control. In emdash, `tsdown` lists `dist/locales` while a +/// sibling task writes into it, so without this the tsdown task invalidates +/// every time a locale is added, even though its own inputs never changed. +const fn is_input_candidate( + history: &PathHistory, + is_directory: bool, + own_derived_state: bool, +) -> bool { + if history.first_content_read.is_none() { + return false; + } + if own_derived_state { + return false; + } + if !is_directory { + return true; + } + // A bare stat of a directory carries only existence and type. It cannot + // detect a content change and is routinely a probe of the task's own output + // directory, where a stored fingerprint could never match on a clean + // checkout. + history.first_listing.is_some() +} + +/// Whether a read path is part of the task's own derived output tree. +/// +/// Two signals have to agree. Version control must call the path derived, and it +/// must sit under a directory this same task wrote into. Requiring both is what +/// keeps the ordinary monorepo dependency intact: a package reading +/// `node_modules//dist/index.mjs` is reading ignored, derived content, but +/// the reader never writes there, so it stays a real input and a change to the +/// dependency still invalidates the consumer. +/// +/// What it does reject is a task reading inside its own output directory. +/// emdash's admin package is the case that forced this: `tsdown` reads +/// `dist/locales//messages.mjs` while a later step in the same build +/// rewrites those files, so fingerprinting them can never settle and the task +/// misses on every run. +fn is_own_derived_state( + path: &RelativePathBuf, + is_directory: bool, + wrote_into_subtree: bool, + context: &ClassifyContext<'_>, + mutated_subtrees: &FxHashSet<&str>, +) -> bool { + if is_directory { + // The listing enumerated names this task produced, or names version + // control considers derived and therefore outside the source graph. + return wrote_into_subtree || (context.is_gitignored)(path); + } + if !(context.is_gitignored)(path) { + return false; + } + let mut remaining = path.as_str(); + while let Some(separator) = remaining.rfind('/') { + remaining = &remaining[..separator]; + if mutated_subtrees.contains(remaining) { + return true; + } + } + false +} + +fn record_input( + classification: &mut Classification, + path: &RelativePathBuf, + history: &PathHistory, + already_fingerprinted: &dyn Fn(&RelativePathBuf) -> bool, +) { + if history.first_listing.is_some() { + classification.listed_directories.insert(path.clone()); + } + if !already_fingerprinted(path) { + classification.inputs.insert(path.clone()); + } +} diff --git a/crates/vite_task/src/session/execute/fingerprint.rs b/crates/vite_task/src/session/execute/fingerprint.rs index 11173d40e..2129e1bde 100644 --- a/crates/vite_task/src/session/execute/fingerprint.rs +++ b/crates/vite_task/src/session/execute/fingerprint.rs @@ -32,12 +32,6 @@ pub enum TrackedEnvQuery { Prefix(Str), } -/// Path read access info -#[derive(Debug, Clone, Copy)] -pub struct PathRead { - pub read_dir_entries: bool, -} - /// Post-run fingerprint capturing file state after execution. /// Used to validate whether cached outputs are still valid. #[derive(SchemaWrite, SchemaRead, Debug, Default, Serialize)] @@ -85,11 +79,13 @@ pub enum PathFingerprint { NotFound, /// File content hash using `xxHash3_64` FileContentHash(u64), - /// Directory with optional entry listing. - /// `Folder(None)` means the directory was opened but entries were not read - /// (e.g., for `openat` calls). - /// `Folder(Some(_))` contains the directory entries sorted by name. - Folder(Option>), + /// Directory entries, sorted by name. + /// + /// A directory only reaches here if the task listed it. A bare stat carries + /// nothing but existence, cannot detect a content change, and is routinely a + /// probe of the task's own output directory, so classification drops those + /// before fingerprinting. + Folder(BTreeMap), } /// Kind of directory entry @@ -118,7 +114,7 @@ impl PostRunFingerprint { /// * `tracked_env_queries` - Tool-requested bulk env queries (query -> match-set hashes) #[tracing::instrument(level = "debug", skip_all, name = "create_post_run_fingerprint")] pub fn create( - inferred_path_reads: &HashMap, + inferred_path_reads: &rustc_hash::FxHashSet, base_dir: &AbsolutePath, globbed_inputs: &BTreeMap, tracked_envs: BTreeMap>, @@ -126,10 +122,10 @@ impl PostRunFingerprint { ) -> anyhow::Result { let inferred_inputs = inferred_path_reads .par_iter() - .filter(|(path, _)| !globbed_inputs.contains_key(*path)) - .map(|(relative_path, path_read)| { + .filter(|path| !globbed_inputs.contains_key(*path)) + .map(|relative_path| { let full_path = Arc::::from(base_dir.join(relative_path)); - let fingerprint = fingerprint_path(&full_path, *path_read)?; + let fingerprint = fingerprint_path(&full_path)?; Ok((relative_path.clone(), fingerprint)) }) .collect::>>()?; @@ -155,10 +151,7 @@ impl PostRunFingerprint { let input_mismatch = self.inferred_inputs.par_iter().find_map_any( |(input_relative_path, path_fingerprint)| { let input_full_path = Arc::::from(base_dir.join(input_relative_path)); - let path_read = PathRead { - read_dir_entries: matches!(path_fingerprint, PathFingerprint::Folder(Some(_))), - }; - let current_path_fingerprint = match fingerprint_path(&input_full_path, path_read) { + let current_path_fingerprint = match fingerprint_path(&input_full_path) { Ok(ok) => ok, Err(err) => return Some(Err(err)), }; @@ -337,7 +330,7 @@ fn determine_change_kind<'a>( (InputChangeKind::ContentModified, None) } (PathFingerprint::Folder(old), PathFingerprint::Folder(new)) => { - determine_folder_change_kind(old.as_ref(), new.as_ref()) + determine_folder_change_kind(old, new) } // Type changed (file ↔ folder) _ => (InputChangeKind::Added, None), @@ -348,13 +341,9 @@ fn determine_change_kind<'a>( /// Both maps are `BTreeMap` so we iterate them in sorted lockstep. /// Returns the specific entry name that was added or removed, if identifiable. fn determine_folder_change_kind<'a>( - old: Option<&'a BTreeMap>, - new: Option<&'a BTreeMap>, + old_entries: &'a BTreeMap, + new_entries: &'a BTreeMap, ) -> (InputChangeKind, Option<&'a Str>) { - let (Some(old_entries), Some(new_entries)) = (old, new) else { - return (InputChangeKind::Added, None); - }; - let mut old_iter = old_entries.iter(); let mut new_iter = new_entries.iter(); let mut o = old_iter.next(); @@ -386,10 +375,7 @@ fn should_ignore_entry(name: &[u8]) -> bool { } /// Fingerprint a single path -pub fn fingerprint_path( - path: &Arc, - path_read: PathRead, -) -> anyhow::Result { +pub fn fingerprint_path(path: &Arc) -> anyhow::Result { let std_path = path.as_path(); let file = match File::open(std_path) { @@ -400,12 +386,12 @@ pub fn fingerprint_path( { if err.kind() == io::ErrorKind::PermissionDenied { // This might be a directory - try reading it as such - return process_directory(std_path, path_read); + return process_directory(std_path); } // On Windows, paths with trailing backslash (from joining empty path) // fail with NotFound (error code 3). Try as directory in this case. if err.raw_os_error() == Some(3) && std_path.to_string_lossy().ends_with('\\') { - return process_directory(std_path, path_read); + return process_directory(std_path); } } if err.kind() != io::ErrorKind::NotFound { @@ -428,11 +414,11 @@ pub fn fingerprint_path( // Is a directory on Unix - use the optimized nix implementation #[cfg(unix)] { - return process_directory_unix(reader.get_ref(), path_read); + return process_directory_unix(reader.get_ref()); } #[cfg(windows)] { - return process_directory(std_path, path_read); + return process_directory(std_path); } } Ok(PathFingerprint::FileContentHash(super::hash::hash_content(reader)?)) @@ -441,14 +427,7 @@ pub fn fingerprint_path( /// Process a directory on Windows using `std::fs::read_dir` #[cfg(windows)] #[expect(clippy::disallowed_types, reason = "Windows fallback uses std::path::Path directly")] -fn process_directory( - path: &std::path::Path, - path_read: PathRead, -) -> anyhow::Result { - if !path_read.read_dir_entries { - return Ok(PathFingerprint::Folder(None)); - } - +fn process_directory(path: &std::path::Path) -> anyhow::Result { let mut entries = BTreeMap::new(); for entry in std::fs::read_dir(path)? { let entry = entry?; @@ -472,18 +451,14 @@ fn process_directory( entries.insert(Str::from(name_str.as_ref()), kind); } - Ok(PathFingerprint::Folder(Some(entries))) + Ok(PathFingerprint::Folder(entries)) } /// Process a directory on Unix using nix for efficiency #[cfg(unix)] -fn process_directory_unix(file: &File, path_read: PathRead) -> anyhow::Result { +fn process_directory_unix(file: &File) -> anyhow::Result { use std::os::fd::AsFd; - if !path_read.read_dir_entries { - return Ok(PathFingerprint::Folder(None)); - } - let fd = file.as_fd(); let mut dir = nix::dir::Dir::from_fd(fd.try_clone_to_owned()?)?; @@ -511,7 +486,7 @@ fn process_directory_unix(file: &File, path_read: PathRead) -> anyhow::Result, +} + +impl WorkspaceGitignore { + /// Build a matcher for the workspace root. + /// + /// Collects `.gitignore` at the root plus `.git/info/exclude`. Nested + /// `.gitignore` files inside subdirectories are picked up by the builder as + /// it walks the ones it is given, and a missing file is simply no rules. + /// + /// A malformed pattern is not fatal. Failing the whole task because one line + /// of a `.gitignore` is unparsable would be worse than treating that line as + /// absent, and the fallback direction is safe: fewer ignore rules means more + /// paths look tracked, which means fewer runs are cached. + pub fn open(workspace_root: &AbsolutePath) -> Self { + let mut builder = GitignoreBuilder::new(workspace_root.as_path()); + // Errors here describe individual bad patterns; the partial matcher + // built from the good ones is still worth having. + drop(builder.add(workspace_root.join(".gitignore").as_path())); + drop(builder.add(workspace_root.join(".git/info/exclude").as_path())); + Self { matcher: builder.build().ok() } + } + + /// Whether version control ignores this path. + /// + /// `false` when there is no matcher at all, which is the input-leaning + /// answer. + pub fn is_ignored(&self, path: &RelativePathBuf) -> bool { + if path + .as_path() + .iter() + .any(|component| ALWAYS_IGNORED.contains(&component.to_string_lossy().as_ref())) + { + return true; + } + + let Some(matcher) = &self.matcher else { + return false; + }; + // A directory and a file with the same name match different patterns + // (`dist/` only matches the directory), so the caller's path is checked + // both ways rather than guessing. + matcher.matched_path_or_any_parents(path.as_path(), false).is_ignore() + || matcher.matched_path_or_any_parents(path.as_path(), true).is_ignore() + } +} + +#[cfg(test)] +mod tests { + use tempfile::TempDir; + use vite_path::AbsolutePathBuf; + + use super::*; + + /// A root `dist/` rule must ignore a nested build output. This is the shape + /// every real monorepo has, and getting it wrong turns every package's + /// output into a modified input. + #[test] + fn root_directory_rule_ignores_nested_output() { + let temp = TempDir::new().unwrap(); + let root = AbsolutePathBuf::new(temp.path().to_path_buf()).unwrap(); + std::fs::write(root.join(".gitignore").as_path(), "dist/\nnode_modules\n").unwrap(); + std::fs::create_dir_all(root.join("packages/auth/dist").as_path()).unwrap(); + std::fs::write(root.join("packages/auth/dist/index.mjs").as_path(), "x").unwrap(); + + let gitignore = WorkspaceGitignore::open(&root); + + assert!( + gitignore.is_ignored(&RelativePathBuf::new("packages/auth/dist/index.mjs").unwrap()), + "a root `dist/` rule must cover nested package output" + ); + assert!( + !gitignore.is_ignored(&RelativePathBuf::new("packages/auth/src/index.ts").unwrap()), + "tracked sources must not be reported as ignored" + ); + } + + /// A workspace with no `.gitignore` must still recognise `node_modules` as + /// derived. Vitest reads and then rewrites its own + /// `node_modules/.vite/vitest/*/results.json`, and calling that a modified + /// input stops the task from ever caching. + #[test] + fn node_modules_is_derived_without_any_gitignore() { + let temp = TempDir::new().unwrap(); + let root = AbsolutePathBuf::new(temp.path().to_path_buf()).unwrap(); + + let gitignore = WorkspaceGitignore::open(&root); + + assert!( + gitignore.is_ignored( + &RelativePathBuf::new("node_modules/.vite/vitest/abc/results.json").unwrap() + ), + "a tool's own cache under node_modules must not become a modified input" + ); + assert!( + gitignore + .is_ignored(&RelativePathBuf::new("packages/auth/node_modules/x/cache").unwrap()), + "nested node_modules must be covered too" + ); + assert!( + !gitignore.is_ignored(&RelativePathBuf::new("src/index.ts").unwrap()), + "sources must still be tracked when there is no .gitignore" + ); + } +} diff --git a/crates/vite_task/src/session/execute/mod.rs b/crates/vite_task/src/session/execute/mod.rs index 1813051e2..f0eeb69ca 100644 --- a/crates/vite_task/src/session/execute/mod.rs +++ b/crates/vite_task/src/session/execute/mod.rs @@ -1,5 +1,9 @@ mod cache_update; +#[cfg(fspy)] +mod classify; pub mod fingerprint; +#[cfg(fspy)] +mod gitignore; pub mod glob; mod hash; pub mod pipe; diff --git a/crates/vite_task/src/session/execute/tracked_accesses.rs b/crates/vite_task/src/session/execute/tracked_accesses.rs index 6e3596512..fc1c4002c 100644 --- a/crates/vite_task/src/session/execute/tracked_accesses.rs +++ b/crates/vite_task/src/session/execute/tracked_accesses.rs @@ -1,27 +1,25 @@ -//! Normalize raw fspy path accesses into workspace-relative, filtered form. +//! Normalize raw fspy path accesses into workspace-relative, ordered form. +//! +//! Order is preserved because the classification rules depend on it: a task that +//! wrote a path before reading it is observing its own output, while one that +//! read before writing consumed something that existed. Collapsing accesses into +//! per-path read and write sets, as this module used to, throws that away. //! //! User-configured negative globs are NOT applied here. They are applied later, //! separately for reads (input config) and writes (output config), since those //! two configs are independent. #![cfg(fspy)] -use std::collections::hash_map::Entry; - -use fspy::{AccessMode, PathAccessIterable}; -use rustc_hash::FxHashSet; +use fspy::PathAccessIterable; use vite_path::{AbsolutePath, RelativePathBuf}; -use super::fingerprint::PathRead; -use crate::collections::HashMap; +use super::classify::TrackedEvent; -/// Tracked file accesses from fspy, normalized to workspace-relative paths. +/// Tracked file accesses from fspy, normalized to workspace-relative paths and +/// kept in observation order. #[derive(Default, Debug)] pub struct TrackedPathAccesses { - /// Tracked path reads - pub path_reads: HashMap, - - /// Tracked path writes - pub path_writes: FxHashSet, + pub events: Vec, } impl TrackedPathAccesses { @@ -29,7 +27,7 @@ impl TrackedPathAccesses { /// normalizing `..` components. `.git/*` paths are skipped. User-configured /// negatives are applied by the caller (see module docs). pub fn from_raw(raw: &PathAccessIterable, workspace_root: &AbsolutePath) -> Self { - let mut accesses = Self::default(); + let mut events = Vec::new(); for access in raw.iter() { // Strip workspace root and clean `..` components in one pass. // fspy may report paths like `packages/sub-pkg/../shared/dist/output.js`. @@ -44,27 +42,9 @@ impl TrackedPathAccesses { continue; }; - if access.mode.contains(AccessMode::READ) { - accesses - .path_reads - .entry(relative_path.clone()) - .or_insert(PathRead { read_dir_entries: false }); - } - if access.mode.contains(AccessMode::WRITE) { - accesses.path_writes.insert(relative_path.clone()); - } - if access.mode.contains(AccessMode::READ_DIR) { - match accesses.path_reads.entry(relative_path) { - Entry::Occupied(mut occupied) => { - occupied.get_mut().read_dir_entries = true; - } - Entry::Vacant(vacant) => { - vacant.insert(PathRead { read_dir_entries: true }); - } - } - } + events.push(TrackedEvent { path: relative_path, mode: access.mode }); } - accesses + Self { events } } } diff --git a/crates/vite_task_bin/src/vtt/main.rs b/crates/vite_task_bin/src/vtt/main.rs index 9e3c7f6fb..939d921c9 100644 --- a/crates/vite_task_bin/src/vtt/main.rs +++ b/crates/vite_task_bin/src/vtt/main.rs @@ -20,7 +20,9 @@ mod print_color; mod print_cwd; mod print_env; mod print_file; +mod publish_dir; mod read_stdin; +mod rename; mod replace_file_content; mod rm; #[cfg(target_os = "linux")] @@ -35,7 +37,7 @@ fn main() { if args.len() < 2 { eprintln!("Usage: vtt [args...]"); eprintln!( - "Subcommands: barrier, check-tty, cp, exit, exit-on-ctrlc, grep-file, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, read-stdin, replace-file-content, rm, small_dev_shm, stat-file, stat_long_filename, touch-file, write-file" + "Subcommands: barrier, check-tty, cp, exit, exit-on-ctrlc, grep-file, list-dir, mkdir, pipe-stdin, print, print-color, print-cwd, print-env, print-file, publish-dir, read-stdin, rename, replace-file-content, rm, small_dev_shm, stat-file, stat_long_filename, touch-file, write-file" ); std::process::exit(1); } @@ -76,6 +78,8 @@ fn main() { Ok(()) } "stat_long_filename" => stat_long_filename::run(&args[2..]), + "publish-dir" => publish_dir::run(&args[2..]), + "rename" => rename::run(&args[2..]), "touch-file" => touch_file::run(&args[2..]), "write-file" => write_file::run(&args[2..]), other => { diff --git a/crates/vite_task_bin/src/vtt/publish_dir.rs b/crates/vite_task_bin/src/vtt/publish_dir.rs new file mode 100644 index 000000000..79ad8a6fc --- /dev/null +++ b/crates/vite_task_bin/src/vtt/publish_dir.rs @@ -0,0 +1,48 @@ +//! Publish an output directory the way real build tools do, in one process. +//! +//! Both modes exist because they stress different parts of auto-output tracking, +//! and both must happen inside a single task: a compound `a && b` command is +//! cached as separate tasks, so splitting the steps would hide the very +//! relationship under test. +//! +//! `atomic` stages the output under a temporary directory and renames that +//! directory into place. Every write lands on the staging path and only the +//! directory carries the rename, so a tracker that ignores directory renames +//! collects nothing. +//! +//! `rebuild` empties the output directory first, which is what makes a build +//! tool stat and list its own output. Those reads must not become inputs. + +use std::path::Path; + +pub fn run(args: &[String]) -> Result<(), Box> { + let [mode, source, directory] = args else { + return Err("Usage: vtt publish-dir ".into()); + }; + let directory = Path::new(directory); + let contents = std::fs::read_to_string(source)?; + + match mode.as_str() { + "atomic" => { + let staging = directory.with_extension("tmp"); + if staging.exists() { + std::fs::remove_dir_all(&staging)?; + } + std::fs::create_dir_all(&staging)?; + std::fs::write(staging.join("out.txt"), &contents)?; + if directory.exists() { + std::fs::remove_dir_all(directory)?; + } + std::fs::rename(&staging, directory)?; + } + "rebuild" => { + if directory.exists() { + std::fs::remove_dir_all(directory)?; + } + std::fs::create_dir_all(directory)?; + std::fs::write(directory.join("out.txt"), &contents)?; + } + other => return Err(format!("unknown mode: {other}").into()), + } + Ok(()) +} diff --git a/crates/vite_task_bin/src/vtt/rename.rs b/crates/vite_task_bin/src/vtt/rename.rs new file mode 100644 index 000000000..a333cb052 --- /dev/null +++ b/crates/vite_task_bin/src/vtt/rename.rs @@ -0,0 +1,15 @@ +//! Rename a file or a whole directory, the way tools publish results atomically. +//! +//! Build tools commonly stage output under a temporary name and rename it into +//! place so readers never see a half-written tree. Renaming a *directory* is the +//! interesting case for tracking: every write lands on the staging path, and only +//! the directory itself carries the rename, so a tracker that ignores directory +//! renames loses every output. + +pub fn run(args: &[String]) -> Result<(), Box> { + if args.len() != 2 { + return Err("Usage: vtt rename ".into()); + } + std::fs::rename(&args[0], &args[1])?; + Ok(()) +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore new file mode 100644 index 000000000..8b69f3a68 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/.gitignore @@ -0,0 +1,3 @@ +node_modules/ +dist/ +.cache/ diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json new file mode 100644 index 000000000..bd710e911 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/package.json @@ -0,0 +1,5 @@ +{ + "name": "auto-output-tracking", + "version": "0.0.0", + "private": true +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json new file mode 100644 index 000000000..7d075639b --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/package.json @@ -0,0 +1,4 @@ +{ + "name": "@test/atomic-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt new file mode 100644 index 000000000..63695f771 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/src/data.txt @@ -0,0 +1 @@ +atomic diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json new file mode 100644 index 000000000..91d5fd62a --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/atomic-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt publish-dir atomic src/data.txt dist" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json new file mode 100644 index 000000000..5fdf310b4 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/package.json @@ -0,0 +1,4 @@ +{ + "name": "@test/clean-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt new file mode 100644 index 000000000..831263020 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/src/data.txt @@ -0,0 +1 @@ +clean diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json new file mode 100644 index 000000000..ceac42463 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/clean-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt publish-dir rebuild src/data.txt dist" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json new file mode 100644 index 000000000..219163dc7 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/package.json @@ -0,0 +1,4 @@ +{ + "name": "@test/sibling-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt new file mode 100644 index 000000000..e620d96dd --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/src/data.txt @@ -0,0 +1 @@ +sibling diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json new file mode 100644 index 000000000..416a32370 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/sibling-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt publish-dir rebuild src/data.txt dist && vtt cp dist/out.txt dist/copied.txt && vtt cp src/data.txt dist/late.txt" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json new file mode 100644 index 000000000..ec1a34c57 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/package.json @@ -0,0 +1,4 @@ +{ + "name": "@test/state-pkg", + "version": "0.0.0" +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt new file mode 100644 index 000000000..ff72b5c73 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/src/data.txt @@ -0,0 +1 @@ +state diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json new file mode 100644 index 000000000..aaef5ca78 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/packages/state-pkg/vite-task.json @@ -0,0 +1,7 @@ +{ + "tasks": { + "task": { + "command": "vtt mkdir .cache && vtt cp src/data.txt .cache/state.txt && vtt replace-file-content .cache/state.txt e E" + } + } +} diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml new file mode 100644 index 000000000..924b55f42 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/pnpm-workspace.yaml @@ -0,0 +1,2 @@ +packages: + - packages/* diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml new file mode 100644 index 000000000..8d02cfe17 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots.toml @@ -0,0 +1,85 @@ +[[e2e]] +name = "atomic_directory_publish_is_cached_and_restored" +comment = """ +A task that stages output under `dist.tmp` and renames the directory into place must still have its outputs collected. Every write lands on the staging path and only the directory itself carries the rename, so a tracker that ignores directory renames archives nothing and a later cache hit restores an incomplete tree. The third run removes `dist` first to prove the outputs really came back from the archive rather than from the previous run leaving them behind. +""" +cwd = "packages/atomic-pkg" +steps = [ + [ + "vt", + "run", + "task", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "rm", + "-rf", + "dist", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "print-file", + "dist/out.txt", + ], +] + +[[e2e]] +name = "output_directory_cleanup_still_caches" +comment = """ +Build tools empty their output directory before writing to it, which means they stat and list it. Those reads must not make the directory an input: its contents are the task's own product, and a fingerprint of them can never match on a clean checkout where the directory does not exist yet. +""" +cwd = "packages/clean-pkg" +steps = [ + [ + "vt", + "run", + "task", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "rm", + "-rf", + "dist", + ], + [ + "vt", + "run", + "task", + ], + [ + "vtt", + "print-file", + "dist/out.txt", + ], +] + +[[e2e]] +name = "gitignored_state_read_then_rewritten_is_an_output" +comment = """ +A path a task reads and then rewrites is either a source it fixed up or state it manages, and access mechanics cannot tell those apart. Version control can: this file is gitignored, so it is derived state and belongs in the archive. Compare `input_read_write_not_cached`, where the same access pattern on a tracked source blocks caching instead. +""" +cwd = "packages/state-pkg" +steps = [["vt", "run", "task"], ["vt", "run", "task"], ["vtt", "print-file", ".cache/state.txt"]] + +[[e2e]] +name = "reading_inside_own_output_tree_still_caches" +comment = """ +A compound command's `&&` segments are separate cached tasks. Here the middle segment reads a file inside its own package's output directory while the last segment writes another file there, which is the shape of emdash's admin build: `tsdown` reads `dist/locales//messages.mjs` and a later `locale:copy` step rewrites those files. Fingerprinting content a sibling task rewrites can never settle, so a read is treated as the task's own derived state when version control calls it derived *and* it sits under a directory the same task wrote into. Requiring both is what keeps `node_modules//dist` a real input, so changing a dependency still invalidates its consumers. +""" +cwd = "packages/sibling-pkg" +steps = [["vt", "run", "task"], ["vt", "run", "task"], ["vt", "run", "task"]] diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md new file mode 100644 index 000000000..09e218965 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/atomic_directory_publish_is_cached_and_restored.md @@ -0,0 +1,38 @@ +# atomic_directory_publish_is_cached_and_restored + +A task that stages output under `dist.tmp` and renames the directory into place must still have its outputs collected. Every write lands on the staging path and only the directory itself carries the rename, so a tracker that ignores directory renames archives nothing and a later cache hit restores an incomplete tree. The third run removes `dist` first to prove the outputs really came back from the archive rather than from the previous run leaving them behind. + +## `vt run task` + +``` +~/packages/atomic-pkg$ vtt publish-dir atomic src/data.txt dist +``` + +## `vt run task` + +``` +~/packages/atomic-pkg$ vtt publish-dir atomic src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt rm -rf dist` + +``` +``` + +## `vt run task` + +``` +~/packages/atomic-pkg$ vtt publish-dir atomic src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt print-file dist/out.txt` + +``` +atomic +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md new file mode 100644 index 000000000..e7553c278 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/gitignored_state_read_then_rewritten_is_an_output.md @@ -0,0 +1,35 @@ +# gitignored_state_read_then_rewritten_is_an_output + +A path a task reads and then rewrites is either a source it fixed up or state it manages, and access mechanics cannot tell those apart. Version control can: this file is gitignored, so it is derived state and belongs in the archive. Compare `input_read_write_not_cached`, where the same access pattern on a tracked source blocks caching instead. + +## `vt run task` + +``` +~/packages/state-pkg$ vtt mkdir .cache + +~/packages/state-pkg$ vtt cp src/data.txt .cache/state.txt + +~/packages/state-pkg$ vtt replace-file-content .cache/state.txt e E + +--- +vt run: 0/3 cache hit (0%). (Run `vt run --last-details` for full details) +``` + +## `vt run task` + +``` +~/packages/state-pkg$ vtt mkdir .cache ◉ cache hit, replaying + +~/packages/state-pkg$ vtt cp src/data.txt .cache/state.txt ◉ cache hit, replaying + +~/packages/state-pkg$ vtt replace-file-content .cache/state.txt e E ◉ cache hit, replaying + +--- +vt run: 3/3 cache hit (100%). (Run `vt run --last-details` for full details) +``` + +## `vtt print-file .cache/state.txt` + +``` +statE +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md new file mode 100644 index 000000000..cc0836622 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/output_directory_cleanup_still_caches.md @@ -0,0 +1,38 @@ +# output_directory_cleanup_still_caches + +Build tools empty their output directory before writing to it, which means they stat and list it. Those reads must not make the directory an input: its contents are the task's own product, and a fingerprint of them can never match on a clean checkout where the directory does not exist yet. + +## `vt run task` + +``` +~/packages/clean-pkg$ vtt publish-dir rebuild src/data.txt dist +``` + +## `vt run task` + +``` +~/packages/clean-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt rm -rf dist` + +``` +``` + +## `vt run task` + +``` +~/packages/clean-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +--- +vt run: cache hit. +``` + +## `vtt print-file dist/out.txt` + +``` +clean +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md new file mode 100644 index 000000000..97dfc5569 --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/snapshots/reading_inside_own_output_tree_still_caches.md @@ -0,0 +1,42 @@ +# reading_inside_own_output_tree_still_caches + +A compound command's `&&` segments are separate cached tasks. Here the middle segment reads a file inside its own package's output directory while the last segment writes another file there, which is the shape of emdash's admin build: `tsdown` reads `dist/locales//messages.mjs` and a later `locale:copy` step rewrites those files. Fingerprinting content a sibling task rewrites can never settle, so a read is treated as the task's own derived state when version control calls it derived *and* it sits under a directory the same task wrote into. Requiring both is what keeps `node_modules//dist` a real input, so changing a dependency still invalidates its consumers. + +## `vt run task` + +``` +~/packages/sibling-pkg$ vtt publish-dir rebuild src/data.txt dist + +~/packages/sibling-pkg$ vtt cp dist/out.txt dist/copied.txt + +~/packages/sibling-pkg$ vtt cp src/data.txt dist/late.txt + +--- +vt run: 0/3 cache hit (0%). (Run `vt run --last-details` for full details) +``` + +## `vt run task` + +``` +~/packages/sibling-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp dist/out.txt dist/copied.txt ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp src/data.txt dist/late.txt ◉ cache hit, replaying + +--- +vt run: 3/3 cache hit (100%). (Run `vt run --last-details` for full details) +``` + +## `vt run task` + +``` +~/packages/sibling-pkg$ vtt publish-dir rebuild src/data.txt dist ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp dist/out.txt dist/copied.txt ◉ cache hit, replaying + +~/packages/sibling-pkg$ vtt cp src/data.txt dist/late.txt ◉ cache hit, replaying + +--- +vt run: 3/3 cache hit (100%). (Run `vt run --last-details` for full details) +``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json new file mode 100644 index 000000000..d0fd62ced --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/auto_output_tracking/vite-task.json @@ -0,0 +1 @@ +{ "cache": true } diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml index e3c51ace1..25bef5d86 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots.toml @@ -22,9 +22,9 @@ cwd = "packages/rw-pkg" steps = [["vt", "run", "-v", "task"]] [[e2e]] -name = "single_O_RDWR_open_is_not_cached" +name = "single_O_RDWR_open_without_writing_is_cached" comment = """ -Opening a single file with `O_RDWR` (e.g. `touch` keeping the file) should count as a read-write overlap and prevent caching, just like separate read+write syscalls. +Opening a file `O_RDWR` and never writing to it leaves the file untouched, so it is a read and not a read-write overlap. Write capability alone must not block caching: formatters such as Biome open a clean source file read-write on every run and only write when there is something to fix, and lock files across cargo, rustc and Parcel are opened `O_RDWR` and only ever flocked. A mutation requires truncation, exclusive creation, or being a rename destination. """ cwd = "packages/touch-pkg" steps = [["vt", "run", "task"], ["vt", "run", "task"]] diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md index 74e809467..649da06e5 100644 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/multi_task_with_read_write_shows_not_cached_in_summary.md @@ -13,7 +13,7 @@ hello ~/packages/rw-pkg$ vtt replace-file-content src/data.txt i ! --- -vt run: 0/3 cache hit (0%). @test/touch-pkg#task (and 1 more) not cached because they modified their inputs. (Run `vt run --last-details` for full details) +vt run: 0/3 cache hit (0%). @test/rw-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) ``` ## `vt run -r task` @@ -22,10 +22,10 @@ vt run: 0/3 cache hit (0%). @test/touch-pkg#task (and 1 more) not cached because ~/packages/normal-pkg$ vtt print hello ◉ cache hit, replaying hello -~/packages/touch-pkg$ vtt touch-file src/data.txt +~/packages/touch-pkg$ vtt touch-file src/data.txt ◉ cache hit, replaying ~/packages/rw-pkg$ vtt replace-file-content src/data.txt i ! --- -vt run: 1/3 cache hit (33%). @test/touch-pkg#task (and 1 more) not cached because they modified their inputs. (Run `vt run --last-details` for full details) +vt run: 2/3 cache hit (66%). @test/rw-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) ``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md deleted file mode 100644 index c940d6b28..000000000 --- a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_is_not_cached.md +++ /dev/null @@ -1,21 +0,0 @@ -# single_O_RDWR_open_is_not_cached - -Opening a single file with `O_RDWR` (e.g. `touch` keeping the file) should count as a read-write overlap and prevent caching, just like separate read+write syscalls. - -## `vt run task` - -``` -~/packages/touch-pkg$ vtt touch-file src/data.txt - ---- -vt run: @test/touch-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) -``` - -## `vt run task` - -``` -~/packages/touch-pkg$ vtt touch-file src/data.txt - ---- -vt run: @test/touch-pkg#task not cached because it modified its input. (Run `vt run --last-details` for full details) -``` diff --git a/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md new file mode 100644 index 000000000..49c4c13be --- /dev/null +++ b/crates/vite_task_bin/tests/e2e_snapshots/fixtures/input_read_write_not_cached/snapshots/single_O_RDWR_open_without_writing_is_cached.md @@ -0,0 +1,18 @@ +# single_O_RDWR_open_without_writing_is_cached + +Opening a file `O_RDWR` and never writing to it leaves the file untouched, so it is a read and not a read-write overlap. Write capability alone must not block caching: formatters such as Biome open a clean source file read-write on every run and only write when there is something to fix, and lock files across cargo, rustc and Parcel are opened `O_RDWR` and only ever flocked. A mutation requires truncation, exclusive creation, or being a rename destination. + +## `vt run task` + +``` +~/packages/touch-pkg$ vtt touch-file src/data.txt +``` + +## `vt run task` + +``` +~/packages/touch-pkg$ vtt touch-file src/data.txt ◉ cache hit, replaying + +--- +vt run: cache hit. +```