diff --git a/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md b/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md new file mode 100644 index 00000000..d811699d --- /dev/null +++ b/.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md @@ -0,0 +1,1749 @@ +# Four issues, measured: #527 (workspace half), #529, #535, #537 + +> Status: analysis and design. No code changed. +> Baseline: `origin/main` @ `f4cb9a1`. The working tree at the time of writing +> was 33 commits behind and predates the `modules/` split, so every line anchor +> below was read from a detached worktree at `origin/main`, not from the +> checkout. +> Measured with: mcpp `2026.8.30.1`, gcc 16.1.0, ninja 1.12.1, Linux x86_64, +> 32 threads. +> Reproductions: six, all recorded below, all reduced to inputs that need +> nothing from the index. +> +> **Scope.** Host-native toolchain and sysroot support — #527 RFC 1, and the +> "make `toolchain = "system"` work" reading of #527 Bug 1 — is out of scope by +> instruction and by the maintainer's A1/A3 on the issue. What remains of those +> two items in this document is a diagnostic obligation, not a feature. #537 is +> likewise treated as a truthfulness defect in an existing guard, never as a +> proposal to support host linking. + +--- + +## 0. Verdicts + +| # | filed as | verdict | what it actually is | +|---|---|---|---| +| #529 | performance regression in `mcpp test` | **real defect, root-caused** | `prepare_build` rewrites `resolution.json` from a fresh object and drops `runtime.loader_tags` and `runtime.symbol_provision`, so the memoisation both post-link ELF passes were built around is destroyed before it can ever be read across invocations | +| #535 | host tool cannot start | **real defect** | the tool store publishes exactly one file, so a host tool's link unit can carry a `DT_NEEDED` that nothing in the store satisfies; the scratch tree that held the provider is then deleted | +| #537 | link-time and run-time providers diverge silently | **real defect** | the closure guard is satisfiability-based; when a SONAME resolves on both sides it is structurally unable to fire | +| #527 Bug 1 | `posix_spawnp('')` under `toolchain = "system"` | **real defect** | the resolved compiler path exists in `tc->binaryPath` and is not handed to the `build.mcpp` compile; the same manifest without a `build.mcpp` builds fine (measured), so this is an unfilled variable, not the host-support boundary | +| #527 Bug 2 | workspace `[build]` not inherited | **real defect, reproduced** | the two inheritance sites merge four keys and `buildConfig` is not among them | +| #527 RFC 2 | sync dialect flags into the `import std` BMI | **mechanism complete, silence is the defect** | `dialect_cxxflags` already reaches the std BMI prebuild; nothing tells the user that is where the flag belongs | +| #527 RFC 3 | `[workspace.*]` inheritance to prevent cross-package BMI drift | **premise incorrect; a larger defect underneath** | drift is impossible — the root's standard is imposed graph-wide — but a dependency's declared `standard` is therefore accepted and discarded with no diagnostic | +| #527 RFC 4 | aggregate `compile_commands.json` | **real gap, small** | per-member CDBs only; no root aggregate | +| #527 RFC 1 | host-native toolchain and sysroot | **out of scope** | answered A3; not revisited here | +| — | not filed | **real defect, found while reviewing D3** | the fast path's staleness sweep is rooted at the project directory, so a **new source file appearing in a `path` dependency** is invisible: `mcpp build` reports `Finished dev in 0.00s` and the module is never compiled (§2.5, D3a) | + +Nothing in this set is a usage error. The closest is #527 RFC 2, where the +correct key exists and the user did not find it — and the reason they did not +find it is that mcpp knows the answer and does not say it, which makes it a +diagnostic defect rather than a usage error. + +--- + +## 1. Two families, and the policy that decides refuse versus warn + +### 1.1 Two families + +Eight of the nine items reduce to two shapes. Naming them is what keeps the +individual fixes from being eight unrelated patches. + +**Family A — a record exists and the decision does not read it.** +`resolution.json` holds verdicts that the next invocation destroys (#529). A +dependency's `standard` is parsed into `Package::standard` and no consumer +reads it (RFC 3). `dialect_cxxflags` is the field that answers the question and +no diagnostic names it (RFC 2). The workspace root's `buildConfig` is loaded +and never merged (Bug 2). The refusal text for a system toolchain is compiled +into the binary and unreachable on the native path (Bug 1). + +**Family B — the run-time search path mixes what was declared with what this +machine happens to have, and the artifact never says which one won.** +`runtime_search_closure` (`src/build/plan.cppm:728`) already tags every +directory with a provenance — `Payload`, `Package`, `Artifact`, `SubosFarm`, +`HostDefault` (`modules/platform/src/runtime_search.cppm:45`). The closure +resolver returns the file that satisfied each `DT_NEEDED`. The two facts are +never joined, so "satisfied by a declared dependency" and "satisfied by +whatever is installed on this developer's machine" are the same green build +(#535, #537). #414 is the same shape, already fixed for one directory: the farm +outranked `$ORIGIN`, and the artifact linked one `libX11` and loaded another. + +### 1.2 The host-dependence policy, stated once + +Family B is a policy question as much as a mechanism, and every diagnostic +proposed below has to answer it the same way or the tool contradicts itself. +The rule, in the maintainer's terms: + +> **mcpp itself, and everything the mcpp ecosystem provides, depends on no +> host. Where something must be depended on, it arrives through the xlings +> system — mcpp-index or the xim package index.** +> +> **A user's own project may depend on the host in the LIBRARIES it links — +> its own `.so`, or one the machine provides. That choice is theirs to +> guarantee. It is not recommended, it is warned about, and it is not refused +> as long as the result builds and runs.** +> +> **The TOOLCHAIN is not that axis. mcpp builds only with toolchains it +> manages: `[toolchain] … = "system"` is refused, with `msvc@system` the single +> exception.** + +**The rule is per axis, and that is the correction this document needed.** An +earlier draft applied one boundary — "does it build and run" — to everything, +and concluded that `toolchain = "system"` should be warned about because it +does build and run (measured, §7). That conclusion was wrong, and the reason is +not that the measurement was wrong: + +> Everything mcpp promises — that `import std` is available, that the runtime +> closure is computable, that two machines and CI produce the same build — is a +> statement about **a compiler mcpp resolved and can identify**. A compiler +> picked off `PATH` makes every one of those unverifiable. The libraries a +> program links carry no such promise: they are the program's own dependencies, +> and the developer who chose them owns the artifact. + +So the compiler is part of mcpp's contract and is not the project's to take +from the host, while the libraries are exactly the project's to choose. Applying +that to every item here: + +| item | axis | severity | +|---|---|---| +| D15 `toolchain = "system"` | toolchain — mcpp's contract | **error**, `msvc@system` excepted | +| unsatisfiable closure (existing) | library, and provably cannot run | error — unchanged | +| D14 dialect flag not in the BMI | toolchain dialect, provably cannot build | error | +| D6 host tool with an unsatisfiable closure | library, cannot run | error, where the tool is built | +| D8 host or farm-supplied provider | library, runs | warning | +| D13 dependency declares a higher standard | neither; a graph-shape statement that still builds | warning | + +Within the library axis the old boundary still holds and is still the right +one: refused when the artifact provably cannot start, warned when it runs. + +**A warning about a host LIBRARY names the way back to the ecosystem.** Not "do +not do this", which the developer has already decided; the actionable sentence +is where the supported version of the same thing lives: + +``` +note: mcpp builds against no host library by default, so this artifact is not + reproducible on another machine and `mcpp pack` cannot bundle it. + The supported route is to declare the provider as a dependency from + mcpp-index. If mcpp-index does not carry it yet, contributing the + package is the path — see docs/… — and the dependency then resolves + the same way on every machine and in CI. +``` + +One vocabulary, used by D8 and the existing `allow_host_libs` note, so a +developer meets the same sentence wherever they touch a host library. Writing +it twice in two wordings is how the two drift. The toolchain refusal is +deliberately NOT that sentence: it is not advice about a trade-off, it is a +statement that the configuration is unsupported. + +--- + +## 2. #529 — `mcpp test` re-derives an unchanged answer on every invocation + +### 2.1 What was measured + +A two-member workspace, no index dependencies: `big` provides 101 module +interfaces that each `import std;`, `app` takes `big` as a `path` dependency +and has nine tests. All artifacts built and unchanged. + +``` +warm `mcpp build -p app` 0.695 s +warm `mcpp test -p app` 1.94 s (three runs: 1.932, 1.983, 1.946) +``` + +`MCPP_VERBOSE=1` decomposes the test invocation (the instrumentation added with +the earlier rule-E work, `src/build/ninja_backend.cppm:2643`): + +``` +build/stage: emit-ninja: 1ms +build/stage: compile-commands: 6ms +build/stage: ninja: 3ms +build/stage: loader-tags: 449ms ← drive 1 +build/stage: symbol-provision: 920ms ← drive 1 +build/stage: emit-ninja: 1ms +build/stage: compile-commands: 6ms +build/stage: ninja: 3ms ← drive 2: neither pass appears at all +``` + +**1.36 s of a 1.94 s invocation — 70 % — is two passes re-reading ten ELF +images totalling 115 MB to reach the answer they reached last time.** Ninja +compiles and links nothing; artifact mtimes are byte-identical before and +after. + +The cost scales with link units, measured on the same tree: + +| test binaries | loader-tags | symbol-provision | +|---|---|---| +| 3 | 151 ms | 307 ms | +| 9 | 449 ms | 912 ms | + +Per link unit it is ~50 ms and ~100 ms for an 11.5 MB image, and the work is a +full read of the image, so it scales with image size — that is, with the +module-interface surface statically linked into it. This is exactly the scaling +law the issue reports ("scales with the module-interface surface of the whole +dependency closure, including workspace `path` dependencies"), and it explains +every one of its side observations: no child processes (the passes are +in-process), nothing written but a handful of `.json` files (the records), and +a 2× spread between runs on the larger member (page-cache sensitivity of +reading hundreds of megabytes). + +**And the real developer loop is worse than the steady state.** The figures +above are `mcpp test` repeated. An edit-test loop alternates `mcpp build` and +`mcpp test`, and the two commands write **different plans into the same output +directory** — measured, `mcpp test`'s plan contains only the nine test +binaries and does not contain `bin/app` at all. Alternating them: + +``` + runtime-validate loader-tags symbol-provision wall +build -p app 146 ms 51 ms 101 ms 0.70 s +test -p app 1192 ms 462 ms 895 ms 3.15 s +build -p app 136 ms 49 ms 101 ms 0.70 s +test -p app 1172 ms 458 ms 925 ms 3.12 s +test -p app (absent) 440 ms 904 ms 1.95 s +``` + +Two facts, and the second one is the important one for the fix: + +- `loader-tags` and `symbol-provision` cost the same in **every** row, + including the repeat, because their record is wiped unconditionally (§2.2). +- `runtime-validate` costs 1.19 s only after a `build`, and **disappears on the + repeat**. Its record lives in a sidecar that survives `prepare_build`, so it + memoises correctly — until the other command prunes it (§2.5, D2). + +So the loop a user actually runs pays **2.55 s of the 3.15 s (81 %)** on +re-deriving answers that are already on disk. + +**Honest limit of this reproduction.** It accounts for 2.55 s, not for the +reporter's 15.6 s. The mechanism is proven and the scaling law matches; the +absolute figure on their tree is not reproduced here, and the fix below should +be validated against their workspace rather than assumed to close the whole +gap. + +### 2.2 Root cause + +Both passes were written with a read-back, and the comment on it is precise +(`src/build/runtime_validation.cppm:720-731`): + +> Re-parsing every image on every drive is what made the loader-tag check cost +> 158.7 s of a 190 s hot run, so an artifact whose stat did not move is skipped +> here too. But skipping it must not DROP its verdict [...] the record would +> shrink to "whatever moved last". + +The read-back reads `/resolution.json`. And +`prepare_build` writes that file from a **fresh** `nlohmann::json` object +(`src/build/prepare.cppm:9283`, written at `:9420-9433`) on every invocation. +The fresh object carries `binding`, `search`, `link_intent`, `validation` and +friends; it carries neither `runtime.loader_tags` nor +`runtime.symbol_provision`. Every invocation therefore begins by deleting the +memo that the invocation's own backend is about to look for. + +### 2.3 The control that proves it + +Two observations, both direct: + +``` +$ python3 -c "keys of runtime{} in resolution.json" # after `mcpp test` +['binding','cxx_runtime_by_role','link_intent','loader_tags','search', + 'symbol_provision','validation'] + +$ mcpp build -p app --configure-only # prepare_build only +$ python3 -c "same" +['binding','cxx_runtime_by_role','link_intent','search','validation'] + loader_tags: absent symbol_provision: absent +``` + +And the positive control is in the verbose trace above: within one invocation, +**drive 2 pays nothing** — neither pass prints a line, so each cost under 1 ms +— because drive 1 wrote the record and nothing wiped it in between. The +memoisation works. It is only ever destroyed across the process boundary. + +The third control is `validate_changed_artifacts`, the one post-link pass that +does *not* record into `resolution.json`: it keeps its own sidecar, +`.mcpp-runtime-verdicts.json` (`src/build/runtime_validation.cppm:149`), keyed +by `contract_hash`. That file survives `prepare_build`, and on a **repeated** +`mcpp test` the pass costs under 1 ms while its two neighbours cost 1.36 s. +The difference between the fast pass and the slow ones is the file they write +to. The same control also shows the limit of the sidecar as it stands: after an +intervening `mcpp build` it costs 1.19 s, because the two commands prune each +other's entries (§2.5, D2). + +### 2.4 Two amplifiers + +Neither is the root cause, and both make it worse in exactly the workspace +shape the issue reports. + +**`mcpp test` has no fast path.** `cmd_build` short-circuits a fully cached +build through `try_fast_build`, which skips `prepare_build` entirely +(`src/cli/cmd_build.cppm:134-144`). `run_tests` calls `prepare_build` +unconditionally (`src/build/execute.cppm:1349`). This alone is most of the +0.695 s vs 1.94 s gap that is not the two ELF passes. + +**`-p` disables the fast path that does exist.** The guard requires +`ov.package_filter.empty()`. Inside a workspace, every per-member command is +spelled `-p `, so no workspace member has ever taken the fast path, +for `build` or for `test`. A `--workspace` sweep pays the full +`prepare_build` plus the full ELF re-derivation once per member. + +### 2.5 Design + +**D1 — the post-link verdict records move out of `resolution.json` into the +sidecar that already survives.** `loader_tags` and `symbol_provision` join +`.mcpp-runtime-verdicts.json`, under the same `contract_hash` invalidation the +runtime verdicts already use. `resolution.json` goes back to being what its +writer treats it as: a record regenerated per configure, published for `mcpp +why runtime`, CI and `doctor`. + +**This is not a new pattern; it is the pattern the file already uses, applied +to the two passes that skipped it.** `sync_resolution_verdict` +(`src/build/runtime_validation.cppm:328`) reads the sidecar and projects the +runtime verdicts into `resolution.json`'s `runtime` block after the link. So +the architecture is already "sidecar is authoritative, `resolution.json` is the +published copy, synced post-link" — `loader_tags` and `symbol_provision` are +simply the two records that were written directly into the published copy +instead, which is why they are the two that get erased. + +Finding this also rules out the alternative that looked simpler: teaching +`prepare_build` to preserve the two keys when it rewrites. That would make +`resolution.json` authoritative for two records and a published copy for a +third, in the same `runtime` object. Fewer lines, worse structure, and the next +post-link record would have to guess which convention to follow. + +**D1a — the invalidation key must cover what the verdict actually depends on, +and today it does not.** This is the correctness half of D1, and skipping it +would trade a slow correct answer for a fast stale one. A memoised +`loader_tags` / `symbol_provision` verdict is a function of more than the +artifact's stat: + +- **The SubOS farm.** `/lib` is a symlink view rewritten by every + `xlings install`, and it is in the artifact's runtime search path. Installing + a package can change which file satisfies a `DT_NEEDED` without touching the + artifact. Today the verdict is re-derived every run and therefore correct by + accident. `try_fast_build` already treats `/.xlings.json`'s mtime as + the farm's version stamp (`src/build/execute.cppm:893-895`); the record must + use the same stamp. +- **The policy inputs that are not in the fingerprint.** `MCPP_ALLOW_HOST_LIBS` + is read from the environment at check time (`host_libs_allowed`, + `src/build/runtime_validation.cppm:194-198`) and does not enter the output + directory's fingerprint, so it can flip the verdict with every input file + unchanged. + +Both fold into the sidecar's existing `contract_hash` slot as additional key +material. The criterion for this is in §2.6, step 4 — and it is a criterion +that only exists because the memoisation is being made durable; nobody needed +it while the answer was recomputed every time. + +**D2 — an artifact leaves the record when it leaves the disk, not when it +leaves the current command's plan.** D1 alone is not sufficient, and the +measurement in §2.1 is why. `mcpp build` and `mcpp test` share one output +directory and have **different link-unit sets** — verified against the emitted +graph: the test plan's `cxx_link` edges are the nine test binaries and +`bin/app` does not appear in it at all. `validate_changed_artifacts` prunes +every entry whose key is absent from the current plan +(`src/build/runtime_validation.cppm:429-438`), so each command deletes the +other's verdicts, and the pass that memoises correctly for a repeated command +still costs 1.19 s in the alternating loop that a developer actually runs. + +Moving `loader_tags` and `symbol_provision` into the same file without changing +this would inherit the same behaviour. The pruning is there to keep the file +from growing without bound, which is a real concern; the correct predicate for +it is **the artifact no longer exists on disk**, not "the current plan does not +mention it". Under that predicate the record is a property of the output +directory — which is what it physically is — rather than of whichever command +last ran against it. + +This is the same collision that `is_plain_build_graph` (#407) had to solve for +`build.ninja`: `mcpp test` and `mcpp build --configure-only` write their plan +into the same file as a plain build, and the file needed a discriminator. The +verdict record has the identical collision and does not have one. + +**D3 — extend the fast path to workspace members, but not before the hole +below is closed.** The guard's purpose is to refuse the fast path when an +override would be silently ignored; `package_filter` is not such an override — +it selects which project directory is being built, which the fast path can +honour by resolving the member directory first and keying the freshness check +on that directory's `build.ninja`. + +**The precondition is a defect found while reviewing this proposal, and it is +not in any of the four issues.** `sources_newer_than` sweeps only +`projectRoot/src/**/*`, `projectRoot/build.mcpp`, the resource scripts, and +`glob_inputs_stale(projectRoot)` — all rooted at the project being built +(`src/build/execute.cppm:659-712`). A `path` dependency's sources are outside +all four. Content edits are still caught, but not by the sweep: ninja rebuilds +the dependency object and relinks, and `artifact_snapshot_unchanged` then +abandons the fast path after the fact (`:917-919`). What nothing catches is a +**new file appearing** in a `path` dependency — the #359 shape ("a GLOB input +changes without any existing file's mtime changing"), whose fix was bounded to +the project root. Measured on a single-package project with one `path` +dependency: + +``` +$ printf 'export module dep.third;\nexport int three(){return 3;}\n' > ../dep/src/third.cppm +$ mcpp build + Finished dev in 0.00s +$ find . ../dep -name 'dep.third*' +(nothing — the module was never compiled, and the build reported success) +``` + +Today this is narrow: it needs a single-package project with a `path` +dependency. **D3 would make it the normal case**, because members of a +workspace depend on each other by `path` and every workspace command is spelled +`-p`. So D3 is conditioned on the sweep first covering every `path` dependency's +source root and manifest — and that is worth doing on its own merits, before +and independently of D3. + +**D3a — close the sweep hole regardless of D3.** `sources_newer_than` takes the +set of directories the plan actually reads from, not one project root. The +resolved package list is available at plan time and is already recorded; the +fast path needs the same list, which means recording the dependency source +roots in `.build_cache` alongside the output directory it already stores. + +**D4 — a fast path for `mcpp test`.** Lower priority, and it should be built on +top of D1–D3 rather than instead of them: with the two ELF passes memoised and +`prepare_build` skippable, the remaining per-invocation cost is small enough +that the value of a separate test fast path should be re-measured before it is +designed. + +### 2.6 Criterion + +The assertion must be on **elapsed time attributable to the two stages, with a +denominator**, and it must be taken across two separate process invocations, +because a single invocation already passes today: + +1. Build a fixture with a known number of link units *N*, all warm. +2. Run `mcpp test` twice. On the second run, assert that + `build/stage: loader-tags` and `build/stage: symbol-provision` are **absent + from the verbose output** (each below the 1 ms print threshold), and assert + that `N` entries are present in the record — the denominator, so that "the + pass was skipped" cannot be satisfied by "the pass had nothing to look at". +3. Touch exactly one source belonging to exactly one link unit. Assert the + record still has `N` entries and that the changed one has a fresh verdict. + This is the step that catches the cheap wrong fix, in which skipping + unchanged artifacts shrinks the record to what was relinked. +4. **Alternate `mcpp build` and `mcpp test`**, and assert on the second `test` + that all three stages are absent and the record holds *N + M* entries — the + union of both plans, not either one. Without this step D1 passes and D2 does + not exist, and the loop a developer runs keeps paying (§2.1). +5. **Staleness, which the memoisation newly makes possible.** With everything + warm, change the SubOS farm (an `xlings install` of a package providing a + SONAME already in the closure) without touching any project file, and assert + the verdict is recomputed. Then set `MCPP_ALLOW_HOST_LIBS=1` on an otherwise + identical invocation and assert the same. Both must fail against a + stat-only key, which is what makes them a criterion for D1a rather than a + restatement of steps 2–4. + +Step 3 is the same trap the earlier rule-E work recorded: a purely no-op +regression test cannot distinguish a correct memoisation from a record that has +quietly become "what moved last". Step 4 is the trap this review found: a +memoisation can be correct for a repeated command and worthless for the pair of +commands anyone actually alternates. + +**Separate criterion for D3a**, because it is a different defect: add a new +source file to a `path` dependency, changing nothing else, and assert the file +is compiled and appears in the dependency's archive. Assert also that the +*count* of objects in that archive went up by one — "it rebuilt" is satisfied +by a full rebuild that happens to be triggered for another reason. + +--- + +## 3. #535 — a host tool cannot carry a runtime closure + +### 3.1 What the store publishes + +`src/build/prepare.cppm:6699-6731`. After the sub-build succeeds, the +provisioning pass copies **one file**: the tool executable, from the sub-build's +output directory into `/bin/`. It then removes the sub-build scratch +tree. If the tool's link unit has a `DT_NEEDED` on a `kind = "shared"` +dependency's output, the file that satisfied it lived in the scratch tree and +no longer exists anywhere the tool will look. + +This is not a missing copy statement. **A store entry whose published form is a +single file cannot represent a program that is more than one file**, and the +entry validator (`tool_store::entry_valid`) accordingly reports such an entry +as valid. The issue's report and this reading agree exactly: `$ORIGIN` is in the +tool's `RPATH` and the directory it names is empty. + +### 3.2 The second half, which is the more dangerous one + +The tool's `RPATH` ends with the SubOS library view. On a machine where any +package has installed a library with the same SONAME, the missing dependency is +satisfied — by a different build of a different version — and the tool starts. +The issue measured precisely this: `libexpat.so.1` resolved to `xim:expat` +2.6.2 rather than to the declared `compat.expat` 2.7.1, and the build was green +on the developer's machine and red on a clean runner. + +That is Family B, and it is the same sentence as #537: the artifact was linked +against one library and loaded another, and nothing said so. + +### 3.3 Design + +**D5 — a host tool's store entry is a directory, not a file.** Publish the +executable together with the runtime files it needs. Both halves already exist +and should be reused rather than re-derived: `mcpp.build.stage` implements the +publish discipline (content compare, write out of place, rename), and +`plan.runtimeDeployFiles` / `LinkIntent::deployFiles` is already the answer to +"which files must sit beside this artifact for it to run", emitted as +`stage_file` edges at `src/build/ninja_backend.cppm:2177-2183` — the mechanism +that puts DLLs next to a PE executable, which has no `RPATH` at all. A host +tool's store entry needs the same list. The store key must fold in the staged +set, so an entry published by an older engine that staged nothing is a miss +rather than a silently incomplete hit. + +Two constraints on how it is staged, both of which a "copy the `.so` next to the +exe" reading would get wrong: + +- **Preserve the sub-build's relative layout, do not flatten to `$ORIGIN`.** + The tool's `RPATH` was written by the linker against the sub-build's + directory shape. In the reported case that shape happened to make `$ORIGIN` + sufficient, but a target laid out as `bin/` + `lib/` carries + `$ORIGIN/../lib`, and flattening would leave a correct-looking entry that + still does not resolve. Publishing the relative paths the sub-build produced + is layout-independent; "beside the exe" is not. +- **Stage the transitive closure, not the direct dependencies.** A staged `.so` + has its own `DT_NEEDED`. Stopping at depth one produces exactly the failure + this item is about, one level down, and D6 is what would catch it. + +**D6 — the tool's own closure is validated before the entry is published.** The +sub-build already runs the same `NinjaBackend::build` as any other build, and +therefore already runs the closure validator — but it validates against the +scratch tree, where the provider still exists. The check that matters is +against the **published** entry. One re-validation after the rename, with the +store entry's directory as the artifact's own directory, converts "the tool +will fail to start when the consumer runs it" into a build error at the point +where the tool is built. + +**D7 — decide, explicitly, whether the SubOS farm belongs in a host tool's +`RPATH`.** The issue proposes removing it. That is the right instinct and the +wrong altitude: the farm is what makes `-lGL` work at all, and removing it from +one artifact class trades a silent wrong answer for a hard failure in a +different set of cases. D8 below is the general form, and it makes the farm +safe to keep by making its contribution visible. If, after D8, a host tool +still has no legitimate use for the farm, removing it becomes a small, +separately justified change rather than a workaround. + +### 3.4 Criterion + +Two link units in one fixture: a host tool with a `kind = "shared"` dependency, +and the same tool with a `kind = "lib"` dependency. Assert on the **store entry +contents**, not on whether the tool ran: + +- the shared case publishes a store entry containing the executable *and* the + `.so`, and `ldd`-equivalent resolution of the published executable names the + staged file and no path under the SubOS farm; +- the static case publishes an entry containing exactly one file — the + denominator, so that "we stage everything" cannot pass by staging nothing; +- with the provider removed from the SubOS entirely, both cases still behave + the same. This is the step that fails today, and it is the one that a + developer machine cannot run without being made clean first. + +--- + +## 4. #537 — the guard is satisfiability-based and therefore cannot fire + +The analysis is in `.agents/docs/2026-08-30-issues-532-533-534-analysis.md` §3 +and is not repeated. What follows is the design it deferred. + +### 4.1 The predicate is already available + +The issue offered two directions and judged the cheaper one (warn on a host +search path in `ldflags`) as not resting on facts, and the better one (compare +what the linker resolved against what the loader will resolve) as requiring +information from both sides. The second is closer to hand than it appeared: + +- `runtime_search_closure` returns an **ordered, provenance-tagged** directory + list (`src/build/plan.cppm:728`, `modules/platform/src/runtime_search.cppm:45`). +- `resolve_runtime_closure` returns, per `DT_NEEDED`, the **file** that + satisfied it (`src/runtime/elf.cppm:67`, `objects`). +- `carries_foreign_link_inputs` already recognises all three spellings of a + link-time search path — `-Ldir`, `-Wl,-Ldir`, `-Wl,--library-path`, plus the + MSVC form (`src/build/linkage_form.cppm:186-198`). + +Joining the first two gives the provenance of every runtime provider. That is +the whole predicate. + +### 4.2 Design + +**D8 — record, per `DT_NEEDED`, the provenance of the directory that satisfied +it, and report the two cases that provenance makes provable.** + +*Case 1 — satisfied by the farm, declared by nobody.* A SONAME whose runtime +provider has `Origin::SubosFarm` and which no resolved dependency in the graph +provides is, by construction, "supplied by this machine". Report it as +degraded, naming the SONAME, the file, and the fact that a clean environment +will not have it. This is the one rule that covers #535's silent wrong +`libexpat` and #537's silent wrong `libgbm`. + +**`Origin::SubosFarm` ALONE IS NOT THE PREDICATE, and taking it as one would +make this rule fire on correct builds.** The farm is a symlink view *of +installed packages*, verified: + +``` +$ ls -l ~/.mcpp/registry/subos/default/lib/ +crt1.o -> /home/…/.mcpp/registry/data/xpkgs/xim-x-glibc/2.44/lib64/crt1.o +``` + +A declared dependency's library is therefore routinely reached *through* the +farm rather than through a `Package`-origin directory. #532's own measurement +is the counter-example that matters: declaring four packages resolves an +eleven-entry closure whose `libexpat`, `libffi` and `libGLdispatch` no manifest +names, and all of those arrive by way of the farm while being perfectly +legitimate transitive contents of declared packages. A rule keyed on the origin +tag would report every one of them. + +The predicate is one step further: **canonicalise the resolved file through the +symlink, read the owning `xim-x-/` segment, and ask whether that +package is in the resolved graph.** Farm-origin plus an owner in the graph is +correct and silent; farm-origin plus an owner outside it — or a farm entry that +is not a symlink into the store at all — is the reportable case. This is the +difference between a rule that means "declared" and a rule that means +"reachable", and it is the whole content of D8. + +*Case 2 — a link-time search path that is not in the runtime closure at all.* +The set of `-L` directories, minus the runtime search closure, is exactly the +set of directories that can contribute ABI and can never contribute a runtime +provider. If a `DT_NEEDED` was produced by a `-l` whose only possible source is +in that set, then the link-time provider and the run-time provider are +different files as a matter of construction — not as a guess. Report it, name +both files, and name the flag line that caused it. + +Case 2 subsumes the cheap direction the issue proposed and is strictly better +than it: it is silent when the user's `-L` names a directory that is also on +the runtime path (where nothing is wrong), and it fires with two file paths +rather than a heuristic when it is. + +**Severity and wording.** Degraded, promoted to an error by `--strict`, per the +§1.2 rule: the artifact builds and runs, so the developer's choice stands. The +sentence that makes it actionable is not "do not do this" but the route back +into the ecosystem — the vocabulary in §1.2, used verbatim here so a developer +meets one sentence and not three. Case 1 will fire on real, currently-green +graphics builds; that is the intended reach of the rule and the reason it does +not refuse. + +**D9 — the provenance goes into the published record, and into the +distribution statement.** `resolution.json` already publishes `search` with +provenance and `requirements`/`providers`; the resolved provider's origin per +SONAME belongs beside them. This makes "this artifact's closure is satisfied +entirely by declared packages" an assertion a CI job can make, which is the +third argument in the issue — today a `--list` on a developer machine cannot +separate "declared" from "installed here". + +**The second consumer is the one that makes this more than a warning.** +`mcpp.pack.host_requirements` exists to answer "what must the TARGET machine +supply", precisely because a vendor driver cannot be bundled; its header states +that two consumers must produce the same list from the same plan — `mcpp pack` +writes it beside the artifact and `mcpp publish` projects it into an xpkg +descriptor's `[runtime].requirements` — and that "deriving it twice is how the +two drift". A host-supplied or farm-supplied provider **is** a host +requirement. So D9 must feed that existing list rather than stand up a parallel +one, and the payoff is concrete: a project that opts into a host library gets a +warning at build time and a truthful `host requirements` entry at pack time, +instead of a bundle that quietly omits the library and fails on the user's +machine. That is the §1.2 policy made operational at the distribution layer — +the developer may depend on the host, and what they must then guarantee is +written down for them rather than left implicit. + +### 4.3 Criterion + +- A fixture linking a library present **only** in the SubOS: Case 1 fires, and + the record names `subos_farm` as the provider origin. +- The same fixture after the library is declared as a dependency: Case 1 is + silent, and the record names `package`. Both halves are needed; the first + alone is satisfied by a rule that always fires. +- A fixture with `-L` naming a directory that *is* on the runtime path: Case 2 + is silent. This is the assertion that keeps Case 2 from degenerating into the + path-substring heuristic it replaced. + +--- + +## 5. #527 Bug 2 and RFC 3 — workspace configuration inheritance + +### 5.1 What is inherited today + +Two sites, one for "the command was issued at the workspace root and `-p` +selected a member" (`src/build/prepare.cppm:1119-1138`) and one for "the +command was issued inside a member directory" (`:1153-1163`). Both merge four +things: + +| key | discipline | +|---|---| +| `[workspace.dependencies]` | **explicit opt-in** — `x.workspace = true` per dependency | +| `[toolchain]` | implicit, member wins if it declares any | +| `[target.]` | implicit, per triple, member wins per triple | +| `[indices]` | implicit, member wins if it declares any | + +Four keys, three disciplines, and the disciplines are not stated anywhere. That +is the actual subject of RFC 3, and it matters more than the list of keys: the +next key added will pick a discipline by whichever neighbour its author read. + +### 5.2 What is not inherited, measured + +Reproduction: a rooted workspace whose root declares +`[build] cxxflags = ["-DFROM_WORKSPACE_ROOT=1"]`, and a member whose source +`#error`s if the macro is absent. + +``` +$ mcpp build -p child + Compiling child v0.1.0 (.) +error: build failed +failed: obj/main.cpp.ddi + child/src/main.cpp:2:2: error: #error "workspace [build] did not reach the member" +``` + +Bug 2 is real and reproduces in one command. Beyond `buildConfig`, neither site +merges `package` metadata (`standard`, `version`, `license`, `edition`), +`profiles`, `featuresMap`, `resources`, `runtimeConfig`, `xlings`, or +`conditionalConfigs`. + +Two further measurements about the workspace model, for completeness: + +- **Each member has its own `target/` and its own `compile_commands.json`.** + The `WorkspaceConfig` doc comment (`modules/manifest/src/types.cppm:919-926`) + states that members "share a unified lock file, target directory". Measured, + they do not share a target directory. A comment that describes a property the + code does not have is a promise, and this one should be corrected in the same + change that settles the inheritance model. +- **A rooted workspace's bare `mcpp build` builds the root package only** and + silently ignores every member, while a virtual workspace's bare `mcpp build` + fans out over all of them (`src/cli/cmd_build.cppm:40-42`). This matches + cargo's default-members semantics and is a defensible decision, not a defect + — but it is undocumented, and the divergence between the two workspace forms + is exactly the kind of thing a `[workspace]` specification exists to state. + +### 5.3 RFC 3's stated motivation does not hold + +The RFC's premise is that a member at `-std=c++23` importing a member's C++26 +BMI produces a hard compiler refusal, and that `[workspace.package] standard` +prevents it. Measured, that drift cannot occur: + +``` +libm/mcpp.toml standard = 26 (kind = "lib") +appm/mcpp.toml standard = 23 ([dependencies] libm = { path = "../libm" }) + +$ mcpp build -p appm + Finished dev [unoptimized + debuginfo] in 0.08s + +$ compile_commands.json +main.cpp ['-std=c++23'] +libm.cppm ['-std=c++23'] ← the dependency's declared 26 is discarded +``` + +The standard is graph-global, taken from the root package, and this is correct: +cross-standard BMIs are hard-incompatible, so a single value per graph is +physics rather than a simplification. It is stated as such in +`.agents/docs/2026-07-31-cpp20-standard-support-design.md` §4.3, whose only +reader is `src/build/plan.cppm:1079`. + +**The defect underneath is the opposite of the one reported.** A package +declares `standard = 26`, mcpp parses it, and no decision reads it. A package +that declares 26 because it *requires* 26 is compiled at whatever the consumer +says, and fails — if it fails at all — with a compiler error inside a +dependency's translation unit that names neither package nor the mechanism. +This is Family A in its purest form: the answer is parsed, and it is not wired +to a decision. + +### 5.4 The precondition was already specified, and it is the same one + +`2026-07-31-cpp20-standard-support-design.md` §9-Q3 declined to add a +dependency floor check and recorded exactly why: + +> The default and an explicit declaration are indistinguishable — +> `xpkg.cppm:1070` and the toml parser both write `"c++23"` when the key is +> absent, so the check would judge the entire mcpp-index ecosystem as +> "requires C++23". + +Verified on `origin/main`: `modules/manifest/src/xpkg.cppm:1173` assigns +`"c++23"` unconditionally, and `modules/manifest/src/toml.cppm:294` leaves the +struct default `"c++23"` in place when the key is absent. + +**The same indistinguishability blocks RFC 3 itself, and this is the finding +that connects the two.** Inheritance means "use the workspace value when the +member did not say". Today "the member did not say" and "the member said +c++23" are the same bytes. `[workspace.package] standard = 26` is therefore not +implementable — it would either silently override a member that deliberately +pinned 23, or silently do nothing. + +So the precondition §9-Q3 wrote down for a future floor check is the *same* +precondition RFC 3 needs, and doing it once serves both: + +> the field goes on `Manifest::package` as `std::optional`, and +> **both** parse paths fill it, or it is the same decision derived in two +> places again. + +### 5.5 Design + +**D10 — declaredness before inheritance.** Make every workspace-inheritable +key optional-typed at the manifest layer, filled by both parse paths, with the +default applied at one point after inheritance rather than at parse time. For +`standard` this is the field §9-Q3 already specified. For `BuildConfig` the +scalar fields with meaningful defaults (`optLevel`, `cStandard`, `linkage`, +`bmiSchedule`, `defaultProfile`) need the same treatment; the vector fields do +not, because empty is unambiguous for them. + +This is the load-bearing change. Every other item in this section is +mechanical once it exists, and none of them is correct without it. + +**D11 — one stated merge discipline, with named exceptions.** The default is +**the member wins when it declared the key; otherwise the workspace value +applies**, which is what `toolchain`, `[target.*]` and `[indices]` already do +and what a reader will expect from all four. `[workspace.dependencies]` +keeps its explicit `x.workspace = true` opt-in, and the reason is worth writing +down rather than leaving as an accident: a dependency is an edge in the +resolution graph, and inheriting edges implicitly would change what a member +resolves without the member's manifest mentioning it. Vectors +(`cxxflags`, `ldflags`, `dialect_cxxflags`) **append**, workspace first, so a +member can add without having to restate; a member that needs to *not* have a +workspace flag is a signal that the flag was declared at the wrong altitude. + +**Not every `[build]` key should be inheritable, and the exclusions must be +named rather than left to whoever writes the merge.** `allow_host_libs` is a +policy escape hatch that disables a correctness gate; a workspace root that +sets it once would silently disable that gate for every member, including +members added later by someone who never read the root manifest. The same +argument applies to any future key whose effect is "turn a refusal off". +Inheritable keys are those that describe *how to build*; keys that describe +*which safety check not to run* stay per-package, where the person turning them +off is the person who owns the artifact. + +**D12 — `[workspace.package]`, `[workspace.build]`, `[workspace.target.]`.** +With D10 and D11 in place these are three table names and one merge function, +not three features. `WorkspaceConfig` currently holds `members`, `exclude`, +`dependencies` and `present` (`modules/manifest/src/types.cppm:927`); the three +new tables are the same shape as the member-side tables they mirror. Both +inheritance sites must call the one merge function — today they are two copies +of the same four merges, and a fifth key added to one of them is a defect that +compiles. + +**D13 — with declaredness available, add the floor check §9-Q3 deferred.** A +dependency that *declared* a standard higher than the resolved graph standard +is reported before compiling, naming both packages, the two values, and the fix +(raise the root's `standard`, or `[workspace.package] standard`). A dependency +that did not declare one is silent, which is the whole point of D10. This +retires the "diagnostic enhancement" stopgap in the cpp20 design doc's §4.3 — appending a hint to a +failing dependency compile — with the check that stopgap was standing in for. + +**It ships degraded first, not as an error, and the reason is not caution for +its own sake.** The condition is not a proven failure: a package declaring +`standard = 26` compiles perfectly well at 23 whenever it happens not to use a +C++26 construct, and today that is a working, green configuration for anyone +who wrote the key aspirationally. Making it an error on first release converts +green builds to red with no defect behind them. Degraded, promoted by +`--strict`, matches how the closure rules and `symbol_provision` were rolled +out. This differs from D14, where the compile provably cannot succeed, and that +difference is exactly why the two carry different severities — and it is the +§1.2 rule applied: it builds and runs, so it is warned about. + +**D13a — and the declaredness bit is not sufficient for index packages. This is +the review's sharpest finding, and it is measurable.** §9-Q3's trap was stated +as a parser problem: both parse paths write `"c++23"` when the key is absent, +so absent and declared are indistinguishable. D10 fixes that for `mcpp.toml`. +It does **not** fix it for the index, because index descriptors declare the +value explicitly. Counted over the local registry (2727 descriptors, 774 with +an mcpp segment): + +``` + 678 language = "c++23" + 60 language = "c++23" + 36 language = "c++20" + 8 language = "c++23" + ──── + 782 declarations — every descriptor that has an mcpp segment declares one + + 756 of those 774 also declare `import_std = false` +``` + +`language` is the descriptor key that feeds `Package::standard` +(`modules/manifest/src/xpkg.cppm:1202-1203`). So **every package in the index +"declares" a C++ standard**, and for the 98 % that are C libraries with +`import_std = false` the declaration is boilerplate rather than a requirement. +A floor check keyed on declaredness would, for a root at c++20, fire against +essentially the entire index — which is precisely the outcome §9-Q3 refused, +arrived at through a different door. + +So D13 is scoped to **manifests the project author controls**: the root +package, workspace members, and `path` dependencies. There, a declared +`standard` was typed by the person reading the diagnostic. Index packages are +out of scope until the ecosystem has a key that means "requires at least", +which is a descriptor-schema change gated on the index floor and is not part of +this work. Recording the boundary matters more than the check: the check +without it is a mass false positive, and the shape of that false positive is +invisible from inside mcpp's own repository. + +### 5.6 Criterion + +- A member that declares nothing inherits `[workspace.build] cxxflags`; a + member that declares its own `cxxflags` gets both, workspace first. Assert on + the **compile command in `compile_commands.json`**, not on build success. +- A member that declares `standard = 23` under a workspace declaring 26 is + compiled at 23 — the assertion that fails if declaredness was faked with a + sentinel value. +- A member that declares nothing under a workspace declaring 26 is compiled at + 26. +- The floor check fires for a dependency that **declared** a higher standard, + and does not fire for the whole existing index, whose packages declare + nothing and are defaulted. The second half is the denominator, and it is the + assertion that would have caught the trap §9-Q3 recorded. +- Both inheritance sites are exercised: the command issued at the workspace + root with `-p`, and the command issued inside the member directory. Two sites + means an assertion on one of them proves nothing about the other. + +--- + +## 6. #527 RFC 2 — dialect flags and the `import std` BMI + +### 6.1 Measured + +```toml +[build] +cxxflags = ["-fno-exceptions"] +``` + +``` +$ mcpp build + Compiling noexc v0.1.0 (.) +error: build failed +failed: obj/main.o +std: error: language dialect differs 'C++23', expected 'C++23/no-exceptions' +std: error: failed to read compiled module: Bad file data +std: note: compiled module file is 'gcm.cache/std.gcm' +``` + +```toml +[build] +dialect_cxxflags = ["-fno-exceptions"] +``` + +``` +$ mcpp build + Finished dev [unoptimized + debuginfo] in 0.61s +``` + +The maintainer's A4/A5 is correct in full: the key exists and it works. + +### 6.2 The mechanism is complete + +`dialect_flags()` (`modules/manifest/src/types.cppm:1217`) returns explicit +`dialect_cxxflags` plus auto-promoted known flags; `prepare_build` appends the +result to the standard flag to form one `stdFlagAndDialect` string +(`src/build/prepare.cppm:7612-7636`) that is shared by the p1689 scan and by +`stdmod::ensure_built`, so scan-time, prebuild-time and compile-time dialect +provably agree. Nothing about the plumbing is missing. + +What is missing is that `-fno-exceptions` and `-fno-rtti` are deliberately +excluded from the auto-promotion list, with the reason recorded in the code +(`modules/manifest/src/types.cppm:1200-1204`): dependencies may assume +exceptions are available. That reason is correct and should stand. + +### 6.3 Why auto-promotion is the wrong fix + +Promotion makes a per-unit flag graph-global and silently changes how every +dependency is compiled. A dependency that uses `try`/`catch` then fails to +build, and the user who wrote one flag in their own `[build]` table has no +reason to look there. The split in the list is not arbitrary and should be +stated as a rule rather than left as a list: + +> A flag is auto-promoted when a graph that mixes it is ill-formed anyway — +> `-freflection`, `-fchar8_t`, `-D_GLIBCXX_USE_CXX11_ABI=` change what the +> standard library headers *declare*, so no dependency can hold a coherent +> different opinion. A flag is not auto-promoted when a dependency can +> legitimately disagree: `-fno-exceptions` and `-fno-rtti` remove a language +> facility that a dependency may use and that the consumer cannot decide on its +> behalf. + +### 6.4 Design + +**D14 — refuse, and name the key.** The condition is provable, not heuristic: +the graph imports `std` (mcpp already computes this as `needsStdModule`, +`src/build/prepare.cppm:7682`, and already gates two refusals on it at `:7828` +and `:7914`), and `[build] cxxflags` contains a +dialect-class flag that is not in `dialect_flags()`. Under those two facts the +compile *will* fail with the message above. Refuse before compiling, with the +line the user can paste: + +``` +error: `-fno-exceptions` in [build] cxxflags changes the language dialect, but + the `import std` BMI is precompiled without it, so every importing TU + will fail with "language dialect differs". + Fix: move it to `[build] dialect_cxxflags`, which is applied to the std + BMI prebuild, the module scan and every TU in the graph — including + dependencies, which is what makes the graph coherent. + + [build] + dialect_cxxflags = ["-fno-exceptions"] +``` + +The recognition list for this check is the one `is_dialect_flag` already +maintains, extended with the two flags that are recognised-but-not-promoted. +One list, two consumers, so the day a third dialect flag is added it cannot be +added to only one of them. + +`-fno-rtti` behaves identically and is therefore in scope, measured rather than +assumed: + +``` +std: error: language dialect differs 'C++23', expected 'C++23/no-rtti' +``` + +**The check reads the EFFECTIVE flags, not `[build] cxxflags`.** The same flag +can arrive from `[profile.] cxxflags`, from +`[target..build] cxxflags`, or from a `cfg(...)` conditional block, and +all of them reach the compile line while none of them reaches the std BMI +prebuild. A check that reads only the one table is silent on three spellings of +the same mistake — which is the shape this whole document keeps finding, and it +would be careless to reproduce it in the fix for it. + +### 6.5 Criterion + +- `cxxflags = ["-fno-exceptions"]` plus `import std` refuses **before** any + compile edge runs, and the message contains the string `dialect_cxxflags`. +- `dialect_cxxflags = ["-fno-exceptions"]` plus `import std` builds. +- `cxxflags = ["-fno-rtti"]` refuses on the same terms, so the list is exercised + and not just its first entry. +- `cxxflags = ["-D_GLIBCXX_USE_CXX11_ABI=0"]` plus `import std` **builds**, and + builds silently. This is the control that keeps the check from swallowing the + auto-promotion path: an auto-promoted flag is already in `dialect_flags()`, so + a check that fires here is testing "is this flag dialect-class" instead of + "did this flag reach the BMI". Verified as green today. +- `cxxflags = ["-fno-exceptions"]` **without** `import std` anywhere in the + graph still builds. This is the denominator: a check that refuses in both + cases has stopped testing the condition it claims to test. +- The same flag placed in `[profile.dev] cxxflags` refuses too — the assertion + that the check reads effective flags. + +--- + +## 7. #527 Bug 1 — a crash where a refusal belonged + +Reproduced verbatim: + +``` +$ cat mcpp.toml +[package] name = "demo" ... +[toolchain] linux = "system" +$ ls build.mcpp +build.mcpp +$ mcpp build + build.mcpp compiling +error: build.mcpp failed to compile (exit 127): +posix_spawnp('') failed (error 2): No such file or directory +``` + +The reporter's source-level diagnosis is exactly right, at shifted line +numbers. `explicit_compiler` is left empty by the `system` branch +(`src/build/prepare.cppm:2160-2161`), and the native branch of +`host_tc_for_build_program` returns it unchanged +(`src/build/prepare.cppm:2825-2826`). + +The interesting part is what sits eight lines further down. The **cross** branch +already refuses this configuration, with a good message: + +``` +build.mcpp under a cross --target needs a resolvable host toolchain — +set one via [toolchain] or `mcpp toolchain default` +``` + +So a classified refusal for "no host toolchain is resolvable" is written and +correct. It is guarded by `!overrides.target_triple.empty()`, and the failing +case is the native one — which is why the native path reaches `posix_spawnp` +with nothing at all. Reading it as "the refusal is unreachable" was this +document's first conclusion, and §1.2 corrects it: the cross branch refuses +because nothing was resolved, whereas on the native path something *was* +resolved and simply was not handed over. + +**D15 — refuse `[toolchain] system`, with `msvc@system` the one exception.** +This reverses what an earlier draft of this document proposed, and the reversal +comes from §1.2's corrected form: the boundary is per axis, and the toolchain is +not the axis a project may take from the host. + +The crash itself is real and its diagnosis stands. `explicit_compiler` is left +empty by the `system` branch — it has nothing to assign until `detect` finds the +PATH compiler and stores the absolute path in `tc->binaryPath` — and the native +branch of `host_tc_for_build_program` returned the local variable, handing `""` +to `posix_spawnp`. The main build read the compiler from `tc` and worked, which +is why only `build.mcpp` died. + +What changed is the conclusion drawn from it. Filling the variable makes the +escape hatch consistent, and a consistent unsupported configuration is still an +unsupported configuration. **A refusal that arrives as a crash three layers down +is not a policy; it is a bug wearing one.** So the fix is the refusal the +configuration always warranted, raised where the specification is read: + +``` +error: [toolchain] linux = "system" is not supported: mcpp builds only with + toolchains it manages. + A compiler taken from PATH cannot be identified or reproduced, so + `import std` availability, the runtime closure and "the same build on + another machine" all stop being things mcpp can promise. + Name one instead — mcpp installs it on first use: + + [toolchain] + linux = "gcc@16.1.0" + + or set a machine default with `mcpp toolchain default gcc@16.1.0`, and + see `mcpp toolchain list` for what is available. + (On Windows, `msvc@system` is different and remains supported: it names + a family whose installation mcpp locates.) + Host LIBRARIES are a separate question and are not refused — a project + may link them and owns the result. +``` + +The last two lines are load-bearing rather than courtesy. A Windows user +reading a blanket "system is not supported" would reasonably conclude +`msvc@system` had been removed, and a user who came here from a host-library +warning would reasonably conclude the two axes had merged. Both are wrong, and +neither is inferable from the refusal without saying so. + +**That the configuration works today is not an argument against refusing it.** +Measured, `[toolchain] system` compiles a project using `import std` in 2.04 s +on a host with a new enough compiler. The measurement was right and the +inference from it was wrong: what mcpp promises is not "this compiled here", +it is "this compiles the same way elsewhere", and a PATH compiler cannot +support that sentence no matter how well it performs on one machine. + +**The cross-target branch is unchanged.** There `explicit_compiler` is empty for +a different reason — no host toolchain was resolved at all — and its classified +`refusal::Code::HostToolToolchain` remains the right answer. + +**Three existing tests referenced the escape hatch, and each needed a different +answer.** Recorded because two of them would have gone on passing: + +| test | what happened | what it needed | +|---|---|---| +| `14_toolchain_fallback` | asserted only that `system` did NOT produce "no toolchain configured" — a predicate any other error also satisfies. It passed while its stated intent inverted. | assert the refusal on its own terms, both halves | +| `293_…_name_one_os` | used `system` to point a Linux compiler at a Windows target; the refusal fires first, so it began taking its skip branch — which its own header says must be earned or the test cannot see a revert | accept the refusal as a PASS branch with its reason: the door it guarded is now closed entirely | +| `105_asm_sources_nasm` | genuinely unaffected — its broken-`MCPP_HOME` bootstrap error still fires first | nothing; verified rather than assumed | + +A negative-only assertion cannot distinguish "it worked" from "it failed +differently", and a skip cannot distinguish "not applicable here" from "this +test stopped testing". Both traps were live in this change. + +**Criterion.** Five assertions, and the last is the one that keeps the fix +honest: + +- `[toolchain] system` is refused with no `build.mcpp`, and with one, and the + refusal reaches the user before the build program is compiled — no + `posix_spawnp`, no `build.mcpp compiling` line. +- `MCPP_TOOLCHAIN=system` is refused identically: one policy cannot have two + answers depending on which channel stated it. +- The message names what to write instead, the command that lists the choices, + the `msvc@system` exception, and the library axis. +- A project with no `[toolchain]` at all **still builds**, and does not see the + refusal — the denominator, without which "refuse everything" satisfies every + assertion above. + +## 8. #527 RFC 4 — the aggregate `compile_commands.json` + +Measured: each member writes its own `compile_commands.json` in its own +directory; there is no root aggregate, and `compile_commands.cppm` has no +workspace-aware path. + +The request is well-founded — a language server rooted at the workspace +directory finds nothing — and the maintainer has deferred it to a later +release. Two notes for whoever builds it: + +- The aggregate must be written by the `--workspace` fan-out, after the last + member, from the per-member files. Building it inside `run_configure_plan` + would make each member's write a partial overwrite of the root file, and the + last member to finish would win. +- Entries from different members can name the same `file` under different + flags, which is legal in the format and which some consumers resolve by + taking the first match. The aggregation order should be the declared member + order, so the result is at least deterministic. + +Deferred here as well; recorded so the later work does not start from a blank +page. + +--- + +## 9. Not recommended + +- **Auto-promoting `-fno-exceptions` / `-fno-rtti` to dialect flags** (#527 RFC + 2 as literally written). §6.3. The existing exclusion is correct; the defect + is the silence, and D14 removes it without changing what the flags mean. +- **`[workspace.package] standard` before the declaredness bit** (#527 RFC 3 as + literally written). §5.4. It is not implementable — "the member did not say" + and "the member said c++23" are the same bytes today, and shipping the table + without D10 produces an inheritance rule that silently overrides deliberate + member pins. +- **Filling in the host compiler so `[toolchain] system` works consistently.** + §7. An earlier draft of this document proposed exactly that, on the ground + that the configuration builds and runs. It does; that is not the question. + The toolchain is mcpp's own contract and is refused, so the reporter's + `explicit_compiler.empty() ? tc->binaryPath : …` patch is NOT adopted. +- **Any of this being read as movement on RFC 1.** `sysroot = "system"`, + suppressing the private `PT_INTERP`, and not injecting RPATH remain out of + scope and untouched. D15 moves in the opposite direction from RFC 1: it + narrows what the host may supply rather than widening it. +- **Refusing a host LIBRARY.** §1.2. That axis is the project's own, and the + answer there stays a warning that names the mcpp-index route. +- **Removing the SubOS farm from host tool `RPATH`s as the fix for #535.** + §3.3/D7. It trades a silent wrong answer for a hard failure in a different + set of cases, and D8 makes the farm's contribution visible without that + trade. +- **Warning on host search paths in `ldflags` as the fix for #537.** §4.2. Case + 2 is the same idea resting on two file paths instead of a path substring, and + it is silent in the case where the substring rule produces noise. +- **`Origin::SubosFarm` as the "undeclared provider" predicate** (D8 as first + drafted). §4.2. The farm is a symlink view of installed packages, so a + declared dependency's library routinely resolves through it; the predicate + has to canonicalise and ask who owns the file. +- **D3 before D3a.** §2.5. Extending the fast path to `-p` while the staleness + sweep is blind to `path` dependencies would promote a narrow existing hole + into the normal workspace case. +- **A blanket refusal of `[toolchain] system`.** §7. Measured, that + configuration builds a project with `import std` today; only the `build.mcpp` + path is broken, and only that path should be refused. + +--- + +## 10. Sequencing + +Ordered by what unblocks what, not by severity. + +| step | items | unblocks | +|---|---|---| +| 1 | D10 (declaredness on inheritable keys, both parse paths) | D12, D13; nothing in §5 is correct without it | +| 2 | D1, D1a, D2 (records move to the surviving sidecar; key covers farm and policy; prune on disk absence, not on plan absence) | #529's dominant cost; independent of everything else. D1 without D1a trades slow-and-correct for fast-and-stale; D1 without D2 leaves the developer loop paying | +| 3 | D11, D12 (stated merge discipline; the three workspace tables) | #527 Bug 2, RFC 3 | +| 4 | D15 (refuse `[toolchain] system`; `msvc@system` excepted) | #527 Bug 1. One refusal plus three existing tests that referenced the escape hatch | +| 5 | D14 (dialect flag refusal), incl. the MSVC spellings or a stated GNU-only scope | #527 RFC 2 | +| 6 | D3a (staleness sweep covers `path` dependency source roots) | a defect on its own; D3's precondition | +| 7 | D8, D9 (provenance per `DT_NEEDED`, published, feeding `host_requirements`) | #537, the safe half of #535, and truthful `mcpp pack` host requirements | +| 8 | D5, D6 (tool store publishes a directory; closure validated after publish) | #535 | +| 9 | D3 (fast path honours `-p`), D13 + D13a (standard floor check, degraded, author-owned manifests only) | after their preconditions land | +| 10 | D4 (test fast path), D7 (farm in tool `RPATH`), RFC 4 | re-measure before designing | + +Steps 2, 4, 5 and 6 are independent of every other step and of each other; any +can go first. Step 2 is the one with a user-visible number attached (§11.1), +and step 4 is the cheapest. + +**One ordering constraint that is not about dependencies.** Steps 4, 7 and 9 +each add a diagnostic channel that fires on builds which are green today (§12.3). +They should not land in the same release. A warning column that grows by three +in one version is read as noise, and the one that matters most — D8's, which is +a genuine correctness statement about what an artifact will load — is the one +that would be discounted. + +--- + +## 11. What changes for a user + +Every example below is the behaviour after the corresponding step, written +against the reproductions in this document. + +### 11.1 The edit-test loop stops paying for answers it already has (D1, D1a, D2) + +Nothing in the interface changes. The numbers do. On the measured fixture — ten +link units, 11.5 MB each, everything warm: + +``` + before after (projected) +mcpp build -p app 0.70 s 0.40 s +mcpp test -p app 3.15 s 0.60 s +mcpp test -p app 1.95 s 0.60 s +``` + +The "after" column is the measured wall clock minus the three stage timings, +which is what the memo is expected to remove; it is a projection and is the +number the criterion in §2.6 should be held to, not a promise. + +One behaviour does change and is worth calling out in release notes: a verdict +that was previously recomputed on every invocation is now read from a record, +so `resolution.json` and the sidecar become load-bearing. Deleting the output +directory remains the way to force a full re-derivation, and D1a's key is what +makes that unnecessary in the cases users actually hit (`xlings install`, +toggling `MCPP_ALLOW_HOST_LIBS`). + +### 11.2 A workspace declares shared settings once (D10, D11, D12) + +Before — every member restates everything, and the root's `[build]` is inert: + +```toml +# mcpp.toml (workspace root) — the [build] table here does nothing today +[workspace] +members = ["apps/compositor", "packages/render", "packages/proto"] + +[build] +cxxflags = ["-Wall", "-Wextra"] # silently ignored by every member +``` + +```toml +# apps/compositor/mcpp.toml — and the same six lines in every other member +[package] +name = "compositor" +version = "0.1.0" +standard = 26 + +[build] +cxxflags = ["-Wall", "-Wextra"] +``` + +After: + +```toml +# mcpp.toml (workspace root) +[workspace] +members = ["apps/compositor", "packages/render", "packages/proto"] + +[workspace.package] +standard = 26 +version = "0.1.0" +license = "Apache-2.0" + +[workspace.build] +cxxflags = ["-Wall", "-Wextra"] +dialect_cxxflags = ["-fno-exceptions"] + +[workspace.target.x86_64-linux-gnu] +toolchain = "gcc@16.1.0" +``` + +```toml +# apps/compositor/mcpp.toml +[package] +name = "compositor" +# standard, version, license inherited; [build] inherited +``` + +A member that wants something different says so, and the member wins: + +```toml +[package] +name = "legacy-shim" +standard = 23 # deliberate, and honoured — this is what D10 buys +``` + +`mcpp build -p legacy-shim` compiles it at 23; every other member at 26. Today +both spellings produce 23 for the whole graph and the workspace table is inert. + +**Migration.** Existing manifests are unaffected: `[workspace.*]` tables are +absent, so nothing is inherited and every member keeps its own values. There is +no index floor concern either — `[workspace.*]` appears only in a workspace +root manifest, which is never published to the index, so no released mcpp is +ever asked to read a key it does not know. + +### 11.3 A misplaced dialect flag says where it belongs (D14) + +Before: + +``` +$ mcpp build + Compiling noexc v0.1.0 (.) +error: build failed +failed: obj/main.o +std: error: language dialect differs 'C++23', expected 'C++23/no-exceptions' +std: error: failed to read compiled module: Bad file data +std: note: compiled module file is 'gcm.cache/std.gcm' +``` + +After: + +``` +$ mcpp build +error: `-fno-exceptions` in [build] cxxflags changes the language dialect, but + the `import std` BMI is precompiled without it, so every importing TU + will fail with "language dialect differs". + Fix: move it to `[build] dialect_cxxflags`. + + [build] + dialect_cxxflags = ["-fno-exceptions"] +``` + +No project that builds today starts failing: the refusal fires only where the +compile already fails. + +### 11.4 A package that needs a newer standard says so (D13) + +``` +$ mcpp build +warning: dependency `render` declares `standard = 26`, and this graph is built + at c++23 (from [package] standard of `compositor`). A C++ module graph + has one standard; the dependency's declaration is not applied. + Fix: raise the consumer's standard, or set it once for the workspace: + + [workspace.package] + standard = 26 +``` + +Degraded, so the build continues; `--strict` promotes it. Packages that never +declared a standard are silent, which is every package in the index today. + +### 11.5 "This closure is satisfied by declared packages" becomes checkable (D8, D9) + +Before — on a machine with Mesa installed, a build that takes its ABI from +`/usr/include` and its runtime library from the SubOS says nothing at all: + +``` +$ mcpp build + Compiling linkab v0.1.0 (.) + Finished dev [unoptimized + debuginfo] in 0.05s +``` + +After: + +``` +$ mcpp build + Compiling linkab v0.1.0 (.) +warning: build/provenance: `libgbm.so.1` is linked from + /usr/lib/x86_64-linux-gnu/libgbm.so.1 (a -L directory that is not on + this artifact's runtime search path) and loaded from + ~/.mcpp/registry/subos/default/lib/libgbm.so.1 (SubOS, provided by + xim:mesa, which no dependency declares). + The program runs against a different build than it was compiled + against. +note: mcpp builds against no host library by default, so this artifact is + not reproducible on another machine and `mcpp pack` will record + libgbm.so.1 as a host requirement rather than bundling it. + The supported route is to declare the provider as a dependency from + mcpp-index. If mcpp-index does not carry it yet, contributing the + package is the path, and the dependency then resolves the same way on + every machine and in CI. + Finished dev [unoptimized + debuginfo] in 0.05s +``` + +The build is not refused: it compiles, it runs on this machine, and the +developer said they wanted this. What changes is that the cost is stated and +the supported alternative is named. + +And the record gains a field a CI job can assert on, which is what makes "zero +host-supplied libraries" a statement rather than an impression: + +```json +"runtime": { + "providers": [ + { "soname": "libgbm.so.1", "file": ".../subos/default/lib/libgbm.so.1", + "origin": "subos_farm", "owner": "xim:mesa", "declared": false } + ] +} +``` + +Expect this to fire on existing green graphics builds. That is the point, and +it is why it is degraded rather than an error on first release. + +### 11.6 A host tool that needs a shared library works on a clean machine (D5, D6) + +Before, on a clean runner: + +``` + Building host tool wayland-scanner:wayland-scanner from wayland-scanner v1.26.0 +error: dependency 'wayland': build.mcpp exited with 1 (build aborted): + .../bin/wayland-scanner: error while loading shared libraries: + libexpat.so.1: cannot open shared object file: No such file or directory +``` + +Before, on a developer machine with `xim:expat` installed: it *works*, against +a different version than the one declared. + +After, both machines behave the same, and the store entry contains what the +tool needs: + +``` +$ ls /v1/tool/freedesktop/wayland-scanner@1.26.0//bin/ +wayland-scanner libexpat.so.1 +``` + +If the closure still cannot be satisfied, the failure moves to where the tool +is built and names it, instead of arriving as a loader error inside an +unrelated package's `build.mcpp`. + +### 11.7 `toolchain = "system"` is refused, and says what to write instead (D15) + +Before, adding a `build.mcpp` to a project that otherwise builds: + +``` +$ mcpp build + build.mcpp compiling +error: build.mcpp failed to compile (exit 127): +posix_spawnp('') failed (error 2): No such file or directory +``` + +After — and with or without the `build.mcpp`, because the configuration itself +is what is refused: + +``` +$ mcpp build +error: [toolchain] linux = "system" is not supported: mcpp builds only with + toolchains it manages. + A compiler taken from PATH cannot be identified or reproduced, so + `import std` availability, the runtime closure and "the same build on + another machine" all stop being things mcpp can promise. + Name one instead — mcpp installs it on first use: + + [toolchain] + linux = "gcc@16.1.0" + + or set a machine default with `mcpp toolchain default gcc@16.1.0`, and + see `mcpp toolchain list` for what is available. + (On Windows, `msvc@system` is different and remains supported.) + Host LIBRARIES are a separate question and are not refused. +``` + +A project that used the escape hatch has to name a toolchain, which mcpp +installs on first use. A project with a declared toolchain sees no new output +at all, and a project that links host **libraries** is untouched — that is the +other axis, and its answer is still a warning. + +### 11.8 Adding a file to a `path` dependency is no longer invisible (D3a) + +Before: + +``` +$ printf 'export module dep.third;\n...' > ../dep/src/third.cppm +$ mcpp build + Finished dev in 0.00s # the module was never compiled +``` + +After: the fast path is abandoned, the module is compiled, and the archive +gains an object. The cost is that a project with `path` dependencies stats +their source trees on every invocation, which is the same cost it already pays +for its own. + +--- + +## 12. The plan under six other lenses + +Reviewed after the design was written, deliberately from angles the design was +not written from. Findings that changed the design are marked; the rest are +constraints the implementation has to carry. + +### 12.1 Cross-platform + +**D8/D9 are Linux-only as designed, and that must be stated rather than +discovered.** `runtime_search_closure` guards on format: `DT_RPATH` exists on +ELF only, so "Mach-O and PE get nothing rather than a branch in every consumer" +(`src/build/plan.cppm:754-772`). The provenance join therefore has no input on +the other two platforms. This is not a defect in D8 — it is the correct scope — +but the vocabulary must not become ELF-shaped, because the question is +universal: + +| platform | the same question | the mechanism that would answer it | +|---|---|---| +| Linux | which directory on the search path supplied this `DT_NEEDED` | `runtime_search_closure` + `resolve_runtime_closure` — exists | +| Windows | was this DLL found beside the exe, or in `System32` / on `PATH` | `deployFiles` already stages the beside-the-exe half; the search-order model does not exist | +| macOS | `@rpath` entry, or a system framework / the dyld shared cache | neither exists | + +So D9's record field should be named for the question ("which origin supplied +this provider"), not for the ELF answer, and the two other platforms should +appear in the record as "not evaluated" rather than as absent — the same +four-valued discipline `RuntimeVerdict` already uses, where "not measured" and +"clean" must not read the same. + +**D5 is a cross-platform unification, not a Linux fix, and this strengthens +it.** On PE there is no RPATH at all, so a shared dependency *must* sit beside +the executable; that is exactly what `deployFiles` already does for a normal +build. A host tool's store entry publishing only the executable is therefore +broken on Windows for a more basic reason than on Linux, where the farm can +paper over it. Reusing `deployFiles` makes one mechanism serve both. + +**D14 is GNU-spelled and would be silent on MSVC.** `is_dialect_flag`'s list is +`-freflection`, `-fcontracts`, `-fchar8_t`, `-D_GLIBCXX_USE_CXX11_ABI=` +(`modules/manifest/src/types.cppm:1200-1214`). The MSVC spellings of the same +dialect axes — `/EHsc` and `/EHs-c-` for exceptions, `/GR-` for RTTI — are not +in it, and `msvc_crt_flag` is already threaded into `stdmod::ensure_built` +precisely because `cl` bakes `_MSVC_MT`/`_MSVC_MD` into the module. The same +class of mismatch exists there and the check would not see it. Either add the +MSVC spellings in the same change, or state in the code that the list is +GNU-only and why — an unstated platform gap in a table like this is how the +next reader assumes coverage. + +**D10–D12 are platform-neutral**, with one interaction to specify: +`[workspace.target.]` and `[target.'cfg(...)']` conditional blocks both +target-scope configuration, and the merge order between an inherited +`[workspace.target.X]` and a member's own `cfg()` block has to be written down. +The safe default is that the member's conditional evaluates after inheritance, +so a member can always narrow. + +### 12.2 Compatibility and migration + +**Nothing here changes a published manifest's schema.** `[workspace.*]` appears +only in a workspace root, which is never published to the index, so no released +mcpp is asked to read a key it does not know — the failure mode where an +unknown key makes a whole manifest unloadable does not apply. `mcpp pack`'s +emitted manifest does not write `standard` at all +(`src/pack/manifest_emit.cppm`, checked), so D10's optional-ness does not leak +into published descriptors either. + +**D13's blast radius is measured, and it is the finding that scoped it.** See +§5.5/D13a: every index descriptor with an mcpp segment declares `language`, so +"declared" cannot mean "authored" for index packages. Scoped to author-owned +manifests, D13 fires on nothing that exists today. + +**D1's record format needs a version, and half of one exists.** The sidecar +carries `schema` and `contract_hash`; adding two record kinds to it means an +older mcpp reading a newer file must ignore what it does not know rather than +discard the file — which is what a schema field is for, provided the reader is +written that way. Worth asserting, because the failure is silent: a discarded +record reads exactly like a cold cache, i.e. as a slow build rather than as an +error. + +**Behaviour that changes for someone whose build is green today:** D8 Case 1 +and D15's warning both add output to builds that currently print none. Neither +fails. That is the whole of the user-visible compatibility surface. + +### 12.3 Usability + +**The inheritance discipline diverges from cargo, deliberately.** Cargo spells +inheritance `x.workspace = true` for package fields as well as dependencies, +and #527's RFC copied that spelling. D11 makes package and build keys implicit +instead, and the argument is the RFC's own goal: if every member must opt in +per key, a member that forgets one drifts silently, which is the drift the RFC +exists to eliminate. Implicit-if-absent makes drift the thing you have to ask +for. The cost is that a cargo user's expectation is wrong once, and the answer +is documentation rather than a second mechanism. + +It is also the choice that keeps the count down: mcpp already ships three +implicitly-inheriting keys (`toolchain`, `[target.*]`, `[indices]`) and one +opt-in key (`dependencies`). Making the new keys opt-in would give four keys +under two disciplines with no rule; making them implicit gives one rule and one +named exception. + +**Warnings must not train people to ignore them.** D8 Case 1 will fire on +existing graphics builds, D15 on every `system`-toolchain build, D13 on +author-declared mismatches. Three new channels arriving at once is how a +warning column becomes noise. They should land in the order of §10, each with +its own release, and each must be silent on a compliant project — the last +assertion in §7's criterion exists for that reason. + +**The best usability outcome in this document is not a message.** It is §11.1: +an edit-test loop that stops costing seconds per iteration. Nothing else here +is felt as often. + +### 12.4 Simplicity + +Counted by what an implementer touches: + +| item | surface | +|---|---| +| D1, D1a, D2 | one record file, one key derivation, one prune predicate; the sync pattern already exists (`sync_resolution_verdict`) | +| D3a | one function's input set, plus one field in `.build_cache` | +| D5, D6 | reuse `deployFiles` + one post-publish validation; no new concept | +| D8, D9 | one join between two existing records; feeds an existing list (`host_requirements`) | +| D10 | optional-ness on a handful of fields, filled in **two** parse paths | +| D11, D12 | one merge function, three table names | +| D14, D15 | two diagnostics, one of which is a one-line assignment | + +The two places where simplicity is at risk, both worth watching in review: + +- **D10 is the only wide change.** `Package::standard` becoming optional + touches every reader. The mitigation is that the default is applied once, + after inheritance, so readers keep seeing a plain string — but if that + single application point turns into three, the change has failed and should + be reconsidered rather than finished. +- **D12's merge function must be one function called from two sites.** The two + inheritance sites are already two copies of the same four merges. Adding + three tables to two copies is six edits, and the sixth is the one that gets + forgotten. + +### 12.5 Architectural clarity + +The plan adds **no new concepts**. Every item is an existing mechanism reaching +one more case: + +| item | the mechanism it extends | +|---|---| +| D1/D2 | `sync_resolution_verdict`'s sidecar-authoritative pattern | +| D5 | `deployFiles`, the beside-the-artifact staging that PE already needs | +| D8/D9 | `search::Origin`, already tagged and already ordered | +| D9 → pack | `host_requirements`, already the two-consumer statement of host dependence | +| D11/D12 | the implicit-if-absent discipline of `toolchain`/`[target.*]`/`[indices]` | +| D13 | the floor check §9-Q3 specified and deferred | +| D14 | `is_dialect_flag`, one list with a second consumer | +| D15 | the toolchain specification is read in one place; the refusal joins the branches already there | + +The one genuinely new statement is §1.2, and it is a policy rather than a +mechanism: **mcpp depends on no host; a user project may, and is warned rather +than refused whenever the result builds and runs.** Everything with a severity +in this document now derives from that sentence, which is why D15 reversed and +D13 softened during review. + +### 12.6 Build and distribution + +**Host dependence is a distribution property first and a build property +second, and the plan was initially written the other way round.** A warning at +build time is advice; the consequence lands at `mcpp pack`, where a +host-supplied library cannot be bundled and the honest output is a statement of +what the target machine must provide. `host_requirements` already exists for +this, already has two consumers (`mcpp pack` and `mcpp publish` → +`[runtime].requirements`), and already warns in its own header that deriving +the list twice is how the two drift. D9 feeding it is therefore not an +enhancement; deriving a second list would be the defect. + +This also gives the §1.2 policy its teeth without a refusal. "You may depend on +the host and you guarantee it" becomes concrete: the guarantee is written into +the bundle, and a consumer of the package can read it. + +**Reproducibility.** Two items in the plan improve it and one must be checked +not to harm it. D15's warning and D8's report both make non-reproducible inputs +visible where they were silent. D1's memoisation makes a build's *diagnostics* +depend on a cached record — if the key is wrong, two machines with identical +sources can print different warnings, which is worse than a slow build. That is +D1a, and it is why D1a is not optional. + +**Offline and sandboxed builds** (#527's second comment, the distro-packaging +argument) are untouched by everything here. That request is about mcpp +resolving toolchains from the network during `%build`, which none of these +items changes; it is a separate question from host dependence and is not +addressed by this document. + +--- + +## 13. Review record + +What this document said before it was reviewed, and what the review changed. +Recorded because three of the corrections are the kind that would have shipped +as defects, and one of them found a defect that no issue reports. + +| claim as first written | what measurement showed | where | +|---|---|---| +| "the record is incomplete because the last drive within an invocation replaced it" | False. `mcpp test`'s plan genuinely contains only the nine test binaries — verified against the emitted `cxx_link` edges. The real collision is **between commands**: `build` and `test` share one output directory with different link-unit sets and prune each other | §2.5, D2 | +| "move the records to the sidecar; that pass is fast, so the sidecar works" | Incomplete. The sidecar's own pass costs 1.19 s after an intervening `build`, for the same pruning reason. D1 alone would have inherited the defect it was fixing | §2.1, §2.3 | +| D1 with no discussion of invalidation | A durable memo is stale when the SubOS farm is re-pointed or `MCPP_ALLOW_HOST_LIBS` is toggled — neither is in the artifact's stat or the output fingerprint. The memoisation *creates* a correctness obligation that did not exist while the answer was recomputed | §2.5, D1a | +| "extend the fast path to `-p`" | Would promote an existing hole into the normal case. Measured: a new source file in a `path` dependency is invisible to the staleness sweep, and `mcpp build` reports success without compiling it. A defect in its own right, in no issue | §2.5, D3a; §0 | +| "a farm-origin provider that no dependency declares" | Wrong predicate. The farm is a symlink view *of installed packages*, so declared dependencies routinely resolve through it; #532's eleven-entry closure would have been reported wholesale. The predicate must canonicalise and ask who owns the file | §4.2, D8 | +| D13 as an error | Would turn green builds red with no defect behind them: a package declaring `standard = 26` compiles fine at 23 unless it uses a C++26 construct. Degraded first, `--strict` promotes | §5.5, D13 | +| D14 reading `[build] cxxflags` | Three other tables reach the compile line and not the BMI prebuild. Reading one table reproduces, inside the fix, the exact shape being fixed | §6.4 | +| "refuse `[toolchain] system`" | Measured: that configuration builds a project with `import std` today, in 2.04 s -- so an earlier draft proposed filling the variable and warning. §1.2 was then corrected per axis and the refusal reinstated: the measurement was right, the inference from it was not. See the third round below | §7, D15 | + +**Second round, after the host-dependence policy was stated (§1.2).** The +policy — mcpp depends on no host; a user project may, and is warned rather than +refused whenever it builds and runs — is not a preference that could have been +guessed from the code, and four items moved once it was written down. + +| claim as it stood | what the policy or a measurement showed | where | +|---|---|---| +| D15 refuses `toolchain = "system"` for `build.mcpp` | Reversed. A user project's opt-in to the host is warned about, not refused. `build.mcpp` failing is an unfilled variable (`tc->binaryPath` is already resolved), not a policy boundary, so the reporter's patch is adopted — on the narrower ground that it fills a variable, not that it adds host support | §7, D15 | +| "this document does not recommend the reporter's patch" | Withdrawn. It was excluded on a scope reading that §1.2 does not support | §9 | +| D13's declaredness bit suffices to keep the index quiet | False, and measurable: **782 of 782** index descriptors with an mcpp segment declare `language` (the key that feeds `Package::standard`), and **756 of 774** also say `import_std = false` — C libraries carrying a boilerplate `"c++23"`. A floor check keyed on declaredness would fire against the whole index for a root at c++20, which is exactly what §9-Q3 refused. D13 is now scoped to author-owned manifests | §5.5, D13a | +| D1 justified by "the sidecar is a cleaner owner" | The pattern already exists: `sync_resolution_verdict` (`runtime_validation.cppm:328`) already treats the sidecar as authoritative and `resolution.json` as a published copy. D1 is applying an existing convention, not choosing between two designs — and that also rules out the "make `prepare_build` merge" alternative, which would give one object two conventions | §2.5, D1 | +| D8/D9 as a build-time warning | Under-scoped. Host dependence is a distribution property: `mcpp.pack.host_requirements` exists to state what the target machine must supply, has two consumers, and warns in its own header that deriving the list twice is how they drift. D9 must feed it rather than stand up a parallel list | §4.2, D9; §12.6 | +| D8/D9 with no platform scope | ELF-only by construction — `runtime_search_closure` gives Mach-O and PE nothing. The record must say "not evaluated" on those platforms rather than be absent, or "not measured" and "clean" read the same | §12.1 | +| D14 as a complete check | GNU-spelled only. `/EHsc`, `/EHs-c-`, `/GR-` are the MSVC spellings of the same axes and are not in the list, while MSVC's std module already threads a CRT flag through for exactly this class of mismatch | §12.1 | + +Three claims were checked and survived unchanged: `-fno-rtti` behaves +identically to `-fno-exceptions` (measured, same error); an auto-promoted +dialect flag in `cxxflags` builds silently (measured, green), now serving as +D14's control; and `mcpp pack`'s emitted manifest does not write `standard`, so +D10 does not leak into published descriptors. + +**What would falsify the central diagnosis.** If, on the reporter's workspace, +the three stage timings do not account for the bulk of the 15.6 s, then §2 has +found a real defect that is not their defect, and the remaining time is +somewhere this document has not looked. The first thing to ask them for is the +`MCPP_VERBOSE=1` stage decomposition of one warm `mcpp test`; it is four lines +and it settles the question. + +**Third round: the host-dependence rule is per AXIS, not one boundary.** + +§1.2's first form applied a single test — "does it build and run" — to +everything, and D15 followed it to "warn, do not refuse". That was wrong, and +the error was not in the measurement: + +| claim as it stood | what the corrected policy showed | where | +|---|---|---| +| one boundary for all host dependence | Two axes with different owners. The **toolchain** is mcpp's contract — `import std` availability, a computable closure, the same build elsewhere are all statements about a compiler mcpp resolved and can name — so it is refused. The **libraries** a program links are the program's own, and stay a warning | §1.2 | +| D15 fills the compiler path and warns | Reinstated as a refusal. Filling it makes an unsupported configuration *consistent*, which is not the same as making it supported; and a refusal that arrives as `posix_spawnp('')` three layers down is a bug wearing a policy | §7, D15 | +| `mcpp.diag::host_route_hint` shared by D8 and D15 | With the toolchain axis refusing rather than warning, and the library work not in this change, the helper had no consumer. Reverted — an unread field is the defect this document is about | §1.2 | +| three existing tests "still pass" | Two passed for the wrong reason. `14_toolchain_fallback`'s only assertion was a negative that any other error satisfies, and `293` began taking a skip its own header says must be earned. Verified by reading the branch each took, not the exit code | §7 | + +The general lesson is the one this document keeps finding from the other side: +**a rule stated once, uniformly, is easier to write than one stated per axis — +and when the axes have different owners, the uniform version is wrong.** The +measurement that `system` builds and runs was correct and load-bearing for the +library axis; carrying it across to the toolchain axis is what produced a +proposal the maintainer had already declined on the issue. diff --git a/CHANGELOG.md b/CHANGELOG.md index 640b4d9b..23973ebe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,95 @@ > 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。 > 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)。 +## [2026.8.30.2] — 2026-08-30 + +六处缺陷,来自 #527 / #529 的分析,外加一处在实现 review 时挖出来、没有人报过的。 +它们分属两族:**记录存在而做决定的代码不读它**,以及**运行期搜索路径把「声明的」 +和「这台机器碰巧装了的」混在一起**。 + +完整分析、量化与设计见 +[`.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md`](.agents/docs/2026-08-30-issues-527-529-535-537-analysis-and-design.md)。 + +> **host 依赖的规则按轴分,而这个分叉是刻意的。** **工具链属于 mcpp 的契约**: +> `import std` 可用、闭包可计算、同一份构建在别的机器和 CI 上一致,都是关于 +> 「一个 mcpp 解析得出、叫得出名字的编译器」的陈述,所以 `[toolchain] = "system"` +> 被**明确拒绝**(`msvc` 是唯一例外)。**而程序链接哪些库是程序自己的事**: +> 工程可以链 host 的库或自己的 `.so`,mcpp 说明代价并指出 mcpp-index 那条路,但不拒绝。 + +### 修复 + +- **`mcpp test` 每次调用都在重算一份没有变化的答案(#529)。** 两个 post-link ELF + pass 都写了读回优化(stat 没变 ⇒ 复用上次判定),而 `prepare_build` 每次调用都用 + 一个**全新的 json 对象**重写 `resolution.json` —— 那里面没有这两条记录。于是每次 + 调用一开始,就把自己后端待会儿要找的备忘录删掉了。 + + 记录改存进 `.mcpp-runtime-verdicts.json`(它本来就活得过 `prepare_build`), + `resolution.json` 继续发布一份副本 —— 这正是 `sync_resolution_verdict` 已有的 + 形态。同时: + - 剪枝的判据从「不在本次 plan 里」改为「产物已不在磁盘上」。`mcpp build` 与 + `mcpp test` 共用一个输出目录而 link unit 集合不同,前一个判据让两条命令互删 + 对方的记录; + - 记录的失效键纳入 SubOS farm 的 `.xlings.json` 时间戳与 `MCPP_ALLOW_HOST_LIBS` + —— 让备忘录持久化,就产生了一条以前不存在的正确性义务。 + + 实测(10 个 link unit,每个 11.5MB,全热): + + | | 之前 | 之后 | + |---|---|---| + | `mcpp build -p ` | 0.70s | 0.32s | + | `mcpp test -p `(紧接 build) | 3.15s | **0.36s** | + | `mcpp test -p `(连续) | 1.95s | 0.40s | + +- **`path` 依赖里新增一个源文件,fast path 看不见。** 陈旧性扫描只覆盖被构建的那个 + 工程,于是 `mcpp build` 报 `Finished dev in 0.00s`,而那个模块从没编译过。内容改动 + 之所以还能被抓到,靠的不是扫描,是 ninja 重链后的事后放弃。这是 #359 那条形态 + ("glob 输入变了而现存文件的 mtime 一个没动")在它当年没有覆盖到的目录里。 + workspace 成员之间就是 `path` 依赖,所以这不是边角情况。 + +- **`[toolchain] system` 现在被明确拒绝,而不再崩溃(#527 Bug 1)。** + 它此前配合 `build.mcpp` 会死在 `posix_spawnp('') failed (error 2)` —— 一条以崩溃形式 + 出现的"拒绝"不是政策,是穿着政策外衣的 bug。 + + **mcpp 只用它自己管理的工具链构建。** `PATH` 上的编译器无法被识别、无法被复现,于是 + `import std` 可用性、运行期闭包、"同一份构建在另一台机器上"全都不再是 mcpp 能承诺的 + 东西。拒绝消息给出该写什么、去哪看可选项,并点明 `msvc` 是**唯一例外** + (它点名的是一个族,mcpp 定位其安装),同时说明**host 库是另一条轴,不在拒绝之列**。 + +- **`standard = 26`(不带引号)被静默忽略。** 键被文档写成字符串,而 `get_string` 对 + 裸整数返回空,于是工程按默认档位编译、零诊断。#527 自己的三处示例就是这么写的。 + 两种拼法现在都接受。 + +### 新增 + +- **`[workspace.package]` 与 `[workspace.build]`(#527 Bug 2 / RFC 3)。** workspace 根 + 的 `[build]` 此前完全没有传给成员;现在标量按「成员**声明过**就成员优先」继承, + 向量按 workspace 在前追加。 + + 「声明过」是**解析时记录的事实**,不是与默认值比较得出的推断 —— 成员在 + `standard = 26` 的 workspace 下刻意写 `standard = "c++23"` 必须保住,而那与默认值 + 同为一串字节。这正是 cpp20 设计文档 §9-Q3 记下的前置条件。 + + `allow_host_libs` 明确不可继承:它关掉的是某个具体产物的检查,workspace 根设一次 + 就等于替所有后来加入的成员也关掉了。`[workspace.package]` / `[workspace.build]` 里 + 不认识的键会被**拒绝**而不是忽略。 + + 没有 `[workspace.target.]`:根里普通的 `[target.]` 本来就按 triple + 被成员继承,为同一能力再加一种拼法只增加接口面。 + +- **依赖声明了高于当前图的标准时会说出来。** C++ 模块图只有一个标准,依赖自己的 + `standard` 不生效 —— 这是对的;缺的是它一直不说。degraded 级别(`--strict` 提升), + 且**只对工程作者自己拥有的 manifest 生效**:索引里带 mcpp 段的描述符 782 个全都声明了 + `language`,其中 756/774 是 `import_std = false` 的 C 库带的样板值,信任「声明过」会 + 让 c++20 的根工程对着整个索引报警。 + +- **方言标志没进 `import std` 预编译时,在编译前拒绝。** `[build] cxxflags` 里的 + `-fno-exceptions` / `-fno-rtti` 会到达每个 TU 却到不了 std BMI 预编译,于是每个 + importer 都在 mcpp 生成的文件里失败,而报错只讲机制不讲那个键。现在提前拒绝并指出 + `dialect_cxxflags`。读的是**生效后**的标志集合(`[build]` / `[profile.*]` / + `[target.…]` 都算),并且在图中没有 `import std` 时不触发。 + + 这两个标志仍然**不自动提升**:依赖可以合法地不同意,消费者无权替它决定。 + ## [2026.8.29.1] — 2026-08-29 构建规则以普通包分发的机制自 2026.8.5.1 就能用,而**规范**一直没有:一个规则包 diff --git a/docs/03-toolchains.md b/docs/03-toolchains.md index 61f4050f..8bacfb1b 100644 --- a/docs/03-toolchains.md +++ b/docs/03-toolchains.md @@ -305,6 +305,55 @@ Pinned toolsets coexist with each other and with a system Visual Studio. > that may have been intended. (The family-less `[toolchain] … = "system"` — the PATH > compiler — is a separate and deliberate escape hatch, and is unaffected.) +### `[toolchain] … = "system"` — refused + +**mcpp builds only with toolchains it manages.** A compiler taken from `PATH` is +not supported, and the configuration is refused rather than warned about: + +``` +error: [toolchain] linux = "system" is not supported: mcpp builds only with + toolchains it manages. + A compiler taken from PATH cannot be identified or reproduced, so + `import std` availability, the runtime closure and "the same build on + another machine" all stop being things mcpp can promise. + Name one instead — mcpp installs it on first use: + + [toolchain] + linux = "gcc@16.1.0" + + or set a machine default with `mcpp toolchain default gcc@16.1.0`, and + see `mcpp toolchain list` for what is available. +``` + +`msvc@system` is **the one exception** and is a different spelling: it names a +*family* whose installation mcpp locates and identifies, on the one platform +where the compiler cannot be redistributed. See the section above. + +#### Why the toolchain and the libraries get different answers + +mcpp's rule about host dependence is not uniform across axes, and the split is +deliberate: + +- **mcpp itself, and everything the mcpp ecosystem publishes, depends on no + host.** Toolchains and payloads come through xlings — the xim index or + mcpp-index. This is what makes a build reproducible across machines and Linux + distributions. +- **The toolchain is part of that contract, so it is not the project's to take + from the host.** Everything mcpp promises — `import std` availability, a + computable runtime closure, the same build on a teammate's machine and in CI + — is a statement about a compiler mcpp resolved and can name. A `PATH` + compiler makes all of it unverifiable, which is why this one is a refusal. +- **The libraries a program links are the program's own business.** A project + may link a host library or its own `.so`. mcpp says what that costs and names + the supported route — declare the provider so it resolves from mcpp-index, and + if the index does not carry it yet, contributing the package is the path — but + it does not refuse, as long as the result builds and runs. The developer owns + the artifact and guarantees it. + +A build that provably *cannot* run stays an error on either axis: a runtime +closure that cannot be satisfied is refused, because the artifact will not +start. See [binary distribution](12-binary-distribution.md). + ### `msvc@system` — the machine's own Visual Studio mcpp locates and identifies an installed Visual Studio / Build Tools; it never diff --git a/docs/05-mcpp-toml.md b/docs/05-mcpp-toml.md index 4b2810f8..c7a53587 100644 --- a/docs/05-mcpp-toml.md +++ b/docs/05-mcpp-toml.md @@ -72,6 +72,47 @@ Two properties worth knowing: If the sources `import std;` at a level the resolved toolchain does not provide the `std` module for, mcpp fails before compiling and names both the toolchain and the project level. +Both spellings of the value are accepted: `standard = "c++26"` and `standard = 26`. + +When a **dependency declares a level above the graph's**, mcpp says so before compiling +rather than letting it fail somewhere inside that dependency's sources. See +[workspace §4.2](06-workspace.md). + +#### Dialect flags and the `import std` BMI + +Some flags change what the standard library's headers declare, so the precompiled `import std` +BMI has to be built with them too. That is what `[build] dialect_cxxflags` is for: it is +applied to the std BMI prebuild, the module scan **and** every translation unit in the graph, +including dependencies. + +```toml +[build] +dialect_cxxflags = ["-fno-exceptions"] +``` + +mcpp promotes a few flags into that channel automatically when it finds them in `cxxflags` +(`-freflection`, `-fchar8_t`, `-D_GLIBCXX_USE_CXX11_ABI=…`) — a graph that mixes those is +ill-formed anyway, so no dependency can hold a different opinion about them. + +`-fno-exceptions` and `-fno-rtti` are **not** promoted, because a dependency can legitimately +disagree: they remove a language facility the dependency may use, and the consumer cannot make +that choice on its behalf. Left in `cxxflags` they reach every TU and not the prebuild, so the +build cannot succeed — mcpp refuses it before compiling and names the key: + +``` +error: `-fno-exceptions` changes the language dialect, but the `import std` BMI is + precompiled without it, so every importing translation unit will fail with + "language dialect differs". + Declare it as a dialect flag instead: + + [build] + dialect_cxxflags = ["-fno-exceptions"] +``` + +The check reads the **effective** flags, so it fires for the same flag written in +`[profile.] cxxflags` or in a `[target.…]` block. It does not fire when nothing in the +graph imports `std`, where the flag is an ordinary per-unit option that works. + ### 2.2 `[targets.]` — Build Targets ```toml diff --git a/docs/06-workspace.md b/docs/06-workspace.md index d1172241..fd083625 100644 --- a/docs/06-workspace.md +++ b/docs/06-workspace.md @@ -151,6 +151,102 @@ linkage = "static" default = "llvm@20.1.7" ``` +### 4.1 `[workspace.package]` and `[workspace.build]` + +Package metadata and build flags shared by every member are declared once at the +workspace root: + +```toml +[workspace] +members = ["libs/core", "libs/http", "apps/server"] + +[workspace.package] +standard = 26 # or "c++26"; both spellings are accepted +version = "0.4.2" +license = "Apache-2.0" +authors = ["example"] + +[workspace.build] +cxxflags = ["-Wall", "-Wextra"] +dialect_cxxflags = ["-fno-exceptions"] +``` + +A member then declares only what is its own: + +```toml +[package] +name = "core" +# standard, version, license and authors are inherited; +# [workspace.build] cxxflags are inherited +``` + +**The merge rule.** + +| kind | rule | +|---|---| +| scalars (`standard`, `version`, `license`, `c_standard`, `linkage`, …) | the member wins **when it declared the key**; otherwise the workspace value applies | +| vectors (`cxxflags`, `ldflags`, `defines`, `dialect_cxxflags`, `include_dirs`, …) | append, **workspace first** — so a member's own flag comes later on the command line, where it wins | +| `[workspace.dependencies]` | explicit opt-in per dependency, `x.workspace = true` (§3) | + +"Declared" means the key was written, not that its value differs from the +default. A member that deliberately pins `standard = "c++23"` under a +`[workspace.package] standard = 26` keeps c++23; a member that says nothing gets +c++26. Those two are the same value and opposite intents, which is why the +distinction is recorded rather than inferred. + +Scalars and vectors are inherited **implicitly**, without a per-key opt-in. The +drift a workspace exists to prevent is a member that forgot to opt in, so +inheritance is the default and overriding is what has to be stated. +Dependencies keep their explicit opt-in because a dependency is an edge in the +resolution graph: inheriting one implicitly would change what a member resolves +without its own manifest naming it. + +**`version` may be omitted by a member** when `[workspace.package]` supplies it. +It remains required overall — a member with neither is refused, naming both the +member and the workspace key that would have supplied it. + +**Not everything is inheritable.** `[workspace.build] allow_host_libs` is +refused. It disables the hermetic-link check for a specific artifact, and a +workspace root able to set it once would disable that check for members added +later by someone who never read the root manifest. Keys that describe *how to +build* are inheritable; keys that describe *which safety check not to run* stay +with the package whose artifact it is. Any other unknown key in +`[workspace.package]` / `[workspace.build]` is refused too, rather than ignored: +a key that is silently dropped from a table whose whole purpose is propagation +produces a workspace that looks configured and is not. + +**There is no `[workspace.target.]`.** A plain `[target.]` block +in the workspace root is already inherited by every member, per triple, with the +member winning. A second spelling for the same capability would be surface with +no function. + +### 4.2 One standard for the whole module graph + +A C++ module graph has exactly one standard: BMIs are not compatible across +levels, so the root package's `standard` is applied to every package in the +graph, including dependencies. A dependency's own `standard` is not applied. + +When a dependency **declares** a level higher than the graph is built at, mcpp +reports it before compiling: + +``` +warning: dependency `render` declares standard = "c++26", and this graph is + built at c++23 + impact: a C++ module graph has one standard, so the dependency's declaration + is not applied and its sources are compiled at the graph's level + hint: raise the consumer's standard to "c++26", or declare it once for + every member: + + [workspace.package] + standard = "c++26" +``` + +This is a warning rather than an error — such a build usually succeeds, and it +is promoted to an error by `--strict`. It is reported only for manifests the +project author controls (the root package, workspace members, and `path` +dependencies): a package resolved from an index carries a `standard` written by +a descriptor generator rather than by the person reading the message. + ## 5. Build Commands ### 5.1 Building & testing from the Workspace Root diff --git a/docs/zh/03-toolchains.md b/docs/zh/03-toolchains.md index 5f5eacba..32cf774a 100644 --- a/docs/zh/03-toolchains.md +++ b/docs/zh/03-toolchains.md @@ -284,6 +284,47 @@ pinned toolset 之间、以及与系统 Visual Studio 之间都可以共存。 > `[toolchain] … = "system"` —— 即 PATH 上的编译器 —— 是另一套、也是有意保留的 > 逃生口,不受影响。) +### `[toolchain] … = "system"` —— 拒绝 + +**mcpp 只用它自己管理的工具链构建。** `PATH` 上现成的编译器不受支持,该配置会被**拒绝**, +而不是提示: + +``` +error: [toolchain] linux = "system" is not supported: mcpp builds only with + toolchains it manages. + A compiler taken from PATH cannot be identified or reproduced, so + `import std` availability, the runtime closure and "the same build on + another machine" all stop being things mcpp can promise. + Name one instead — mcpp installs it on first use: + + [toolchain] + linux = "gcc@16.1.0" + + or set a machine default with `mcpp toolchain default gcc@16.1.0`, and + see `mcpp toolchain list` for what is available. +``` + +`msvc@system` 是**唯一的例外**,而且是另一种拼法:它点名的是一个**族**,mcpp 负责定位并识别 +其安装 —— 那是唯一一个编译器不能被重新分发的平台。见上一节。 + +#### 为什么工具链与库得到的答案不同 + +mcpp 对 host 依赖的规则并不是各条轴统一的,这个分叉是刻意的: + +- **mcpp 自身、以及 mcpp 生态发布的一切,都不依赖任何 host。** 工具链与 payload 都经由 + xlings 获得 —— xim 索引或 mcpp-index。这正是构建能跨机器、跨 Linux 发行版复现的原因。 +- **工具链属于这份契约,所以它不是工程可以从 host 拿的东西。** mcpp 承诺的每一件事 —— + `import std` 可用、运行期闭包可计算、同一份构建在同事机器上和 CI 里一致 —— 都是关于 + **一个 mcpp 解析出来、叫得出名字的编译器**的陈述。`PATH` 上的编译器让这些全部无法核验, + 这就是这一条是拒绝的原因。 +- **程序链接哪些库,是程序自己的事。** 工程可以链 host 的库,也可以链自己的 `.so`。mcpp 会 + 说明这样做的代价,并指出受支持的路径 —— 声明该 provider 让它从 mcpp-index 解析;索引尚未 + 收录时,**把包贡献进 mcpp-index** 就是那条路 —— 但只要结果能构建、能运行,就不强行拒绝。 + 产物是开发者的,由他保证。 + +而"证明跑不起来"的构建在两条轴上都仍然是错误:运行期闭包不可满足时会被拒绝,因为产物根本 +起不来。见[二进制分发](12-binary-distribution.md)。 + ### `msvc@system` —— 机器自己的 Visual Studio mcpp 只负责定位并识别已安装的 Visual Studio / Build Tools,**从不**安装、 diff --git a/docs/zh/05-mcpp-toml.md b/docs/zh/05-mcpp-toml.md index 2cbde30e..e7e8f063 100644 --- a/docs/zh/05-mcpp-toml.md +++ b/docs/zh/05-mcpp-toml.md @@ -70,6 +70,44 @@ repo = "https://github.com/user/myapp" # 仓库地址(可选) 如果源码在某个档位上 `import std;` 而解析出的工具链在该档位不提供 `std` 模块, mcpp 会在编译前失败,并同时报出工具链与工程档位。 +值的两种拼法都接受:`standard = "c++26"` 与 `standard = 26`。 + +当**依赖声明的档位高于当前图**时,mcpp 会在编译前说出来,而不是让它在那个依赖的源码里 +某处失败。见 [workspace §4.2](06-workspace.md)。 + +#### 方言标志与 `import std` BMI + +有些标志会改变标准库头文件**声明出什么**,因此预编译的 `import std` BMI 也必须带着它们一起 +构建。这就是 `[build] dialect_cxxflags` 的用途:它会被施加到 std BMI 预编译、模块扫描 +**以及**图中每一个 TU(依赖也包括在内)。 + +```toml +[build] +dialect_cxxflags = ["-fno-exceptions"] +``` + +其中少数几个标志,mcpp 在 `cxxflags` 里发现时会自动提升进这条通道 +(`-freflection`、`-fchar8_t`、`-D_GLIBCXX_USE_CXX11_ABI=…`)—— 混用这些标志的图本来就是 +病态的,任何依赖都不可能对它们持有另一种自洽的意见。 + +`-fno-exceptions` 与 `-fno-rtti` **不会**被自动提升,因为依赖可以合法地不同意:它们移除的是 +依赖可能正在使用的语言设施,而消费者无权替它做这个决定。留在 `cxxflags` 里,它们会到达每一个 +TU 却到不了预编译,于是构建不可能成功 —— mcpp 在编译前就拒绝,并指出该用哪个键: + +``` +error: `-fno-exceptions` changes the language dialect, but the `import std` BMI is + precompiled without it, so every importing translation unit will fail with + "language dialect differs". + Declare it as a dialect flag instead: + + [build] + dialect_cxxflags = ["-fno-exceptions"] +``` + +这项检查读的是**生效后的**标志集合,所以同一个标志写在 `[profile.] cxxflags` 或 +`[target.…]` 块里同样会被抓到。而当图中根本没有 `import std` 时它不触发 —— 那里它就是一个 +正常工作的按 TU 选项。 + ### 2.2 `[targets.]` — 构建目标 ```toml diff --git a/docs/zh/06-workspace.md b/docs/zh/06-workspace.md index c8ac3fca..baae2cff 100644 --- a/docs/zh/06-workspace.md +++ b/docs/zh/06-workspace.md @@ -151,6 +151,89 @@ linkage = "static" default = "llvm@20.1.7" ``` +### 4.1 `[workspace.package]` 与 `[workspace.build]` + +所有成员共享的包元信息与构建标志,在 workspace 根声明一次: + +```toml +[workspace] +members = ["libs/core", "libs/http", "apps/server"] + +[workspace.package] +standard = 26 # 也可写 "c++26",两种拼法都接受 +version = "0.4.2" +license = "Apache-2.0" +authors = ["example"] + +[workspace.build] +cxxflags = ["-Wall", "-Wextra"] +dialect_cxxflags = ["-fno-exceptions"] +``` + +成员只声明属于它自己的部分: + +```toml +[package] +name = "core" +# standard / version / license / authors 继承自 workspace +# [workspace.build] 的 cxxflags 也继承 +``` + +**合并规则。** + +| 类别 | 规则 | +|---|---| +| 标量(`standard`、`version`、`license`、`c_standard`、`linkage` 等) | 成员**声明了该键**时成员优先;否则取 workspace 的值 | +| 向量(`cxxflags`、`ldflags`、`defines`、`dialect_cxxflags`、`include_dirs` 等) | 追加,**workspace 在前** —— 成员自己的标志排在命令行后面,后者生效 | +| `[workspace.dependencies]` | 逐依赖显式选择加入,`x.workspace = true`(§3) | + +"声明了"指的是**这个键被写过**,而不是它的值与默认值不同。成员在 +`[workspace.package] standard = 26` 之下刻意写 `standard = "c++23"`,得到的就是 +c++23;什么都不写的成员得到 c++26。这两种情况的值相同而意图相反,所以这个事实是被 +**记录**下来的,而不是推断出来的。 + +标量与向量是**隐式继承**,不需要逐键选择加入。workspace 要消除的漂移正是"某个成员忘了 +选择加入",所以继承是默认行为,覆盖才是需要主动表达的动作。依赖保留显式选择加入,因为 +依赖是解析图上的一条**边**:隐式继承一条边,会在成员自己的 manifest 只字未提的情况下改变 +它解析到什么。 + +**成员可以省略 `version`**,只要 `[workspace.package]` 提供了它。这个字段整体上仍是必需的 +—— 两边都没有时会被拒绝,并同时指出成员文件和本该提供它的 workspace 键。 + +**并非所有键都可继承。** `[workspace.build] allow_host_libs` 会被拒绝:它关掉的是某个具体 +产物的 hermetic 链接检查,而 workspace 根若能设置一次,就等于替所有后来加入的成员也关掉了 +这项检查 —— 而那些成员的作者可能从没读过根 manifest。**描述"如何构建"的键可继承;描述 +"不要跑哪项安全检查"的键留在产物所属的那个包里。** `[workspace.package]` / +`[workspace.build]` 中其他不认识的键同样会被拒绝而不是忽略:一个以"传播"为唯一目的的表, +若能静默丢弃某个键,产出的就是"看起来配置好了、实际没有"的 workspace。 + +**没有 `[workspace.target.]`。** workspace 根里一个普通的 `[target.]` 块 +本来就会按 triple 逐项被所有成员继承(成员优先)。为同一能力再加一种拼法,只会增加接口面 +而不增加功能。 + +### 4.2 整个模块图只有一个标准 + +C++ 模块图有且只有一个标准:BMI 跨档位不兼容,因此根包的 `standard` 会施加到图中每一个包, +依赖也不例外。依赖自己的 `standard` 不会被应用。 + +当依赖**声明**了高于当前图的档位时,mcpp 在编译前就报出来: + +``` +warning: dependency `render` declares standard = "c++26", and this graph is + built at c++23 + impact: a C++ module graph has one standard, so the dependency's declaration + is not applied and its sources are compiled at the graph's level + hint: raise the consumer's standard to "c++26", or declare it once for + every member: + + [workspace.package] + standard = "c++26" +``` + +这是 warning 而不是 error —— 这类构建通常仍然成功;`--strict` 会把它提升为错误。它只对 +**工程作者自己拥有的 manifest** 生效(根包、workspace 成员、`path` 依赖):从索引解析来的 +包,其 `standard` 是描述符生成器写的,不是读到这条消息的人写的。 + ## 5. 构建命令 ### 5.1 从工作空间根目录构建与测试 diff --git a/mcpp.toml b/mcpp.toml index 411b6723..39ca2d93 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "2026.8.30.1" +version = "2026.8.30.2" description = "Modern C++ build & package management tool" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/modules/manifest/src/toml.cppm b/modules/manifest/src/toml.cppm index 1c61e222..ec293cc8 100644 --- a/modules/manifest/src/toml.cppm +++ b/modules/manifest/src/toml.cppm @@ -83,9 +83,28 @@ std::string version_req_problem(std::string_view spec) { export namespace mcpp::manifest { +// WHAT A MEMBER MANIFEST IS ALLOWED TO LEAVE OUT. +// +// `package.name` and `package.version` are required, and the parser cannot see +// that a manifest is a workspace MEMBER: a member has no `[workspace]` table of +// its own, so the file that would relax the rule is the one above it. Passing +// the fact in keeps the required-field check where it is while letting +// `[workspace.package]` actually be inheritable — a key that nothing could +// consume would be a recorded field with no reader, which is the defect these +// tables exist to remove rather than one to add. +// +// The requirement does not disappear. `inherit_workspace_config`'s caller +// raises it after inheritance, where "still missing" is knowable and the +// message can name both files. +struct LoadContext { + bool insideWorkspace = false; +}; + std::expected parse_string(std::string_view content, - const std::filesystem::path& origin = "mcpp.toml"); -std::expected load(const std::filesystem::path& path); + const std::filesystem::path& origin = "mcpp.toml", + LoadContext ctx = {}); +std::expected load(const std::filesystem::path& path, + LoadContext ctx = {}); // For `mcpp new` scaffolding. std::string default_template(std::string_view packageName); @@ -235,7 +254,8 @@ std::optional find_disallowed_array_of_tables( } // namespace std::expected parse_string(std::string_view content, - const std::filesystem::path& origin) { + const std::filesystem::path& origin, + LoadContext ctx) { auto doc = t::parse(content); if (!doc) { return std::unexpected(error(origin, doc.error().message, doc.error().where)); @@ -270,7 +290,7 @@ std::expected parse_string(std::string_view content, return std::unexpected(error(origin, "missing required [package] section")); auto name = doc->get_string("package.name"); - if (!name && !has_workspace) + if (!name && !has_workspace && !ctx.insideWorkspace) return std::unexpected(error(origin, "missing required field 'package.name'")); if (name) m.package.name = *name; @@ -280,7 +300,7 @@ std::expected parse_string(std::string_view content, if (auto v = doc->get_string("package.namespace")) m.package.namespace_ = *v; auto version = doc->get_string("package.version"); - if (!version && !has_workspace) + if (!version && !has_workspace && !ctx.insideWorkspace) return std::unexpected(error(origin, "missing required field 'package.version'")); if (version) m.package.version = *version; @@ -291,7 +311,29 @@ std::expected parse_string(std::string_view content, if (auto v = doc->get_string_array("package.platforms")) m.package.platforms = *v; // [package].standard (M5.0 new home) - if (auto v = doc->get_string("package.standard")) m.package.standard = *v; + if (auto v = doc->get_string("package.standard")) { + m.package.standard = *v; + // Recorded HERE, where the key's presence is a fact rather than an + // inference. Both spellings count as a declaration; the deprecated + // `[language] standard` below is the same statement in an older place. + m.package.standardDeclared = true; + } else if (auto n = doc->get_int("package.standard")) { + // `standard = 26` — WRITTEN BY USERS AND SILENTLY IGNORED UNTIL NOW. + // + // The key is documented as a string, `get_string` returns nothing for a + // bare integer, and the project compiled at the default with no + // diagnostic. Measured on the released engine: `standard = 26` produced + // `-std=c++23`. Issue #527 writes it that way in three of its examples, + // so a reader following the issue got a build that ignored the line + // they were told to add. + // + // Accepted rather than refused because the mapping is unambiguous and + // the intent is not in question; an integer that is not a standard + // level still goes through `normalize_cpp_standard` below and is + // refused there, with that function's list of accepted spellings. + m.package.standard = std::format("c++{}", *n); + m.package.standardDeclared = true; + } // [language] (M5.0: deprecated, kept for backward compat — drop in M6) // Reads to old fields AND mirrors to new package.standard if [package].standard not set. @@ -300,6 +342,7 @@ std::expected parse_string(std::string_view content, m.language.standard = *v; // mirror to new home only if [package].standard wasn't explicitly set if (!doc->get_string("package.standard")) m.package.standard = *v; + m.package.standardDeclared = true; } else { m.language.standard = m.package.standard; // keep old field consistent with new } @@ -1987,6 +2030,112 @@ std::expected parse_string(std::string_view content, if (auto v = doc->get_string_array("workspace.exclude")) m.workspace.exclude = *v; + // [workspace.package] — package metadata every member inherits unless + // it declares its own. `name` is absent on purpose: two members cannot + // share one, and a workspace able to set it would be describing a + // single package. + if (auto* wpkg = doc->get_table("workspace.package")) { + auto& inh = m.workspace.inherited; + // Both spellings, for the same reason `[package] standard` takes + // both: the integer form is what people write. + std::optional wsStd; + if (auto v = doc->get_string("workspace.package.standard")) wsStd = *v; + else if (auto n = doc->get_int("workspace.package.standard")) + wsStd = std::format("c++{}", *n); + if (wsStd) { + // Normalised HERE so a member inheriting it gets the same + // canonical spelling a member declaring it would, and so an + // invalid value is reported against the line that wrote it + // rather than against whichever member inherited it first. + auto cfg = normalize_cpp_standard(*wsStd); + if (!cfg) return std::unexpected(error(origin, std::format( + "[workspace.package].standard: {}", cfg.error()))); + inh.standard = cfg->canonical; + inh.standardDeclared = true; + } + if (auto v = doc->get_string("workspace.package.version")) + inh.version = *v; + if (auto v = doc->get_string("workspace.package.license")) + inh.license = *v; + if (auto v = doc->get_string("workspace.package.description")) + inh.description = *v; + if (auto v = doc->get_string("workspace.package.repo")) + inh.repo = *v; + if (auto v = doc->get_string_array("workspace.package.authors")) + inh.authors = *v; + static constexpr std::string_view kKnown[] = { + "standard", "version", "license", "description", "repo", "authors", + }; + for (auto& [key, ignored] : *wpkg) { + (void)ignored; + if (std::ranges::find(kKnown, key) != std::end(kKnown)) continue; + // REFUSED, not ignored. A key in a table whose entire purpose + // is to propagate is either propagated or reported; silently + // dropping it produces a workspace that looks configured and + // is not, which is the defect this table was added to fix. + return std::unexpected(error(origin, std::format( + "[workspace.package] has no key '{}'. Supported: " + "standard, version, license, description, repo, authors. " + "`name` is per-member by definition.", key))); + } + } + + // [workspace.build] — the inheritable subset of [build]. + if (auto* wbuild = doc->get_table("workspace.build")) { + auto& b = m.workspace.inherited.build; + m.workspace.inherited.buildPresent = true; + if (auto v = doc->get_string_array("workspace.build.cflags")) b.cflags = *v; + if (auto v = doc->get_string_array("workspace.build.cxxflags")) b.cxxflags = *v; + if (auto v = doc->get_string_array("workspace.build.ldflags")) b.ldflags = *v; + if (auto v = doc->get_string_array("workspace.build.defines")) b.defines = *v; + if (auto v = doc->get_string_array("workspace.build.dialect_cxxflags")) + b.dialectCxxflags = *v; + if (auto v = doc->get_string_array("workspace.build.include_dirs")) + for (auto& d : *v) b.includeDirs.emplace_back(d); + if (auto v = doc->get_string_array("workspace.build.include_dirs_after")) + for (auto& d : *v) b.includeDirsAfter.emplace_back(d); + if (auto v = doc->get_string_array("workspace.build.private_include_dirs")) + for (auto& d : *v) b.privateIncludeDirs.emplace_back(d); + if (auto v = doc->get_string("workspace.build.c_standard")) b.cStandard = *v; + if (auto v = doc->get_string("workspace.build.linkage")) b.linkage = *v; + if (auto v = doc->get_string("workspace.build.target")) b.target = *v; + if (auto v = doc->get_string("workspace.build.cxx_runtime")) b.cxxRuntime = *v; + if (auto v = doc->get_string("workspace.build.dependency_linkage")) + b.dependencyLinkage = *v; + if (auto v = doc->get_string("workspace.build.macos_deployment_target")) + b.macosDeploymentTarget = *v; + static constexpr std::string_view kKnown[] = { + "cflags", "cxxflags", "ldflags", "defines", "dialect_cxxflags", + "include_dirs", "include_dirs_after", "private_include_dirs", + "c_standard", "linkage", "target", "cxx_runtime", + "dependency_linkage", "macos_deployment_target", + }; + for (auto& [key, ignored] : *wbuild) { + (void)ignored; + if (std::ranges::find(kKnown, key) != std::end(kKnown)) continue; + // `allow_host_libs` is named explicitly because refusing it is + // a decision and not an omission: it turns a correctness gate + // off, and a workspace root that could set it once would + // disable that gate for members added later by someone who + // never read this file. Keys that say HOW TO BUILD are + // inheritable; keys that say WHICH CHECK NOT TO RUN stay with + // the package whose artifact it is. + if (key == "allow_host_libs") + return std::unexpected(error(origin, + "[workspace.build] allow_host_libs is not inheritable. " + "It disables the hermetic-link check for a specific " + "artifact, so it belongs in that package's own [build] " + "table where the person turning it off owns the result.")); + return std::unexpected(error(origin, std::format( + "[workspace.build] has no key '{}' (or it is not " + "inheritable). Supported: cflags, cxxflags, ldflags, " + "defines, dialect_cxxflags, include_dirs, " + "include_dirs_after, private_include_dirs, c_standard, " + "linkage, target, cxx_runtime, dependency_linkage, " + "macos_deployment_target.", key))); + } + } + // [workspace.dependencies] — versions that members inherit via .workspace = true. if (auto* wdeps = doc->get_table("workspace.dependencies")) { for (auto& [k, v] : *wdeps) { @@ -2216,7 +2365,8 @@ void apply_defaults_and_infer(Manifest& m, const std::filesystem::path& root) { } // namespace -std::expected load(const std::filesystem::path& path) { +std::expected load(const std::filesystem::path& path, + LoadContext ctx) { std::ifstream is(path); if (!is) { return std::unexpected(ManifestError{ @@ -2225,7 +2375,7 @@ std::expected load(const std::filesystem::path& path) { } std::stringstream ss; ss << is.rdbuf(); - auto m = parse_string(ss.str(), path); + auto m = parse_string(ss.str(), path, ctx); if (!m) return m; // M5.0: defaults + target inference (uses filesystem context relative to mcpp.toml). diff --git a/modules/manifest/src/types.cppm b/modules/manifest/src/types.cppm index 593e3f62..5ce20798 100644 --- a/modules/manifest/src/types.cppm +++ b/modules/manifest/src/types.cppm @@ -39,6 +39,32 @@ struct Package { std::string namespace_; // xpkg V1 namespace field (0.0.6+); empty = infer from name std::string version; std::string standard = "c++23"; // C++ standard (M5.0: moved from [language]) + // DID THE AUTHOR WRITE `standard`, or is this the default? + // + // The two are the same bytes in `standard` and they need opposite + // treatment. `[workspace.package] standard` means "use the workspace value + // where the member did not say", which is unanswerable without this bit: + // inheriting over a member that deliberately pinned c++23 and inheriting + // into one that said nothing are the same operation otherwise. + // + // The cpp20 design doc's §9-Q3 recorded this as the precondition for a + // dependency floor check and declined to add it while nothing consumed it. + // Workspace inheritance is that consumer, and the floor check is now its + // second one. + // + // MUST BE SET BY BOTH PARSE PATHS. `toml.cppm` reads an author's + // `mcpp.toml`; `xpkg.cppm` synthesises a manifest from an index descriptor + // and writes `"c++23"` unconditionally before reading `language`. Setting it + // in one place only would be the same decision derived twice, which is what + // §9-Q3 warned about. + // + // NOTE FOR THE FLOOR CHECK: an index descriptor declaring `language` sets + // this too, and measured over the local registry every descriptor with an + // mcpp segment does (782 of 782, 756 of them C libraries carrying a + // boilerplate "c++23"). Declaredness alone therefore does NOT mean "the + // author asked for it" outside author-owned manifests — see the scope gate + // in prepare.cppm. + bool standardDeclared = false; std::string description; std::string license; std::vector authors; @@ -924,10 +950,74 @@ struct PackConfig { // // Virtual workspace (no [package]): pure management node. // Rooted workspace ([package] + [workspace]): root is also a package. +// THE MERGE DISCIPLINE, STATED ONCE. +// +// Four keys were inherited before these tables existed — `[toolchain]`, +// `[target.]`, `[indices]` and `[workspace.dependencies]` — under three +// different rules, none of them written down. That is how the fifth key gets +// whichever rule its author happened to read. The rule for everything here: +// +// SCALARS the member wins when it DECLARED the key; otherwise the +// workspace value applies. "Declared" is a fact the parser +// records, not a comparison against a default — see +// Package::standardDeclared for why the difference is load-bearing. +// +// VECTORS append, workspace first. A member adds to the shared set rather +// than restating it, and the workspace flag comes first so a +// member's flag can override it on the command line where later +// wins. A member that needs to NOT have a workspace flag is a +// signal the flag was declared at the wrong altitude. +// +// DEPENDENCIES keep their explicit `x.workspace = true` opt-in, and that +// exception is deliberate rather than historical: a dependency is +// an EDGE in the resolution graph, and inheriting edges implicitly +// would change what a member resolves without the member's manifest +// naming it. +// +// Implicit-if-absent rather than cargo's per-key `x.workspace = true` for +// package and build keys, because the drift this exists to remove is a member +// that FORGOT to opt in. Making inheritance the default makes drift the thing +// you have to ask for. +// +// NOT EVERY KEY IS INHERITABLE. `[build] allow_host_libs` turns a correctness +// gate off; a workspace root setting it once would disable that gate for every +// member, including members added later by someone who never read the root +// manifest. Keys that describe HOW TO BUILD are inheritable; keys that describe +// WHICH SAFETY CHECK NOT TO RUN stay with the package whose artifact it is. +struct WorkspaceInherited { + // `[workspace.package]` — the subset that is meaningful to share. `name` is + // deliberately absent: two members cannot have one name, and a workspace + // that could set it would be describing a single package. + std::string standard; + bool standardDeclared = false; + std::string version; + std::string license; + std::string description; + std::string repo; + std::vector authors; + // `[workspace.build]` — the INHERITABLE SUBSET of `[build]`, and the subset + // is a stated list rather than "whatever [build] happens to carry". A key + // that is not in it is refused at parse time with the reason, because + // silently ignoring a key in a table whose whole purpose is propagation is + // the failure this table exists to remove. + BuildConfig build; + bool buildPresent = false; + + // THERE IS NO `[workspace.target.]`, DELIBERATELY. + // + // A plain `[target.]` block in the workspace root manifest is + // ALREADY inherited by every member, per triple, member-wins — that + // predates these tables. Adding a second spelling for a capability that + // exists would be surface with no function, and two spellings of one rule + // is how the two acquire different behaviour later. Documented in docs/05 + // rather than implemented here. +}; + struct WorkspaceConfig { std::vector members; // relative paths to member dirs std::vector exclude; // paths to exclude std::map dependencies; // [workspace.dependencies] + WorkspaceInherited inherited; // [workspace.package|build|target.*] bool present = false; }; @@ -1221,6 +1311,27 @@ std::vector dialect_flags(const BuildConfig& bc); // libstdc++/libc++ headers declare, or participates in BMI dialect checks). bool is_dialect_flag(std::string_view flag); +// True when `flag` changes the language dialect the standard library BMI is +// compiled with, but is deliberately NOT auto-promoted into the graph-global +// set (`-fno-exceptions`, `-fno-rtti`, and their MSVC spellings). See the +// implementation for why the list is split in two rather than merged. +// +// Disjoint from `is_dialect_flag` by construction: a flag is promoted or it is +// recognised-and-refused, never both, so a caller cannot double-count one. +bool is_unpromoted_dialect_flag(std::string_view flag); + +// The dialect-class flags present in `flags` that will NOT reach the `import +// std` BMI prebuild — the exact set that makes an importing TU fail with +// "language dialect differs". Empty when there is nothing to say. +// +// `flags` is the EFFECTIVE per-unit set, not `[build] cxxflags`: the same flag +// arrives from `[profile.] cxxflags`, from `[target..build]`, and +// from a `cfg(...)` block, and all three reach the compile line while none +// reaches the prebuild. A check that reads one table is silent on three +// spellings of one mistake. +std::vector dialect_flags_missing_from_prebuild( + std::span flags, std::span prebuild); + // The lib root's CONVENTIONAL name: `src/.cppm`, or `[lib] path` // when the manifest states one. It does not touch the filesystem, so it is the // right answer for a diagnostic or a validator's expectation and the wrong one @@ -1283,8 +1394,9 @@ std::optional validate_target_soname(const Target& t, bool is_dialect_flag(std::string_view flag) { // Deliberately conservative first list (design doc §1.3a): - // -fno-exceptions / -fno-rtti stay per-unit until separately reviewed - // (dependencies may assume exceptions are available). + // -fno-exceptions / -fno-rtti stay per-unit — see + // `is_unpromoted_dialect_flag` below for why, and for what now happens + // instead of silence. static constexpr std::string_view exact[] = { "-freflection", "-fno-reflection", // P2996 (GCC 16+) "-fcontracts", "-fno-contracts", // P2900 @@ -1297,6 +1409,66 @@ bool is_dialect_flag(std::string_view flag) { return false; } +bool is_unpromoted_dialect_flag(std::string_view flag) { + // THE SECOND TIER, AND WHY THE LIST IS SPLIT AT ALL. + // + // A flag is AUTO-PROMOTED (the list above) when a graph that mixes it is + // ill-formed anyway: `-freflection`, `-fchar8_t` and the libstdc++ dual-ABI + // macro change what the standard library headers DECLARE, so no dependency + // can hold a coherent different opinion and promoting is the only outcome + // that can work. + // + // A flag is NOT auto-promoted when a dependency can legitimately disagree. + // `-fno-exceptions` and `-fno-rtti` remove a language facility the + // dependency may use, and the consumer cannot make that decision on its + // behalf: promoting them would compile every dependency without exceptions + // because the root package asked for it, and the failure would land in + // source the user does not own. + // + // WHAT WAS MISSING WAS THE THIRD OPTION. Left in `cxxflags`, these flags + // reach every TU but not the `import std` BMI prebuild, so the compiler + // refuses the BMI it was handed: + // + // std: error: language dialect differs 'C++23', expected + // 'C++23/no-exceptions' + // + // Recognising them here does not promote them. It lets the build refuse + // BEFORE compiling, naming `dialect_cxxflags` — the key that does apply to + // the prebuild — instead of leaving the user with a compiler message about + // a file mcpp generated. + static constexpr std::string_view exact[] = { + "-fno-exceptions", "-fexceptions", + "-fno-rtti", "-frtti", + // The MSVC spellings of the same two axes. `cl` bakes the choice into + // the module the same way it bakes `_MSVC_MT`/`_MSVC_MD` in — which + // `stdmod::ensure_built` already threads through for the CRT — so the + // mismatch class exists there too. A GNU-only list would make this + // check silent on one of the three supported toolchains, and silence + // is what it exists to remove. + "/EHsc", "/EHs-c-", "/EHa", "/EHac", "/GR", "/GR-", + }; + for (auto e : exact) + if (flag == e) return true; + return false; +} + +std::vector dialect_flags_missing_from_prebuild( + std::span flags, std::span prebuild) { + std::vector out; + for (auto const& f : flags) { + // An auto-promoted flag is already in `prebuild` by construction, so + // asking the membership question covers both tiers with one rule + // rather than special-casing the promoted list here — and it stays + // correct on the day a flag moves from one tier to the other. + if (!is_dialect_flag(f) && !is_unpromoted_dialect_flag(f)) continue; + if (std::find(prebuild.begin(), prebuild.end(), f) != prebuild.end()) + continue; + if (std::find(out.begin(), out.end(), f) == out.end()) + out.push_back(f); + } + return out; +} + std::vector dialect_flags(const BuildConfig& bc) { std::vector out; auto add = [&](const std::string& f) { diff --git a/modules/manifest/src/xpkg.cppm b/modules/manifest/src/xpkg.cppm index eb526114..1d3b8cda 100644 --- a/modules/manifest/src/xpkg.cppm +++ b/modules/manifest/src/xpkg.cppm @@ -1201,6 +1201,15 @@ synthesize_from_xpkg_lua(std::string_view luaContent, if (!v.empty()) { m.language.standard = v; m.package.standard = v; + // The second of the two parse paths that must fill this, per + // Package::standardDeclared. It is set for the same reason it + // is set in toml.cppm — the key was present — and NOT as a + // statement that the package requires that level: a descriptor + // generator writes `language` for every package, including C + // libraries with `import_std = false`. Consumers that need + // "the author asked for it" must additionally scope to + // author-owned manifests. + m.package.standardDeclared = true; } } else if (key == "import_std") { diff --git a/modules/versioning/src/version.cppm b/modules/versioning/src/version.cppm index d8bd4e53..fb5f0929 100644 --- a/modules/versioning/src/version.cppm +++ b/modules/versioning/src/version.cppm @@ -31,6 +31,6 @@ import std; export namespace mcpp { -inline constexpr std::string_view MCPP_VERSION = "2026.8.30.1"; +inline constexpr std::string_view MCPP_VERSION = "2026.8.30.2"; } // namespace mcpp diff --git a/src/build/execute.cppm b/src/build/execute.cppm index 1799ac68..0bf3eaf4 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -101,6 +101,20 @@ struct BuildCacheEntry { // Exact immutable snapshot used by the build. Optional distinguishes a // current cache from one written by an older mcpp (or a corrupt payload). std::optional runtimeBinding; + // Source trees outside `projectRoot` that this build read — `path` + // dependencies, which is what workspace members are to each other. The + // staleness sweep has to cover them or a NEW FILE appearing in one is + // invisible: ninja has no edge for a file that did not exist when + // build.ninja was written, so `mcpp build` replays the stale graph and + // reports success. See BuildContext::depSourceRoots. + std::vector depSourceRoots; + // Was the block present at all? An EMPTY list is a legitimate answer — a + // project with no path dependencies has none — so it cannot stand in for + // "this cache predates the field", and the two need opposite treatment: + // the first takes the fast path, the second must fall through once so the + // list gets written. Same discipline as `subosRecorded` above, and for the + // same reason. + bool depSourceRootsRecorded = false; }; std::vector read_build_cache(const std::filesystem::path& projectRoot) { @@ -194,6 +208,18 @@ std::vector read_build_cache(const std::filesystem::path& proje e.cacheMode = line.substr(10); haveNextLine = static_cast(std::getline(f, line)); } + // Count-prefixed, like `runTargets=` above and for the same reason: a + // zero-length list and an absent block must not read the same. Absent + // means the cache predates the field, and the fast path then declines + // once so the next write records it. + if (haveNextLine && line.starts_with("depSourceRoots=")) { + std::size_t n = 0; + try { n = std::stoul(line.substr(15)); } catch (...) { n = 0; } + for (std::size_t i = 0; i < n && std::getline(f, line); ++i) + e.depSourceRoots.push_back(line); + e.depSourceRootsRecorded = true; + haveNextLine = static_cast(std::getline(f, line)); + } entries.push_back(std::move(e)); if (!haveNextLine || line.empty()) break; } @@ -217,7 +243,8 @@ void write_build_cache(const std::filesystem::path& projectRoot, const std::string& runEnvValue = "", const std::string& profile = "", const std::string& cacheMode = "", - const mcpp::platform::runtime::RuntimeBinding& runtimeBinding = {}) { + const mcpp::platform::runtime::RuntimeBinding& runtimeBinding = {}, + std::vector depSourceRoots = {}) { auto path = projectRoot / kBuildCacheFile; auto entries = read_build_cache(projectRoot); @@ -236,6 +263,8 @@ void write_build_cache(const std::filesystem::path& projectRoot, /*subosRecorded=*/true, profile, cacheMode}; newEntry.runtimeBinding = runtimeBinding; + newEntry.depSourceRoots = std::move(depSourceRoots); + newEntry.depSourceRootsRecorded = true; entries.insert(entries.begin(), std::move(newEntry)); // Trim to LRU capacity. @@ -274,6 +303,8 @@ void write_build_cache_entries(const std::filesystem::path& path, << '\n'; f << "profile=" << e.profile << '\n'; f << "cacheMode=" << e.cacheMode << '\n'; + f << "depSourceRoots=" << e.depSourceRoots.size() << '\n'; + for (auto& r : e.depSourceRoots) f << r << '\n'; } } @@ -605,7 +636,14 @@ export int run_build_plan(BuildContext& ctx, bool verbose, bool no_cache, r->runtimeEnvValue, std::move(runTargets), runEnvKey, runEnvValue, ctx.profile, std::string(cache_mode_name(ctx.cacheMode)), - ctx.plan.runtimeBinding); + ctx.plan.runtimeBinding, + [&] { + std::vector v; + v.reserve(ctx.depSourceRoots.size()); + for (auto const& r : ctx.depSourceRoots) + v.push_back(r.generic_string()); + return v; + }()); } // The one place the --strict policy is settled. Degradations reported by @@ -711,6 +749,48 @@ bool sources_newer_than(const std::filesystem::path& projectRoot, return false; } +// The same question, asked of a source tree that is not the project's own. +// +// WHY THIS IS NOT COVERED BY THE SWEEP ABOVE, AND WHY NINJA DOES NOT COVER IT +// EITHER. A `path` dependency's translation units are compiled by edges in the +// SAME build.ninja, so an EDIT to one of its existing files is caught: ninja +// rebuilds the object, the link output moves, and `artifact_snapshot_unchanged` +// abandons the fast path afterwards. A NEW FILE has no edge at all. Nothing in +// the graph mentions it, every recorded timestamp is unmoved, and the fast path +// replays a build.ninja that predates it — measured, `mcpp build` printed +// `Finished dev in 0.00s` and the module was never compiled. That is #359's +// shape ("a GLOB input changes without any existing file's mtime changing") +// applied to a directory the original fix did not reach. +// +// It matters most exactly where it is hardest to notice: members of a workspace +// depend on one another by `path`, so for a workspace this is not an edge case +// but the ordinary arrangement. +// +// The dependency's own manifest is swept too. A member that gains a target, a +// `[modules] sources` glob or a `[build]` flag changes what the graph SHOULD +// be, and none of that is visible from its source files' timestamps. +bool dep_sources_newer_than(const std::vector& depSourceRoots, + std::filesystem::file_time_type ninjaTime, + const mcpp::ExtensionTable& extTable) { + std::error_code ec; + for (auto const& rootStr : depSourceRoots) { + std::filesystem::path depRoot(rootStr); + // A dependency directory that has gone away is a resolution question, + // not a staleness one: fall through to prepare_build, which reports it + // with the dependency's name instead of a bare missing path. + if (!std::filesystem::is_directory(depRoot, ec)) { ec.clear(); return true; } + auto tomlTime = std::filesystem::last_write_time(depRoot / "mcpp.toml", ec); + if (ec) { ec.clear(); return true; } + if (tomlTime > ninjaTime) return true; + for (auto& f : mcpp::modgraph::expand_glob(depRoot, "src/**/*")) { + if (!mcpp::affects_graph_shape(mcpp::classify(f, extTable))) continue; + auto ft = std::filesystem::last_write_time(f, ec); + if (ec || ft > ninjaTime) return true; + } + } + return false; +} + // mcpp#225: run ninja quietly against an already-verified-fresh build.ninja. // Shared by try_fast_build (which just reports "Finished" on success) and // try_fast_run (which goes on to locate + exec a binary). Returns nullopt @@ -916,6 +996,14 @@ export std::optional try_fast_build(const std::filesystem::path& projectRoo // instead of a hand-rolled recursive_directory_iterator over src/. if (sources_newer_than(projectRoot, ninjaTime, want->resourceScripts, want->extTable)) return std::nullopt; + // A cache written before this field existed cannot say whether the build + // had `path` dependencies, and answering "assume none" is the wrong + // half of that guess: it would keep replaying a stale graph for exactly + // the projects the field was added for. Decline once; the write below + // records the list and every later invocation is fast again. + if (!match->depSourceRootsRecorded) return std::nullopt; + if (dep_sources_newer_than(match->depSourceRoots, ninjaTime, want->extTable)) + return std::nullopt; auto validatedBefore = mcpp::build::runtime_validation::validated_artifact_snapshot( @@ -1042,6 +1130,12 @@ std::optional try_fast_run(const std::filesystem::path& projectRoot, if (sources_newer_than(projectRoot, ninjaTime, want->resourceScripts, want->extTable)) return std::nullopt; + // Same gate as try_fast_build's, and it has to be BOTH places: `mcpp run` + // reaches its binary through this path, so a `run` that skipped the check + // would execute an artifact built from a source set that no longer exists. + if (!match->depSourceRootsRecorded) return std::nullopt; + if (dep_sources_newer_than(match->depSourceRoots, ninjaTime, want->extTable)) + return std::nullopt; auto validatedBefore = mcpp::build::runtime_validation::validated_artifact_snapshot( diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index fdccc3ec..4a1a9aa6 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -588,6 +588,23 @@ export struct BuildContext { mcpp::xlings::runtime::RuntimeSelection runtimeSelection; mcpp::platform::runtime::RuntimeBinding runtimeBinding; std::filesystem::path projectRoot; + // THE SOURCE TREES THIS BUILD READ THAT ARE NOT UNDER `projectRoot`. + // + // A `path` dependency — which is what every workspace member is to its + // siblings — contributes translation units from a directory the fast path + // has no way to name. `sources_newer_than` sweeps the project being built, + // so a NEW FILE appearing in such a tree is invisible to it: ninja cannot + // report an edge that was never emitted, and the fast path replays a + // build.ninja that predates the file. Measured before this field existed: + // `mcpp build` printed `Finished dev in 0.00s` and the module was never + // compiled. + // + // Recorded rather than re-derived, because the authoritative answer is + // which packages this build ACTUALLY read from source — the fast path + // cannot resolve dependencies without becoming prepare_build, and a second + // derivation would drift from the first exactly when a resolution rule + // changes. Written into `.build_cache`; see BuildCacheEntry::depSourceRoots. + std::vector depSourceRoots; std::filesystem::path outputDir; std::filesystem::path stdBmi; std::filesystem::path stdObject; @@ -1024,6 +1041,17 @@ prepare_build(bool print_fingerprint, ? std::expected( *overrides.preloaded_manifest) : mcpp::manifest::load(*root / "mcpp.toml"); + // A COMMAND ISSUED INSIDE A MEMBER DIRECTORY loads that member's manifest + // here, before anything knows a workspace is above it — so a member relying + // on `[workspace.package]` for a required field would be refused by the + // parser before inheritance could supply it. Retried, not reordered: the + // workspace lookup walks the tree reading manifests, and paying that on + // every build to serve the error path would be the wrong trade. The + // requirement still holds; it is enforced after inheritance, where "still + // missing" is knowable. + if (!m && !overrides.preloaded_manifest + && !mcpp::project::find_workspace_root(*root).empty()) + m = mcpp::manifest::load(*root / "mcpp.toml", {.insideWorkspace = true}); if (!m) return std::unexpected(m.error().format()); // ⚠️ AND ONLY FOR THE ROOT. A layer name this engine does not know is a @@ -1088,7 +1116,8 @@ prepare_build(bool print_fingerprint, // Virtual workspace: find a member with a binary target, or use last member. for (auto& mp : m->workspace.members) { auto memberDir = *root / mp; - auto mm = mcpp::manifest::load(memberDir / "mcpp.toml"); + auto mm = mcpp::manifest::load(memberDir / "mcpp.toml", + {.insideWorkspace = true}); if (!mm) continue; for (auto& t : mm->targets) { if (t.kind == mcpp::manifest::Target::Binary) { @@ -1112,30 +1141,18 @@ prepare_build(bool print_fingerprint, } runtimeWorkspaceRoot = *root; wsManifest = std::move(*m); // preserve workspace manifest - m = mcpp::manifest::load(memberDir / "mcpp.toml"); + m = mcpp::manifest::load(memberDir / "mcpp.toml", + {.insideWorkspace = true}); if (!m) return std::unexpected(std::format( "workspace member '{}': {}", targetMember, m.error().format())); - // Merge workspace dependency versions/paths. `*root` is still the - // WORKSPACE root here (the `root = memberDir` reassignment below - // hasn't happened yet), so it anchors any relative `path` in - // `[workspace.dependencies]` (#224). - mcpp::project::merge_workspace_deps(*m, *wsManifest, *root); - - // Inherit workspace toolchain if member doesn't define one - if (m->toolchain.byPlatform.empty()) { - m->toolchain = wsManifest->toolchain; - } - // Inherit workspace target overrides - for (auto& [triple, entry] : wsManifest->targetOverrides) { - if (!m->targetOverrides.contains(triple)) { - m->targetOverrides[triple] = entry; - } - } - // Inherit workspace indices if member doesn't define any. `*root` - // is still the workspace root here, which is what a relative - // `[indices].path` was written against (#224). - mcpp::project::inherit_workspace_indices(*m, *wsManifest, *root); + // ONE call, not a hand-copied list. `*root` is still the WORKSPACE + // root here (the `root = memberDir` reassignment below has not + // happened yet), which is what a relative `[indices].path` or + // `[workspace.dependencies] path` was written against (#224). + mcpp::project::inherit_workspace_config(*m, *wsManifest, *root); + if (auto bad = mcpp::project::workspace_inheritance_error(*m, memberDir)) + return std::unexpected(*bad); mcpp::ui::status("Workspace", std::format("building member '{}'", targetMember)); root = memberDir; @@ -1148,19 +1165,13 @@ prepare_build(bool print_fingerprint, if (wsm && wsm->workspace.present) { runtimeWorkspaceRoot = wsRoot; wsManifest = std::move(*wsm); - // #224: anchor relative `path`/`[indices].path` to the - // workspace root, not this member's own directory. - mcpp::project::merge_workspace_deps(*m, *wsManifest, wsRoot); - if (m->toolchain.byPlatform.empty()) { - m->toolchain = wsManifest->toolchain; - } - for (auto& [triple, entry] : wsManifest->targetOverrides) { - if (!m->targetOverrides.contains(triple)) { - m->targetOverrides[triple] = entry; - } - } - // Inherit workspace indices if member doesn't define any - mcpp::project::inherit_workspace_indices(*m, *wsManifest, wsRoot); + // The SECOND inheritance site, and it calls the same function + // as the first for that reason. #224: relative `path` and + // `[indices].path` anchor to the workspace root, not to this + // member's own directory. + mcpp::project::inherit_workspace_config(*m, *wsManifest, wsRoot); + if (auto bad = mcpp::project::workspace_inheritance_error(*m, *root)) + return std::unexpected(*bad); } } } @@ -1188,6 +1199,7 @@ prepare_build(bool print_fingerprint, std::error_code wdEc; std::filesystem::create_directories(workRoot, wdEc); } + if (m->package.sourceProvenance.empty()) { m->package.sourceProvenance = "path+" + root->lexically_normal().generic_string(); @@ -1468,6 +1480,22 @@ prepare_build(bool print_fingerprint, pr.ldflags.begin(), pr.ldflags.end()); } + // Every directory a package payload may legitimately have been INSTALLED + // into: the global registry, plus the two project-local data roots a custom + // git index installs into. Defined HERE, above its first use, because three + // separate questions now depend on the same answer — where a dependency's + // cache address is anchored, whether its sources came from a store at all, + // and whether a `standard` declaration in its manifest was written by an + // author or by a descriptor generator. One definition, three uses; deriving + // the same fact twice is how two of them start disagreeing. + const auto storeRoots = [&]() -> std::vector { + std::vector roots; + if (auto c = get_cfg()) roots.push_back((*c)->xlingsHome() / "data" / "xpkgs"); + for (auto& d : mcpp::config::project_xlings_data_roots(workRoot)) + roots.push_back(d / "xpkgs"); + return roots; + }(); + // [package] platforms — fixed vocabulary owned by mcpp (it owns the // target/triple system). Unknown values: warning, or error under --strict. for (auto& pf : m->package.platforms) { @@ -2158,7 +2186,52 @@ prepare_build(bool print_fingerprint, chosenBy)); } } else if (tcSpec.has_value() && *tcSpec == "system") { - // Explicit user opt-in to system PATH compiler — kept as escape hatch. + // REFUSED. THE COMPILER IS THE ONE AXIS THAT IS NOT THE PROJECT'S TO + // TAKE FROM THE HOST. + // + // mcpp's host-dependence policy is not uniform across axes, and the + // split is the point rather than an inconsistency: + // + // LIBRARIES are the program's business. A project may link a host + // library or its own `.so`; mcpp says what that costs and what the + // supported route is, and does not refuse as long as the result + // builds and runs. The developer owns the artifact and guarantees it. + // + // THE TOOLCHAIN is mcpp's own contract. Everything mcpp promises — + // that `import std` is available, that the runtime closure is + // computable, that two machines and CI produce the same build — is a + // statement about a compiler mcpp resolved and can identify. A + // compiler picked off `PATH` makes every one of those promises + // unverifiable, and a build tool that cannot state what it built with + // is answering in the wrong version (see + // `.agents/docs/…a-build-must-be-able-to-state-its-own-version`). + // + // So this is refused rather than warned about, and it is refused HERE, + // before any resolution work, so the message is the first thing the + // user sees rather than a consequence three layers down. + // + // `msvc@system` is a different spelling and stays supported: it names a + // FAMILY whose installation mcpp locates and identifies, on the one + // platform where the compiler cannot be redistributed. + return std::unexpected(std::format( + "[toolchain] {} = \"system\" is not supported: mcpp builds only " + "with toolchains it manages.\n" + " A compiler taken from PATH cannot be identified or " + "reproduced, so `import std` availability, the runtime closure and " + "\"the same build on another machine\" all stop being things mcpp " + "can promise.\n" + " Name one instead — mcpp installs it on first use:\n" + "\n" + " [toolchain]\n" + " {} = \"gcc@16.1.0\"\n" + "\n" + " or set a machine default with `mcpp toolchain default " + "gcc@16.1.0`, and see `mcpp toolchain list` for what is available.\n" + " (On Windows, `msvc@system` is different and remains " + "supported: it names a family whose installation mcpp locates.)\n" + " Host LIBRARIES are a separate question and are not refused " + "— a project may link them and owns the result.", + kCurrentPlatform, kCurrentPlatform)); } else if (mcpp::platform::env::offline_mode() || mcpp::platform::env::no_auto_install()) { // CI / offline / test opt-out: hard-error instead of silently @@ -2822,8 +2895,32 @@ prepare_build(bool print_fingerprint, t.cAbiPrebuilt = true; return t; }; + // `explicit_compiler` IS EMPTY FOR ONE RESOLUTION PATH, AND THIS IS + // THE ONLY CALLER THAT NOTICED BY CRASHING (#527). + // + // Every branch that resolves a toolchain from the index assigns + // `explicit_compiler`; the `[toolchain] system` branch does not, because + // it has nothing to assign yet — `detect` finds the PATH compiler a few + // hundred lines below and stores the resolved ABSOLUTE path in + // `tc->binaryPath`. The main build reads the compiler from `tc` and is + // fine; this closure returned the local variable and handed "" to + // `posix_spawnp`, which is `exit 127: posix_spawnp('') failed`. + // + // AND THE FIX IS NOT "SUPPORT THE HOST". `tc->binaryPath` is the + // compiler this build is ALREADY using for every other translation + // unit; build.mcpp is compiled with the project's toolchain by + // definition (see this lambda's header). Reading it from the place it + // was resolved makes the two paths agree — it grants no capability the + // project did not already have, and the host-dependence warning at the + // `system` branch is what states the cost. + // + // The CROSS branch below is a different question and deliberately + // unchanged: there `explicit_compiler` is empty because NO host + // toolchain was resolved at all, and its classified refusal is correct. if (overrides.target_triple.empty()) - return std::pair{explicit_compiler, as_host(*tc)}; + return std::pair{ + explicit_compiler.empty() ? tc->binaryPath : explicit_compiler, + as_host(*tc)}; if (hostTcCache) return std::pair{hostTcCache->first, as_host(hostTcCache->second)}; if (!tcSpec || *tcSpec == "system" || tcSpecIsMsvc) { @@ -7680,6 +7777,147 @@ prepare_build(bool print_fingerprint, } bool needsStdModule = graph_or_targets_import_std(scan.graph, *m, *root); + + // A DEPENDENCY THAT DECLARED A HIGHER STANDARD THAN THE GRAPH IS BUILT AT. + // + // A C++ module graph has ONE standard — cross-level BMIs are hard + // incompatible — so the root package's level is imposed graph-wide, and a + // dependency's `standard` is parsed and then discarded. That is correct and + // is not the defect. The defect is the silence: a package that declared + // c++26 because it needs c++26 is compiled at whatever the consumer says, + // and fails — if it fails at all — with a compiler error inside a + // translation unit the user does not own, naming neither package nor the + // mechanism. + // + // SCOPED TO MANIFESTS THE PROJECT AUTHOR CONTROLS, and that scope is the + // whole reason this check is shippable. The cpp20 design doc's §9-Q3 + // declined it because the default and a declaration were indistinguishable; + // `standardDeclared` fixes that for `mcpp.toml`, and NOT for the index: + // measured over the local registry, every descriptor with an mcpp segment + // declares `language` (782 of 782), and 756 of those 774 packages are C + // libraries with `import_std = false` carrying a boilerplate "c++23". A + // check that trusted declaredness everywhere would fire against essentially + // the whole index for any root at c++20 — exactly the outcome §9-Q3 + // refused, reached through a different door. + // + // DEGRADED, NOT AN ERROR. The condition is not a proven failure: a package + // declaring c++26 compiles perfectly well at c++23 whenever it happens not + // to use a C++26 construct, and that is a working configuration today for + // anyone who wrote the key aspirationally. `--strict` promotes it. + { + const auto graphLevel = m->cppStandard.level; + for (std::size_t i = 1; i < packages.size(); ++i) { + auto const& pkg = packages[i]; + if (!pkg.manifest.package.standardDeclared) continue; + // The scope gate. A package whose root is under a store directory + // arrived from an index and its declaration was written by a + // descriptor generator, not by the person reading this diagnostic. + if (mcpp::build::path_is_under_any(pkg.root, storeRoots)) + continue; + auto declared = mcpp::manifest::normalize_cpp_standard( + pkg.manifest.package.standard); + if (!declared || declared->level <= graphLevel) continue; + mcpp::diag::degraded( + "build/standard", + std::format("dependency `{}` declares standard = \"{}\", and " + "this graph is built at {}", + pkg.manifest.package.name, + declared->canonical, m->cppStandard.canonical), + "a C++ module graph has one standard, so the dependency's " + "declaration is not applied and its sources are compiled at the " + "graph's level", + std::format( + "raise the consumer's standard to \"{}\", or declare it " + "once for every member:\n\n [workspace.package]\n " + "standard = \"{}\"", declared->canonical, declared->canonical)); + } + } + + // A DIALECT FLAG THAT REACHES EVERY TU AND NOT THE `import std` PREBUILD + // IS A BUILD THAT CANNOT SUCCEED, AND MCPP KNOWS IT BEFORE COMPILING. + // + // `[build] cxxflags = ["-fno-exceptions"]` is applied to each translation + // unit; the std BMI in `stdFlagAndDialect` is precompiled without it, + // because only `dialect_flags()` rides that channel. Every importer then + // fails inside a file mcpp generated: + // + // std: error: language dialect differs 'C++23', expected + // 'C++23/no-exceptions' + // std: error: failed to read compiled module: Bad file data + // + // The message names the mechanism and not the key, so the way out + // (`dialect_cxxflags`, which IS applied to the prebuild, the scan and every + // TU) is not discoverable from it. Both facts are known here: whether the + // graph imports `std`, and which flags reached the prebuild. + // + // REFUSED RATHER THAN WARNED, and that is the same rule the host-dependence + // diagnostics follow from the other side: this build provably cannot + // succeed, so there is no user decision to respect. Contrast + // `[toolchain] system`, which builds and runs and is therefore warned about. + // + // GATED ON `needsStdModule` — without `import std` in the graph there is no + // prebuilt BMI to disagree with, and `-fno-exceptions` is then an ordinary + // per-unit flag that works. A check that refused in both cases would have + // stopped testing the condition it claims to test. + if (needsStdModule) { + const auto prebuilt = mcpp::toolchain::cppfly::effective_dialect_flags( + *tc, m->cppStandard.experimental, + mcpp::manifest::dialect_flags(m->buildConfig)); + // THE ROOT PACKAGE ONLY, and the narrowing is a correctness bound + // rather than a shortcut. + // + // A dependency carrying the same flag fails identically — but only if + // ITS OWN translation units import `std`. `needsStdModule` is a + // property of the whole graph: a C++ wrapper package that uses no std + // module can carry `-fno-exceptions` in its `[build] cxxflags` and + // compile perfectly well inside a graph whose ROOT imports std. + // Refusing there would stop a build that works, which is the one thing + // a refusal must never do — the rule is "provably cannot build", and + // for a dependency this evidence does not prove it. + // + // Extending it needs a per-package answer to "does this package import + // std", which the scan graph holds and does not expose in that shape. + // Recorded here so the next person meets the reason and not the gap. + for (auto const& pkg : std::span{packages}.first(1)) { + auto missing = mcpp::manifest::dialect_flags_missing_from_prebuild( + pkg.manifest.buildConfig.cxxflags, prebuilt); + if (missing.empty()) continue; + std::string list; + for (auto const& f : missing) { + if (!list.empty()) list += ", "; + list += '`'; list += f; list += '`'; + } + // NAMES THE FLAG, NOT THE TABLE IT CAME FROM. The same flag + // reaches the compile line from `[build] cxxflags`, from + // `[profile.] cxxflags` and from a `[target.…]` / `cfg(...)` + // block; by the time it is read here they have been merged, and + // asserting one of them would be wrong two times in three. + return std::unexpected(std::format( + "{} changes the language dialect{}, but the `import std` BMI is " + "precompiled without it, so every importing translation unit " + "will fail with \"language dialect differs\".\n" + " Declare it as a dialect flag instead — that channel is " + "applied to the std BMI prebuild, the module scan and every TU " + "in the graph:\n" + "\n" + " [build]\n" + " dialect_cxxflags = [{}]\n" + "\n" + " It belongs in `[build]` and not in a profile or a " + "per-target block: a dialect the standard library was not built " + "with cannot be held by one package or one profile alone.", + list, std::string{}, + [&] { + std::string q; + for (auto const& f : missing) { + if (!q.empty()) q += ", "; + q += '"'; q += f; q += '"'; + } + return q; + }())); + } + } + // A standard library that came from a PACKAGE brings its own module // source, because the compiler cannot be asked for one it does not have. // @@ -8016,13 +8254,40 @@ prepare_build(bool print_fingerprint, // whether a package's sources really came from a store. ONE definition, // two uses — deriving the same fact twice is how the object layout and the // cache key drifted apart in the first place (#344). - const auto storeRoots = [&]() -> std::vector { + // Which source trees does the fast path have to watch besides this one? + // + // A package whose root is neither under `projectRoot` nor under a directory + // mcpp OWNS is a `path` dependency — the shape every workspace member takes + // towards its siblings — and its sources are read on every build. See + // BuildContext::depSourceRoots for what the list is for. + // + // WHAT IS EXCLUDED, AND WHY IT IS "WHO WROTE THE DIRECTORY" RATHER THAN + // "WHICH KIND OF DEPENDENCY". An xpkg payload under the store is written + // once at install time and never edited. A git checkout under + // `/git/` is a pinned revision in a hash-addressed + // directory: changing the revision changes the directory name, and the + // manifest that names it is already swept. Neither can change under a warm + // build, so sweeping them would buy nothing and cost a directory walk per + // dependency on every invocation — which is the fast path this whole change + // exists to keep. + // + // A `path` dependency is the opposite on both counts: it is the user's + // working tree, and editing it is the point. + { + std::vector owned = storeRoots; + owned.push_back(mcpp::home::root()); std::vector roots; - if (auto c = get_cfg()) roots.push_back((*c)->xlingsHome() / "data" / "xpkgs"); - for (auto& d : mcpp::config::project_xlings_data_roots(workRoot)) - roots.push_back(d / "xpkgs"); - return roots; - }(); + for (std::size_t i = 1; i < packages.size(); ++i) { + const auto& pkgRoot = packages[i].root; + if (pkgRoot.empty()) continue; + if (mcpp::build::path_is_under_any(pkgRoot, owned)) continue; + auto normalized = pkgRoot.lexically_normal(); + if (normalized == root->lexically_normal()) continue; + if (std::find(roots.begin(), roots.end(), normalized) == roots.end()) + roots.push_back(std::move(normalized)); + } + ctx.depSourceRoots = std::move(roots); + } // ─── Prebuilt dependencies: check before planning to link them ───── // // Here rather than at each place a dependency manifest is loaded, because diff --git a/src/build/runtime_validation.cppm b/src/build/runtime_validation.cppm index 0aeecaff..e3248d3b 100644 --- a/src/build/runtime_validation.cppm +++ b/src/build/runtime_validation.cppm @@ -5,6 +5,13 @@ // ELF parses. Verdicts are persisted beside build.ninja and keyed by artifact // stat + RuntimeBinding contract so doctor can explain the last result without // probing the host again. +// +// THE RECORD LIVES IN `.mcpp-runtime-verdicts.json`, NOT IN `resolution.json`. +// `prepare_build` rewrites the latter from an empty object at the start of +// every invocation, so a verdict recorded there is deleted before the next run +// can read it back -- which is what made two of these three passes re-parse +// every image on every command (#529). `resolution.json` publishes a copy after +// the link and stays the documented place to read one. export module mcpp.build.runtime_validation; @@ -116,8 +123,18 @@ ArtifactVerdict artifact_identity_verdict( // // `before` is the pre-ninja snapshot, same as validate_changed_artifacts takes: // an artifact whose stat did not move was not produced by this run, so its -// verdict is READ BACK from resolution.json instead of re-derived from the ELF. -// The returned vector still covers every artifact either way. +// verdict is READ BACK instead of re-derived from the ELF. The returned vector +// still covers every artifact either way. +// +// READ BACK FROM THE SIDECAR, NOT FROM `resolution.json`, and the distinction +// is the whole of #529. `prepare_build` regenerates `resolution.json` from an +// empty object at the start of every invocation, so a verdict recorded there +// was deleted before the next run could find it and the read-back never fired +// across processes — 1.36 s of a 1.94 s warm `mcpp test`, every time. +// `.mcpp-runtime-verdicts.json` survives, and `resolution.json` keeps +// publishing a copy after the link, which is how `sync_resolution_verdict` +// already handled the runtime verdicts. One authoritative writer, one +// published view. std::vector check_and_record_loader_tags(const mcpp::build::BuildPlan& plan, const ArtifactSnapshot& before); @@ -261,6 +278,173 @@ std::string cache_key(const mcpp::build::BuildPlan& plan, : relative.lexically_normal().generic_string(); } +// ── The durable home of the post-link verdicts ────────────────────────────── +// +// WHY THESE RECORDS LIVE IN THE SIDECAR AND NOT IN `resolution.json`. +// +// Both post-link passes were written with a read-back: an artifact whose stat +// did not move keeps the verdict already on file instead of being re-parsed, +// because re-reading every image on every drive is what made the loader-tag +// check cost 158.7 s of a 190 s hot run. The read-back was correct and it never +// fired across invocations, because it read `resolution.json` — and +// `prepare_build` rewrites that file from a FRESH json object at the start of +// every invocation, carrying neither key. Every run therefore began by deleting +// the memo its own backend was about to look for. +// +// Measured on a ten-link-unit tree with nothing to do: 1.36 s of a 1.94 s +// `mcpp test`, every time. +// +// `resolution.json` keeps publishing both records — `mcpp why runtime`, doctor, +// e2e 214 and e2e 307 read them there — but it publishes a COPY, written after +// the link, exactly as `sync_resolution_verdict` already publishes the runtime +// verdicts. One authoritative writer, one published view. +constexpr std::string_view kLoaderTagsRecord = "loader_tags"; +constexpr std::string_view kSymbolProvisionRecord = "symbol_provision"; + +// WHAT INVALIDATES A STORED VERDICT BESIDES THE ARTIFACT ITSELF. +// +// Making the memo durable creates a correctness obligation that did not exist +// while every answer was recomputed: a verdict about which file satisfies a +// DT_NEEDED is a function of more than the artifact's stat. +// +// the SubOS farm `/lib` is a symlink view rewritten by every +// `xlings install`, and it sits on the artifact's runtime +// search path. Installing a package can change which file +// answers, with the artifact untouched. `.xlings.json` is +// that view's version stamp — `try_fast_build` already +// treats it as one. +// the policy `MCPP_ALLOW_HOST_LIBS` is read from the environment at +// check time and enters no fingerprint, so it can flip a +// verdict with every input file unchanged. +// +// Folded into one key rather than compared field by field, so a third input +// added later has one place to go. +std::string post_link_key(const mcpp::build::BuildPlan& plan) { + std::string material = plan.runtimeBinding.contractHash; + material += '\x1f'; + if (!plan.runtimeBinding.subosDir.empty()) { + std::error_code ec; + auto stampPath = plan.runtimeBinding.subosDir / ".xlings.json"; + auto size = std::filesystem::file_size(stampPath, ec); + if (!ec) material += std::to_string(size); + ec.clear(); + auto when = std::filesystem::last_write_time(stampPath, ec); + if (!ec) + material += std::to_string( + static_cast(when.time_since_epoch().count())); + } + material += '\x1f'; + material += host_libs_allowed(plan) ? "host-libs" : "hermetic"; + std::uint64_t hash = 0xcbf29ce484222325ull; + for (unsigned char c : material) { hash ^= c; hash *= 0x100000001b3ull; } + return std::format("{:016x}", hash); +} + +// The stored entries for one pass, or an empty array when nothing usable is on +// file. A key mismatch reads as "nothing stored", which re-derives everything +// once — the safe direction, and the only one that keeps a stale farm from +// answering for a fresh one. +nlohmann::json stored_post_link(const nlohmann::json& doc, + std::string_view name, + std::string_view key) { + if (!doc.is_object()) return nlohmann::json::array(); + if (doc.value("post_link_key", "") != key) return nlohmann::json::array(); + auto it = doc.find(std::string(name)); + if (it == doc.end() || !it->is_array()) return nlohmann::json::array(); + return *it; +} + +// Merge this drive's findings over what is on file, and drop only what has +// LEFT THE DISK. +// +// Pruning on "not in the current plan" is the shape that made the sidecar's own +// pass cost 1.19 s in an edit-test loop: `mcpp build` and `mcpp test` share one +// output directory and have different link-unit sets — measured, the test plan +// contains the nine test binaries and not `bin/app` — so each command deleted +// the other's verdicts and both paid full price on every alternation. The +// record is a property of the output directory, not of whichever command last +// ran against it. +nlohmann::json merge_post_link(const nlohmann::json& stored, + nlohmann::json fresh, + const std::filesystem::path& outputDir) { + std::set covered; + for (auto const& e : fresh) + if (e.is_object()) covered.insert(e.value("path", "")); + for (auto const& e : stored) { + if (!e.is_object()) continue; + auto rel = e.value("path", ""); + if (rel.empty() || covered.contains(rel)) continue; + std::error_code ec; + if (!std::filesystem::exists(outputDir / rel, ec)) continue; + fresh.push_back(e); + } + std::sort(fresh.begin(), fresh.end(), [](auto const& a, auto const& b) { + return a.value("path", "") < b.value("path", ""); + }); + return fresh; +} + +// Store the authoritative copy, then publish the readable one. +// +// The publish half is not decoration: `mcpp why runtime`, `mcpp doctor` and two +// e2e tests read `runtime.` out of `resolution.json`, and that file is the +// documented place to look (docs/05). What changed is which copy survives an +// invocation — the sidecar's — so the published one can be regenerated from it +// rather than being the only one there was. +void persist_post_link(const mcpp::build::BuildPlan& plan, + std::string_view name, std::string_view key, + const nlohmann::json& entries) { + auto doc = read_cache(plan.outputDir); + if (!doc.is_object()) doc = nlohmann::json::object(); + // A key change invalidates the OTHER pass's entries too — they were derived + // under the same farm and the same policy — so they go with it rather than + // being silently carried across as if they had been re-checked. + const bool keyMoved = doc.value("post_link_key", "") != key; + if (keyMoved) { + doc.erase(std::string(kLoaderTagsRecord)); + doc.erase(std::string(kSymbolProvisionRecord)); + } + if (keyMoved || doc.value(std::string(name), nlohmann::json::array()) != entries) { + doc["post_link_key"] = std::string(key); + doc[std::string(name)] = entries; + write_cache(plan.outputDir, doc); + } + + // THE PUBLISHED COPY IS REWRITTEN UNCONDITIONALLY, and the store above is + // not. They have opposite lifetimes: the sidecar survives `prepare_build`, + // which is the whole point, while `resolution.json` was regenerated from a + // fresh object at the start of this very invocation and therefore carries + // nothing yet. Skipping the publish when the CONTENT had not changed left + // `runtime.symbol_provision` absent on every warm build — the record was + // correct and the documented place to read it was empty, which is the + // failure this whole change exists to remove, moved one file over. + const auto path = plan.outputDir / "resolution.json"; + nlohmann::json resolution; + { + std::ifstream input(path); + resolution = nlohmann::json::parse(input, nullptr, false); + } + if (resolution.is_discarded() || !resolution.is_object()) return; + auto runtime = resolution.find("runtime"); + if (runtime == resolution.end() || !runtime->is_object()) return; + (*runtime)[std::string(name)] = entries; + + std::error_code ec; + auto tmp = path; + tmp += ".tmp"; + if (std::ofstream output(tmp); output) { + output << resolution.dump(2) << '\n'; + output.close(); + std::filesystem::rename(tmp, path, ec); + if (ec) { + ec.clear(); + std::filesystem::remove(path, ec); + ec.clear(); + std::filesystem::rename(tmp, path, ec); + } + } +} + std::vector runtime_search_dirs(const mcpp::build::BuildPlan& plan) { std::vector out; @@ -421,15 +605,24 @@ ValidationReport validate_changed_artifacts( doc["schema"] = 1; doc["contract_hash"] = plan.runtimeBinding.contractHash; auto searchDirs = runtime_search_dirs(plan); - std::set currentKeys; - for (auto const& [artifact, ignored] : before) { - (void)ignored; - currentKeys.insert(cache_key(plan, artifact)); - } + // AN ARTIFACT LEAVES THIS RECORD WHEN IT LEAVES THE DISK, NOT WHEN IT + // LEAVES THE CURRENT COMMAND'S PLAN. + // + // Pruning against the current plan is correct only if one plan owns the + // output directory, and none does: `mcpp build` and `mcpp test` share it + // and have different link-unit sets — measured, the test plan carries the + // test binaries and not the package's own `bin/`. Each command therefore + // deleted the other's verdicts, and an edit-test loop paid the full ELF + // re-parse on every alternation (1.19 s of a 3.15 s `mcpp test` on a + // ten-unit tree, with nothing to rebuild). + // + // Disk existence keeps the file bounded — which is what the pruning is for + // — without making the record a property of whichever command ran last. if (auto artifacts = doc.find("artifacts"); artifacts != doc.end() && artifacts->is_object()) { for (auto it = artifacts->begin(); it != artifacts->end();) { - if (!currentKeys.contains(it.key())) { + std::error_code existsEc; + if (!std::filesystem::exists(plan.outputDir / it.key(), existsEc)) { it = artifacts->erase(it); changedCache = true; } else { @@ -581,32 +774,9 @@ check_and_record_loader_tags(const mcpp::build::BuildPlan& plan, std::vector findings; if constexpr (!mcpp::platform::is_linux) return findings; - const auto path = plan.outputDir / "resolution.json"; - nlohmann::json resolution; - { - std::ifstream input(path); - resolution = nlohmann::json::parse(input, nullptr, false); - } - - // What this run actually produced. The parameter used to be spelled - // `produced` and then be given the BEFORE snapshot, so every drive - // re-parsed every link artifact in the plan — and `mcpp test` drives the - // backend once per test on an already-built tree. - // MEASURED on the 83-test suite: 158.7 s of a 190 s hot run, 1.87 s x 85 - // drives, for artifacts that nothing had touched. - // - // An unchanged artifact keeps the verdict already written to - // resolution.json rather than being dropped: a violation must keep being - // reported on every build, and "checked and compliant" must stay - // distinguishable from "never checked" — which is exactly what a shorter - // fix (skip unchanged, record only the fresh ones) would have destroyed. - const auto recorded = [&]() -> nlohmann::json { - auto rt = resolution.is_object() ? resolution.find("runtime") : resolution.end(); - if (rt == resolution.end() || !rt->is_object()) return nlohmann::json::array(); - auto tags = rt->find("loader_tags"); - if (tags == rt->end() || !tags->is_array()) return nlohmann::json::array(); - return *tags; - }(); + const auto key = post_link_key(plan); + const auto recorded = + stored_post_link(read_cache(plan.outputDir), kLoaderTagsRecord, key); auto recorded_entry = [&](const std::string& rel) -> const nlohmann::json* { for (auto const& e : recorded) if (e.is_object() && e.value("path", "") == rel) return &e; @@ -625,7 +795,6 @@ check_and_record_loader_tags(const mcpp::build::BuildPlan& plan, return Tag::None; }; - bool anyFresh = false; for (auto const& [artifact, oldStamp] : before) { auto now = stamp(artifact); if (!now.exists) continue; @@ -656,14 +825,9 @@ check_and_record_loader_tags(const mcpp::build::BuildPlan& plan, auto finding = loader::check_artifact(artifact); if (finding.form == loader::Form::NotElf) continue; - anyFresh = true; findings.push_back(std::move(finding)); } - if (findings.empty() || !anyFresh) return findings; - - if (resolution.is_discarded() || !resolution.is_object()) return findings; - auto runtime = resolution.find("runtime"); - if (runtime == resolution.end() || !runtime->is_object()) return findings; + if (findings.empty()) return findings; nlohmann::json entries = nlohmann::json::array(); for (auto const& finding : findings) { @@ -684,22 +848,15 @@ check_and_record_loader_tags(const mcpp::build::BuildPlan& plan, ? "violation" : "not_checked"}, }); } - (*runtime)["loader_tags"] = std::move(entries); - - std::error_code ec; - auto tmp = path; - tmp += ".tmp"; - if (std::ofstream output(tmp); output) { - output << resolution.dump(2) << '\n'; - output.close(); - std::filesystem::rename(tmp, path, ec); - if (ec) { - ec.clear(); - std::filesystem::remove(path, ec); - ec.clear(); - std::filesystem::rename(tmp, path, ec); - } - } + auto merged = merge_post_link(recorded, std::move(entries), plan.outputDir); + // The union is what gets stored, and the store is only rewritten when it + // moved -- but the PUBLISHED copy in resolution.json is rewritten every + // drive, because `prepare_build` regenerated that file from an empty object + // at the start of this invocation. `anyFresh` was the wrong condition on + // both counts: a drive that read every verdict back but contributed a link + // unit the record had never seen would leave it out, and an absent entry + // reads exactly like "checked and clean". + persist_post_link(plan, kLoaderTagsRecord, key, merged); return findings; } @@ -727,22 +884,9 @@ check_symbol_provision(const mcpp::build::BuildPlan& plan, // record would shrink to "whatever moved last", a conflict found on // Monday would stop being reported on Tuesday, and — worse — the absence // of an entry would read exactly like "checked and clean". - const auto resolutionPath = plan.outputDir / "resolution.json"; - nlohmann::json resolution; - { - std::ifstream input(resolutionPath); - resolution = nlohmann::json::parse(input, nullptr, false); - } - const auto recorded = [&]() -> nlohmann::json { - auto rt = resolution.is_object() ? resolution.find("runtime") - : resolution.end(); - if (rt == resolution.end() || !rt->is_object()) - return nlohmann::json::array(); - auto entries = rt->find("symbol_provision"); - if (entries == rt->end() || !entries->is_array()) - return nlohmann::json::array(); - return *entries; - }(); + const auto key = post_link_key(plan); + const auto recorded = + stored_post_link(read_cache(plan.outputDir), kSymbolProvisionRecord, key); auto stored_for = [&](const std::string& rel) -> const nlohmann::json* { for (auto const& entry : recorded) if (entry.is_object() && entry.value("path", "") == rel) return &entry; @@ -765,7 +909,6 @@ check_symbol_provision(const mcpp::build::BuildPlan& plan, return closureCache.emplace(object, std::move(names)).first->second; }; - bool anyFresh = false; for (auto const& [artifact, oldStamp] : before) { auto now = stamp(artifact); if (!now.exists) continue; @@ -807,8 +950,6 @@ check_symbol_provision(const mcpp::build::BuildPlan& plan, continue; } } - anyFresh = true; - // Which link unit is this? Its own flags matter as much as the // global ones, and the static side of any report is attributed from // the objects it links. Spelled EXACTLY as `snapshot_link_artifacts` @@ -885,14 +1026,14 @@ check_symbol_provision(const mcpp::build::BuildPlan& plan, } // Record, for the same reason the loader-tag contract records: a warning - // scrolls past, and `resolution.json` is what CI, `mcpp why runtime` and a - // test can read. It also gives a test a FIELD to assert on instead of a + // scrolls past, and the record is what CI, `mcpp why runtime` and a test + // can read. It also gives a test a FIELD to assert on instead of a // substring of a message — a message whose wording is free to improve. - if (!findings.empty() && anyFresh) { - const auto& path = resolutionPath; - if (!resolution.is_discarded() && resolution.is_object()) { - if (auto runtime = resolution.find("runtime"); - runtime != resolution.end() && runtime->is_object()) { + // Stored in the sidecar and published into `resolution.json`; see the + // module header for why those are two different files. + if (!findings.empty()) { + { + { nlohmann::json entries = nlohmann::json::array(); for (auto const& finding : findings) { std::error_code ec; @@ -922,22 +1063,9 @@ check_symbol_provision(const mcpp::build::BuildPlan& plan, } entries.push_back(std::move(entry)); } - (*runtime)["symbol_provision"] = std::move(entries); - - std::error_code ec; - auto tmp = path; - tmp += ".tmp"; - if (std::ofstream output(tmp); output) { - output << resolution.dump(2) << '\n'; - output.close(); - std::filesystem::rename(tmp, path, ec); - if (ec) { - ec.clear(); - std::filesystem::remove(path, ec); - ec.clear(); - std::filesystem::rename(tmp, path, ec); - } - } + auto merged = merge_post_link(recorded, std::move(entries), + plan.outputDir); + persist_post_link(plan, kSymbolProvisionRecord, key, merged); } } } diff --git a/src/project.cppm b/src/project.cppm index 85c7285f..e5f61b85 100644 --- a/src/project.cppm +++ b/src/project.cppm @@ -128,6 +128,144 @@ export void inherit_workspace_indices(mcpp::manifest::Manifest& member, } } +// EVERYTHING A MEMBER INHERITS FROM ITS WORKSPACE ROOT, IN ONE FUNCTION. +// +// There are two inheritance SITES in prepare_build — the command issued at the +// workspace root with `-p `, and the command issued inside a member +// directory — and until this function existed they were two hand-copied lists +// of the same merges. A fifth key added to one of them is a defect that +// compiles, which is exactly how `[build]` came to be inherited by neither +// (#527 Bug 2). +// +// The discipline is stated on `WorkspaceInherited`: scalars are taken when the +// member did not DECLARE the key, vectors append with the workspace first, and +// dependencies keep their explicit `.workspace = true` opt-in because they are +// graph edges. `[toolchain]`, `[target.]` and `[indices]` were already +// inherited before these tables existed and keep the behaviour they had. +// +// `wsRoot` anchors relative paths: an `[indices].path` or a +// `[workspace.dependencies] path` was written against the WORKSPACE ROOT, and +// re-anchoring it to the member directory is #224. +export void inherit_workspace_config(mcpp::manifest::Manifest& member, + const mcpp::manifest::Manifest& workspace, + const std::filesystem::path& wsRoot) { + merge_workspace_deps(member, workspace, wsRoot); + + if (member.toolchain.byPlatform.empty()) + member.toolchain = workspace.toolchain; + for (auto& [triple, entry] : workspace.targetOverrides) + if (!member.targetOverrides.contains(triple)) + member.targetOverrides[triple] = entry; + inherit_workspace_indices(member, workspace, wsRoot); + + const auto& inh = workspace.workspace.inherited; + + // `[workspace.package]`. The standard is the load-bearing one: a C++ module + // graph has ONE standard, so a workspace that states it once is how a + // monorepo stops depending on every member remembering to. + // + // `standardDeclared` and not `standard != "c++23"`: a member that + // deliberately pins c++23 under a c++26 workspace must keep it, and that is + // indistinguishable from the default without the bit. + if (inh.standardDeclared && !member.package.standardDeclared) { + member.package.standard = inh.standard; + member.language.standard = inh.standard; + member.package.standardDeclared = true; + // `cppStandard` was normalised by the parser from the member's own + // value; it has to be re-derived, or the inherited spelling would sit + // in `package.standard` while every build surface kept reading the + // default out of the normalised copy. Same class of defect as a + // recorded field with no reader, one struct over. + if (auto cfg = mcpp::manifest::normalize_cpp_standard(inh.standard)) + member.cppStandard = *cfg; + } + if (member.package.version.empty()) member.package.version = inh.version; + // (the "still missing after inheritance" refusal is in + // `workspace_inheritance_error` below — one predicate, both call sites) + if (member.package.license.empty()) member.package.license = inh.license; + if (member.package.description.empty()) member.package.description = inh.description; + if (member.package.repo.empty()) member.package.repo = inh.repo; + if (member.package.authors.empty()) member.package.authors = inh.authors; + + // `[workspace.build]`. Vectors append workspace-FIRST so a member's own + // flag lands later on the command line, where the compiler lets it win. + if (inh.buildPresent) { + auto& b = member.buildConfig; + const auto& w = inh.build; + auto prepend = [](auto& dst, const auto& src) { + if (src.empty()) return; + dst.insert(dst.begin(), src.begin(), src.end()); + }; + prepend(b.cflags, w.cflags); + prepend(b.cxxflags, w.cxxflags); + prepend(b.ldflags, w.ldflags); + prepend(b.defines, w.defines); + prepend(b.dialectCxxflags, w.dialectCxxflags); + // A RELATIVE INCLUDE DIRECTORY IN THE WORKSPACE MANIFEST WAS WRITTEN + // AGAINST THE WORKSPACE ROOT, and every member would otherwise resolve + // it against its own directory. + // + // This is #224 for a new key: `[indices].path` and + // `[workspace.dependencies] path` are anchored for exactly this reason, + // and a third relative-path key that skipped it would silently point at + // `/shared/inc` for a directory that lives at + // `/shared/inc`. The failure is a missing header, three + // members deep, naming neither the manifest that declared it nor the + // root it was declared against. + // + // Anchored rather than refused: an absolute include directory is + // already accepted by `expandIncludeDirs`, so the anchored form needs + // no new handling downstream. + auto anchored = [&](const std::vector& src) { + std::vector out; + out.reserve(src.size()); + for (auto const& d : src) + out.push_back(d.is_absolute() ? d + : (wsRoot / d).lexically_normal()); + return out; + }; + prepend(b.includeDirs, anchored(w.includeDirs)); + prepend(b.includeDirsAfter, anchored(w.includeDirsAfter)); + prepend(b.privateIncludeDirs, anchored(w.privateIncludeDirs)); + if (b.cStandard.empty()) b.cStandard = w.cStandard; + if (b.linkage.empty()) b.linkage = w.linkage; + if (b.target.empty()) b.target = w.target; + if (b.cxxRuntime.empty()) b.cxxRuntime = w.cxxRuntime; + if (b.dependencyLinkage.empty()) b.dependencyLinkage = w.dependencyLinkage; + if (b.macosDeploymentTarget.empty()) + b.macosDeploymentTarget = w.macosDeploymentTarget; + } +} + +// The required-field check, asked at the one point where it is answerable. +// +// `mcpp.manifest`'s parser cannot enforce `package.name` / `package.version` on +// a workspace member, because a member manifest carries no evidence that it is +// one. Deferring the check is not relaxing it: it runs here, after inheritance, +// and the message can name the workspace table that would have supplied the +// value — which the parser could not have done either. +export std::optional workspace_inheritance_error( + const mcpp::manifest::Manifest& member, + const std::filesystem::path& memberDir) { + auto missing = [&](std::string_view field, std::string_view wsKey) + -> std::optional { + return std::format( + "{}: missing required field '{}', and the workspace root does not " + "supply it either.\n" + " Declare it in the member, or once for every member:\n" + "\n" + " [workspace.package]\n" + " {} = \"...\"", + (memberDir / "mcpp.toml").string(), field, wsKey); + }; + if (member.package.name.empty()) + return std::format("{}: missing required field 'package.name'. " + "A workspace cannot supply it: members do not share " + "a name.", (memberDir / "mcpp.toml").string()); + if (member.package.version.empty()) return missing("package.version", "version"); + return std::nullopt; +} + // Resolve which member directory a workspace command acts on, for the // single-member case. Shares the match rule (basename OR member path) with // prepare_build's member switch, so `build -p X` and `test -p X` agree. diff --git a/tests/e2e/14_toolchain_fallback.sh b/tests/e2e/14_toolchain_fallback.sh index a6d2a0ed..b219fed7 100755 --- a/tests/e2e/14_toolchain_fallback.sh +++ b/tests/e2e/14_toolchain_fallback.sh @@ -29,16 +29,32 @@ echo "$out" | grep -q 'no toolchain configured' || { echo "$out" | grep -q 'mcpp toolchain install' || { echo "FAIL: error doesn't suggest install: $out"; exit 1; } -# Adding a project-level [toolchain] = "system" still works as escape hatch. +# A project-level [toolchain] = "system" is REFUSED, and refused on its own +# terms rather than falling through to "no toolchain configured". +# +# ⚠️ THIS USED TO ASSERT THE OPPOSITE — that `system` "still works as escape +# hatch" — and the assertion was only ever `grep -q 'no toolchain configured'` +# being absent. That predicate stays satisfied by ANY other error, so when the +# escape hatch became a refusal the test went on passing while its stated +# intent had inverted. A negative-only assertion cannot tell "it worked" from +# "it failed differently"; both halves are checked now. +# +# The policy: mcpp builds only with toolchains it manages. A compiler taken +# from PATH cannot be identified or reproduced, so `import std` availability +# and "the same build on another machine" stop being things mcpp can promise. +# `msvc@system` is the single exception and is not this spelling. Host +# LIBRARIES are a different axis and remain the project's own choice. cat >> mcpp.toml <<'EOF' [toolchain] default = "system" EOF out=$("$MCPP" build 2>&1) || true -# 'system' opt-in path uses $CXX or PATH g++ — host-dependent; we don't -# verify success, only that the hard-error path doesn't fire. echo "$out" | grep -q 'no toolchain configured' && { - echo "FAIL: 'system' opt-in still hard-errored: $out"; exit 1; } + echo "FAIL: 'system' fell through to the unconfigured path: $out"; exit 1; } +echo "$out" | grep -q 'toolchains it manages' || { + echo "FAIL: 'system' was not refused on its own terms: $out"; exit 1; } +echo "$out" | grep -q 'msvc@system' || { + echo "FAIL: the refusal does not name the one supported exception: $out"; exit 1; } echo "OK" diff --git a/tests/e2e/293_the_requested_target_and_the_resolved_one_name_one_os.sh b/tests/e2e/293_the_requested_target_and_the_resolved_one_name_one_os.sh index 0a804b03..e795b5da 100755 --- a/tests/e2e/293_the_requested_target_and_the_resolved_one_name_one_os.sh +++ b/tests/e2e/293_the_requested_target_and_the_resolved_one_name_one_os.sh @@ -34,9 +34,17 @@ make_project() { # dir target-section # ── Half one: it refuses, and says which two systems ────────────────────── # -# `toolchain = "system"` is the escape hatch that hands the build the PATH -# compiler. Pointing it at a Windows target is the one arrangement that -# reproduces CI's fallback without uninstalling anything. +# `toolchain = "system"` hands the build the PATH compiler; pointing it at a +# Windows target is the one arrangement that reproduces CI's fallback without +# uninstalling anything. +# +# ⚠️ AND SINCE THE SYSTEM TOOLCHAIN IS REFUSED, THAT REFUSAL IS ALSO AN ANSWER +# TO THIS TEST'S QUESTION. mcpp builds only with toolchains it manages +# (`msvc@system` excepted), and the refusal fires before target resolution — so +# a Windows target can no longer reach a Linux host compiler through this door +# at all. The invariant holds by a stronger mechanism than the one this test was +# written against, and BOTH refusals count. What must never happen, and what the +# else-branch below still fails on, is the build going ahead. make_project "$work/mismatch" '[target.x86_64-windows-gnu] toolchain = "system"' out="$(cd "$work/mismatch" && "$MCPP" build --target x86_64-windows-gnu 2>&1 || true)" @@ -47,9 +55,22 @@ out="$(cd "$work/mismatch" && "$MCPP" build --target x86_64-windows-gnu 2>&1 || # green-by-silence rather than red. The arrangement either reproduced (the # report names two systems) or it did not; only the second is a skip. reported="$(printf '%s\n' "$out" | grep -oP 'Target \K\S+ → \S+' | head -1)" +half_one_done=0 case "$out" in *"different operating systems"*) echo " ok it refuses rather than building for the wrong system" ;; + *"is not supported: mcpp builds only with toolchains it manages"*) + # The stronger refusal: the arrangement cannot be expressed any more, so a + # Windows target never reaches a Linux compiler through this door. + # + # ⚠️ AND IT MUST NOT `exit 0` HERE. Half two is independent of half one and + # asserts that correct cross builds still go through; leaving early skips + # it, and the ecosystem job that runs this file checks that each test "ran + # to its conclusion" precisely so an early exit cannot masquerade as a + # pass. Caught by that job, not by a local run. + echo " ok the system toolchain is refused outright, so the mismatch" + echo " this test guards cannot be reached through it" + half_one_done=1 ;; *) asked="${reported%% → *}" resolved="${reported##* → }" @@ -73,15 +94,21 @@ esac # ⭐ AND THE MESSAGE NAMES BOTH. A refusal that does not say what it resolved # to leaves the reader with the same question the report used to answer. -ok=1 -printf '%s\n' "$out" | grep -q "x86_64-windows-gnu" || ok=0 -printf '%s\n' "$out" | grep -q "linux" || ok=0 -if [ "$ok" = 1 ]; then - echo " ok and it names the target asked for and the one resolved" -else - echo "FAIL: the refusal does not name both systems" - printf '%s\n' "$out" | head -4 | sed 's/^/ /' - exit 1 +# +# Only asked of the OS-mismatch refusal. The toolchain refusal is a different +# sentence about a different decision — it never resolved a target at all — +# and demanding both triples from it would be asserting on the wrong object. +if [ "$half_one_done" = 0 ]; then + ok=1 + printf '%s\n' "$out" | grep -q "x86_64-windows-gnu" || ok=0 + printf '%s\n' "$out" | grep -q "linux" || ok=0 + if [ "$ok" = 1 ]; then + echo " ok and it names the target asked for and the one resolved" + else + echo "FAIL: the refusal does not name both systems" + printf '%s\n' "$out" | head -4 | sed 's/^/ /' + exit 1 + fi fi # ── Half two: every correct cross build still goes through ──────────────── diff --git a/tests/e2e/321_workspace_inheritance.sh b/tests/e2e/321_workspace_inheritance.sh new file mode 100755 index 00000000..186a3e0a --- /dev/null +++ b/tests/e2e/321_workspace_inheritance.sh @@ -0,0 +1,152 @@ +#!/usr/bin/env bash +# requires: gcc +# 321_workspace_inheritance.sh — [workspace.package] / [workspace.build] (#527). +# +# THE ASSERTION IS ON THE COMPILE COMMAND, NOT ON BUILD SUCCESS. A member that +# inherits nothing still builds; only the flags say whether inheritance +# happened, and only they say WHICH value won when both sides declared one. +# +# Four members, chosen so that each one falsifies a different wrong +# implementation: +# +# silent declares nothing → inherits standard and cxxflags +# pinned declares standard = "c++23" → KEEPS it under a c++26 workspace. +# Fails if declaredness was faked +# by comparing against the default, +# since the pin IS the default. +# adds declares its own cxxflags → gets BOTH, workspace first +# inside built from inside its own dir → the second inheritance site; +# an assertion on the first proves +# nothing about it +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +mkdir -p silent/src pinned/src adds/src shared/inc +printf '#define SHARED_HEADER_FOUND 1\n' > shared/inc/shared.h + +cat > mcpp.toml <<'EOF' +[workspace] +members = ["silent", "pinned", "adds"] + +[workspace.package] +standard = 26 +version = "0.4.2" +license = "Apache-2.0" + +[workspace.build] +cxxflags = ["-DFROM_WORKSPACE=1"] +include_dirs = ["shared/inc"] +EOF + +# `version` is deliberately absent from every member: it is a required field, +# and the workspace supplying it is the half of this feature that the parser +# cannot enforce on its own. +printf '[package]\nname = "silent"\n' > silent/mcpp.toml +printf '[package]\nname = "pinned"\nstandard = "c++23"\n' > pinned/mcpp.toml +printf '[package]\nname = "adds"\n\n[build]\ncxxflags = ["-DFROM_MEMBER=1"]\n' > adds/mcpp.toml +for m in silent pinned adds; do + # The include also proves the workspace-relative path was anchored to the + # WORKSPACE ROOT and not to each member: `shared/inc` lives at + # `/shared/inc`, and resolving it per-member would look for + # `/shared/inc` and fail with a missing header three members deep, + # naming neither the manifest that declared it nor the root it meant. + printf '#include \n#if !defined(FROM_WORKSPACE) || !defined(SHARED_HEADER_FOUND)\n#error "workspace [build] did not reach the member"\n#endif\nint main(){return 0;}\n' > "$m/src/main.cpp" +done + +flags_of() { # $1 = member dir + grep -oE '\-std=c\+\+[0-9a-z]+|\-DFROM_[A-Z]+=1' "$1/compile_commands.json" \ + | sort -u | tr '\n' ' ' +} + +for m in silent pinned adds; do + "$MCPP" build -p "$m" > "build_$m.log" 2>&1 || { cat "build_$m.log"; exit 1; } +done + +# ── silent: inherits both ─────────────────────────────────────────────────── +got=$(flags_of silent) +case "$got" in + *"-DFROM_WORKSPACE=1"*) ;; + *) echo "FAIL: member 'silent' did not inherit [workspace.build] cxxflags" + echo " got: $got"; exit 1 ;; +esac +case "$got" in + *"-std=c++26"*) ;; + *) echo "FAIL: member 'silent' did not inherit [workspace.package] standard" + echo " got: $got"; exit 1 ;; +esac +grep -q '"version": *"0.4.2"' silent/target/*/*/resolution.json 2>/dev/null \ + || grep -q "0.4.2" build_silent.log \ + || { echo "FAIL: member 'silent' did not inherit [workspace.package] version" + cat build_silent.log; exit 1; } + +# ── pinned: its own declaration wins ──────────────────────────────────────── +# +# THE ONE THAT CATCHES A FAKED DECLAREDNESS BIT. c++23 is also the default, so +# an implementation that decided "did the member declare it?" by comparing +# against the default would overwrite this pin with c++26 and every other +# assertion in this file would still pass. +got=$(flags_of pinned) +case "$got" in + *"-std=c++23"*) ;; + *) echo "FAIL: member 'pinned' declared standard = c++23 and did not get it" + echo " got: $got"; exit 1 ;; +esac +case "$got" in + *"-DFROM_WORKSPACE=1"*) ;; + *) echo "FAIL: member 'pinned' should still inherit [workspace.build]" + echo " got: $got"; exit 1 ;; +esac + +# ── adds: both, workspace first ───────────────────────────────────────────── +got=$(flags_of adds) +for want in "-DFROM_WORKSPACE=1" "-DFROM_MEMBER=1"; do + case "$got" in + *"$want"*) ;; + *) echo "FAIL: member 'adds' is missing $want"; echo " got: $got"; exit 1 ;; + esac +done +# Order is load-bearing: later wins on a compiler command line, so a member's +# own flag has to come after the workspace's. The CDB is pretty-printed with one +# argument per line, so the question is which line number comes first — matching +# both on a single line would silently never fire. +ws_at=$(grep -n 'FROM_WORKSPACE=1' adds/compile_commands.json | head -1 | cut -d: -f1) +mem_at=$(grep -n 'FROM_MEMBER=1' adds/compile_commands.json | head -1 | cut -d: -f1) +[ -n "$ws_at" ] && [ -n "$mem_at" ] || { + echo "FAIL: could not locate both flags in the compile command"; exit 1; } +[ "$ws_at" -lt "$mem_at" ] || { + echo "FAIL: workspace cxxflags must precede the member's own" + echo " workspace at line $ws_at, member at line $mem_at"; exit 1; } + +# ── inside: the second inheritance site ───────────────────────────────────── +( cd silent && "$MCPP" build > ../build_inside.log 2>&1 ) \ + || { cat build_inside.log; exit 1; } +got=$(flags_of silent) +case "$got" in + *"-DFROM_WORKSPACE=1"*"-std=c++26"* | *"-std=c++26"*"-DFROM_WORKSPACE=1"*) ;; + *) echo "FAIL: inheritance did not happen when the command was issued" + echo " inside the member directory (the second site)" + echo " got: $got"; exit 1 ;; +esac + +# ── a member with no version and no workspace value is still refused ──────── +# +# THE DENOMINATOR. Deferring the required-field check must not delete it: with +# the workspace value removed, the same member must fail, or this feature has +# quietly made `version` optional for everyone. +sed -i 's/^version = "0.4.2"$//' mcpp.toml +rm -rf silent/target +if ( cd silent && "$MCPP" build > ../novers.log 2>&1 ); then + echo "FAIL: a member with no version and no [workspace.package] version built" + cat novers.log + exit 1 +fi +grep -q "package.version" novers.log || { + echo "FAIL: the refusal does not name the missing field" + cat novers.log + exit 1 +} + +echo "PASS: 321_workspace_inheritance" diff --git a/tests/e2e/322_dialect_flag_refusal.sh b/tests/e2e/322_dialect_flag_refusal.sh new file mode 100755 index 00000000..203d2481 --- /dev/null +++ b/tests/e2e/322_dialect_flag_refusal.sh @@ -0,0 +1,110 @@ +#!/usr/bin/env bash +# requires: gcc +# 322_dialect_flag_refusal.sh — a dialect flag that misses the std BMI (#527). +# +# `[build] cxxflags = ["-fno-exceptions"]` reaches every translation unit and +# not the precompiled `import std` BMI, so every importer fails inside a file +# mcpp generated, with a message that names the mechanism and not the key that +# fixes it. The build cannot succeed, which is why this is refused rather than +# warned about — contrast the host-dependence diagnostics, which fire on builds +# that work. +# +# FIVE CASES, AND THREE OF THEM MUST STAY GREEN. A check that refuses whenever +# it sees `-fno-exceptions` would pass the first two and has stopped testing +# the condition it claims to test. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" +mkdir -p src + +writes_manifest() { # $1 = the [build] body + printf '[package]\nname = "dia"\nversion = "0.1.0"\nstandard = "c++23"\n\n[build]\n%s\n' "$1" > mcpp.toml +} +uses_import_std() { + printf 'import std;\nint main(){ std::println("ok"); return 0; }\n' > src/main.cpp +} +no_import_std() { + printf '#include \nint main(){ std::printf("ok\\n"); return 0; }\n' > src/main.cpp +} + +# ── 1. refused, and the message names the key that fixes it ───────────────── +uses_import_std +writes_manifest 'cxxflags = ["-fno-exceptions"]' +rm -rf target +if "$MCPP" build > r1.log 2>&1; then + echo "FAIL: -fno-exceptions in cxxflags + import std should be refused" + cat r1.log; exit 1 +fi +grep -q 'dialect_cxxflags' r1.log || { + echo "FAIL: the refusal does not name \`dialect_cxxflags\`" + cat r1.log; exit 1 +} +# Refused BEFORE compiling: a refusal that arrives after the compiler has +# already produced the confusing error has added a line, not removed one. +# +# NOT `grep 'language dialect differs'`. The refusal QUOTES that phrase, so +# the assertion matched its own message and failed on a correct implementation. +# The signal that no compile ran is the absence of run_build_plan's "Compiling" +# banner, which is printed after prepare_build returns. +grep -q '^ *Compiling' r1.log && { + echo "FAIL: the compiler ran — the check fired too late" + cat r1.log; exit 1 +} +[ ! -d target ] || [ -z "$(find target -name '*.o' 2>/dev/null)" ] || { + echo "FAIL: objects were produced before the refusal" + exit 1 +} + +# ── 2. -fno-rtti is the same axis, so the list is exercised past entry one ── +writes_manifest 'cxxflags = ["-fno-rtti"]' +rm -rf target +if "$MCPP" build > r2.log 2>&1; then + echo "FAIL: -fno-rtti in cxxflags + import std should be refused"; cat r2.log; exit 1 +fi + +# ── 3. the effective flag set, not one table ──────────────────────────────── +# +# The same flag arrives from a profile and from a `[target...]` block, and both +# reach the compile line while neither reaches the prebuild. Reading only +# `[build] cxxflags` would be silent here. +printf '[package]\nname = "dia"\nversion = "0.1.0"\nstandard = "c++23"\n\n[profile.dev]\ncxxflags = ["-fno-exceptions"]\n' > mcpp.toml +rm -rf target +if "$MCPP" build > r3.log 2>&1; then + echo "FAIL: a dialect flag in [profile.dev] cxxflags should be refused too" + cat r3.log; exit 1 +fi + +# ── 4. GREEN: the correct spelling builds ─────────────────────────────────── +writes_manifest 'dialect_cxxflags = ["-fno-exceptions"]' +rm -rf target +"$MCPP" build > g1.log 2>&1 || { + echo "FAIL: dialect_cxxflags is the documented fix and must build" + cat g1.log; exit 1 +} + +# ── 5. GREEN: an AUTO-PROMOTED dialect flag in cxxflags is silent ─────────── +# +# `-D_GLIBCXX_USE_CXX11_ABI=` is already promoted into the prebuild by +# `dialect_flags()`, so it does reach the BMI. A check keyed on "is this flag +# dialect-class" instead of "did it reach the prebuild" would refuse here. +writes_manifest 'cxxflags = ["-D_GLIBCXX_USE_CXX11_ABI=0"]' +rm -rf target +"$MCPP" build > g2.log 2>&1 || { + echo "FAIL: an auto-promoted dialect flag must still build" + cat g2.log; exit 1 +} + +# ── 6. GREEN: no `import std` in the graph, so there is nothing to disagree ─ +# +# THE DENOMINATOR. +no_import_std +writes_manifest 'cxxflags = ["-fno-exceptions"]' +rm -rf target +"$MCPP" build > g3.log 2>&1 || { + echo "FAIL: without import std, -fno-exceptions is an ordinary per-unit flag" + cat g3.log; exit 1 +} + +echo "PASS: 322_dialect_flag_refusal" diff --git a/tests/e2e/323_post_link_record_durability.sh b/tests/e2e/323_post_link_record_durability.sh new file mode 100755 index 00000000..9a1062d7 --- /dev/null +++ b/tests/e2e/323_post_link_record_durability.sh @@ -0,0 +1,134 @@ +#!/usr/bin/env bash +# requires: elf +# 323_post_link_record_durability.sh — the post-link verdicts survive an +# invocation, and survive the OTHER command (#529). +# +# Both post-link ELF passes were written with a read-back: an artifact whose +# stat did not move keeps the verdict already on file. It never fired across +# invocations, because the file it read was `resolution.json` and +# `prepare_build` rewrites that from an empty object at the start of every run. +# Measured before the fix: 1.36 s of a 1.94 s warm `mcpp test`. +# +# WHY A SINGLE-COMMAND TEST CANNOT SEE ANY OF THIS. Within one invocation +# the read-back already worked — the second ninja drive paid nothing. And +# repeating one command hides the second half: `mcpp build` and `mcpp test` +# share an output directory with DIFFERENT link-unit sets, and pruning the +# record against the current plan makes each delete the other's verdicts. So +# the assertions below are across processes AND across commands. +# +# The timing is deliberately not asserted — it is a machine property. What is +# asserted is the record: its CONTENT is what makes the memo correct, and a +# memo that is correct is what makes the build fast. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +mkdir -p src tests +printf '[package]\nname = "dur"\nversion = "0.1.0"\nstandard = "c++23"\n' > mcpp.toml +printf 'int main(){ return 0; }\n' > src/main.cpp +for t in one two; do printf 'int main(){ return 0; }\n' > "tests/t_$t.cpp"; done + +"$MCPP" build > b1.log 2>&1 || { cat b1.log; exit 1; } +OUT=$(dirname "$(find target -name build.ninja | head -1)") +[ -n "$OUT" ] || { echo "FAIL: no build.ninja"; exit 1; } + +REC="$OUT/.mcpp-runtime-verdicts.json" +RES="$OUT/resolution.json" + +paths_in() { # $1 = file, $2 = record name + sed -n "/\"$2\"/,/^ *\]/p" "$1" 2>/dev/null \ + | grep -oE 'bin/[A-Za-z_0-9]+' | sort -u | tr '\n' ' ' +} + +# ── the record must survive `prepare_build`, which is the whole defect ────── +[ -f "$REC" ] || { echo "FAIL: no durable record was written"; exit 1; } +grep -q 'post_link_key' "$REC" || { + echo "FAIL: the record carries no invalidation key — a stale SubOS farm or" + echo " a flipped MCPP_ALLOW_HOST_LIBS would be answered from cache" + exit 1 +} + +after_build=$(paths_in "$REC" loader_tags) +case "$after_build" in + *bin/dur*) ;; + *) echo "FAIL: the package binary is not in the record after a build" + echo " got: $after_build"; exit 1 ;; +esac + +# ── `mcpp test` must ADD to it, not replace it ────────────────────────────── +# +# THE ASSERTION WITH A DENOMINATOR. `mcpp test`'s plan contains the test +# binaries and NOT `bin/dur`; a record pruned against the current plan would +# come back holding only the tests, and "the pass was skipped" would then be +# satisfied by "the pass had nothing to look at". +"$MCPP" test > t1.log 2>&1 || { cat t1.log; exit 1; } +after_test=$(paths_in "$REC" loader_tags) +for want in bin/dur bin/t_one bin/t_two; do + case "$after_test" in + *"$want"*) ;; + *) echo "FAIL: after 'mcpp test' the record lost or never gained $want" + echo " got: $after_test"; exit 1 ;; + esac +done + +sp_after_test=$(paths_in "$REC" symbol_provision) +for want in bin/dur bin/t_one bin/t_two; do + case "$sp_after_test" in + *"$want"*) ;; + *) echo "FAIL: symbol_provision record is missing $want" + echo " got: $sp_after_test"; exit 1 ;; + esac +done + +# ── and a following `mcpp build` must not delete the test verdicts ────────── +"$MCPP" build > b2.log 2>&1 || { cat b2.log; exit 1; } +after_build2=$(paths_in "$REC" loader_tags) +for want in bin/dur bin/t_one bin/t_two; do + case "$after_build2" in + *"$want"*) ;; + *) echo "FAIL: 'mcpp build' pruned the test binaries out of the record." + echo " The two commands share one output directory; pruning" + echo " against the current plan makes each erase the other." + echo " got: $after_build2"; exit 1 ;; + esac +done + +# ── resolution.json keeps publishing both, on every build ─────────────────── +# +# The sidecar is authoritative; `resolution.json` is the documented place to +# read the verdicts (docs/05, `mcpp why runtime`, e2e 214 and 307). Publishing +# only when the content changed left it empty on every warm build — the record +# correct and the place people look for it empty. +for name in loader_tags symbol_provision; do + grep -q "\"$name\"" "$RES" || { + echo "FAIL: resolution.json does not publish $name after a warm build" + exit 1 + } + published=$(paths_in "$RES" "$name") + case "$published" in + *bin/dur*) ;; + *) echo "FAIL: resolution.json's $name is empty on a warm build" + echo " got: '$published'"; exit 1 ;; + esac +done + +# ── an artifact that leaves the DISK leaves the record ────────────────────── +# +# The pruning still has to happen, or the file grows forever. What changed is +# the predicate: gone from disk, not absent from this command's plan. +rm -f "$OUT/bin/t_two" +"$MCPP" build > b3.log 2>&1 || { cat b3.log; exit 1; } +final=$(paths_in "$REC" loader_tags) +case "$final" in + *bin/t_two*) echo "FAIL: a deleted artifact stayed in the record" + echo " got: $final"; exit 1 ;; +esac +case "$final" in + *bin/dur*) ;; + *) echo "FAIL: pruning removed more than the deleted artifact" + echo " got: $final"; exit 1 ;; +esac + +echo "PASS: 323_post_link_record_durability" diff --git a/tests/e2e/324_fast_path_sees_path_deps.sh b/tests/e2e/324_fast_path_sees_path_deps.sh new file mode 100755 index 00000000..4c91a7ad --- /dev/null +++ b/tests/e2e/324_fast_path_sees_path_deps.sh @@ -0,0 +1,84 @@ +#!/usr/bin/env bash +# requires: gcc +# 324_fast_path_sees_path_deps.sh — a new file in a `path` dependency is not +# invisible to the fast path. +# +# The staleness sweep was rooted at the project being built, so a source tree +# reached through `path = "../dep"` was outside all of it. An EDIT to an +# existing file was still caught, but not by the sweep: ninja rebuilt the object +# and the relink made the fast path abandon afterwards. A NEW FILE has no edge +# at all — nothing in the graph mentions it and every recorded timestamp is +# unmoved — so the fast path replayed a build.ninja that predated it. Measured +# before the fix: +# +# $ mcpp build +# Finished dev in 0.00s # and the module was never compiled +# +# This is #359's shape ("a glob input changes without any existing file's mtime +# changing") in a directory that fix did not reach. It matters most where it is +# hardest to notice: workspace members depend on each other by `path`. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +mkdir -p dep/src app/src +printf '[package]\nname = "dep"\nversion = "0.1.0"\nstandard = "c++23"\n[targets.dep]\nkind = "lib"\n' > dep/mcpp.toml +printf 'export module dep;\nexport int one(){ return 1; }\n' > dep/src/dep.cppm +printf '[package]\nname = "app"\nversion = "0.1.0"\nstandard = "c++23"\n[dependencies]\ndep = { path = "../dep" }\n' > app/mcpp.toml +printf 'import dep;\nint main(){ return one()==1 ? 0 : 1; }\n' > app/src/main.cpp + +cd app +"$MCPP" build > b1.log 2>&1 || { cat b1.log; exit 1; } +# Second build records the dependency source roots and arms the fast path. +"$MCPP" build > b2.log 2>&1 || { cat b2.log; exit 1; } + +# The fast path must actually be in play, or the rest of this test is asserting +# on a path it never took. A fast-path build prints no "Compiling" line. +"$MCPP" build > warm.log 2>&1 || { cat warm.log; exit 1; } +if grep -q "Compiling" warm.log; then + echo "NOTE: the fast path was not taken on a warm build; the assertion below" + echo " still holds but tests less than it means to." + cat warm.log +fi + +count_gcm() { find . ../dep -name '*.gcm' 2>/dev/null | wc -l | tr -d ' '; } +before=$(count_gcm) + +# ── the case that was invisible ───────────────────────────────────────────── +sleep 1 +printf 'export module dep.extra;\nexport int two(){ return 2; }\n' > ../dep/src/extra.cppm +"$MCPP" build > add.log 2>&1 || { cat add.log; exit 1; } + +if ! find . ../dep -name 'dep.extra.gcm' | grep -q .; then + echo "FAIL: a new source file in a path dependency was never compiled," + echo " and the build reported success:" + cat add.log + exit 1 +fi + +# THE DENOMINATOR. "It rebuilt" is satisfied by a full rebuild triggered for an +# unrelated reason; the count going up by exactly one says the new unit is what +# was added. +after=$(count_gcm) +[ "$after" -gt "$before" ] || { + echo "FAIL: the module-interface count did not grow ($before -> $after)" + exit 1 +} + +# ── and the dependency's MANIFEST is watched too ──────────────────────────── +# +# A member that gains a target or a glob changes what the graph should be, and +# none of that is visible from its source files' timestamps. +"$MCPP" build > steady.log 2>&1 +sleep 1 +printf '\n[build]\ncxxflags = ["-DDEP_TOUCHED=1"]\n' >> ../dep/mcpp.toml +"$MCPP" build > toml.log 2>&1 || { cat toml.log; exit 1; } +grep -q "DEP_TOUCHED" compile_commands.json || { + echo "FAIL: editing the path dependency's mcpp.toml did not re-plan" + cat toml.log + exit 1 +} + +echo "PASS: 324_fast_path_sees_path_deps" diff --git a/tests/e2e/325_system_toolchain_refused.sh b/tests/e2e/325_system_toolchain_refused.sh new file mode 100755 index 00000000..9d60ec1d --- /dev/null +++ b/tests/e2e/325_system_toolchain_refused.sh @@ -0,0 +1,117 @@ +#!/usr/bin/env bash +# requires: gcc +# 325_system_toolchain_refused.sh — mcpp builds only with toolchains it +# manages (#527 Bug 1). +# +# THE POLICY, AND WHY IT IS NOT UNIFORM ACROSS AXES. +# +# LIBRARIES are the program's business. A project may link a host library or +# its own `.so`; mcpp says what that costs and does not refuse, because the +# developer owns the artifact and guarantees it. +# +# THE TOOLCHAIN is mcpp's own contract. Everything mcpp promises — that +# `import std` is available, that the runtime closure is computable, that two +# machines and CI produce the same build — is a statement about a compiler +# mcpp resolved and can identify. A compiler picked off PATH makes every one +# of those unverifiable, so `[toolchain] … = "system"` is refused. +# +# `msvc@system` is the single exception and is a different spelling: it names a +# FAMILY whose installation mcpp locates, on the one platform where the +# compiler cannot be redistributed. +# +# What this replaces: the same configuration used to die as +# `posix_spawnp('') failed (error 2)` as soon as the project had a build.mcpp, +# because the resolved compiler path sat in `tc->binaryPath` and was never +# handed to the build.mcpp closure. A refusal that arrives as a crash three +# layers down is not a policy, it is a bug wearing one. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" +mkdir -p src +printf '#include \nint main(){ std::printf("hi\\n"); return 0; }\n' > src/main.cpp + +manifest() { # $1 = extra body + printf '[package]\nname = "systc"\nversion = "0.1.0"\nstandard = "c++23"\n%s' "$1" > mcpp.toml +} + +expect_refusal() { # $1 = log file, $2 = what was being built + grep -q 'toolchains it manages' "$1" || { + echo "FAIL: $2 was not refused on the toolchain policy's own terms" + cat "$1"; exit 1; } + # A refusal has to be actionable, or it is only a complaint. Three things + # make it so: what to write instead, where to see the choices, and the one + # exception — a Windows user reading a blanket "system is not supported" + # would otherwise conclude `msvc@system` had been removed too. + grep -q 'gcc@16.1.0' "$1" || { + echo "FAIL: the refusal does not show what to write instead"; cat "$1"; exit 1; } + grep -q 'mcpp toolchain' "$1" || { + echo "FAIL: the refusal does not name the command that lists the choices" + cat "$1"; exit 1; } + grep -q 'msvc@system' "$1" || { + echo "FAIL: the refusal does not name the one supported exception" + cat "$1"; exit 1; } + # And it must not read as a ban on host libraries, which are a different + # axis with the opposite answer. + grep -qi 'librar' "$1" || { + echo "FAIL: the refusal does not distinguish the library axis" + cat "$1"; exit 1; } +} + +# ── refused without a build.mcpp ──────────────────────────────────────────── +manifest ' +[toolchain] +linux = "system" +macos = "system" +windows = "system" +' +rm -rf target +if "$MCPP" build > plain.log 2>&1; then + echo "FAIL: [toolchain] system built; mcpp builds only with managed toolchains" + cat plain.log; exit 1 +fi +expect_refusal plain.log "a plain project" + +# ── refused WITH a build.mcpp, and not as a spawn failure ─────────────────── +# +# THE ORIGINAL DEFECT. The refusal has to reach the user before anything tries +# to compile the build program, or they get exit 127 and an empty program name. +printf '#include \nint main(){ return 0; }\n' > build.mcpp +rm -rf target +if "$MCPP" build > bp.log 2>&1; then + echo "FAIL: [toolchain] system with a build.mcpp built" + cat bp.log; exit 1 +fi +expect_refusal bp.log "a project with a build.mcpp" +grep -q "posix_spawnp" bp.log && { + echo "FAIL: still spawning an empty compiler path instead of refusing" + cat bp.log; exit 1; } +grep -q "build.mcpp compiling" bp.log && { + echo "FAIL: the refusal arrived after the build program had started" + cat bp.log; exit 1; } + +# ── the same via the environment side channel ─────────────────────────────── +rm -f build.mcpp +manifest '' +rm -rf target +if MCPP_TOOLCHAIN=system "$MCPP" build > env.log 2>&1; then + echo "FAIL: MCPP_TOOLCHAIN=system built; the policy must not have two answers" + cat env.log; exit 1 +fi +expect_refusal env.log "MCPP_TOOLCHAIN=system" + +# ── THE DENOMINATOR: a managed toolchain still builds ─────────────────────── +# +# Without this, "everything is refused" satisfies every assertion above. +manifest '' +rm -rf target +"$MCPP" build > managed.log 2>&1 || { + echo "FAIL: a project with no [toolchain] at all must still build" + cat managed.log; exit 1 +} +grep -q 'toolchains it manages' managed.log && { + echo "FAIL: the refusal fired for a project that did not ask for system" + cat managed.log; exit 1; } + +echo "PASS: 325_system_toolchain_refused" diff --git a/tests/e2e/326_dependency_standard_floor.sh b/tests/e2e/326_dependency_standard_floor.sh new file mode 100755 index 00000000..ba317a4b --- /dev/null +++ b/tests/e2e/326_dependency_standard_floor.sh @@ -0,0 +1,88 @@ +#!/usr/bin/env bash +# requires: gcc +# 326_dependency_standard_floor.sh — a dependency that DECLARED a higher +# standard than the graph is built at (#527 RFC 3). +# +# A C++ module graph has one standard: cross-level BMIs are hard incompatible, +# so the root's level is imposed graph-wide and a dependency's `standard` is +# parsed and then discarded. That is correct. The defect was the silence — a +# package declaring c++26 because it needs c++26 is compiled at whatever the +# consumer says, and fails, if it fails at all, inside a translation unit the +# user does not own. +# +# SCOPED TO AUTHOR-OWNED MANIFESTS, AND THE NEGATIVE IS THE HARD HALF. +# `standardDeclared` says the key was present, not that a human meant it: every +# index descriptor with an mcpp segment declares `language` (measured: 782 of +# 782 in the local registry, 756 of those 774 packages being C libraries with +# `import_std = false` carrying a boilerplate "c++23"). A check that trusted +# declaredness everywhere would fire against the whole index for any root at +# c++20 — which is exactly why the cpp20 design doc deferred it. +# +# DEGRADED, NOT AN ERROR: a package declaring c++26 compiles fine at c++23 +# whenever it does not use a C++26 construct, and that is a green build today. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +mkdir -p lib/src app/src +printf '[package]\nname = "flib"\nversion = "0.1.0"\nstandard = "c++26"\n[targets.flib]\nkind = "lib"\n' > lib/mcpp.toml +printf 'export module flib;\nexport int f(){ return 1; }\n' > lib/src/flib.cppm +printf '[package]\nname = "fapp"\nversion = "0.1.0"\nstandard = "c++23"\n[dependencies]\nflib = { path = "../lib" }\n' > app/mcpp.toml +printf 'import flib;\nint main(){ return f()==1 ? 0 : 1; }\n' > app/src/main.cpp + +cd app + +# ── it fires, and the build still succeeds ────────────────────────────────── +"$MCPP" build > warn.log 2>&1 || { + echo "FAIL: the floor check must be degraded, not an error" + cat warn.log; exit 1 +} +grep -q 'flib' warn.log && grep -q 'c++26' warn.log || { + echo "FAIL: the diagnostic does not name the package and the level" + cat warn.log; exit 1 +} +grep -q 'workspace.package' warn.log || { + echo "FAIL: the hint does not name the one-place fix" + cat warn.log; exit 1 +} + +# ── --strict promotes it, in the one place that policy lives ──────────────── +rm -rf target +if "$MCPP" build --strict > strict.log 2>&1; then + echo "FAIL: --strict did not promote the degradation" + cat strict.log; exit 1 +fi + +# ── SILENT when the dependency did not declare one ────────────────────────── +# +# THE DENOMINATOR for "declared". Same graph, same levels, the key removed. +printf '[package]\nname = "flib"\nversion = "0.1.0"\n[targets.flib]\nkind = "lib"\n' > ../lib/mcpp.toml +rm -rf target +"$MCPP" build > silent.log 2>&1 || { cat silent.log; exit 1; } +grep -q 'declares standard' silent.log && { + echo "FAIL: fired for a dependency that declared nothing" + cat silent.log; exit 1 +} + +# ── SILENT when the dependency declares a level at or below the graph's ───── +printf '[package]\nname = "flib"\nversion = "0.1.0"\nstandard = "c++23"\n[targets.flib]\nkind = "lib"\n' > ../lib/mcpp.toml +rm -rf target +"$MCPP" build > equal.log 2>&1 || { cat equal.log; exit 1; } +grep -q 'declares standard' equal.log && { + echo "FAIL: fired for a dependency at the graph's own level" + cat equal.log; exit 1 +} + +# ── and raising the consumer silences it, which is what the hint promises ─── +printf '[package]\nname = "flib"\nversion = "0.1.0"\nstandard = "c++26"\n[targets.flib]\nkind = "lib"\n' > ../lib/mcpp.toml +printf '[package]\nname = "fapp"\nversion = "0.1.0"\nstandard = "c++26"\n[dependencies]\nflib = { path = "../lib" }\n' > mcpp.toml +rm -rf target +"$MCPP" build > raised.log 2>&1 || { cat raised.log; exit 1; } +grep -q 'declares standard' raised.log && { + echo "FAIL: the documented fix does not silence the diagnostic" + cat raised.log; exit 1 +} + +echo "PASS: 326_dependency_standard_floor"