The compile side closes the host's C library the link side already dropped (#662) - #664
Merged
Merged
Conversation
…opped (#662) Target-side header isolation had a half. The link side has read `plan.targetSide.cAbi.prebuilt()` since #511 and dropped `-nostdlib` whenever a package supplies the target's C library; the compile side never got the same treatment, so clang's own driver kept searching the host's system directories. On a machine that happens to have the matching host headers (mingw-w64, in the issue's repro) the two C libraries' declarations collide and the error names neither the host header nor why it was read; on a machine without them the failure is a bare "file not found" with no explanation. There is a same-shaped C++ twin: the existing `-nostdinc++` condition asked `!graphSuppliesTarget && !cxxFromPayload`, which is false exactly when BOTH layers come from the graph — the openkal shape — so the driver kept finding the host's libstdc++ beside itself. The issue's own minimal repro only `import std`s and never text-includes a header, so it exposed neither half. - `mcpp.toolchain.hostflags` appends `-nostdlibinc` when the C library comes from the graph, and fixes the `-nostdinc++` condition to read only whether the C++ layer comes from the graph. Both tokens live in the global `cflags`/`cxxflags`/`asmflags` producer, so every consumer (C, C++, asm, `clang-scan-deps`, the std module precompile) gets them identically. - GCC has no one-token equivalent. `can_isolate_graph_c_library` states the capability in the toolchain model (Clang only); `prepare.cppm` refuses the combination at resolution when it would otherwise reach an unisolated build, rather than silently skipping isolation. No target row needs the GCC form today. - `mcpp::build::graph_c_library_isolation_advice`, the compile-side sibling of `link_failure_advice`: when a failed build's raw ninja output carries both the isolating token and a "file not found", it appends a note naming the C library and two remedies (adapt on `cfg(c-abi = "...")`, or bring the platform headers into the graph as a private dependency of the package that needs them) without rewriting the compiler's own line. - M5: verified rather than assumed that a `[feature-deps.<feature>]` entry can already stay private to its declaring package (`visibility = "private"`, folding only into `privateBuild`, never into `publicUsage`) — measured both directions with local path packages before writing e2e 739. The capability already existed and was undocumented; docs/06 now states the pattern and docs/24 gains a "boundary" section (kernel-abi vs c-abi, one C/C++ runtime per image, adapt on the differing layer, no source-level implementation detection) — both in English and zh. - Unit tests: the option matrix (`cAbiPrebuilt` x `cxxFromGraph` x {clang, gcc}), a byte-identical regression guard for payload-served builds, and `graph_c_library_isolation_advice`'s own matching rules. - e2e 738: openkal's `x86_64-windows-gnu` build, each C/C++ unit's actual command from the build database re-run with `-v -fsyntax-only`, asserting clang's own header search list names only store paths. Red before this fix (confirmed locally, both the C and the C++ twin), green after. Gated on a `mingw-host-headers` capability (the criterion has no discriminating power without a host mingw to have leaked in); `openkal-cross.yml`'s `ecosystem-e2e` job installs `mingw-w64` so it holds in CI, since `llvm` — which 738 also requires — is never in `run_all.sh`'s detected CAPS on any sharded runner. - e2e 739: the private-feature-dep guarantee, in both directions on the same two packages so the harness cannot pass vacuously. - Version 2026.9.17.3.
CI's check_docs_structure.sh caught this — python3 .github/tools/gen_agents_index.py.
docs/06's new 'A platform SDK dependency stays private' section cited [05 — Dependencies] for the visibility field, and that field (public / private / interface, defaulting to public) was not documented anywhere. Adds it, in English and zh, at the point in docs/05 where the other per-edge dependency-spec fields are introduced.
CI's "invariants (linux-x86_64)" job turned up what local testing had not:
268, 282 and 303 all went red. The common shape is a `provides =
["mcpp:c-abi=..."]` fixture package with no real headers, used to exercise
something the target-side report says — none of them about isolation —
under this host's ambient default toolchain, which is gcc. The refusal
fired before the target-side report line ever printed, which is exactly
what 268's own comment says must not happen ("the resolution is reported
during planning ... before a single object is compiled ... complete
whether or not the link afterwards succeeds"): a hard refusal at that
point is not a stricter report, it is the report's replacement.
GCC's behaviour on this combination reverts to precisely what it was before
#662: `hostflags.cppm` never emitted `-nostdlibinc` for GCC in the first
place (`bypassCfg` is already false there — no `<driver>.cfg` beside a GCC
binary), so nothing about GCC's own compile command changes. What is
removed is the extra, invented gate in `prepare_build` and the
`can_isolate_graph_c_library` predicate it read, both added preemptively
and unsupported by anything the plan actually required — measured against
the real test suite rather than against a hypothesis about it.
- `prepare.cppm`: the `LayerRequirement` refusal for "graph-supplied C
library + a compiler that cannot isolate" is removed.
- `modules/toolchain-model`: `can_isolate_graph_c_library` is removed —
unused once nothing enforces it, and its comment's claim about
`prepare.cppm` would otherwise go stale silently.
- `hostflags.cppm`, `test_hostflags.cpp`: comments corrected to state GCC's
actual, unchanged behaviour instead of a removed enforcement.
- Re-verified: 268, 282, 303 (the three CI caught), plus 292, 304, 328, 619,
281 (every other e2e using a synthetic c-abi/kernel-abi provider), 286
(the real openkal stack), 295-302 (the native-target and toolchain-origin
invariants), 130/133 (freestanding), 736, and 738/739 (this change's own
new tests) — all still green. Full unit suite: 121/121 files, 0 failed.
…aded The revert (bcbb3e0) removed the hard refusal for "graph-supplied C library + a compiler that cannot isolate" because it broke the "report before refusing" contract. That left GCC silent again on this combination -- which is what D4 actually ruled out. This adds the third option: a degradation, not a refusal and not silence. `mcpp::diag::degraded("target/c-abi-isolation", ...)` fires once resolution knows the C library came from the graph and the resolved compiler is not Clang-family. It names the C library (interface and package@version) and the compiler, points at a Clang-family toolchain as the one that isolates, and changes nothing about the command line -- `hostflags.cppm` was already not emitting an isolation token for GCC, and still is not. `--strict` promotes it to an error through the existing `diag::flush` policy, which is the one place that decision already lives. - `prepare.cppm`: the check sits right where `resolvedTargetSide` becomes available, beside the existing `target/compiler-runtime` degradation, and before `format_report` prints -- so the report is unaffected either way, which is what the earlier refusal got wrong. - e2e 740: names the library and the compiler in the warning, asserts the target-side report still prints (the exact thing the reverted refusal broke), and controls against a Clang-family toolchain over the same graph getting no such note. 268, 282, 303 re-verified green. - CHANGELOG's D4 bullet rewritten to describe what this branch actually does, not the removed refusal.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #662.
The link side has read
plan.targetSide.cAbi.prebuilt()since #511 and drops-nostdlibwhenever a package supplies the target's C library. The compileside never got the same treatment: clang's own driver kept searching the
host's system directories even when a graph package supplied the C library,
so a build over openkal could silently mix the host's headers with the
graph's C library, or fail with a bare "file not found" on a host that lacked
the matching headers. There is a same-shaped C++ twin: the existing
-nostdinc++condition was false exactly when BOTH the C library and the C++runtime came from the graph — the arrangement openkal actually ships — so the
driver kept finding the host's libstdc++ beside itself. Neither half is
reached by the issue's own minimal repro, which only
import stds.mcpp.toolchain.hostflagsappends-nostdlibincwhen the Clibrary comes from the graph, and the
-nostdinc++condition now readsonly whether the C++ layer comes from the graph. Both live in the shared
cflags/cxxflags/asmflagsproducer, so C, C++, asm,clang-scan-depsand the std module precompile all see them.
-nostdincplus-isystem <gcc -print-file-name=include>and<…/include-fixed>), sohostflags.cppmmakes no change to GCC's commandline at all — it never emitted an isolation token before this PR and still
does not. What GCC gets is a degradation, not silence and not a refusal:
mcpp::diag::degradedreports once, inprepare.cppm, when the resolvedC library comes from the graph and the resolved compiler is not
Clang-family — naming the library and the compiler, pointing at a
Clang-family toolchain as the one that isolates.
--strictpromotes it toan error through the existing
diag::flushpolicy. This shape is the thirdone tried: a hard refusal at resolution was tried first and reverted after
CI caught it firing before the target-side report prints, which broke
three pre-existing e2e fixtures (268, 282, 303) that use a synthetic
provides = ["mcpp:c-abi=..."]package on this host's native, GCC-defaulttarget for something unrelated to isolation — see the review history on
this PR for the measurement. 268/282/303 are re-verified green with the
degradation in place (e2e 740).
mcpp::build::graph_c_library_isolation_advice, the compile-sidesibling of
link_failure_advice: on a failed build whose raw ninja outputcarries both the isolating token and "file not found", it appends a note
naming the C library and two remedies, without rewriting the compiler's
own line.
[feature-deps.<feature>]entry canalready stay private to its declaring package (
visibility = "private").The capability already existed and was undocumented; docs/06 and docs/24
(English + zh) now state the pattern and the layer boundary.
CHANGELOG entry describing upgrade impact, including the corrected D4
account.
mcpp.toml,modules/versioning).Test plan
mcpp test test_hostflags— the option matrix(
cAbiPrebuilt x cxxFromGraph x {clang, gcc}), the openkal-shapedouble-graph regression test, and the byte-identical guard for
payload-served builds (GCC's command line is unaffected by this PR,
asserted directly). All pass.
mcpp test test_ninja_backend—graph_c_library_isolation_advicematching rules. All pass.
mcpp test, no filter) — 121/121 files pass, 0 failed(re-run after every substantive change on this branch).
x86_64-windows-gnu, each unit's real commandre-run with
-v -fsyntax-only, header search list asserted store-only.Confirmed RED on
main(host mingw and host libstdc++ both leakin) and GREEN on this branch, reproduced multiple times each way.
[feature-deps]entry does not reach theconsumer, both directions on the same two packages.
degradation, names the library and the compiler, the target-side
report still prints, and a Clang-family toolchain over the same graph
gets no such note (the harness's own validity check).
broke: 268, 281, 282, 292, 295-298, 301-304, 328, 619 (target-side /
toolchain-origin invariants), 286 (full openkal stack — static link,
no INTERP, runs), 130/133 (freestanding), 736 — all green.
bash .github/tools/check_version_pins.sh,check_docs_style.sh,check_docs_structure.sh— OK.iOS - what this runner actually provides).Review history
An earlier revision of this PR (commits up to
d82137b6) implemented D4 asa hard refusal in
prepare_build. CI'sinvariants (linux-x86_64)jobcaught what local testing had not: it broke 268, 282 and 303 by firing
before the target-side report prints, which is exactly what 268's own
comment says must never happen.
bcbb3e04reverted the refusal;356c77ecreplaces it with the degradation described above, which keeps the same three
files green while still surfacing the gap rather than leaving it silent.
Not done / deferred (by design, see plan §3.2 and §5)
compat.zlibPOSIX branch undercfg(all(windows, c-abi = "musl"))(issue's actualcompat:libarchivefailure) is a separate PRin
mcpplibs/mcpp-index, pinned to this release, per the plan's I1–I3.It cannot land before this PR is released.
covered by existing e2e (130/133/730s) staying green plus the unit-test
guard against a duplicated
-nostdinc++; no macOS runner was availablelocally to re-verify the iOS leg directly (CI's iOS job is green).