feat(scratch): reference a scratch cache by addr from a consuming target - #407
Draft
raphaelvigee wants to merge 4 commits into
Draft
feat(scratch): reference a scratch cache by addr from a consuming target#407raphaelvigee wants to merge 4 commits into
raphaelvigee wants to merge 4 commits into
Conversation
Adds the reference half: `scratch = ["//build:gocache"]` on an exec target, resolved against the declaration and validated. Still nothing mounts or locks — the edge exists in the graph and the errors land, which is what the storage work builds on. A reference is an Input with `hashed: false, runtime: false` — the one combination nothing else uses, and both halves are load-bearing. It materializes no artifacts because a declaration has none, and it must not touch the consumer's cache key because a target's outputs are required to be identical whether its scratch is warm, cold, or absent. That second half is the property most likely to be "fixed" later into a bug, so it is asserted three ways: the def hash is byte-identical with and without a reference, referencing a scratch does not change what a target produces, and bumping a scratch's `version` leaves every consumer's cached result a hit. The tempting alternative — fold the declaration into the consumer's hashin so a `version` bump rebuilds users of the cache — is an over-hash: a fresh slot changes nothing about the outputs, so the rebuild is pure waste, and if it *did* change them the target is already broken and rebuilding is not the fix. Nothing new crosses the plugin ABI. The reference rides on `Input.annotations`, host-visible and already the producer→host channel (`read_only`, `stage_per_file` are the precedent), and the settings are read from the declaration's spec config rather than travelling on the edge. So: no proto change, no `ABI_SEMVER` bump, and a third-party driver participates without being recompiled. Validation lives in the engine rather than the driver, because the properties that matter are only visible across a *set* of resolved references — and putting them there means every driver gets the checks, not just pluginexec: - a referenced addr that is not a `scratch` target, naming both ends and suggesting `deps`, since that is the likely intent - two references claiming one environment variable, which would silently make one of two real caches unreachable; the fix is on a declaration, so the error says so - two mounts overlapping, which would write one cache through the other. Compared component-wise, not by string prefix, so `.cache/go` and `.cache/golang` are correctly not a collision - the same scratch referenced twice, rather than quietly collapsing it Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FArWjycMDyWeSfHHtpgtoU
`link.rs` filters `runtime: false` inputs out of `LinkedTargetDef`, which is what makes a scratch reference a pure graph edge — and is also why it never becomes a `RunInput`, so the resolved mount cannot travel on `RunInput` annotations as §5.3 claimed. Declaration and reference stay ABI-free (and are implemented that way); mounting will need an additive `RunRequest` field. Also sharpens the testing section: asserting `hashout` does not prove a target's key did not move, because a target whose `hashin` changed still produces identical bytes. The def hash has to be compared directly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FArWjycMDyWeSfHHtpgtoU
A target whose `hashin` moved still produces identical bytes, so comparing `hashout` passes while the cache misses on every run — the assertion looked like it guarded §6.3 and did not. Compare the def hash instead, for both the reference and the `version` bump. Adds a precondition that `def_hash` distinguishes two genuinely different targets, so the equality assertions cannot pass vacuously. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FArWjycMDyWeSfHHtpgtoU
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FArWjycMDyWeSfHHtpgtoU
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds the reference half:
scratch = ["//build:gocache"]on an exec target,resolved against the declaration and validated. Still nothing mounts or locks —
the edge exists in the graph and the errors land, which is what the storage work
builds on.
A reference is an Input with
hashed: false, runtime: false— the onecombination nothing else uses, and both halves are load-bearing. It materializes
no artifacts because a declaration has none, and it must not touch the
consumer's cache key because a target's outputs are required to be identical
whether its scratch is warm, cold, or absent.
That second half is the property most likely to be "fixed" later into a bug, so
it is asserted three ways: the def hash is byte-identical with and without a
reference, referencing a scratch does not change what a target produces, and
bumping a scratch's
versionleaves every consumer's cached result a hit. Thetempting alternative — fold the declaration into the consumer's hashin so a
versionbump rebuilds users of the cache — is an over-hash: a fresh slotchanges nothing about the outputs, so the rebuild is pure waste, and if it did
change them the target is already broken and rebuilding is not the fix.
Nothing new crosses the plugin ABI. The reference rides on
Input.annotations,host-visible and already the producer→host channel (
read_only,stage_per_fileare the precedent), and the settings are read from thedeclaration's spec config rather than travelling on the edge. So: no proto
change, no
ABI_SEMVERbump, and a third-party driver participates withoutbeing recompiled.
Validation lives in the engine rather than the driver, because the properties
that matter are only visible across a set of resolved references — and putting
them there means every driver gets the checks, not just pluginexec:
scratchtarget, naming both ends andsuggesting
deps, since that is the likely intentone of two real caches unreachable; the fix is on a declaration, so the error
says so
Compared component-wise, not by string prefix, so
.cache/goand.cache/golangare correctly not a collisionCo-Authored-By: Claude Opus 5 (1M context) noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01FArWjycMDyWeSfHHtpgtoU
Stack created with GitHub Stacks CLI • Give Feedback 💬