Skip to content

Commit f176abd

Browse files
2026.9.25.1: workspace members compile the same way in every position, and a consumer's headers stay in the consumer (#690) (#691)
* docs: #690 design record and implementation plan * fix(workspace): one inheritance pipeline for every member, keyed defines, one key table (#690) A member reached as a dependency now inherits [workspace.package], x.workspace = true entries and [workspace.build] at its load site, before the conditional merge and the defines fold, as the root does. The snapshot in makePackageRoot no longer inherits and refuses unfolded defines with an internal error. A member of a git-hosted workspace inherits from its own repository. [build] defines is a set keyed by macro name, and !NAME removes an inherited name. The inheritable [build] subset is one exported table, which accepts ios_deployment_target. The e2e harness links payloads per version, so a version installed by a test no longer lands in the developer's registry. * fix(build): a consumer's include directories stay in the consumer (#690 W6) The root's [build] include_dirs and include_dirs_after were written into the file-level $cxxflags/$cflags/$asmflags/$nasmflags of build.ninja, which every unit in the graph reads. A root header named like a system header shadowed it inside a dependency, private_include_dirs included, and because no cache-key axis contained the broadcast, a dependency object in the global cache could be compiled against another project's root headers (measured: cJSON_Compare answered 1 in an unrelated project). - flags.cppm: no include directory is broadcast; every unit keeps its own package's directories through $local_includes (C, C++, GAS and NASM). - cache_key.cppm: kCacheEpoch 3 -> 4, orphaning entries written while the broadcast existed. - ninja_backend.cppm / execute.cppm: consumer_include_scope_advice names the consumer directory that holds a header a dependency now fails to find, on the full path and, through a sidecar beside build.ninja, on the fast path. - tests: unit tests for the file-level channel, the unit channel's absolutisation, the advice and the sidecar; e2e 765. * docs: workspace inheritance in every position, keyed defines, include scope; SPEC-004 1.6 (#690) * docs: noun-phrase heading for the defines and include scope section * docs: #690 sandbox verification script * publish: the effective manifest, a normalised published form, and a reproducible archive (#690 W4, W5) * test: renumber the published-form e2e scripts to 772 and 773 * fix: every reader of a member manifest reads the effective manifest; 2026.9.25.1 (#690 W4) prepare_build loads through load_effective_manifest and no longer inherits a preloaded host-tool manifest a second time. sbom, index list/update, the index refresh of mcpp update, the fast-path identity and test discovery read the effective manifest. * chore: xlings pin 2026.9.16.1 -> 2026.9.20.1 * fix: a member inside an index archive inherits its workspace; publish reads the one key table (#690) inherit_as_workspace_member is the one function for a sibling path dependency, a git-hosted member and a member inside an index package's archive (Form A pointer), searched no higher than the install root. normalize.cppm writes [workspace.build] back through kWorkspaceBuildKeys instead of a second copy of the key set. e2e 774. * docs: publishing a workspace member, the index-archive member, CHANGELOG 2026.9.25.1, SPEC-004 criteria 13-14 * publish: the archive commit ignores commit.gpgSign and the manifest blob ignores .gitattributes * fix: a host-tool sub-build merges its dependency's conditional sections once (#690 F12); self-review record * test: host-spelled fixture path in 772, build-directory words excluded from 765 (c), cmd.exe quoting in 775 --------- Co-authored-by: speak-agent <x.d2learn.org@gmail.com> Co-authored-by: speak-agent <248744407+speak-agent@users.noreply.github.com>
1 parent 2687444 commit f176abd

51 files changed

Lines changed: 3680 additions & 327 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
---
2+
subject: plan
3+
status: active
4+
---
5+
6+
# #690: implementation plan
7+
8+
- Design: [2026-09-25-issue-690-workspace-build-inheritance-consistency.md](2026-09-25-issue-690-workspace-build-inheritance-consistency.md)
9+
- Base: `origin/main` b4824697 (mcpp 2026.9.24.1). Release target: **mcpp 2026.9.25.1**.
10+
- Delivery: one pull request in `mcpp-community/mcpp` that carries every workstream, followed by the release chain in the repositories listed in section 5. The design record's per-workstream pull requests (its section 7) are merged into one, as requested in review. Each workstream keeps its own criterion inside that pull request.
11+
12+
---
13+
14+
## 1. Tasks
15+
16+
| Id | Workstream | Content | Files (owner) |
17+
|---|---|---|---|
18+
| T1 | W1 | Build inheritance moves to the dependency load site. `makePackageRoot` stops inheriting. Snapshot post-condition. | `src/build/prepare.cppm` (lead) |
19+
| T2 | W2 | One `[workspace.build]` key table. `ios_deployment_target` accepted. | `modules/manifest/src/toml.cppm` (lead) |
20+
| T3 | W3 | A git-hosted member inherits its repository's `[workspace.build]`. | `src/build/prepare.cppm` (lead) |
21+
| T4 | W4 | `load_effective_manifest`. `publish`, `pack`, `emit xpkg` and `toolchain list` read it. | `src/project.cppm`, `src/publish/pipeline.cppm`, `src/pack/route.cppm`, `src/toolchain/lifecycle.cppm` (agent B) |
22+
| T5 | W5 | Normalised published manifest, sibling-edge rewrite, reproducible archive from git objects, descriptor `deps` from the normalised manifest. | `src/pm/publisher.cppm`, `modules/libs/src/toml.cppm` (serialiser), new `src/publish/normalize.cppm` (agent B) |
23+
| T6 | W6 | Root include broadcast removed (C, C++, NASM). `kCacheEpoch` 3 to 4. Consumer-include advice on a dependency's missing header. Command-identity unit test. | `src/build/flags.cppm`, `src/build/cache_key.cppm`, `src/build/ninja_backend.cppm`, `src/build/execute.cppm` (agent A) |
24+
| T7 | W7 | `[build] defines` is a keyed set. `!NAME` removes an inherited entry. | `src/build/prepare.cppm` (`fold_build_defines_into_flags`, lead) |
25+
| T8 | harness | `_inherit_toolchain.sh` links payloads per version, so a version installed by a test lands in the test's home and not in the developer's registry (#293 recurrence, measured 2026-09-25). | `tests/e2e/_inherit_toolchain.sh` (lead) |
26+
| T9 | docs | `docs/07`, `docs/04`, `docs/11` (English and 简体中文), SPEC-004 §8 amendment and §9, CHANGELOG, version 2026.9.25.1. | lead, after T1 to T8 |
27+
28+
## 2. Dependencies
29+
30+
```
31+
T2 ----------------------------+
32+
T1 --> T3 --> T7 --------------+--> T9 --> PR --> CI --> review --> merge --> release chain
33+
T4 --> T5 ---------------------+
34+
T6 ----------------------------+
35+
T8 ----------------------------+
36+
```
37+
38+
- T3 edits the same load site as T1 and follows it.
39+
- T7 edits the fold that T1's post-condition names, and follows T1.
40+
- T5 needs T4's loader.
41+
- T6 and T8 are independent of every other task.
42+
- Agents A (T6) and B (T4, T5) work in their own worktrees from the plan commit. The lead integrates by cherry-pick. File ownership in section 1 is exclusive. A task that needs a line in another owner's file reports it instead of editing it.
43+
44+
## 3. Review angles
45+
46+
Each angle names the property it requires and the evidence that shows it.
47+
48+
| Angle | Requirement | Evidence |
49+
|---|---|---|
50+
| Architecture | One pipeline per member. One effective-manifest loader. Private requirements stay inside their package. | Removal of the `makePackageRoot` inheritance block and of the `flags.cppm` broadcast. Every raw `manifest::load` of a project manifest is reviewed and recorded. |
51+
| Stability | No silent state. Internal invariants fail loudly. | Snapshot post-condition. Cache epoch increment. Harness fix T8. |
52+
| Simplicity | No new manifest keys. One new value form (`!NAME`). | Diff of the parser's key table. |
53+
| User experience | Every refusal names the file, the key and the fix. The dependency-header advice names the consumer directory. | e2e output assertions. |
54+
| Compatibility | Older engines read every published normalised manifest. No previously valid manifest is refused by the build. | e2e with the published 2026.9.24.1 binary as a consumer of a normalised archive. |
55+
| Cross-platform | Windows, macOS and Linux CI. Path relativisation and archive creation use git and `std::filesystem`, not host tools with platform-specific flags. | CI matrix. |
56+
| Consistency | `-p lib` and `-p app` compile `lib` identically. A git consumer compiles the member as its repository does. | e2e 321 counts. Git-member e2e. |
57+
| Seamless upgrade | No user action is required. The cost is one cold dependency-cache rebuild (epoch 4) and one rebuild of fingerprints whose duplicated flags disappear. | CHANGELOG. |
58+
| Test coverage | Every workstream has a criterion that fails with its fix removed. | Section 4. |
59+
60+
## 4. Criteria per task
61+
62+
| Task | Criterion | Kind |
63+
|---|---|---|
64+
| T1 | e2e 321: each workspace word exactly once, in both positions, in `.cpp` and `.c` entries, workspace before member. | e2e |
65+
| T1 | Snapshot with unfolded `defines` yields the internal error. | unit |
66+
| T2 | A workspace declaring every table row parses, and every value is inherited. | unit |
67+
| T3 | `file://` repository member: the consumer build passes an `#error` guard, and the flag occurs once. | e2e |
68+
| T4 | In a member that omits `version`, `emit xpkg` and `publish --dry-run` succeed. In a member without `[toolchain]`, `toolchain list` marks the workspace's toolchain. | e2e |
69+
| T5 | The archive's `mcpp.toml` carries the inherited values and a version edge. `mcpp.toml.orig` is present. The descriptor lists the sibling. A consumer of the unpacked archive builds, under the new engine and under 2026.9.24.1. An edge without `version` is refused with the sibling's version in the message. Two runs produce byte-identical archives. The serialiser round-trips. | e2e, unit |
70+
| T6 | A root private `limits.h` containing `#error` does not reach a path dependency or an index dependency. The dependency's compile command is identical under two roots that differ in include settings. The root's units carry each root directory once. The advice names the root directory. | e2e, unit |
71+
| T7 | `defines = ["X=1"]` inherited and `defines = ["X=2"]` in the member emit only `-DX=2`. `!X` emits no `-DX`. | e2e, unit |
72+
| T8 | After a test installs a version that the developer registry lacks, the developer registry still lacks it. | e2e harness check |
73+
74+
## 5. Release chain
75+
76+
1. PR in `mcpp-community/mcpp` with the version 2026.9.25.1 in `mcpp.toml` and `modules/versioning/src/version.cppm`, and the internal xlings pin at the latest xlings release (2026.9.20.1). All required checks green. Squash merge.
77+
2. `origin/main` HEAD run green.
78+
3. `release.yml` dispatched. Each archive is uploaded to GitCode with the local tools the moment it appears (`tools/mirror_res.sh` from xlings, GitCode leg only). GET 200 and byte comparison on both mirrors.
79+
4. `openxlings/xim-pkgindex` bump pull request merged. The published index artifact is read to confirm `latest`.
80+
5. `.xlings.json` (the mcpp that builds mcpp) is not moved: review of 2026-09-25 asked for no separate bootstrap pull request unless a build needs it.
81+
6. `mcpp-community/mcpp-index`: one pull request that moves its CI pins to 2026.9.25.1, which runs every member on every platform against the release. Before the release, 16 members were run locally with the candidate binary (all passed), and no member declares `include_dirs`, so the W6 change has no consumer-side reliance to break there.
82+
7. Sandbox verification: `.agents/docs/2026-09-25-issue-690-verify.sh` in `xlings subos use <n> --sandbox --cmd ...`, with the CN mirror set for both xlings and mcpp inside the sandbox, and the same script against 2026.9.24.1 as the control.
Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
---
2+
subject: review
3+
status: active
4+
---
5+
6+
# #690: self-review before release, engine and ecosystem
7+
8+
- Pull request: mcpp-community/mcpp#691 (2026.9.25.1)
9+
- Design: [2026-09-25-issue-690-workspace-build-inheritance-consistency.md](2026-09-25-issue-690-workspace-build-inheritance-consistency.md); plan: [2026-09-25-issue-690-implementation-plan.md](2026-09-25-issue-690-implementation-plan.md)
10+
- Method: the full `src/` and `modules/` diff read against the principles of the design record (P1 to P8), each finding checked by a measurement or a code citation, and the ecosystem consumers enumerated.
11+
12+
---
13+
14+
## 1. Findings of the review, and what was done
15+
16+
| # | Finding | Evidence | Resolution |
17+
|---|---|---|---|
18+
| R1 | The archive commit made by `mcpp publish` honoured `commit.gpgSign`. On a host that signs commits, the object would carry a signature timestamp, which breaks reproducibility, or it would fail where no key is available. | Code: `git commit-tree` without `--no-gpg-sign`. After the fix, measured with `commit.gpgsign = true` and `gpg.program = /bin/false` on the repository: `publish --dry-run` succeeds twice with the same sha256. | `--no-gpg-sign` added. |
19+
| R2 | The manifest blob was hashed with the repository's filters, and the scratch file lies inside the repository (`target/dist`). | Code: `git hash-object -w` without `--no-filters`. | `--no-filters` added. |
20+
| R3 | The publisher carried a second copy of the inheritable key set, written in parallel with `kWorkspaceBuildKeys` because the two tasks started from the same base. | Code: `kStringVectors`, `kPathVectors`, `kScalars` in `normalize.cppm`. | Replaced by `kWorkspaceBuildKeys`. |
21+
| R4 | A host-tool sub-build receives its dependency's manifest preloaded. After W1 that manifest is already inherited, and the sub-build's member branch would have inherited it a second time. | Code: `prepare_build` member branch before W4. | The preloaded manifest is treated as effective. Only its workspace is recorded, for the membership test of its own dependencies. |
22+
| R5 | Two e2e scripts from the parallel tasks used the number 770, the same as the lead's. | Directory listing. | Renumbered to 772 and 773. |
23+
| R7 | A host-tool sub-build merged its dependency's conditional sections a second time (design record F12). Found as an open item of this review and then measured: `-include once.h` from a matching section reached the tool twice, on 2026.9.24.1 as well. | e2e 775 fails on 2026.9.24.1 and passes on the candidate; the package builds on its own (control). | The sub-build receives `Manifest::beforeConditionalMerge`. |
24+
| R6 | A member inside an index archive (a Form A descriptor pointing at `*/<dir>/mcpp.toml`) did not inherit its archive's workspace, which is the same position independence gap as F4 for a third route. | Measured, design record F11 and e2e 774. | Applied through `inherit_as_workspace_member`, bounded by the install root. |
25+
26+
## 2. Principles, checked
27+
28+
| Principle | Holds because | Residual |
29+
|---|---|---|
30+
| P1 position independence | One function (`inherit_as_workspace_member`) serves the sibling, git and index-archive routes. The root inherits at load through the effective loader. e2e 770 and 774 count the words in each position. | None known. |
31+
| P2 merge, normalise, snapshot | `makePackageRoot` performs no merge and refuses unfolded `defines`. | The layer-conditional second pass folds again by design, and the keyed fold removes superseded words across passes (unit test `SecondPassRemovesAWordTheFirstPassFolded`). |
32+
| P3 one source of truth | One key table, one loader, one inheritance function. | `inherit_workspace_build` in `project.cppm` still lists its fields explicitly. The unit test `EveryTableRowIsParsedAndInherited` fails if the table and that function disagree. |
33+
| P4 scope | No include directory is broadcast; the std module, the scanner and every rule read per-unit includes. | A consumer-supplied configuration header has no channel. None is needed today (section 3). |
34+
| P5 cache soundness | `kCacheEpoch` 4; a dependency's command is shown identical under two roots (e2e 765 (c)). | None known. |
35+
| P6 published form | Normalised manifest, reproducible archive, `.orig` kept, released 2026.9.24.1 client builds it (e2e 772 with `MCPP_BOOT`). | `[indices]` inherited from the workspace is not written into the published manifest; a member whose dependencies resolve through a workspace-declared index publishes a manifest that names no index for them. This matches a non-member package, which also publishes no `[indices]`. |
36+
| P7 loud invariants | Internal error text follows `plan.cppm`'s form. | None. |
37+
| P8 measured blast radius | Section 3. | The full mcpp-index matrix runs after release, on the pin-moving pull request. |
38+
39+
## 3. Ecosystem review
40+
41+
- **mcpp-index members.** 166 test members. None declares `include_dirs` (the two matches are comments), so W6 removes nothing a member relied on. The root workspace declares no `[workspace.build]`, so W1 changes no member's flags. 16 members with C sources, `defines` and include directories (`cjson`, `zlib`, `brotli`, `c-ares`, `expat`, `libpng`, `sqlite3`, `pcre2`, `spdlog-compiled`, `fmtlib.fmt`, `yaml-cpp`, `xxhash`, `md4c`, `libffi`, `mimalloc`, `yyjson`) pass `mcpp test -p` with the candidate.
42+
- **Index packages with a workspace in their archive.** 22 are installed on the measuring machine. None declares `[workspace.package]`, `[workspace.build]` or `[workspace.dependencies]`, so F11's inheritance changes none of them.
43+
- **Descriptor comments.** `compat.godot-cpp.lua:163` states that "a consumer-side header shadow never reaches" the package. That statement was false for an uncached compile before this release and is true after it; no descriptor change is needed.
44+
- **Published clients.** The normalised manifest uses only keys 2026.9.24.1 accepts. `!NAME` in `defines` needs 2026.9.25.1, and docs/04 states the floor.
45+
- **Caches.** Epoch 4 orphans every dependency-cache entry once. mcpp-index's CI caches already key on `MCPP_VERSION`, so the pin move costs the same cold run it always does.
46+
- **xlings.** The internal pin moves to 2026.9.20.1 (openxlings/xlings#610). `check_version_pins.sh` passes. The bootstrap `.xlings.json` does not move (review decision 2026-09-25).
47+
48+
## 4. Open items outside #690
49+
50+
- **The e2e harness still shares existing payload versions by link.** An in-place rewrite of an existing payload by a test still reaches the developer's registry (#293, first shape).

0 commit comments

Comments
 (0)