Skip to content

test-tonic: use a writable per-machine CARGO_HOME - #34291

Closed
robobun wants to merge 1 commit into
mainfrom
farm/0dd9a54e/test-tonic-cargo-home
Closed

test-tonic: use a writable per-machine CARGO_HOME#34291
robobun wants to merge 1 commit into
mainfrom
farm/0dd9a54e/test-tonic-cargo-home

Conversation

@robobun

@robobun robobun commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

test/js/third_party/grpc-js/test-tonic.test.ts went red on the :darwin: 14 aarch64 lane in build 73477:

error: tonic server exited (101) before reporting an address:
error: failed to open `/opt/rust/registry/cache/index.crates.io-1949cf8c6b5b557f/http-body-util-0.1.4.crate`

Caused by:
  Permission denied (os error 13)

Cause

The tonic-server fixture has no Cargo.lock, so every run resolves against the current crates.io index. http-body-util 0.1.4 was published on 2026-07-13, so cargo now wants to download it into $CARGO_HOME/registry/cache/.

On darwin-test-arm64-1 the agent was moved from administrator to ciadmin on Jul 9 (see #33970), but /opt/rust (the box's shared CARGO_HOME/RUSTUP_HOME) is still owned by administrator with the registry/ subtree at drwxr-xr-x, so ciadmin can read existing crates but cannot write new ones. The test forwarded CARGO_HOME: process.env.CARGO_HOME, so cargo tried to create the new .crate file under /opt/rust/registry/cache/ and got EACCES.

All other darwin test boxes still run the agent as administrator; darwin-test-arm64-1 is the only one with this user mismatch.

Fix

Point CARGO_HOME at the same per-machine cache directory that already holds CARGO_TARGET_DIR ($TMPDIR/bun-test-tonic-cache/cargo-home). That directory is created by the test runner's own user, so cargo can always write its registry there, regardless of which user owns the system rust install. RUSTUP_HOME stays inherited (rustup only needs read access to locate the toolchain). The registry cache is ~100 MB and persists alongside the 500 MB target dir that was already there, so warm runs are unchanged.

Verification

With CARGO_HOME inherited from a location the runner cannot write to:

  • before: tonic server exited (101) ... Permission denied (os error 13)
  • after: cargo downloads to $TMPDIR/bun-test-tonic-cache/cargo-home, server builds and listens, flow control should work in both directions passes (cold ~36s, warm ~16s).

Test-only change; no src/ diff.

The box-level user/ownership mismatch on darwin-test-arm64-1 is being handled separately.


no test proof · iteration 0 · Platform-specific test-only change; deferring to CI.

The fixture has no Cargo.lock, so cargo resolves fresh and will pick up
new patch versions of transitive deps. When http-body-util 0.1.4 landed
on 2026-07-13, the darwin-aarch64 lane failed because the agent user on
that box cannot write to the shared /opt/rust registry cache:

  error: failed to open `/opt/rust/registry/cache/.../http-body-util-0.1.4.crate`
  Caused by: Permission denied (os error 13)

Point CARGO_HOME at the existing bun-test-tonic-cache directory (already
used for CARGO_TARGET_DIR) so cargo writes its registry to a location the
test runner always owns, instead of inheriting a system CARGO_HOME that
may belong to a different user. RUSTUP_HOME stays inherited since rustup
only needs read access to locate the toolchain.
@robobun

robobun commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 6:47 PM PT - Jul 15th, 2026

@robobun, your commit 017086d is building: #73565

@coderabbitai

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Changes

Tonic Cargo environment

Layer / File(s) Summary
Configure cached Cargo home
test/js/third_party/grpc-js/test-tonic.test.ts
The spawned Rust server now uses join(cacheDir, "cargo-home") for CARGO_HOME, while retaining inherited Rust toolchain settings and updating the explanatory comment.

Possibly related PRs

  • oven-sh/bun#33970: Updates the same tonic test’s Cargo and Rust toolchain spawn environment.

Suggested reviewers: jarred-sumner

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title is concise and accurately describes the main change: making CARGO_HOME writable per machine.
Description check ✅ Passed The description explains the problem, fix, and verification, covering the template's intent even though the headings differ.

Comment @coderabbitai help to get the list of available commands.

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — straightforward test-infra fix pointing CARGO_HOME at the same writable per-machine cache dir already used for CARGO_TARGET_DIR.

Checked that cargo auto-creates $CARGO_HOME (and its parent) on first use, so no explicit mkdir is needed even on a fresh box where cacheDir doesn't exist yet. RUSTUP_HOME is still inherited, so toolchain discovery is unaffected. The registry cache persists across runs alongside the existing target cache, so warm-run timing shouldn't regress.

Extended reasoning...

Overview

Single-file test-infrastructure change to test/js/third_party/grpc-js/test-tonic.test.ts. Replaces CARGO_HOME: process.env.CARGO_HOME in the spawned cargo run env with CARGO_HOME: join(cacheDir, "cargo-home"), where cacheDir is the existing $TMPDIR/bun-test-tonic-cache directory that already hosts CARGO_TARGET_DIR. The accompanying comment is updated to explain why (agent user may not have write access to the system CARGO_HOME).

Security risks

None. This only changes an environment variable passed to a test-fixture subprocess that builds a local Rust gRPC server. No auth, crypto, or user-facing surface is touched.

Level of scrutiny

Low. Test-only, no src/ diff, and the change mirrors the exact pattern already in use one line below for CARGO_TARGET_DIR. The failure mode it addresses (EACCES writing to a read-only shared /opt/rust) and the fix (redirect to a user-writable tmpdir cache) are both well-understood cargo behaviors.

Other factors

  • Cargo creates $CARGO_HOME and its subdirectories on demand, so there's no need for the test to pre-create cargo-home even when cacheDir itself doesn't exist yet — same as the existing CARGO_TARGET_DIR handling.
  • RUSTUP_HOME/RUSTUP_TOOLCHAIN remain inherited, so rustup can still locate the read-only system toolchain; only the writable registry cache is relocated.
  • The cache persists in tmpdir() across runs on persistent CI agents, matching the intent of the surrounding code (protoc zip and target dir already live there).
  • No prior reviewer comments to address; bug hunter found nothing.

@robobun

robobun commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator Author

Closing: the root cause was the host agent on darwin-test-arm64-1 running as ciadmin while /opt/rust (the shared CARGO_HOME) is owned by administrator. That box has been moved back to the same system LaunchDaemon configuration as the other macOS test hosts (agent runs as administrator), so cargo can write its registry cache again and the test on main passes without any code change.

The redirect in this PR would also have dropped the only cache that actually persisted across runs (runner.node.mjs sets a per-test-file TMPDIR and removes it afterward, so join(tmpdir(), ...) never survives), which is a regression relative to the persistent /opt/rust registry.

@robobun robobun closed this Jul 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant