Skip to content

Commit 30e9382

Browse files
committed
refactor(modules): nine subsystems, and the build.mcpp contract among them
The unit under `modules/` is a SUBSYSTEM, not a file. One file per package would turn a directory listing into nine manifests and call it architecture. libs json + toml -- text-format parsers, vendored and in-house log leveled diagnostics versioning mcpp.version + mcpp.version_req source-kind the source-file role table dyndep ninja dyndep emission platform the OS abstraction: fs, process, axis, shell, terminal manifest mcpp.toml and the xpkg descriptor, plus the vocabulary they are written in toolchain-model what a toolchain IS -- triple, model, dialect, cppfly, fingerprint, linkmodel buildmcpp the build.mcpp CONTRACT -- program_protocol, directives, provisions, tool_store Dependencies between them are declared and enforced rather than assumed, so `mcpp test -p <member>` builds each one ALONE and a subsystem relying on something it does not declare fails there while compiling fine in the root. ## Three cuts, one mistake Every group-level cycle was produced by a countable set of back-edges, and each one was a module named for where it sat rather than for what it did. `src/platform/xlings/` was never platform: it is the xlings integration, which by its nature knows about packages, indexes and manifests. Its modules were named `mcpp.platform.xlings` while their namespace already said `mcpp::xlings`; both now agree, in `src/xlings/`. `runtime_binding.cppm` imported the whole of `mcpp.config` to read ONE path out of it, which put the OS layer above configuration, xlings and the package manager. The caller passes the path; the file describes the xlings runtime contract rather than the OS, so it moved to `src/runtime/` with its only dependent. `toolchain.fingerprint` imported `toolchain.detect` for a type -- `Toolchain` -- that `toolchain.model` defines and `detect` merely re-exports. The import named a CONSUMER of the type rather than its provider and dragged toolchain detection behind it. That one is load-bearing: every module on the build-program side needs `fingerprint` for `hash_file`, so while that edge pointed at `detect` the whole build.mcpp contract sat transitively above the package manager. One import line was the entire distance. ## What stays in src/, and why it is not a defect `build_program.cppm` and `hostprogram.cppm` -- the half that compiles and runs the program -- need `toolchain.registry` and `toolchain.stdmod`, which reach into detection, the compiler families and xlings. A thing that compiles and runs a program is a toolchain consumer by construction; moving it would relocate the dependency rather than remove it. ## Tests at two altitudes Each package carries its own tests, and the root keeps a suite that crosses layers. A subsystem test going red means the contract changed; the root's cross-check going red means the contract held and a consumer drifted. `tests/unit/test_subsystem_contracts.cpp` is the new one: it asserts that the protocol number the bundled module announces is SUBSTITUTED rather than spelled (so the announced value cannot drift from the checked one), and that the compiled binary's version equals the root manifest's. ⚠️ `mcpp test -p <member>` exits 0 for a member with no tests, so CI's loop is green either way and "has no tests" reads exactly like "tests pass". `check_modules_wiring.sh` prints which members have none, and does not fail on them. `versioning` is not called `version`: that word is a reserved key inside a dependency specification, so `[dependencies.mcpp] version = { path = … }` parses as the mcpp dependency's version field and fails with an error about the wrong table.
1 parent b4088c0 commit 30e9382

116 files changed

Lines changed: 777 additions & 195 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.agents/docs/2026-08-29-build-rule-package-spec.md

Lines changed: 109 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -567,6 +567,39 @@ before, once for the directive cache and once for the advisory channel.
567567
limit. The question is which objects join the build program's link, and it
568568
has to be answered before a rule can depend on an ordinary library.
569569

570+
4. **`role = "check"` cannot be written portably, and this is the finding the
571+
second rule produced.** Section 4 predicted that writing the next rule
572+
without consulting the guidance is what would expose the gaps; attempting a
573+
`clangtidy` package is what found this one.
574+
575+
A check's output is a stamp and **the command has to create it**. Analysers
576+
do not: clang-tidy's verdict is its exit code, and it writes no file on
577+
success. So a check action needs a wrapper — and an action's `command` is an
578+
argv with no shell assumed, which is correct for Windows and is exactly what
579+
makes the wrapper unwritable there. `tests/e2e/188` and
580+
`examples/08-build-rules` both reach for `sh`, and both are therefore POSIX
581+
only.
582+
583+
The consequence is that the role with no ecosystem consumer also has no
584+
portable way to acquire one, and that is not a coincidence.
585+
586+
The stamp is bookkeeping the graph needs, not something an author should
587+
have to produce — the contract of a check is already "the exit code is the
588+
verdict". Two shapes worth weighing:
589+
590+
- **mcpp creates the stamp when a check command exits zero** and its
591+
declared output does not exist. Backward compatible, since a command that
592+
already writes the stamp keeps working, and it needs no new process. It
593+
does mean ninja's output is written by something other than the edge's own
594+
command.
595+
- **mcpp wraps the command** (`mcpp __check --stamp <p> -- <argv…>`). One
596+
extra process per check, and the engine is already on disk on every
597+
platform, so it is portable by construction.
598+
599+
Neither is implemented. Until one is, a check rule should say in its own
600+
documentation that it requires a POSIX shell, rather than appearing to be
601+
portable and failing on a Windows consumer's first build.
602+
570603
---
571604

572605
## 9a. The work, and what depends on what
@@ -926,20 +959,35 @@ manifest removes all eight edges at once.
926959
927960
### 12.3 The layering that falls out of the measurement
928961
962+
**The unit is a SUBSYSTEM, not a file.** A package under `modules/` is a group
963+
of modules that answer one question and are used together; one file per package
964+
would turn a directory listing into nine manifests and call it architecture.
965+
929966
```
930-
modules/ independent packages, each with its own mcpp.toml, used by path
931-
M0 log, version, version_req, source_kind, dyndep, libs(json)
932-
6 modules, zero mcpp imports; extractable with no edge cut at all
933-
M1 platform, minus platform/xlings/
934-
20 files, 4,852 lines; one edge to cut
935-
M2 vocabulary: dep_spec, index_spec, compat, dependency_selector
936-
~680 lines; removes manifest's dependency on pm
937-
M3 manifest
938-
4 files, 5,560 lines
939-
src/ mcpp itself, including main.cpp and the binary target;
940-
everything not yet separated, and shrinking it is the direction
967+
modules/ independent packages, each with its own mcpp.toml, used by path
968+
libs json + toml — text-format parsers, vendored and in-house
969+
log leveled diagnostics
970+
versioning mcpp.version + mcpp.version_req
971+
source-kind the source-file role table
972+
dyndep ninja dyndep emission
973+
platform the OS abstraction: fs, process, axis, shell, terminal
974+
manifest mcpp.toml and the xpkg descriptor, plus the vocabulary they
975+
are written in (dep_spec, dependency_selector, index_spec,
976+
compat, glob, mangle, targetside)
977+
toolchain-model what a toolchain IS — triple, model, dialect, cppfly,
978+
fingerprint, linkmodel
979+
buildmcpp the build.mcpp CONTRACT — program_protocol, directives,
980+
provisions, tool_store
981+
src/ the build tool's skeleton: prepare, plan, execute, the
982+
toolchain families and detection, pm, pack, cli, xlings,
983+
runtime — and shrinking it is the direction
941984
```
942985
986+
Dependencies between them are declared and enforced, not assumed:
987+
`toolchain-model` → `platform`, `versioning`; `manifest` → `libs`, `platform`,
988+
`source-kind`, `versioning`; `buildmcpp` → `libs`, `manifest`, `source-kind`,
989+
`toolchain-model`. `platform` and the five below it declare nothing at all.
990+
943991
**Two directories, not three.** xlings additionally separates `apps/` from
944992
`modules/`, and that separation earns its place there because xlings produces
945993
two binaries — `xlings` and `xlings-gui` — so "what becomes a binary" is a
@@ -1024,7 +1072,53 @@ from the release, so the pinned version must already understand workspaces plus
10241072
path dependencies. xlings demonstrates the feature works; the pin is the thing
10251073
to check, not the feature.
10261074

1027-
**Order.** M0 first: six modules, zero edges to cut, and it exercises the whole
1028-
mechanism — workspace member, path dependency, bootstrap, cache, CI — against
1029-
the smallest possible blast radius. If M0 measures badly, nothing further should
1030-
proceed.
1075+
**Order.** The leaves first: six packages with no edges to cut, exercising the
1076+
whole mechanism — workspace member, path dependency, bootstrap, cache, CI —
1077+
against the smallest possible blast radius, before anything with a cut in it.
1078+
1079+
### 12.7 Where the boundary actually fell, and why
1080+
1081+
Three edges had to be cut, and each was the same mistake wearing different
1082+
clothes: a module named for where it sat rather than for what it did.
1083+
1084+
| Cut | What it was |
1085+
|---|---|
1086+
| `src/platform/xlings/``src/xlings/` | Not platform. The xlings integration, which by its nature knows about packages, indexes and manifests. Its module names said `mcpp.platform.xlings` while its namespace already said `mcpp::xlings`; the rename made the two agree. |
1087+
| `runtime_binding.cppm``src/runtime/` | Imported the whole of `mcpp.config` to read **one** path out of it, which put the OS layer above configuration, xlings and the package manager. The caller passes the path now. |
1088+
| `fingerprint → toolchain.detect` | The type it needed, `Toolchain`, is defined in `mcpp.toolchain.model`; `detect` merely re-exports it. The import named a **consumer** of the type rather than its provider, and dragged toolchain detection behind it. |
1089+
1090+
That third one is the load-bearing one. Every module on the build-program side
1091+
needs `fingerprint` for `hash_file`, so while that edge pointed at `detect` the
1092+
entire build.mcpp contract sat transitively above the package manager and could
1093+
not be separated from it. One import line was the whole distance.
1094+
1095+
**What stays in `src/`, and why it is not a defect.** `build_program.cppm` and
1096+
`hostprogram.cppm` — the half that compiles and runs the program — need
1097+
`toolchain.registry` and `toolchain.stdmod`, which reach into detection, the
1098+
compiler families and xlings. A thing that compiles and runs a program is a
1099+
toolchain consumer by construction. Moving it would relocate the dependency
1100+
rather than remove it, and the boundary between the contract and its executor
1101+
is the honest place to stop.
1102+
1103+
### 12.8 Tests at two altitudes
1104+
1105+
Each package under `modules/` carries its own tests, and the root keeps a suite
1106+
that crosses layers. The pair produces two distinguishable failures:
1107+
1108+
| Red | Meaning |
1109+
|---|---|
1110+
| the subsystem's own test | the contract changed |
1111+
| the root's cross-check | the contract held and a consumer drifted |
1112+
| both | the change was made once and propagated — the normal case |
1113+
1114+
The subsystem tests are also built in a configuration the root suite never
1115+
produces: the package **alone**, with only its declared dependencies present. A
1116+
subsystem that has quietly come to rely on something it does not declare
1117+
compiles in the root build and fails there, which is the reason to run both
1118+
rather than pick one.
1119+
1120+
⚠️ `mcpp test -p <member>` exits 0 for a member with no tests, so the CI loop is
1121+
green either way and "has no tests" reads exactly like "tests pass".
1122+
`check_modules_wiring.sh` therefore prints which members have none. It does not
1123+
fail on them: a package of vendored parsers legitimately has nothing of its own
1124+
to state.

.github/actions/bootstrap-mcpp/action.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ inputs:
1313
xlings-version:
1414
description: xlings release to bootstrap from
1515
required: false
16-
# Must equal `pinned::kXlingsVersion` in src/platform/xlings/xlings.cppm — enforced by
16+
# Must equal `pinned::kXlingsVersion` in src/xlings/xlings.cppm — enforced by
1717
# .github/tools/check_version_pins.sh, not by this comment. (The previous
1818
# comment here listed the files to keep in lock-step, and that list was
1919
# already incomplete; that is why the check exists.)

.github/tools/check_modules_wiring.sh

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ set -uo pipefail
3131
cd "$(dirname "$0")/../.."
3232

3333
fail=0
34+
untested=()
3435
bad() { echo "FAIL: $*" >&2; fail=1; }
3536
note() { echo " $*"; }
3637

@@ -71,8 +72,31 @@ for d in "${dirs[@]}"; do
7172
# (4) the two package-manager-less builds
7273
[[ -d "$d/src" ]] \
7374
|| bad "$d has no src/ — the xmake source lists glob modules/*/src/**.cppm and would silently miss it"
75+
76+
# (5) tests, REPORTED rather than required.
77+
#
78+
# `mcpp test -p <member>` exits 0 for a member with no tests, so CI's
79+
# per-subsystem loop is green either way and "has no tests" is
80+
# indistinguishable from "tests pass" in its output. Naming them here is
81+
# what makes the difference visible; failing would be wrong, because a
82+
# package of vendored parsers legitimately has nothing of its own to state.
83+
n=0
84+
[[ -d "$d/tests" ]] && n=$(find "$d/tests" -name '*.cpp' | wc -l)
85+
if (( n == 0 )); then
86+
untested+=("$name")
87+
else
88+
note "$name: $n subsystem test file(s)"
89+
grep -q '^\[dev-dependencies' "$d/mcpp.toml" \
90+
|| bad "$d has tests but declares no [dev-dependencies] — they cannot link a framework"
91+
fi
7492
done
7593

94+
if (( ${#untested[@]} )); then
95+
echo
96+
echo " no subsystem tests (allowed, and reported so it is not read as a pass):"
97+
printf ' %s\n' "${untested[@]}"
98+
fi
99+
76100
# The two xmake source lists must actually carry the glob. Asserted by content
77101
# rather than by trusting the loop above: a renamed pattern would leave every
78102
# per-module check passing while compiling nothing.
@@ -87,11 +111,11 @@ done
87111
# appears in three places and has already moved once.
88112
for f in scripts/bootstrap-macos.sh .github/workflows/bootstrap-macos.yml \
89113
bench/projects/mcpp/xmake.lua; do
90-
grep -qF 'modules/json/src/json' "$f" \
91-
|| bad "$f does not add the json include dir (modules/json/src/json)"
114+
grep -qF 'modules/libs/src/json' "$f" \
115+
|| bad "$f does not add the json include dir (modules/libs/src/json)"
92116
done
93-
[[ -f modules/json/src/json/json.hpp ]] \
94-
|| bad "modules/json/src/json/json.hpp is missing, but three files name that path"
117+
[[ -f modules/libs/src/json/json.hpp ]] \
118+
|| bad "modules/libs/src/json/json.hpp is missing, but three files name that path"
95119

96120
if (( fail )); then
97121
echo

.github/tools/check_version_pins.sh

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,13 @@
55
# Two invariants that used to live only in a comment:
66
#
77
# 1. Every xlings version pinned anywhere in .github/ equals
8-
# `pinned::kXlingsVersion` in src/platform/xlings/xlings.cppm — which is the version
8+
# `pinned::kXlingsVersion` in src/xlings/xlings.cppm — which is the version
99
# `mcpp self env` reports and the one release.yml bundles into the
1010
# tarball as <install>/registry/bin/xlings.
1111
#
1212
# 2. mcpp's own version is identical in all four places that carry it.
1313
#
14-
# Why this exists: src/platform/xlings/xlings.cppm used to say "keep in lock-step with the
14+
# Why this exists: src/xlings/xlings.cppm used to say "keep in lock-step with the
1515
# XLINGS_VERSION pins in release.yml / cross-build-test.yml / ci-linux-e2e.yml"
1616
# and that list was ALREADY incomplete — it omitted both composite actions,
1717
# which sat on 0.4.30 while everything else moved to 0.4.69. CI's sandbox
@@ -38,12 +38,12 @@ strip_comments() { sed 's/#.*//'; }
3838

3939
# ── 1. xlings pins ────────────────────────────────────────────────────────
4040

41-
XLINGS_EXPECTED=$(grep -oE 'kXlingsVersion[[:space:]]*=[[:space:]]*"[^"]+"' src/platform/xlings/xlings.cppm \
41+
XLINGS_EXPECTED=$(grep -oE 'kXlingsVersion[[:space:]]*=[[:space:]]*"[^"]+"' src/xlings/xlings.cppm \
4242
| grep -oE '"[^"]+"' | tr -d '"' | head -1)
4343
[ -n "$XLINGS_EXPECTED" ] || {
44-
echo "FAIL: could not read kXlingsVersion from src/platform/xlings/xlings.cppm" >&2; exit 1; }
44+
echo "FAIL: could not read kXlingsVersion from src/xlings/xlings.cppm" >&2; exit 1; }
4545

46-
note "expected xlings pin: $XLINGS_EXPECTED (src/platform/xlings/xlings.cppm)"
46+
note "expected xlings pin: $XLINGS_EXPECTED (src/xlings/xlings.cppm)"
4747

4848
# Anchored patterns only — a bare "version-looking number on a line mentioning
4949
# xlings" would also match `xlings install llvm@20.1.7`, which pins LLVM, not
@@ -100,13 +100,13 @@ done < <(find .github -type f \( -name '*.yml' -o -name '*.yaml' -o -name '*.sh'
100100
# ── 2. mcpp's own version ─────────────────────────────────────────────────
101101

102102
v_toml=$(awk -F '"' '/^version[[:space:]]*=/{print $2; exit}' mcpp.toml)
103-
v_src=$(grep -oE 'MCPP_VERSION[[:space:]]*=[[:space:]]*"[^"]+"' src/version.cppm \
103+
v_src=$(grep -oE 'MCPP_VERSION[[:space:]]*=[[:space:]]*"[^"]+"' modules/versioning/src/version.cppm \
104104
| grep -oE '"[^"]+"' | tr -d '"' | head -1)
105105
v_xl=$(grep -oE '"mcpp"[[:space:]]*:[[:space:]]*"[^"]+"' .xlings.json \
106106
| grep -oE '"[^"]+"$' | tr -d '"' | head -1)
107-
note "mcpp version: building=$v_toml (src/version.cppm=$v_src) bootstrap pin=$v_xl"
107+
note "mcpp version: building=$v_toml (modules/versioning/src/version.cppm=$v_src) bootstrap pin=$v_xl"
108108

109-
for n in "mcpp.toml:$v_toml" "src/version.cppm:$v_src" \
109+
for n in "mcpp.toml:$v_toml" "modules/versioning/src/version.cppm:$v_src" \
110110
".xlings.json:$v_xl"; do
111111
[ -n "${n##*:}" ] || bad "${n%:*} — could not read the mcpp version"
112112
done
@@ -131,7 +131,7 @@ fi
131131
# same number by definition — release.yml derives the tag from the former
132132
# and the smoke test greps the latter out of `mcpp --version`.
133133
[ -z "$v_src" ] || [ "$v_src" = "$v_toml" ] \
134-
|| bad "src/version.cppm has '$v_src' but mcpp.toml has '$v_toml'"
134+
|| bad "modules/versioning/src/version.cppm has '$v_src' but mcpp.toml has '$v_toml'"
135135

136136
# (b) The version BOOTSTRAPPED FROM (.xlings.json) names a mcpp that is already
137137
# published, and is NOT required to equal the version being built. It

.github/workflows/bootstrap-macos.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ jobs:
7878
-- directly.
7979
add_files("modules/*/src/**.cppm")
8080
add_packages("cmdline")
81-
add_includedirs("modules/json/src/json")
81+
add_includedirs("modules/libs/src/json")
8282
set_policy("build.c++.modules", true)
8383
-- Static link libc++ for minimal runtime dependencies
8484
add_ldflags("-static-libstdc++", {force = true})

.github/workflows/ci-aarch64-fresh-install.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ on:
2020
branches: [ main ]
2121
paths:
2222
- 'src/build/build_program.cppm'
23-
- 'src/platform/process.cppm'
23+
- 'modules/platform/src/process.cppm'
2424
- 'tests/e2e/168_build_mcpp_musl_host_static.sh'
2525
- '.github/workflows/ci-aarch64-fresh-install.yml'
2626
push:

.github/workflows/ci-linux.yml

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,32 @@ jobs:
108108
run: |
109109
# Use freshly-built mcpp for test (it has --mirror support)
110110
MCPP_FRESH=$(realpath "$(find target -type f -name mcpp -printf '%T@ %p\n' | sort -rn | head -1 | cut -d' ' -f2)")
111+
echo "MCPP_FRESH=$MCPP_FRESH" >> "$GITHUB_ENV"
111112
"$MCPP_FRESH" self config --mirror GLOBAL
112113
"$MCPP_FRESH" test
113114
115+
# Each package under `modules/` carries its own tests, and they are built
116+
# as that package ALONE -- a configuration the root suite never produces,
117+
# since there every module is linked together. A subsystem that has
118+
# quietly come to depend on something it does not declare compiles in the
119+
# root build and fails here, which is the whole reason to run both.
120+
#
121+
# Every member is run, and the loop is derived from the manifest rather
122+
# than written out: a list maintained by hand is a list that stops
123+
# matching, and `check_modules_wiring.sh` cannot see this file.
124+
- name: Per-subsystem tests (`mcpp test -p <member>`)
125+
run: |
126+
set -euo pipefail
127+
members=$(sed -n '/^\[workspace\]/,/^\[/p' mcpp.toml \
128+
| grep -oE '"modules/[a-z0-9-]+"' | tr -d '"' | sed 's|modules/||')
129+
[ -n "$members" ] || { echo "no workspace members found in mcpp.toml"; exit 1; }
130+
echo "members: $(echo $members | tr '\n' ' ')"
131+
for m in $members; do
132+
echo "::group::mcpp test -p $m"
133+
"$MCPP_FRESH" test -p "$m"
134+
echo "::endgroup::"
135+
done
136+
114137
# A cold, from-scratch self-host build with the manifest-pinned GCC: the
115138
# property `build-test` cannot cover, because it builds incrementally on a
116139
# restored target/. `mcpp test` is deliberately NOT repeated here — it would

.github/workflows/cross-build-test.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ jobs:
106106
- name: Bootstrap mcpp via xlings
107107
env:
108108
XLINGS_NON_INTERACTIVE: '1'
109-
# Must equal `pinned::kXlingsVersion` (src/platform/xlings/xlings.cppm) and the
109+
# Must equal `pinned::kXlingsVersion` (src/xlings/xlings.cppm) and the
110110
# xlings the release bundles — enforced by
111111
# .github/tools/check_version_pins.sh.
112112
#

0 commit comments

Comments
 (0)