fix: a host-tool sub-build merges its dependency's conditional sections once (#690 F12); self-review record

Author: speak-agent

.agents/docs/2026-09-25-issue-690-self-review.md

@@ -0,0 +1,50 @@
+---
+subject: review
+status: active
+---
+
+# #690: self-review before release, engine and ecosystem
+
+- Pull request: mcpp-community/mcpp#691 (2026.9.25.1)
+- 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)
+- 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.
+
+---
+
+## 1. Findings of the review, and what was done
+
+| # | Finding | Evidence | Resolution |
+|---|---|---|---|
+| 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. |
+| 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. |
+| 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`. |
+| 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. |
+| 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. |
+| 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`. |
+| 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. |
+
+## 2. Principles, checked
+
+| Principle | Holds because | Residual |
+|---|---|---|
+| 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. |
+| 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`). |
+| 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. |
+| 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). |
+| P5 cache soundness | `kCacheEpoch` 4; a dependency's command is shown identical under two roots (e2e 765 (c)). | None known. |
+| 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]`. |
+| P7 loud invariants | Internal error text follows `plan.cppm`'s form. | None. |
+| P8 measured blast radius | Section 3. | The full mcpp-index matrix runs after release, on the pin-moving pull request. |
+
+## 3. Ecosystem review
+
+- **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.
+- **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.
+- **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.
+- **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.
+- **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.
+- **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).
+
+## 4. Open items outside #690
+
+- **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).

.agents/docs/2026-09-25-issue-690-workspace-build-inheritance-consistency.md

@@ -77,6 +77,8 @@ The decisions below are derived from these rules. Each rule names its source in
 
 **F11 (measured during implementation): a member inside an index package's archive does not inherit its archive's workspace.** A descriptor may point at a member manifest (`mcpp = "*/mcpp/cairo/mcpp.toml"`; 22 installed index packages on the measuring machine have a workspace root in their archive). The resolver loaded that manifest as a stand-alone file, so a member that omits `version` was refused and `[workspace.build]` was ignored, while the same commit consumed through `git` inherits (D1). None of the 22 installed archives declares `[workspace.package]`, `[workspace.build]` or `[workspace.dependencies]`, so applying the inheritance changes no existing package. It is applied through the same function as the sibling and git cases, searching no higher than the install root (e2e 774).
 
+**F12 (measured during review): a dependency built as a host tool merges its conditional sections twice.** The resolver merges a dependency's `[target.<selector>.build]` sections for the consumer's target, and the host-tool sub-build received that merged manifest and merged it again for the host. A package whose matching section adds `-include once.h` (a header without a guard) builds on its own and fails as a host tool with `redefinition of 'int once_counter'`, on 2026.9.24.1 as well. The comment above the sub-build called the manifest "pristine". The manifest now records its state before the first merge (`Manifest::beforeConditionalMerge`), and the sub-build receives that state (e2e 775).
+
 **F8 (reasoned): the F1 dependency's cache key records a define its compile does not carry.** `cache_key.cppm:534` reads `pkg.manifest.buildConfig.defines`, which retains the unfolded entries.
 
 Root cause: the build half of inheritance runs inside the snapshot, after the fold.

.agents/docs/README.md

@@ -18,7 +18,7 @@ superseded_by: 2026-09-07-....md  # when status is superseded
 ---
 ```
 
-308 records.
+309 records.
 
 ## By subject
 
@@ -65,6 +65,7 @@ Records that declare one. Everything else is listed by date below.
 
 ### review
 
+- [#690: self-review before release, engine and ecosystem](2026-09-25-issue-690-self-review.md) — active
 - [本轮生态级自审](2026-09-20-wave-self-review.md) — active
 - [#674 设计方案评审：`-include unistd.h` 在 Windows + `presents = "posix"` 上的可行性](2026-09-20-issue-674-design-review.md) — active
 - [`__cxa_thread_atexit` 在 openkal-Windows 上:两层都已定位并修复](2026-09-20-cxa-thread-atexit-finding.md) — landed
@@ -100,6 +101,7 @@ Records that declare one. Everything else is listed by date below.
 ### 2026-09
 
 - [Workspace inheritance, flag scoping and the published form: a unified repair plan (#690)](2026-09-25-issue-690-workspace-build-inheritance-consistency.md) — active
+- [#690: self-review before release, engine and ecosystem](2026-09-25-issue-690-self-review.md) — active
 - [#690: implementation plan](2026-09-25-issue-690-implementation-plan.md) — active
 - [工具链选择与载荷可信度：实施计划](2026-09-24-toolchain-selection-implementation-plan.md) — landed
 - [MSVC toolset 的选择、#685、#687 与工具链管理规范：总体设计](2026-09-24-toolchain-selection-and-payload-trust-design.md) — active

CHANGELOG.md

@@ -48,6 +48,13 @@
 之后指出该头文件所在的使用方目录。依赖需要通过自己的 `include_dirs` 或它的某个依赖找到头文件。
 依赖缓存的 epoch 由 3 升为 4,升级后首次构建重建一次依赖缓存,不需要任何操作。
 
+### 作为 host tool 构建的依赖只合并一次条件节
+
+依赖的 `[target.<selector>.build]` 条件节先按使用方的目标合并,host tool 子构建此前拿到的正是合并后的
+清单,又按宿主合并一次,同时命中两者的条目因此到达工具两次。实测:条件节中的 `-include once.h`
+(无 include guard)让单独构建正常的包作为工具时报重定义错误,2026.9.24.1 同样如此。清单现在记录
+第一次合并之前的状态,子构建从该状态按宿主合并。
+
 ### 成员清单只有一种读法
 
 `mcpp publish`、`mcpp pack`、`mcpp emit xpkg`、`mcpp toolchain list`、`mcpp sbom`、`mcpp index list`/`update`、

modules/manifest/src/types.cppm

@@ -2083,6 +2083,14 @@ struct Manifest {
     // values are paths. Kept as text so this module's interface names no JSON
     // type.
     std::string                 packageMetadataJson;
+    // The manifest as it was before its first conditional merge, recorded by
+    // `merge_conditional_config`. The merge evaluates `[target.<selector>]`
+    // sections for ONE target, and a package's manifest can be needed for a
+    // second one: a host-tool sub-build compiles a dependency for the host,
+    // and merging the already merged manifest again applied the consumer's
+    // entries and then the host's, so a flag in a matching section reached
+    // the tool twice (#690, F12). Null when no merge has run.
+    std::shared_ptr<const Manifest> beforeConditionalMerge;
 };
 
 struct ManifestError {

src/build/prepare.cppm

@@ -402,6 +402,9 @@ void replace_dependencies(
 export void merge_conditional_config(mcpp::manifest::Manifest& m,
                                     const cfgpred::Ctx& ctx)
 {
+    // Recorded before the first merge; see Manifest::beforeConditionalMerge.
+    if (!m.beforeConditionalMerge)
+        m.beforeConditionalMerge = std::make_shared<const mcpp::manifest::Manifest>(m);
     // A DISTRIBUTION package may carry a leg's link line twice: as `ldflags`
     // (GNU spelling, which is all an older mcpp reads) and as the neutral
     // `[target.<pred>.runtime]` pair, which mcpp renders per dialect. Applying
@@ -10374,11 +10377,18 @@ prepare_build(bool print_fingerprint,
                     // twice. A `compat` (Form B) package has no mcpp.toml on
                     // disk at all, so without this the sub-build could not read
                     // a manifest for it in the first place.
+                    //
+                    // UNMERGED, because the sub-build targets the HOST: the
+                    // resolver merged this manifest's conditional sections for
+                    // the consumer's target, and the sub-build merges them for
+                    // its own (#690, F12).
                     if (depIdx >= 1 && depIdx - 1 < dep_manifests.size()
-                        && dep_manifests[depIdx - 1])
-                        sub.preloaded_manifest =
-                            std::make_shared<const mcpp::manifest::Manifest>(
-                                *dep_manifests[depIdx - 1]);
+                        && dep_manifests[depIdx - 1]) {
+                        auto const& dep = *dep_manifests[depIdx - 1];
+                        sub.preloaded_manifest = dep.beforeConditionalMerge
+                            ? dep.beforeConditionalMerge
+                            : std::make_shared<const mcpp::manifest::Manifest>(dep);
+                    }
                     sub.inherited_runtime_selection = std::make_shared<
                         const mcpp::xlings::runtime::RuntimeSelection>(
                             runtimeSelection);

tests/e2e/775_a_host_tool_merges_its_conditional_sections_once.sh

@@ -0,0 +1,79 @@
+#!/usr/bin/env bash
+# 775 -- a dependency compiled as a host tool receives each entry of its
+# matching `[target.<selector>.build]` sections once (#690, design record F12).
+#
+# The resolver merges a dependency's conditional sections for the consumer's
+# target. A host-tool sub-build used to receive that merged manifest and merge
+# it again for the host, so an entry of a section matching both reached the
+# tool twice. The section here adds `-include once.h`, and `once.h` defines a
+# variable without an include guard: included twice, the tool does not compile.
+# The package builds on its own, which is the control.
+set -e
+
+TMP=$(mktemp -d)
+trap 'rm -rf "$TMP"' EXIT
+export MCPP_HOME="$TMP/home"
+source "$(dirname "$0")/_inherit_toolchain.sh"
+fail() { echo "FAIL: $1"; [ -n "${2:-}" ] && cat "$2"; exit 1; }
+
+mkdir -p "$TMP/toolpkg/src" "$TMP/toolpkg/inc" "$TMP/app/src"
+echo 'int once_counter = 1;' > "$TMP/toolpkg/inc/once.h"
+cat > "$TMP/toolpkg/mcpp.toml" <<'EOF'
+[package]
+name    = "toolpkg"
+version = "0.1.0"
+
+[targets.gen]
+kind = "bin"
+main = "src/gen.cpp"
+
+[build]
+include_dirs = ["inc"]
+
+[target.'cfg(not(os = "none"))'.build]
+cxxflags = ["-include once.h"]
+EOF
+cat > "$TMP/toolpkg/src/gen.cpp" <<'EOF'
+#include <cstdio>
+int main(int argc, char** argv) {
+    if (argc < 2) return 2;
+    std::FILE* f = std::fopen(argv[1], "w");
+    if (!f) return 3;
+    std::fprintf(f, "int generated_answer() { return %d; }\n", 40 + once_counter);
+    std::fclose(f);
+    return 0;
+}
+EOF
+cat > "$TMP/app/mcpp.toml" <<'EOF'
+[package]
+name    = "app"
+version = "0.1.0"
+
+[dependencies]
+toolpkg = { path = "../toolpkg", tools = ["gen"] }
+EOF
+cat > "$TMP/app/src/main.cpp" <<'EOF'
+#include <cstdio>
+int generated_answer();
+int main() { std::printf("ANSWER=%d\n", generated_answer()); }
+EOF
+cat > "$TMP/app/build.mcpp" <<'EOF'
+import std;
+import mcpp;
+int main() {
+    const char* tool = mcpp::dep_bin("toolpkg", "gen");
+    if (!tool || !*tool) return 1;
+    std::string out = std::string(mcpp::out_dir()) + "/gen.cpp";
+    std::string cmd = std::string("\"") + tool + "\" \"" + out + "\"";
+    if (std::system(cmd.c_str()) != 0) return 1;
+    mcpp::generated(out.c_str());
+}
+EOF
+
+( cd "$TMP/toolpkg" && "$MCPP" build > "$TMP/direct.log" 2>&1 ) \
+    || fail "control: the tool package does not build on its own" "$TMP/direct.log"
+cd "$TMP/app"
+"$MCPP" build > build.log 2>&1 || fail "the host tool received a conditional entry twice" build.log
+"$MCPP" run > run.log 2>&1 || fail "the program did not run" run.log
+grep -q '^ANSWER=41' run.log || fail "wrong answer" run.log
+echo "PASS: 775_a_host_tool_merges_its_conditional_sections_once"