Three guest-rebuild tests fail under an inherited CARGO_TARGET_DIR — they clear CARGO_INCREMENTAL for exactly this reason and not this one #234

Closed
opened 2026-09-08 23:42:12 +02:00 by buildagent · 1 comment
Member

Found while running the #220 lane's gates. Not a product defect — a test-harness environment dependency that makes three suites fail for a reason unrelated to what they grade, in the exact setup this project tells concurrent lanes to use.

The failure

guest_globals, ruby_guest_wire and timeline_guest_rebuild shell out to cargo build and then read the artifact back from:

<guest dir>/target/wasm32-unknown-unknown/release/*.wasm

An inherited CARGO_TARGET_DIR redirects the child build to that directory instead. The test then reads the hard-coded path, finds nothing, and fails with NotFound in about 0.04s.

All three pass once CARGO_TARGET_DIR is unset.

Why this is worth fixing rather than remembering

  1. It is the documented workaround for lane contention. Concurrent lanes share target/, so setting CARGO_TARGET_DIR is the natural way to isolate a build — and doing so silently breaks three suites in a way that looks like a code defect. Someone will spend an hour on it.

  2. The tests already know about this class of hazard and guard one instance of it. They clear CARGO_INCREMENTAL deliberately, for precisely the reason that an inherited cargo env var changes what the child build produces. CARGO_TARGET_DIR is the same hazard, one variable over, unguarded.

  3. The failure mode is fast and wrong-looking. 0.04s and NotFound reads as "the artifact was never built" rather than "the artifact is somewhere else", which points the reader at the guest build script instead of at their own environment.

The fix

Either:

  • clear CARGO_TARGET_DIR for the child build alongside CARGO_INCREMENTAL, so the hard-coded read path is correct by construction; or
  • ask cargo where it put the artifact (--message-format=json and read filenames off the compiler-artifact record) instead of assuming <guest dir>/target/..., which removes the assumption rather than defending it.

The second is better — it makes the path a measurement rather than a constant that happens to be right — but the first is honest and small.

What a fix must prove

  • All three suites pass with CARGO_TARGET_DIR set to a scratch directory. This must go RED against today's code; if it passes, the harness is not actually exporting the variable to the child.
  • All three still pass with it unset (no regression).
  • Anti-vacuity: break the guest source so the rebuilt artifact genuinely differs, and confirm the byte-comparison still goes RED. Otherwise "read whatever cargo says it wrote" could degrade into comparing an artifact against itself.

Note on scope

timeline_guest_rebuild is the leg that closed the in-image half of de.h-dv.timeline's reproducibility claim, and ruby_guest_wire is its long-standing model. Both are load-bearing, which is the argument for making their artifact path measured rather than assumed.

Found while running the #220 lane's gates. Not a product defect — a **test-harness environment dependency** that makes three suites fail for a reason unrelated to what they grade, in the exact setup this project tells concurrent lanes to use. ## The failure `guest_globals`, `ruby_guest_wire` and `timeline_guest_rebuild` shell out to `cargo build` and then read the artifact back from: ``` <guest dir>/target/wasm32-unknown-unknown/release/*.wasm ``` An inherited `CARGO_TARGET_DIR` **redirects the child build** to that directory instead. The test then reads the hard-coded path, finds nothing, and fails with `NotFound` in about **0.04s**. All three pass once `CARGO_TARGET_DIR` is unset. ## Why this is worth fixing rather than remembering 1. **It is the documented workaround for lane contention.** Concurrent lanes share `target/`, so setting `CARGO_TARGET_DIR` is the natural way to isolate a build — and doing so silently breaks three suites in a way that looks like a code defect. Someone will spend an hour on it. 2. **The tests already know about this class of hazard and guard one instance of it.** They clear `CARGO_INCREMENTAL` deliberately, for precisely the reason that an inherited cargo env var changes what the child build produces. `CARGO_TARGET_DIR` is the same hazard, one variable over, unguarded. 3. **The failure mode is fast and wrong-looking.** 0.04s and `NotFound` reads as "the artifact was never built" rather than "the artifact is somewhere else", which points the reader at the guest build script instead of at their own environment. ## The fix Either: * **clear `CARGO_TARGET_DIR` for the child build** alongside `CARGO_INCREMENTAL`, so the hard-coded read path is correct by construction; or * **ask cargo where it put the artifact** (`--message-format=json` and read `filenames` off the `compiler-artifact` record) instead of assuming `<guest dir>/target/...`, which removes the assumption rather than defending it. The second is better — it makes the path a *measurement* rather than a constant that happens to be right — but the first is honest and small. ## What a fix must prove * All three suites pass **with `CARGO_TARGET_DIR` set to a scratch directory**. This must go RED against today's code; if it passes, the harness is not actually exporting the variable to the child. * All three still pass with it unset (no regression). * **Anti-vacuity**: break the guest source so the rebuilt artifact genuinely differs, and confirm the byte-comparison still goes RED. Otherwise "read whatever cargo says it wrote" could degrade into comparing an artifact against itself. ## Note on scope `timeline_guest_rebuild` is the leg that closed the in-image half of `de.h-dv.timeline`'s reproducibility claim, and `ruby_guest_wire` is its long-standing model. Both are load-bearing, which is the argument for making their artifact path measured rather than assumed.
Author
Member

Fixed in f9ddfaf by the package author — and the fix found the half of this issue I got wrong.

I reported the loud failure. The dangerous one is silent.

I described a single mode: the child build is redirected, the hard-coded read finds nothing, NotFound in 0.04s. That is real, and it is only what happens on a clean tree.

The measurement in the fix:

  • on a CLEAN tree the artifact is simply absent and the read panics — loud, and how #234 was reported;
  • on a tree that has EVER built this guest, the leftover is read instead. MEASURED: a 19,765-byte artifact from an earlier run — the child built elsewhere and the leftover was compared instead.

So under an inherited CARGO_TARGET_DIR the mutation goes GREEN on stale bytes. A rebuild test that compares against a leftover is not comparing anything: it is precisely the artifact-bound-to-a-constant defect this suite was written to remove, reintroduced by an environment variable.

That is strictly worse than the failure I filed, and my write-up would have led someone to fix only the visible half — teach the paths to follow CARGO_TARGET_DIR, and the silent mode survives untouched.

Why env_remove rather than following the variable

I suggested reading cargo's --message-format=json output so the path becomes a measurement instead of a constant. The author took the other option, and gave the reason that makes it the right one:

a test that reads whatever is on disk is not binding the artifact to its source.

Following the variable makes the path honest while leaving the comparison free to run against whatever that path happens to contain. Clearing it forces the child to build where the test reads, so the artifact under comparison is necessarily the one just produced. The two variables are now cleared together with one stated reason — reproduce build.sh's environment, not the ambient one — where before CARGO_INCREMENTAL was cleared and CARGO_TARGET_DIR was not.

The mutation was run in both worlds, which is what caught it

Removing .env_remove("CARGO_TARGET_DIR") and running under an inherited one: RED on a clean tree, GREEN on a tree holding a previous build. Running that mutation in only the first state would have confirmed the fix and missed the reason for it.

Closing. My original "what a fix must prove" list is superseded by that pair — the anti-vacuity arm I asked for was "break the guest source and confirm the comparison still goes RED", and the sharper version is "run the mutation on a tree that already holds an artifact."

Fixed in `f9ddfaf` by the package author — and **the fix found the half of this issue I got wrong.** ## I reported the loud failure. The dangerous one is silent. I described a single mode: the child build is redirected, the hard-coded read finds nothing, `NotFound` in 0.04s. That is real, and it is only what happens **on a clean tree**. The measurement in the fix: > * on a CLEAN tree the artifact is simply absent and the read panics — loud, and how #234 was reported; > * on a tree that has EVER built this guest, **the leftover is read instead. MEASURED: a 19,765-byte artifact from an earlier run** — the child built elsewhere and the leftover was compared instead. So under an inherited `CARGO_TARGET_DIR` the mutation goes **GREEN on stale bytes**. A rebuild test that compares against a leftover is not comparing anything: it is precisely the artifact-bound-to-a-constant defect this suite was written to remove, reintroduced by an environment variable. That is strictly worse than the failure I filed, and my write-up would have led someone to fix only the visible half — teach the paths to follow `CARGO_TARGET_DIR`, and the silent mode survives untouched. ## Why `env_remove` rather than following the variable I suggested reading cargo's `--message-format=json` output so the path becomes a measurement instead of a constant. The author took the other option, and gave the reason that makes it the right one: > a test that reads whatever is on disk is not binding the artifact to its source. Following the variable makes the *path* honest while leaving the *comparison* free to run against whatever that path happens to contain. Clearing it forces the child to build where the test reads, so the artifact under comparison is necessarily the one just produced. The two variables are now cleared together with one stated reason — reproduce `build.sh`'s environment, not the ambient one — where before `CARGO_INCREMENTAL` was cleared and `CARGO_TARGET_DIR` was not. ## The mutation was run in both worlds, which is what caught it Removing `.env_remove("CARGO_TARGET_DIR")` and running under an inherited one: **RED on a clean tree, GREEN on a tree holding a previous build.** Running that mutation in only the first state would have confirmed the fix and missed the reason for it. Closing. My original "what a fix must prove" list is superseded by that pair — the anti-vacuity arm I asked for was "break the guest source and confirm the comparison still goes RED", and the sharper version is "run the mutation on a tree that already holds an artifact."
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
h-dv/code-index#234
No description provided.