Skip to content

Pin the toolchain and stop shipping machine-specific state (review PRs 1 + 2) - #1

Merged
codeitlikemiley merged 3 commits into
mainfrom
claude/bazel-setup-review-2gcgzi
Aug 2, 2026
Merged

Pin the toolchain and stop shipping machine-specific state (review PRs 1 + 2)#1
codeitlikemiley merged 3 commits into
mainfrom
claude/bazel-setup-review-2gcgzi

Conversation

@codeitlikemiley

@codeitlikemiley codeitlikemiley commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Three independently reviewable commits from the build-system review. Read them one at a time — they do unrelated work.

  • 1a4844f PR 1 — stop shipping generated and machine-local files
  • a73a9ac PR 2 — pin the Rust toolchain, the edition and the Bazel version
  • 47e8810 PR 2 follow-up.bazelrc amendments after running Bazel end to end

Why

Bazel and Cargo were not building the same program. The most consequential case: every Cargo.toml says edition = "2024", but no BUILD target set edition and no rust.toolchain() was registered, so rules_rust fell back to its own rust.toolchain(edition = "2021") at DEFAULT_RUST_VERSION = "1.86.0". The rust-project.json that was committed to this repo — produced by a real gen_rust_project run — confirms it: all five first-party crates carried "edition": "2021" with a rust_analyzer_1.86.0_tools sysroot, while cargo reports 1.94.1.

That is not only a syntax-compatibility gap. Demonstrated with rustc 1.94.1: an if let holding a MutexGuard whose else arm re-locks deadlocks under --edition 2021 and completes under --edition 2024, because 2024 changed when the scrutinee temporary is dropped. cargo test would pass, bazel test would hang, and nobody could reproduce it locally.

PR 1 — stop shipping machine-specific state

No build logic changes.

Change Reason
Untrack + gitignore rust-project.json 60 of 65 crate roots pointed into /private/var/tmp/_bazel_uriah/…, 5 into /Users/uriah/Code/yoyo, proc-macros at darwin_arm64 .dylibs. Also 3 commits stale, with 2 dead target labels. rust-analyzer's ProjectManifest::discover returns on finding it and never reaches the Cargo fallback — which works today. Shipping it is worse than not having it.
Harden build-ra.sh set -euo pipefail, resolve the repo root, prefer bazelisk, fail clearly when neither binary is present, accept target patterns.
Clean .cargo-runner.json Drop linked_projects (5 absolute /Users/uriah paths). Delete combos/.cargo-runner.jsoncombos/ is a virtual workspace root with no [package]. Strip combos/frontend's override, whose match.file_path is another absolute path that can never fire.
.gitignore Delete the *.md catch-all — it silently swallowed every future doc. Delete cargo-bazel-lock.json, which is the crate_universe lockfile artifact and must be committed. .vscode/.vscode/* + negations. Remove the 5 byte-identical leaf .gitignore files.
README IDE section It pointed at three things that don't exist (.vscode/tasks.json, a Cmd+R Cmd+R binding, ./refresh-rust-analyzer.sh) and told readers to hand-write "rust-analyzer.linkedProjects": ["rust-project.json"] — which would now point at nothing and suppress the working fallback.

Verified with git check-ignore that target/, bazel-*, .DS_Store and rust-project.json are still ignored at every depth from the root file alone, and that CONTRIBUTING.md is now addable.

PR 2 — pin the toolchain

  • MODULE.bazelrust.toolchain(edition = "2024", versions = ["1.94.1"], sha256s = {…}) + register_toolchains. Pinned to 1.94.1, the version verified to build all three workspaces, rather than latest (1.97.1).
  • sha256s (27 entries) — rules_rust 0.63.0 predates 1.94.1, so known_shas.bzl has no entry and repository_utils.bzl:516-521 would download the toolchain unverified. Hashes come from the channel manifest at static.rust-lang.org.
  • rust-toolchain.toml at the same version, plus rust-version = "1.94" in all four [package] tables.
  • edition = "2024" on all nine source-compiling targets. Deliberately not on the five rust_test(crate = …) targets — rust.bzl:374 makes them inherit crate.edition, so it would be a no-op there.
  • .bazelversion8.7.0. Not 9.x: rules_rust 0.63.0 predates it and declares a 7.4.1 minimum in its own CI config.
  • .bazelrc — the base config the repo never had, though .gitignore already reserved .bazelrc.user. Notably --compilation_mode=dbg, since Bazel's default fastbuild is opt-level 0 with no debug info while cargo's dev profile is -C debuginfo=2.
  • .bazelignore, and MODULE.bazel.lock is no longer ignored.
  • tools/pin_rust_toolchain.py — regenerates the hash dict so a version bump is one command.

One proposed flag was dropped after checking it

--incompatible_strict_action_env does not exist in Bazel 8.7.0 (absent from CoreOptions.java). It would have broken every invocation. Every other flag was verified present in Bazel 8.7.0 or in rules_rust 0.63.0's rust/settings/settings.bzl.

--@rules_rust//rust/settings:clippy.toml=//:clippy.toml and its rustfmt twin remain deferred — those files don't exist yet and the root BUILD.bazel exports nothing, so the label_flag in @rules_rust//rust/settings can't see them.

Verification — Bazel was run end to end

Bazel 8.7.0 installed from the official APT bucket on storage.googleapis.com (SHA256 matching the published index). bcr.bazel.build is unreachable from the build environment, so module metadata came from a local mirror of bazelbuild/bazel-central-registry with sources redirected to git clones. That scaffolding is entirely outside the repo plus a gitignored .bazelrc.user; none of it is committed.

Check Result
bazel build //... OK — 603 actions, 17 targets
bazel test //... 9 of 9 pass
bazel build --config=release //... OK
bazel build --config=ci //... OK
gen_rust_project all 8 first-party crates edition=2024; sysroot rust_analyzer_1.94.1_tools
toolchain integrity tampering one sha256s entry fails the fetch with a checksum mismatch
cargo side cargo build --all-targets + cargo test --locked pass in all three workspaces; no Cargo.lock changed

The central claim is now measured rather than argued:

backend_bin edition=2024   frontend_bin edition=2024   corex        edition=2024
bench       edition=2024   axum_example edition=2024   proxy        edition=2024
server_bin  edition=2024   integration_tests_tests_just_test_test edition=2024

EDITIONS ACROSS FIRST-PARTY: {'2024'}

The sha256s block needed proving specifically, because a wrong key degrades silently to an unverified download rather than failing. Corrupting one entry and rebuilding with a cold repository cache gives Checksum was 0a6b16ca…87e1 but wanted 0000…, and 0a6b16ca…87e1 is exactly what tools/pin_rust_toolchain.py emits — so the keys match produce_tool_path() and the hashes are enforced.

Not verified

bazel coverage, for an environment reason only: Bazel's own coverage_output_generator is hosted on mirror.bazel.build, which the build environment's egress policy blocks with a 403. The coverage flags parse.

MODULE.bazel.lock is deliberately not committed

It generates fine, but the build ran against a file:// registry, so Bazel records no registry file hashes — registryFileHashes is empty where a real BCR build records thousands — and its moduleExtensions entries were evaluated against git clones rather than release artifacts. Committing it would look like progress while guaranteeing failure the moment CI enables --lockfile_mode=error. It must be generated on a machine with real BCR access first; the .bazelrc comment says so.

Known follow-ups (deliberately out of scope)

  • Adding lockfile = "//…:cargo-bazel-lock.json" to the three crate.from_cargo tags. PR 1 removed the .gitignore line that was pre-blocking it.
  • size = "small" on the test targets — all nine are declared MODERATE and finish in ~0.1s. test:ci --test_verbose_timeout_warnings now surfaces this.
  • BUILD-file correctness, the server library split, CI, and collapsing the three Cargo.lock files into one workspace — the only fix for server_bin currently linking two distinct serde rlibs.

claude added 3 commits August 1, 2026 23:20
Removes generated and machine-local files from version control, and fixes the
docs and ignore rules that were keeping them there. No build logic changes.

rust-project.json (untracked, now gitignored)
  Generated by gen_rust_project on a macOS machine: 60 of its 65 crate roots
  point into /private/var/tmp/_bazel_uriah/..., the remaining 5 into
  /Users/uriah/Code/yoyo, the sysroot into a dead rust_analyzer_1.86.0_tools
  path, and all four proc_macro_dylib_path entries at darwin_arm64 .dylib
  files. It is also three commits stale: two of its five target labels no
  longer exist (server:server_tests, server:integrated_tests_suite_tests/
  just_test_test) and src/bin/proxy.rs, examples/axum.rs and
  benches/fibonacci_benchmark.rs have no entry at all.

  Shipping it is strictly worse than not having it. rust-analyzer's
  ProjectManifest::discover returns as soon as it finds rust-project.json in a
  parent directory and never reaches the Cargo fallback -- and that fallback
  works today, discovering corex/, server/ and combos/ one level down.

build-ra.sh
  set -euo pipefail, resolve the repo root instead of assuming the caller's
  cwd, prefer bazelisk and fail with a clear message when neither binary is on
  PATH, and accept target patterns instead of hardcoding //....

.cargo-runner.json
  Drop the linked_projects array: five absolute /Users/uriah/Code/yoyo paths
  that resolve nowhere else. Delete combos/.cargo-runner.json -- combos/ is a
  virtual workspace root with no [package], so the file configures nothing.
  Strip combos/frontend's override, whose match.file_path is another absolute
  /Users/uriah path and can never fire, plus its redundant "command": "bazel"
  already set at the root.

.gitignore
  - Delete the *.md catch-all with its two exceptions. It silently swallowed
    every future doc; CONTRIBUTING.md was unaddable without a -f.
  - Delete cargo-bazel-lock.json. That is the crate_universe `lockfile`
    artifact and must be committed for deterministic crate resolution --
    ignoring it pre-blocks the fix, the same way ignoring MODULE.bazel.lock
    does.
  - .vscode/ -> .vscode/* plus negations, so shared editor config can be
    committed without dragging in per-user state.
  - Add rust-project.json.
  - Remove the five leaf .gitignore files (combos/, combos/backend/,
    combos/frontend/, corex/, server/). All five were byte-identical
    (cb20a5d3...) and were the root file minus the *.md block. Verified with
    git check-ignore that target/, bazel-*, .DS_Store and rust-project.json
    are still ignored at every depth from the root file alone.

README.md
  The IDE section pointed at three things that do not exist in this repo
  (.vscode/tasks.json, a Cmd+R Cmd+R binding, ./refresh-rust-analyzer.sh) and
  told the reader to hand-write "rust-analyzer.linkedProjects":
  ["rust-project.json"] -- which, now that the file is gitignored, would point
  at nothing and suppress the working Cargo fallback. Replaced with what
  actually works.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D3MwkT9fzAzZweK4birHYz
Closes the three divergences that were wrong on EVERY build, silently:
Bazel and Cargo were using different language editions, different compilers,
and different debug-info settings on the same source.

MODULE.bazel -- register a toolchain
  rules_rust rust/extensions.bzl:93 is
      toolchains = root.tags.toolchain or rules_rust.tags.toolchain
  so with no rust.toolchain() in the root module the fallback is rules_rust's
  own, which is `rust.toolchain(edition = "2021")` (its MODULE.bazel:48) at
  DEFAULT_RUST_VERSION = "1.86.0" (rust/private/common.bzl:34).

  Evidence this was live, from the rust-project.json that a real gen_rust_project
  run produced on the author's machine: all five first-party crates carried
  "edition": "2021" while all four Cargo.toml files said edition = "2024", and
  the sysroot pointed at rust_analyzer_1.86.0_tools while cargo here is 1.94.1.

  That gap is not only a syntax-compatibility question. Demonstrated with rustc
  1.94.1: an `if let` holding a MutexGuard whose else-arm re-locks deadlocks
  under --edition 2021 and completes under --edition 2024, because 2024 changed
  when the scrutinee temporary is dropped. Same class of hazard for static mut
  refs, unsafe_op_in_unsafe_fn and RPIT lifetime capture. cargo test would pass
  and bazel test would hang, with no way to reproduce it locally.

  Pinned to 1.94.1 -- the version verified to build all three workspaces here --
  rather than latest (1.97.1), so the pin is one we have actually exercised.
  Bumping is a two-line change plus one script run.

  sha256s: rules_rust 0.63.0 predates 1.94.1, so rust/known_shas.bzl has no
  entry and repository_utils.bzl:516-521 would fall back to sha256 = "",
  downloading the toolchain unverified and marking the repo non-reproducible.
  The 27 hashes come from the signed channel manifest at static.rust-lang.org
  and were spot-checked by downloading rust-src-1.94.1.tar.xz and comparing
  (cb375615...b66c matches). Key format "<tool>-<version>-<triple>.tar.xz"
  matches produce_tool_path() + extension; all 27 archive names were validated
  against the manifest's own xz_url basenames.

rust-toolchain.toml -- the cargo half of the same pin, at the same version.
  Plus rust-version = "1.94" in all four [package] tables, so cargo refuses an
  older local toolchain instead of deferring the failure to Bazel.

edition = "2024" on all nine source-compiling targets
  Belt and braces with the toolchain default, and it makes each target
  self-describing. Deliberately NOT added to the five rust_test targets that
  set `crate = ...`: rust.bzl:374 makes them inherit crate.edition, so the
  attribute there is a no-op.

.bazelversion -- 8.7.0, the newest 8.x LTS. Not 9.x: rules_rust 0.63.0 predates
  it and declares a minimum of 7.4.1 in its own CI config.

.bazelrc -- the base config the repo never had, even though .gitignore already
  reserved .bazelrc.user and .bazelversion.user. Notably --compilation_mode=dbg,
  because Bazel's default fastbuild is opt-level 0 with no debug info while
  cargo's dev profile is -C debuginfo=2.

  Two flags from the review's proposal were dropped after checking them against
  the pinned versions:
    - --incompatible_strict_action_env does not exist in Bazel 8.7.0
      (absent from CoreOptions.java); it would have broken every invocation.
    - --@rules_rust//rust/settings:clippy.toml=//:clippy.toml and its rustfmt
      twin are deferred: the files do not exist yet and the root BUILD.bazel
      exports nothing, so the label_flag in @rules_rust//rust/settings could
      not see them.
  Every remaining flag was verified present in Bazel 8.7.0 or in rules_rust
  0.63.0's rust/settings/settings.bzl.

.bazelignore -- keep Bazel's package loader out of the five cargo target/ dirs.

.gitignore -- stop ignoring MODULE.bazel.lock. It is what makes a bzlmod build
  reproducible and what --lockfile_mode=error checks in CI.

tools/pin_rust_toolchain.py -- regenerates the sha256s dict from the channel
  manifest so a version bump stays a one-command change.

Verified: cargo build --all-targets and cargo test --locked still pass in all
three workspaces, and no Cargo.lock changed. NOT verified by running Bazel --
neither releases.bazel.build nor bcr.bazel.build is reachable from this
environment. First run on a networked machine should be:
    bazel build //... && bazel run @rules_rust//tools/rust_analyzer:gen_rust_project -- //...
then confirm every first-party crate in rust-project.json reports
"edition": "2024", and commit the generated MODULE.bazel.lock.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D3MwkT9fzAzZweK4birHYz
Bazel now actually runs in this environment, so the parts of PR 2 that were
previously reasoned-only have been executed. Three changes fall out of that.

Add pipelined_compilation
  Held back in PR 2 as an untested behaviour change. Verified: a full
  `bazel build //...` with
  --@rules_rust//rust/settings:pipelined_compilation=True builds all 17 targets
  successfully on rules_rust 0.63.0 / Bazel 8.7.0.

Add test:ci --test_verbose_timeout_warnings
  Also held back as unverified; Bazel itself suggests it. Scoped to --config=ci
  rather than the default `test` config, because it is currently noisy: every
  test in the repo is declared MODERATE and finishes in about 0.1s, so all nine
  warn. The real fix is size = "small" on the test targets, which belongs to
  the BUILD-file sweep, and this makes CI say so.

Sharpen the --lockfile_mode=error prerequisite
  MODULE.bazel.lock is deliberately NOT committed in this change. bcr.bazel.build
  is unreachable from this sandbox, so the build was run against a local mirror
  of the Bazel Central Registry served over file://. Bazel does not record
  registry file hashes for a file:// registry, so the lock generated here has an
  empty registryFileHashes map and would be judged out of date the moment
  anyone builds against the real BCR. It has to be generated on a machine with
  BCR access before --config=ci can be switched on.

Verified in this environment (Bazel 8.7.0, rules_rust 0.63.0, Rust 1.94.1):
  bazel build //...                      603 actions, 17 targets, OK
  bazel test //...                       9 of 9 tests pass
  bazel build --config=release //...     OK
  bazel build --config=ci //...          OK
  gen_rust_project                       all 8 first-party crates edition=2024,
                                         sysroot rust_analyzer_1.94.1_tools
  toolchain integrity                    tampering one sha256s entry makes the
                                         fetch fail with a checksum mismatch, so
                                         the pinned hashes are enforced, not
                                         silently ignored

Not verified: `bazel coverage`. It fails here only because Bazel's own
coverage_output_generator lives on mirror.bazel.build, which this sandbox's
egress policy blocks with a 403. The coverage flags themselves parse.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D3MwkT9fzAzZweK4birHYz

Copy link
Copy Markdown
Owner Author

Verified by actually running Bazel

The PR description said Bazel could not run here. That is no longer true — 47e8810 is the follow-up. Bazel 8.7.0 was installed from the official APT bucket on storage.googleapis.com (SHA256 889831cc…a6bd, matching the published index), and bcr.bazel.build was replaced with a local mirror of bazelbuild/bazel-central-registry fetched through the session git proxy, with module sources redirected to git clones. All of that is verification scaffolding in /tmp and a gitignored .bazelrc.user; nothing about it is committed.

Results

Check Result
bazel build //... OK — 603 actions, 17 targets, 128s
bazel test //... 9 of 9 pass
bazel build --config=release //... OK
bazel build --config=ci //... OK
gen_rust_project all 8 first-party crates edition=2024; sysroot rust_analyzer_1.94.1_tools
toolchain integrity tampering one sha256s entry fails the fetch with a checksum mismatch

The central claim of the review is now measured rather than argued:

backend_bin    edition=2024      frontend_bin   edition=2024
corex          edition=2024      bench          edition=2024
axum_example   edition=2024      proxy          edition=2024
server_bin     edition=2024      integration_tests_tests_just_test_test edition=2024

EDITIONS ACROSS FIRST-PARTY: {'2024'}
sysroot: .../rules_rust++rust+rust_analyzer_1.94.1_tools

Before this branch those same crates read edition=2021 with a rust_analyzer_1.86.0 sysroot.

The sha256s block needed proving specifically, because a wrong key degrades silently to an unverified download rather than failing. Corrupting one entry and rebuilding with a cold repository cache gives:

Checksum was 0a6b16ca476461c6238f48efcb74a7aa08a3983e61adf587f208ba48504d87e1
        but wanted 0000000000000000000000000000000000000000000000000000000000000000

0a6b16ca…87e1 is exactly what tools/pin_rust_toolchain.py emitted, so the keys match produce_tool_path() and the hashes are enforced.

What changed as a result

  • Added pipelined_compilation — held back in PR 2 as an untested behaviour change; now verified across all 17 targets.
  • Added test:ci --test_verbose_timeout_warnings — also previously unverified. Scoped to --config=ci because it is noisy today: every test is declared MODERATE and finishes in ~0.1s, so all nine warn. The fix is size = "small", which belongs to the BUILD-file sweep.

One thing deliberately not done

MODULE.bazel.lock is not committed. It generates fine, but because the build ran against a file:// registry, Bazel records no registry file hashes — registryFileHashes is empty — so this lock would be judged out of date the moment anyone builds against the real BCR. It has to be generated on a machine with BCR access before --config=ci (which sets --lockfile_mode=error) can be switched on. The .bazelrc comment now says so explicitly.

Still unverified

bazel coverage — and only for an environment reason: Bazel's own coverage_output_generator is hosted on mirror.bazel.build, which this sandbox's egress policy blocks with a 403. The coverage flags parse; the tool behind them can't be fetched here.


Generated by Claude Code

@codeitlikemiley
codeitlikemiley marked this pull request as ready for review August 1, 2026 23:58
@codeitlikemiley
codeitlikemiley merged commit c878c58 into main Aug 2, 2026
codeitlikemiley added a commit that referenced this pull request Aug 2, 2026
Takes the repo from "CI would be red on day one" to a green, trustworthy gate.

Fixed a real bug: the fibonacci bench swept n to 100, but fib(94) exceeds
u64::MAX, so cargo test --all-targets exited 101 on an overflow panic. Release
builds were silently wrapping instead, identically across all four
implementations, so the file's own consistency check could never catch it.

Cleared 9 clippy errors and 8 rustfmt hunks. Vacuous tests were made real where
possible (corex doctests, three tokio tests over the real server handlers) and
deleted where not -- including two #[should_panic] blocks asserting only that
assert_eq! panics. bazel test //... goes from 9 targets to 4, but 5 of the 9
asserted nothing and would have stayed green through any regression.

Adds .github/workflows/ci.yml gating both build systems, --config=lint
(rustfmt + clippy aspects, -D warnings), rustfmt.toml, clippy.toml and
.editorconfig, with test sizes declared so runs are warning-free.

All four checks green on 1edde1b. The bazel job resolved from the real
bcr.bazel.build and downloaded Rust 1.94.1 via the pinned sha256s, which also
confirms the toolchain pin from #1 outside the sandbox it was developed in.
codeitlikemiley pushed a commit that referenced this pull request Aug 2, 2026
Preserves the round-2 review that drove PRs #1-#7. It is kept for the
reasoning, not the findings: how each defect was found, what evidence settled
it, and which plausible-sounding theories turned out to be wrong under
adversarial checking.

It is fronted with a prominent ARCHIVED header, because the body is written in
the present tense about a repo that no longer exists in that state -- it opens
with "nothing in this repo currently fails to build" and describes three
lockfiles, an unpinned toolchain and a binary-only server. Without the header
this would be exactly the kind of confidently-wrong document the review itself
spent most of its length cataloguing. The header maps each area to the merge
commit that resolved it, and records the one item deliberately not done (the
release path).

Also fixes .gitignore:1. `bazel-*` was unanchored, so it matched any path
segment starting with "bazel-" at any depth, not just the bazel-bin/bazel-out
convenience symlinks at the repo root it was written for. This file was silently
unaddable because of it, and so would docs/bazel-guide.md or
tools/bazel-wrapper.sh be. Anchored to `/bazel-*`; verified that bazel-bin,
bazel-out, bazel-testlogs and bazel-complex-bazel-setup are still ignored and
that nested bazel-*-named files now are not.

Same shape as the `*.md` catch-all removed in the first PR of this series: an
over-broad ignore pattern quietly deciding what can exist in the repo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01D3MwkT9fzAzZweK4birHYz
codeitlikemiley added a commit that referenced this pull request Aug 2, 2026
…ig bugs

Archives docs/bazel-review-2026-08.md, the round-2 review that drove #1-#7,
behind a prominent ARCHIVED header -- the body is written in the present tense
about a repo that no longer exists in that state, so unmarked it would be the
same class of confidently-wrong document the review itself catalogued. The
header maps each finding area to the commit that resolved it.

Two silent bugs surfaced while doing it, both the same shape: config quietly
deciding what exists.

.gitignore:1 `bazel-*` was unanchored, matching any path segment starting with
"bazel-" at any depth rather than the root bazel-bin/bazel-out symlinks it was
written for. The new doc was unaddable; so would docs/bazel-guide.md be.
Anchored to /bazel-*, verified in both directions.

.github/dependabot.yml declared package-ecosystem "bazel-modules", which does
not exist. Dependabot rejects the entire file on one invalid value, so cargo and
github-actions updates were dead too -- the repo has had no dependency
automation since #5 while appearing to have three ecosystems configured. Fixed
to "bazel"; the .github/dependabot.yml check passes for the first time.

Also documents an ecosystem deliberately left off: rust-toolchain is valid and
would bump rust-toolchain.toml alone, resplitting cargo and Bazel onto different
compilers -- the original defect. Both halves of that pin move together, by hand.
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.

2 participants