Skip to content

Fix atomic object-cache publication - #151

Merged
itsfuad merged 6 commits into
mainfrom
fix/atomic-object-cache-publication
Oct 4, 2026
Merged

itsfuad merged 6 commits into
mainfrom
fix/atomic-object-cache-publication

Conversation

@itsfuad

@itsfuad itsfuad commented Oct 3, 2026 •

Copy link
Copy Markdown
Member

Summary

  • Compile managed cache objects in unique staging directories on the destination filesystem, avoiding cross-device publication failures.
  • Publish completed objects with atomic hard-link creation. Publication never overwrites, renames away, truncates, or deletes an existing cache winner.
  • Reuse a competing publisher only after validating the final directory entry as a regular file with os.Lstat; directories, symlinks, and other non-regular entries are rejected and preserved.
  • Scope staging cleanup to each object compilation; cover concurrency, failure cleanup, permissions, cache reuse, and native readers.
  • Run native cache regressions in the existing Linux/macOS/Windows amd64/arm64 Actions matrix.

Production ownership

  • cmd/build.go owns native object compilation. compileObject is consumed directly by buildExecutable and owns the per-object staging lifetime.
  • Reuses objectCachePath, toolchain.Profile.ObjectArgs, runCompilerTool, destination-local staging, and one inspectCachedObject rule shared by initial hits and competing winners. Cache keys and compiler arguments are preserved.
  • Removed backup-first and overwrite-style cache publication. No process-global lock, retry loop, production test hook, wrapper, stale alias, or experimental scaffolding remains.
  • Temporary cmd/build.go.bugfix.md is absent and docs/end-to-end-code-review.md matches the PR base exactly.

Validation

Passed locally on the intended final worktree:

go test -count=1 ./cmd
go test -race -count=1 -run "^TestCompileObject" ./cmd
go test -count=50 -run "^TestCompileObjectConcurrentReaders$" ./cmd
go test -count=20 -run "^TestCompileObject" ./cmd
go test -count=1 ./...
go vet ./...
go run ./scripts/bundle.go
PEEPER_BIN="$PWD/build/bin/peeper" go test -count=1 -run "^TestManagedObjectCacheFixture$" -v ./x_test
env GOOS=windows GOARCH=amd64 CGO_ENABLED=0 go test -c -o /tmp/peeper-pr151-windows-amd64.test.exe ./cmd
env GOOS=darwin GOARCH=arm64 CGO_ENABLED=0 go test -c -o /tmp/peeper-pr151-darwin-arm64.test ./cmd
git diff --check

Regression sensitivity verified:

  • Backup-first publication failed Linux filesystem-event and concurrent-reader tests.
  • The pre-existing directory regression returned a directory as a successful cache hit before regular-entry validation.
  • A symlink to a regular file was accepted before switching from os.Stat to os.Lstat.
  • Overwrite-style os.Rename changed the published file identity under the concurrent-reader test. Native Windows amd64 run 37150250680 also observed ERROR_SHARING_VIOLATION while publisher 2 replaced the winner.

Risks and follow-up

  • os.Link is the standard-library create-if-absent primitive: destination existence does not overwrite the winner. After any link failure, actual destination state is inspected; a regular winner is reused, while no winner or a non-regular entry returns the wrapped *os.LinkError.
  • Publication requires source and destination on one hard-link-capable filesystem. Same-directory staging guarantees one filesystem/volume. Mainstream Linux filesystems, APFS/HFS+, and NTFS support regular-file hard links; FAT/exFAT/ReFS and some network/FUSE providers do not and receive a clear publication error rather than an unsafe fallback.
  • Native Windows locked-reader behavior remains covered by TestCompileObjectReusesLockedWinner; native validation of the new mechanism is required in Actions. Local Windows/macOS checks are cross-compilation only.
  • The Linux integration fixture uses real system Clang with target.SystemLLVMTriple so output links and runs against runner libc. It proves real object compilation, destination-filesystem staging, cross-filesystem behavior, publication, reuse, linking, and fixture execution; it does not prove the managed musl ABI/toolchain path.
  • Fresh CI and non-author human approval are required after the follow-up commit is pushed.

itsfuad and others added 6 commits October 4, 2026 00:22
Stage managed objects on the cache filesystem and publish without
moving existing cache entries aside. Reuse a competing publisher's
complete object when replacement fails.

Scope staging cleanup to each object compilation. Add concurrency,
failure-cleanup, cross-filesystem, and cache-reuse regressions, and
run native cache tests in the GitHub Actions matrix.
Validate cached paths before reuse and preserve existing directories on
collision. Add regression coverage and refresh end-to-end review
documentation.
@itsfuad
itsfuad enabled auto-merge October 4, 2026 06:17
@itsfuad
itsfuad disabled auto-merge October 4, 2026 06:17
@itsfuad
itsfuad enabled auto-merge October 4, 2026 06:17
@itsfuad
itsfuad disabled auto-merge October 4, 2026 06:24
@itsfuad
itsfuad merged commit 877fe5c into main Oct 4, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant