fix(build): stage BMIs through mcpp instead of an in-place shell copy (2026.7.30.1, fixes #311) - #312
Merged
Merged
Conversation
…#311) Windows-only symptom, three overlapping defects. 1. `rule cp_bmi` overwrote the destination IN PLACE (`powershell Copy-Item -Force` / `cp -f`). mcpp writes the staged std BMI path into compile_commands.json so clangd can resolve `import std;`, clangd memory-maps that very file, and Windows refuses to replace a file with an open user-mapped section — error 1224, reported as a bare "build failed". Staging now runs through `mcpp stage` (new internal subcommand, same shape as `mcpp dyndep`): skip when the destination is already equivalent, else temp-file + rename, else in-place, with retries, and a failure message that names the file and the likely holder. Never downgraded to a warning — a stale BMI turns into a confusing "module 'std' not found" or a silently mismatched link. The rule is shared with Windows runtime-DLL deployment, which had the same hazard against a program still running from a previous `mcpp run`. 2. `default_cache_root()` was a private copy of the home resolution, unchanged since v0.0.1: no %USERPROFILE% branch, no self-contained detection. On Windows PowerShell (no $HOME) the std BMI cache landed in the *current working directory* as `.mcpp-bmi/` while dep BMIs went to %USERPROFILE%\.mcpp\bmi — two roots for one cache, the second cwd-dependent, which is what made a re-stage a routine event. New leaf module `mcpp.home` is now the single resolver (config, stdmod, prepare's git cache and two more copies in doctor all route through it). 3. The staging rule carried no `restat`, so any re-stage recompiled everything that imports std even when the bytes were identical. Adding `restat = 1` on top of the no-write path fixes that — measured: touching the cache-side BMI now runs the stage edge alone, and the next build is "no work to do". Note for the record: aligning the destination's mtime with the source defeats restat and re-triggers the cascade, so a skipped stage touches no timestamps at all. Also: `FAILED: <target>` survives the ninja output filter (normalized to `failed: <target>`) — dropping it is why the report couldn't tell a staging failure from a compile error; `mcpp new` ignores `.mcpp/`; `mcpp doctor` points at a leftover `.mcpp-bmi/`. Design + plan: .agents/docs/2026-07-30-issue311-*.md Tests: unit test_home (4), test_build_stage (10), test_ninja_backend (+4), e2e 170 (no-cascade + cache root), e2e 171 (held destination, PowerShell MemoryMappedFile on Windows — reproduces #311 without needing clangd).
Bootstrap pin stays at 2026.7.29.1: it names an already-published mcpp and is bumped separately, after the release exists in xim-pkgindex.
…railing pad Two cosmetics found by driving a real staging failure end-to-end through ninja: the `FAILED: <target>` line ninja emits carries a trailing space, and `$out` is relative because ninja runs staging with cwd = the build directory — the reader of that diagnostic is the person who has to go unlock the file.
…ong reason Two holes in 171 as first written: a PowerShell holder that failed to start left the build trivially succeeding (silent pass), and `chmod 444` is toothless under root — which the container e2e job runs as. The holder now signals readiness through a sentinel and the test fails if it never appears, and the load-bearing assertion is inode+mtime+size invariance of the destination: 'nothing was written', not 'the write happened to succeed'.
…py through it Windows CI caught two things the Linux-only self-checks structurally could not. 1. `rule runtime_alias` was a SECOND `Copy-Item -Force`. PE has no soname symlink, so an alias is a copy of a freshly built DLL — the same hazard as BMI staging (a program still running from a previous `mcpp run` holds the old one). Its Windows branch now goes through `$mcpp stage` too; POSIX keeps `ln -s`, where the symlink is semantics and not merely how the file is written. The 'no Copy-Item anywhere' assertion is a no-op on Linux, which is why only the Windows job could find this. 2. Size-only equivalence was unsound for exactly those DLL payloads: PE section padding makes 'genuinely rebuilt, identical size' ordinary, so a stale DLL could survive in the build dir. Content comparison is unconditionally correct — a destination equal to the source needs no write — and it only runs when ninja has already decided the edge is dirty. It is now the default; `--verify size` / MCPP_STAGE_VERIFY=size stays for callers that know the source is fingerprint-scoped, and an unrecognized value falls back to the SAFE mode rather than the fast one. Also from CI: a read-only destination is replaceable on POSIX (rename rewrites the directory entry) but not on Windows, so that test now asserts both outcomes per platform instead of one; the e2e scripts unescape ninja node names (a Windows drive letter arrives as `C$:/Users/...`) and use BSD `stat -f` when GNU `stat -c` is absent (macOS).
`wait` on a job we just killed returns 143, and it sat as the last command of an `&&` list under `set -e` — so on Windows the script died the moment the mapping was released, before asserting anything about the build it had just run. Probed the shell semantics directly rather than guessing: the old form exits 143 on Linux too, it only never ran there because HOLDER is empty on POSIX.
Found while attributing an e2e 156 failure on this branch (NOT caused by the staging change — proven by diffing the generated build.ninja for that exact project: it differs only in an unused rule's text). `process.cppm::merged_environ` strips mcpp's private-glibc payload entries from an INHERITED LD_LIBRARY_PATH, and plan.cppm's comment states that guarantee. But merged_environ takes explicit `extra` overrides verbatim, and `env::prepend_path_list` — which composes exactly such an override for ninja and for run targets — appended the inherited value RAW. So the strip was bypassed precisely in the case it exists for: a nested `mcpp run` → `mcpp test` chain, where a payload tool patched against a different glibc then segfaults inside the dynamic linker before main (bare `__vdso_time`, then SIGSEGV). The predicate moves to mcpp.platform.env, next to path-list composition, since both halves of the guarantee need it; prepend_path_list now sanitizes only the inherited TAIL, so a payload dir the caller passed explicitly — the entry the sandbox binary actually needs — always survives. PATH is untouched. Why it looked like a regression: the two CI runs restored DIFFERENT sandbox caches (…-01baa227… vs …-0e74cc64…), so the payload version set differed and the latent bug only showed on one side. Before this fix, 156 was green only when the poisoned payload version happened to match what the tools expected.
…ing holder can't fail the build The Windows job's own repro exposed the gap: my holder maps the destination with FileShare.None, so `same_content` cannot even OPEN it → equivalence is undecidable → mcpp attempts the write → ERROR_SHARING_VIOLATION(32) → build failed. Which is precisely the #311 outcome, arrived at from the other side. Reading needs an open; SIZE comes from directory metadata and does not. So the verify mode is now per-edge rather than one global setting: std BMI / std.o / std.compat.* --verify size fingerprint-scoped: the cache dir and the build dir share the fp covering compiler identity, triple, stdlib, std source hash and dialect flags ⇒ equal size IS equivalence Windows DLL deploy / runtime_alias content not fp-scoped, and a rebuilt DLL keeps its size (PE sections are page-padded) So #311's actual path — clangd mapping the std BMI — now survives even an EXCLUSIVE lock, while a possibly-stale DLL is still gated byte-for-byte. Carried on a per-edge `$verify` ninja variable; the separating space lives in the rule's command string because ninja trims trailing whitespace in variable values. Verified on Linux with chmod 000 (the closest analogue of a read-denying holder): the fp-scoped edge skips and the build succeeds. Also: bootstrap pin 2026.7.29.1 → 2026.7.29.2. Unrelated to this work and pre-existing on main — the index no longer serves .1 (`available: 2026.7.29.2`), so the aarch64 fresh-install job fails on main too. .2 is released and indexed, and the guard's rule (both pins equal, never newer than the building version) still holds.
Sunrisepeak
force-pushed
the
fix/issue311-bmi-staging
branch
from
July 30, 2026 05:32
99a13c4 to
56ebace
Compare
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.
Fixes #311.
Windows-only symptom, three overlapping defects on one chain. Design + plan:
.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md,.agents/docs/2026-07-30-issue311-implementation-plan.md.What actually happens
mcpp writes the staged std BMI path into
compile_commands.json— deliberately,so clangd can resolve
import std;(flags.cppm:369-382says as much). clangdthen memory-maps that file. And
rule cp_bmioverwrote the same path in placevia
powershell Copy-Item -Force. Windows refuses to replace a file with an openuser-mapped section (error 1224), the edge fails, and the build reports a bare
error: build failed.POSIX never saw it: GNU
cp -funlinks and recreates when it cannot open thedestination, and POSIX allows overwriting a mmapped file anyway — so CI stayed
green while Windows users hit it 100% of the time with clangd running.
The report's "compile and link already succeeded" only holds in the incremental
case; cold builds are genuinely blocked, since the staged BMI is an implicit
input of every importer. What creates that incremental case is defect 2.
D1 — staging primitive
New internal subcommand
mcpp stage(same shape asmcpp dyndep), replacing theper-platform shell copy:
holder (clangd / editor indexer / antivirus / a program still running from a
previous
mcpp run)Never downgraded to a warning: a stale or missing BMI becomes either a confusing
module 'std' not foundor a silently mismatched link."Equivalent" is sound because the destination (
target/<triple>/<fp>) and thesource (
$MCPP_HOME/bmi/<fp>) share a fingerprint that already covers compileridentity, target triple, stdlib, std source hash and the standard/dialect flags.
The dep BMI cache has always worked this way (
bmi_cache.cppm: "Existingproject outputs are left untouched") — std staging was the only forced
overwrite in the tree, and that asymmetry was the defect.
The rule is shared with Windows runtime-DLL deployment, so overwriting a DLL a
running program still holds is fixed by the same change.
D2 — one BMI cache root
toolchain/stdmod.cppm::default_cache_root()was a private copy of the homeresolution, unchanged since v0.0.1 (
git log -L179,187→92f1335): noUSERPROFILEbranch, no self-contained detection. Consequences:MCPP_HOMEset$MCPP_HOME/bmixlings install~/.mcpp/bmi<root>/bin/mcpp)<root>/bmi~/.mcpp/bmi❌$HOME)%USERPROFILE%\.mcpp\bmi<cwd>\.mcpp-bmi❌.mcpp-bmiin the issue's log is that third fallback firing. Being cwd-relative,it also means "run from another directory ⇒ different cache root ⇒ std rebuilt ⇒
staging edge dirty while everything else is up to date" — exactly the state the
report describes.
New leaf module
mcpp.homeis the single resolver. Converged:config.cppm,stdmod.cppm,prepare.cppm's git cache, and two more copies indoctor.cppm— one of which (
self init --force) would wipe~/.mcppfor a self-containedinstall.
D3 — no more rebuild cascade, readable failures
restat = 1on the staging rule, on top of the no-write path. Measured on a demoproject: touching the cache-side BMI now runs the stage edge alone (
main.cppisnot recompiled) and the next build is
no work to do. Before, every importer ofimport stdrebuilt.Recorded in the code and the design doc because it cost a cycle to find: aligning
the destination's mtime with the source defeats restat and re-triggers the
cascade (ninja only spares outputs whose mtime the command did not change), so
a skipped stage touches no timestamps at all.
Also:
FAILED: <target>survives the output filter (normalized tofailed: <target>) — dropping it is half of why #311's output was unreadable;mcpp's own path joins the command-prefix set so the echoed command line is
filtered while its diagnostic is kept;
mcpp newignores.mcpp/;mcpp doctorpoints at a leftover.mcpp-bmi/.Upgrade impact
Windows users and self-contained installs get a one-time std module rebuild
(10–60 s) as the cache root moves. Leftover
.mcpp-bmi/is unused; doctor namesit, nothing deletes it automatically.
Tests
tests/unit/test_home.cpp(4) — resolution order, and the D2 gatedefault_cache_root() == mcpp::home::bmi_root()tests/unit/test_build_stage.cpp(10) — skip/copy decisions, "mtime nottouched at all", read-only destination via rename, failure message content
tests/unit/test_ninja_backend.cpp(+4) — rule text +restat,$mcppboundwith dyndep off, all four std edges + DLL deploy on the shared rule, filter
behaviour
tests/e2e/170_bmi_staging_no_cascade.sh— no cascade on re-stage, edgesettles, and the staging source is under
$MCPP_HOME/bmitests/e2e/171_bmi_staging_locked_dest.sh— on Windows a backgroundPowerShell holds a
MemoryMappedFileon the staged BMI, reproducing Bug: Windows 上 clangd 导致 mcpp build 假失败 #311without needing clangd; POSIX uses a read-only destination; both assert the
loud-failure path keeps its hint
Local: 40/40 unit test binaries; e2e 170, 171, 02, 05, 19, 21, 84, 164, 169, 22
all pass. Negative control: e2e 170 fails against the previously released binary.
Added during review: two things CI found that the Linux-only self-checks could not
runtime_aliaswas a secondCopy-Item -Force. PE has no soname symlink, soan alias is a copy of a freshly built DLL — the same hazard class as BMI staging
(a program still running from a previous
mcpp runholds the old one). ItsWindows branch now goes through
$mcpp stagetoo; POSIX keepsln -s, where thesymlink is semantics, not merely how the file is written. Note the shape of this
miss: my own "no
Copy-Itemanywhere" check is vacuously true on Linux, so onlythe Windows job could ever fail it.
Size-only equivalence was unsound for exactly those DLLs. PE section padding
makes "genuinely rebuilt, identical size" ordinary, so a stale DLL could survive
in the build dir. Content comparison is unconditionally correct — a destination
equal to the source needs no write — and it only runs when ninja has already
decided the edge is dirty. It is now the default;
--verify sizestays forcallers that know the source is fingerprint-scoped, and an unrecognized value
falls back to the SAFE mode rather than the fast one.
Plus: a read-only destination is replaceable on POSIX but not on Windows, so that
test asserts both outcomes per platform instead of one; the e2e scripts unescape
ninja node names (a Windows drive letter arrives as
C$:/Users/...) and fall backto BSD
stat -fon macOS; andwaiton the killed mmap holder returned 143,which under
set -ekilled test 171 before it asserted anything.Also included: a pre-existing env bug, deliberately not disguised as mine
e2e
156_nested_ld_library_pathfailed on this branch and passed on main, whichlooked like a regression. It is not, and the attribution is mechanical rather
than argued: diffing the generated
build.ninjafor that exact project shows thegraphs differ only in an unused rule's text. What actually differed was the CI
sandbox cache the two runs restored (
…-01baa227…vs…-0e74cc64…) — hence adifferent payload version set, which is what decides whether a latent bug shows.
The latent bug:
process.cppm::merged_environstrips mcpp's private-glibcentries from an inherited
LD_LIBRARY_PATH(plan.cppm documents that guarantee),but it takes explicit
extraoverrides verbatim — andenv::prepend_path_list,which composes exactly such an override for ninja and for run targets, appended
the inherited value raw. The strip was therefore bypassed precisely in the
case it exists for: a nested
mcpp run→mcpp testchain, where a payload toolpatched against a different glibc segfaults in the dynamic linker before main
(bare
__vdso_time, then SIGSEGV).Fix: the predicate moves to
mcpp.platform.envnext to path-list composition(both halves of the guarantee need it), and
prepend_path_listsanitizes onlythe inherited tail — a payload dir the caller passed explicitly, the entry
the sandbox binary actually needs, always survives. PATH untouched.
tests/unit/test_platform_env.cpplocks all four cases. Before this, 156 wasgreen only when the poisoned payload version happened to match what the tools
expected.
Round 2 of the Windows job: the verify mode has to be per-edge
My own repro is stricter than clangd:
MemoryMappedFile.CreateFromFile(path, FileMode.Open)takesFileShare.None, so the destination cannot even beopened — content comparison is undecidable, mcpp attempted the write and got
ERROR_SHARING_VIOLATION(32). That is #311's outcome reached from the other side,and it is the argument for splitting the mode per edge rather than picking one
globally:
std.o/std.compat.*--verify sizeruntime_aliasNet effect: #311's actual path — clangd mapping the std BMI — now survives even an
exclusive lock, while a possibly-stale DLL is still gated byte-for-byte.
Verified on Linux with
chmod 000as the closest analogue of a read-denyingholder. Carried on a per-edge
$verifyninja variable (the separating space livesin the rule's command string, because ninja trims trailing whitespace in variable
values).
One more pre-existing item, so CI can be green on its own merits
The aarch64
fresh installjob fails on main too: the bootstrap pin is2026.7.29.1and the index no longer serves it (available: 2026.7.29.2) — theroutine post-release pin bump that main never got. Bumped both sites to
2026.7.29.2(released and indexed). The guard's rule still holds: both pinsequal, never newer than the version being built.