From 71f4b58a260923e3e20f50cca448e2259ae12622 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:16:54 +0800 Subject: [PATCH 01/13] =?UTF-8?q?feat(diag):=20single=20degradation/warnin?= =?UTF-8?q?g=20sink=20=E2=80=94=20impact-bearing=20records,=20one=20--stri?= =?UTF-8?q?ct=20policy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every branch that does LESS work because a precondition was not met used to be silent or debug-logged, which is the same thing from the user's side. mcpp.diag makes that reporting explicit and mandatory: - degraded() requires an 'impact' string, forcing the author to state what the user will actually experience — the sentence missing from every silent-degradation bug this channel exists to prevent. - records dedupe on their whole payload and render as they are reported, so warning/status interleaving and early-return behaviour are unchanged. - flush(strict) settles the --strict policy in one place. Migrates prepare.cppm's eight raw println(stderr, "warning: ...") sites (schema, platforms, feature forwarding/request, scanner, validator) onto the sink. Rendering still goes through ui::warning, so output is byte-identical and existing e2e greps keep matching. The 11 ui::warning call sites in other files are left alone — out of scope for this batch. Design: .agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md section 2 --- .../docs/2026-07-22-issue-triage-254-261.md | 326 +++++++++++ ...026-07-22-v0.0.102-batch-254-261-design.md | 532 ++++++++++++++++++ src/build/prepare.cppm | 18 +- src/diag.cppm | 138 +++++ tests/unit/test_diag.cpp | 81 +++ 5 files changed, 1087 insertions(+), 8 deletions(-) create mode 100644 .agents/docs/2026-07-22-issue-triage-254-261.md create mode 100644 .agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md create mode 100644 src/diag.cppm create mode 100644 tests/unit/test_diag.cpp diff --git a/.agents/docs/2026-07-22-issue-triage-254-261.md b/.agents/docs/2026-07-22-issue-triage-254-261.md new file mode 100644 index 00000000..18c983b6 --- /dev/null +++ b/.agents/docs/2026-07-22-issue-triage-254-261.md @@ -0,0 +1,326 @@ +# Issue 分析报告 —— #254 / #256 / #257 / #258 / #259 / #261 + +> 日期:2026-07-22 +> 基线:HEAD = `af25d18`(mcpp **0.0.101**),工作树 clean +> 方法:每条 issue 的断言都回到 HEAD 源码逐行核对;#256、#257 的关键行为在本机用 mcpp 自带工具链(llvm 20.1.7 / 22.1.8 / gcc 16.1.0 / 系统 clang 18.1.3)**实证**,不依赖 issue 正文的转述。 +> 架构判据沿用批次总账 §5.2:**一数据模型、多文法、单一收敛漏斗;同一决策不许两处推导**;外加一条本批次凸显的:**失败必须响,不许静默降级**。 + +--- + +## 0. 结论速览 + +| # | 主题 | 裁决 | 断言准确度 | 归属 | 优先级 | +|---|------|------|-----------|------|--------| +| **#261** | windows scan-deps `cmd /c` 撞 8191 | **真 bug(构造性必现)** | 全部属实 | mcpp | **P0** | +| **#257** | clang 路径 purview `#include` 不触发重建 | **真 bug(correctness,静默错误结果)** | 全部属实,且是代码里已登记的 follow-up | mcpp | **P0** | +| **#259** | llvm 装完 exec 127 | **症状真,归因错**;顺带暴露 3 个 mcpp 侧真 bug | 现象属实,**根因假设被证伪** | 需重定向(见 §5) | **P1** | +| **#258** | `[target.'cfg(os)'.build].flags` | **合理的特性请求**,且内含 1 个真 bug(键被静默丢弃) | 全部属实 | mcpp | **P1** | +| **#254** | per-OS 段按宿主而非 target 拼接 | **真 bug(设计层不自洽)**,作者已在 #253 设计文档 §2.3 自记 | 全部属实 | mcpp | **P2** | +| **#256** | clang 20/22 模块 BMI 崩溃 | **上游 clang 回归,非 mcpp 缺陷**;三条 ask 均合理 | **本机完整复现** | LLVM 上游 + mcpp 文档/CI | **P2**(canary P1) | + +一句话总览:**六条里五条成立,一条(#259)方向要改**;而且四条(#257/#258/#259 + #261 的 `cmd /c`)共享同一条失效模式——**引擎在遇到自己没准备好的情况时选择静默降级**,详见 §7。 + +--- + +## 1. #261 —— windows scan-deps `cmd /c` 撞 8191 字符 **【真 bug,P0】** + +### 1.1 核实 + +`src/build/ninja_backend.cppm:646-677` 的 `cxx_scan` 规则三分支: + +```cpp +if (msvcDeps) { // /scanDependencies $out —— 文件参数,无重定向 +} else if (plan.scanDepsPath.empty()) { // GCC:-fdeps-file=$out —— 文件参数,无重定向 +} else { // Clang:clang-scan-deps + if constexpr (mcpp::platform::is_windows) { + append(" command = cmd /c \"$scan_deps -format=p1689 -- " + "$cxx $local_includes $cxxflags $unit_cxxflags -c $in -o $compile_target > $out\"\n"); + } else { ... } +} +``` + +断言逐条成立: + +- `cmd /c` 包裹确实存在,且**全仓仅此一处**发射到 ninja 规则(`:299` 的另一处是 `compile_commands.json` 过滤器的字符串前缀判断,非规则)。 +- 触发面精确:**windows × clang 工具链 × dyndep 开启 × `clang-scan-deps` 存在于 `clang++` 旁**。GCC(`-fdeps-file=$out`)与 MSVC(`/scanDependencies $out`)因为把输出写成**参数**而不是 shell 重定向,天然不需要 `cmd /c`——这本身就说明重定向是可以避免的。 +- `-o` 可用性我在本机实证(不是查文档): + + ``` + $ clang-scan-deps --help | grep '^ -o' + -o Destination of the primary output # 20.1.7 与 22.1.8 均有 + $ clang-scan-deps -format=p1689 -o out.ddi -- clang++ -std=c++23 -c m.cppm -o m.o + rc=0 size=488 → 合法 P1689 JSON + ``` + + 故 issue 提议的替换**功能等价且已验证**。仓库无任何最低 LLVM 版本闸(`src/toolchain/clang.cppm:246-252` 的 `find_scan_deps` 只做路径探测,不查版本;`provider.cppm:88-93` 直接 `has_scan_deps = true`),实际下限就是自带工具链(20.1.7),`-o` 从 LLVM 17 起存在,兼容面无风险。 + +### 1.2 架构判断:issue 的修法对,但只对了一半 + +`cmd /c` 是**放大器**,不是唯一病灶。长度来源是 `local_include_flags()`(`ninja_backend.cppm:102-127`)每依赖一个 `-I`,无上界。去掉 `cmd /c` 把天花板从 8191 抬到 32767 —— 4 倍余量,对 48 个 `-I` 够,但对下一个 ffmpeg/opencv 级包不一定够。而且 `cxx_object` / `cxx_module` / `c_object` 在 windows 上**同样没有任何长度兜底**(rspfile 目前只在链接三规则上,`:602` `useRsp = separateLinker || is_windows`,#247 引入)。今天 scan 先炸,只是因为它的命令行 = 编译命令行 + `$scan_deps -format=p1689 -- ` 前缀 + `> $out` 后缀,**严格更长**。 + +工业界基准很清楚:CMake/Ninja 生成器对**所有**可能超长的规则统一走 response file(`RULE_LAUNCH_*` / `CMAKE_NINJA_FORCE_RESPONSE_FILE`),而不是逐规则救火。仓库自己在 #247 已经建立了这个先例并写下了理由注释(`:594-602`)——只是没推广到编译/扫描边。 + +### 1.3 建议 + +- **P0(本周)**:合并两分支为 `-o $out`,消灭全仓唯一的 `cmd /c`。改动 8 行,`cxx_scan` 的 POSIX 命令文本也一并简化(去掉 `> $out`),两平台同形——符合"单一收敛漏斗"。 +- **P1(随后)**:把 `useRsp` 的判据从"链接规则"上移为"windows 上所有含 `$local_includes` 的规则",`cxx_scan`/`cxx_module`/`cxx_object`/`c_object` 统一 `rspfile`。注意 #247 的教训(rsp 内容按 GNU 文法分词,反斜杠是转义符 → 节点名必须 `generic_string()`)已经在 `link_rule` 里踩过,复用即可。 +- 单测:`tests/unit/test_ninja_backend.cpp:516-519` 已锁链接规则的 rsp 形状,照抄一条锁 `cxx_scan` 无 `cmd /c`。 + +--- + +## 2. #257 —— clang 路径 purview `#include` 不触发模块接口重建 **【真 bug,P0】** + +### 2.1 核实:这是代码里**明写的已知缺口** + +`src/build/ninja_backend.cppm:496-502`: + +```cpp +const bool posixDepfile = !msvcDeps && !mcpp::platform::is_windows + && plan.toolchain.compiler == mcpp::toolchain::CompilerId::GCC; +``` + +于是 clang 上 `mmd_flag` / `mmd_filter` 皆空,`append_cxx_deps()` 落到 `append_deps()` 而后者对非 MSVC 什么都不发 → **`cxx_module` 与 `cxx_object` 在 clang 上完全没有 depfile**。`.inc` 对整个构建图不可见。 + +这个闸不是疏忽,是 0.0.97 主动加的。`78bb1c9` "fix(build): scope #235 compile-edge depfile to GCC" 的提交信息与 `:486-495` 的注释都写着: + +> Clang's module system (.pcm) does not emit those and its module-TU depfile shape is **different**; applying the GCC-shaped filter there is **untested** and could yield a wrong dep set. Until Clang is verified, scope this to GCC —— header/purview rebuild tracking on Clang is a **follow-up**. + +`tests/e2e/118_purview_include_rebuild.sh:2` 是 `# requires: gcc`,所以 clang 腿零覆盖。scanner / p1689 都不回读 textual include(`p1689.cppm:355` 生成 `.dep` 却从不解析),也不存在任何源文件内容哈希/mtime 兜底——**除 depfile 外没有第二道防线**。 + +### 2.2 关键新证据:那个"形状不同"的顾虑,实测不成立 + +当年 gate 掉的唯一理由是"clang 的 module-TU depfile 形状未知,GCC 的 awk 过滤器套上去可能出错"。我在本机把两边的形状打出来了: + +``` +$ clang++ -std=c++23 -fmodule-output=x.pcm -MMD -MF x.d -c x.cppm -o x.o # 20.1.7 与 22.1.8 同 +x.o: x.cppm ops.inc ← 干净的单条 make rule,ninja 直接可吃 + +$ g++ -std=c++23 -fmodules -MMD -MF x.d -c x.cppm -o x.o # gcc 16.1.0 +x.o gcm.cache/x.gcm: x.cppm ops.inc +x.c++-module: gcm.cache/x.gcm ← 正是 awk 过滤器要剥掉的 "reversed rules" +.PHONY: x.c++-module +gcm.cache/x.gcm:| x.o +``` + +结论:**clang 根本不产生需要过滤的东西**。正确修法不是"把 GCC 的过滤器移植到 clang",而是把两件事拆开—— + +- `posixDepfile`(是否发 `-MMD` + `deps = gcc` + `depfile`)→ 放开到 **GCC ∪ Clang**; +- `mmd_filter`(awk 剥离 reversed rules)→ **保持 GCC-only**。 + +代码上就是把 `:496-502` 的单一 bool 拆成两个谓词。风险极低,且 e2e 118 的 `# requires: gcc` 可以直接删掉,变成两工具链共同覆盖。 + +### 2.3 严重性:被 issue 低估了 + +issue 说"Harmless for CI (cold builds) but dangerous locally"。我认为要往上提一档: + +- 失效模式是**静默给出错误产物**,不是报错。构建系统的失效等级里,silent staleness 是最差的一档——它把编译期错误延后成运行期错误,而且怀疑对象总是先落到用户代码上。issue 自己就记录了它已经造成了一次误诊(在 #256 的 clang 调试过程中)。 +- 影响面不止 `.inc`:任何 purview 里的 `#include`(含 `module;` 之后再包的私有头)、任何被模块接口文本包含的生成头,在 clang 上都不重建。而 clang 恰是 macOS 默认腿与 windows 托管腿的编译器。 +- 从"依赖跟踪必须完备"这条构建系统第一原则看,当前状态是 mcpp 在 clang 上**不满足最小正确性契约**。0.0.97 当时的取舍(不 regress、留 follow-up)在发布压力下合理,但已经拖了 4 个版本,而实测表明代价只有几行。 + +--- + +## 3. #258 —— `[target.'cfg(os)'.build]` 缺 per-glob `flags` **【合理的特性请求 + 1 个真 bug,P1】** + +### 3.1 核实 + +| 断言 | 结论 | +|---|---| +| `ConditionalConfig` 无 globFlags 字段 | 属实,`types.cppm:276-292` 只有 cflags/cxxflags/ldflags/sources/三类 deps | +| `[target..build]` 只读四个键 | 属实,`toml.cppm:976-987` 四次 `read_list` | +| `merge_conditional_sources_flags` 不碰 globFlags | 属实,`prepare.cppm:429-441`,加一行 `insert` 即可 | +| xpkg 描述符**有**per-OS `flags` | 属实,机制是 `xpkg.cppm:888-894` 的**文本体拼接**——OS 段体追加到 base 体后跑同一解析循环,所以 `flags` 连同其余顶层键"免费"支持 per-OS,且 OS 条目自然排在 base 之后(last-wins 顺序天然正确) | +| 死 glob 告警按**构建的源集**判定,不看文件系统 | 属实,`globFlagHits` 只在 `apply_glob_flags` 内自增,而它只被 `all_files` 循环调用(`scanner.cppm:900/919`,`all_files` 由 `modules.sources` 展开)→ issue 评论里"磁盘上放锚点文件无效"的实测结论有代码依据 | +| `features..flags` 在 mcpp.toml 里全局-only | 属实,`toml.cppm:320-327` 在顶层 `[features]` 里;`[target.*]` 没有 `features` 子键 → **OS 轴 × feature 轴在 mcpp.toml 里不可组合** | +| 无 `optional` / 允许零匹配的逃生口 | 属实,且两个解析器对未知键都**硬错**,所以今天写 `optional = true` 是 parse error 而非被忽略 | + +### 3.2 顺带发现的真 bug:`flags` 在条件段里被**静默丢弃** + +`toml.cppm:976-987` 的读法是"认得的四个键各读一次",**没有未知键拒绝**。作者在 `[target.'cfg(windows)'.build]` 里写 `flags = [{...}]`,今天的行为是:解析成功、零告警、规则凭空消失、构建出一个 flags 缺失的产物。 + +这与本仓已确立的纪律直接冲突:xpkg 未知 feature 子键进 `xpkgUnknownKeys` 报 did-you-mean(0.0.100)、`[build].flags` 条目未知子键硬错、`scan_overrides` 零命中硬错。条件段是这条纪律的一个**破口**。即便 #258 的特性最终不做,这个静默吞键也该单独修(#227 封闭文法 allowlist 应覆盖 `target.*.build.*`)。 + +### 3.3 架构判断:请求成立,而且是补齐既有原则而非新增机制 + +- **文法平价**是本仓写在设计文档里的原则("一数据模型两文法")。今天的实况是 xpkg 描述符能表达、mcpp.toml 不能——即"manifest 包是二等公民"这个论证站得住。而且不对称的成因是**偶然**(xpkg 靠文本拼接免费获得,toml 走结构化解析所以要逐键实现),不是有意的能力分级。 +- **removal 论证成立**。`-U` 之所以救不了,是因为它自身也需要 OS 条件化,递归回同一缺口;issue 正文对此的推理正确。而 `merge_conditional_sources_flags` 的 append 位置天然给出 last-wins 覆盖权,这正是表达"windows 上撤销 unix 的 `-DHAVE_UNISTD_H`"所需要的。 +- **实施成本**:issue 给的 4 处清单准确无误,四处全部复用既有代码(`parse_glob_flags_value` 的签名 `(value, ctx, std::vector& dst)` 可直接指向 `cc.globFlags`;下游 scanner/backend/fingerprint 零改动,因为 0.0.101 已经把 globFlags 全量序列化进 per-package fingerprint)。唯一要盯的顺序不变量:merge 必须在 `makePackageRoot` 快照 `buildConfig` 之前(`prepare.cppm:900-909` 已有注释)——三个调用点 `:912 / :1871 / :2762` 都在其前。 +- **对 703 个 stub 文件的评估**:这不是"包作者偷懒",是**引擎缺表达力把语义泄漏成了文件系统结构**。0.0.97 刚删掉过同类的 #233-era tu-stub 机制,现在因为另一个缺口在下游重新长出来——这是判断该做的强信号。 + +### 3.4 对 stopgap `optional = true` 的反对意见 + +issue 评论里提议的 `optional=true` / `?`-前缀 我**不建议做**,理由是它会引入**第二套抑制机制**,与 #253 刚确立的路线("条目按条件*存在*,所以无从告警")相悖——那正是同一决策两处推导。#253 已经证明了正确形状:让规则在不该生效的维度上**不进入 `globFlags`**。OS 轴照抄即可,不需要新语义。如果 #258 主体要拖,宁可先只做 §3.2 的静默吞键修复(半天工作量,且能让作者立刻发现自己写的 `flags` 没生效)。 + +--- + +## 4. #254 —— per-OS 段按宿主拼接而非 resolved target **【真 bug,P2】** + +### 4.1 核实 + +两轴不自洽,逐条属实: + +- `platform/common.cppm:95-105` 的 `xpkg_platform` 是 `constexpr` + `#if defined(_WIN32)/__APPLE__/__linux__` —— **mcpp 自己被编译时的宿主**,无任何运行期输入。 +- `xpkg.cppm:888-889` `osKey = osOverride.empty() ? xpkg_platform : osOverride`。`osOverride` 的**全部**非测试调用点:`prepare.cppm:1589 / 1637 / 1814` 三个**构建路径**全部走默认(宿主),唯一传 override 的是 lint 命令 `mcpp xpkg parse --all-os`(`cmd_xpkg.cppm:132`)。`overrides.target_triple` 从未穿进来。 +- `cfgpred::context_for`(`prepare.cppm:76-107`)注释明写 "Context = the RESOLVED target's coordinates",三个调用点(`:911/:1872/:2763`)都传 `overrides.target_triple`。 +- 影响面比 issue 列的还宽一格:`xpm` 版本/资产表也走宿主键(`xpkg.cppm:711-772` 的 `list_xpkg_versions(luaContent, platform)`,`pm/resolver.cppm:108` 传 `kXpkgPlatform = xpkg_platform`),即**交叉编译时连"有哪些版本可选/下哪个 tarball"都按宿主判定**,连错误文案都把宿主平台名硬写进去。 +- 交叉编译不是纸面能力:`tests/e2e/102_mingw_cross_wine.sh`、`112_build_mcpp_cross.sh` 真跑 `--target x86_64-windows-gnu` 并用 wine 执行。**但**没有一条 cross e2e 使用带 per-OS `mcpp` 段的 xpkg 依赖 → 缺口对 CI 完全不可见。 + +### 4.2 架构判断 + +这是教科书级的"**同一决策两处推导**":平台选择这一个决策,在 xpkg 通道按宿主编译期常量推,在 manifest 通道按 resolved target 运行期推。native 构建下 host == target 使两者巧合一致,于是缺陷被三平台 CI 完全掩盖——**恰恰是 0.0.99 复盘里记下的那类隐性架构债**(加新语义会变构建失败:#253 刚给 per-OS 段加了 `features`,这个错误轴上又多挂了一项)。 + +作者的处理方式是对的:在 #253 设计文档 §2.3 记录、明确不入该 PR、拆成独立 issue 并回引设计节。这是应该保持的工作方式。 + +### 4.3 建议(比 issue 的方向再收紧一步) + +issue 说"pm/install 侧需单独辨析",正确但太软。建议直接上**类型层**约束,否则这类 bug 会反复: + +- 引入两个不可互换的类型(如 `HostPlatform` / `TargetPlatform`,或一个带 `enum class PlatformAxis { Host, Target }` 的强类型 wrapper),让 `synthesize_from_xpkg_lua` / `list_xpkg_versions` 在签名上**必须**声明自己要哪一轴。今天二者都收 `std::string_view`,混用没有任何编译期阻力。 +- 分类结论(已核实,可直接照用):`prepare.cppm:1589/1637/1814` 三处 = 编进用户构建的**源码库包** → **target**;`toolchain/lifecycle.cppm:135` = **宿主工具**(且注释已写明为何要 host)→ 保持 **host**;`pm/resolver.cppm:108` = 用户项目的库依赖 → **target**;`cmd_xpkg.cppm` = lint,遍历全 OS。 +- 锁定:e2e 加一条"host == target 时拼接结果逐字节不变"的回归,再加一条 mingw-cross 腿消费带 per-OS 段的 xpkg dep。 + +优先级 P2 合理——今天没有真实用户被咬(交叉编译尚未与 per-OS xpkg 依赖组合使用),但它是**能力增长的路障**:每加一个 per-OS 键,错误面就扩大一次。 + +--- + +## 5. #259 —— llvm 装完 exec 127 **【症状真,根因假设被证伪 → 需重定向,P1】** + +### 5.1 现象属实 + +本机 ELF 证据与 issue 完全一致: + +``` +xim-x-llvm/22.1.8/bin/clang++ → interpreter …/xpkgs/xim-x-glibc/2.39/lib64/ld-linux-x86-64.so.2 +RUNPATH …/xim-x-llvm/22.1.8/lib : …/xim-x-glibc/2.39/lib64 : …/xim-x-zlib/1.3.1/lib : … +``` + +解释器路径是**绝对的、机器本地的、安装时写入的**(不在发布 tarball 里)。glibc payload 缺失 ⇒ 所有 clang 二进制 exec 127。到这里都对。 + +### 5.2 但归因错了 + +issue 的判断是"looks like a **missing runtime dependency in the llvm xim package**"。核对索引: + +```lua +-- xim-pkgindex pkgs/l/llvm.lua:25-31 +xpm = { linux = { + deps = { "xim:glibc@2.39", "xim:linux-headers@5.11.1", "xim:zlib@1.3.1", "xim:libxml2@2.13.5" }, +``` + +**llvm 声明了 glibc**,自 `8e432d99`(#182,首次加 Linux x86_64 支持)起从未变过,与 gcc.lua 的声明形态一致。三份本地 checkout 全同。而且 llvm.lua 自己的 install hook 在 glibc 缺失时是**故意 fail-loud** 的(`:234-239` `log.error("glibc payload not found … refusing to write a host-dependent clang cfg"); return false`)。 + +所以"llvm 包漏声明 glibc"不成立。真正的问题一定在**声明有、实际没装上,而且没人喊**这条链上。目前证据指向三个 mcpp 侧的静默点: + +**(a) 依赖安装结果被丢弃** —— `toolchain/lifecycle.cppm:554-568`: + +```cpp +for (auto dep : {"xim:glibc", "xim:linux-headers"}) { + auto depPayload = fetcher.resolve_xpkg_path(dep, /*autoInstall=*/true, &progress); + mcpp::log::debug("toolchain", std::format("dep {} result: {}", dep, depPayload ? "ok" : ...)); +} +``` + +结果只进 **debug 日志**,不检查、不中断。裸 runner 上 glibc 安装失败,流程原样继续去装 llvm。 + +**(b) llvm 的 post-install fixup 在 glibc 缺失时静默跳过,还把这个状态记成"成功"** —— `post_install.cppm:364-390`:`if (!glibcLibDir.empty() && exists(patchelfBin)) { … }`,glibc 找不到时整段跳过且**一句告警都没有**(gcc 路径至少有 `:358-361` 的 warning)。更糟的是随后 `fixup_clang_cfg`(`:249-284`)会把 `bin/` 下每个 `*.cfg` 重写成**省略 glibc `-B/-L/--dynamic-linker/-isystem` 的版本**——正好覆盖掉 llvm.lua 本来会拒绝写、且会 fail-loud 的那份 cfg;然后幂等标记 `.mcpp-fixup.json` 把 `glibcLib: ""` 记录为一次**成功**的 fixup,后续重装也不会重试。 + +**(c) 装完不验活** —— `lifecycle.cppm:578-585` 只 `std::filesystem::exists(bin)`。全仓唯一的 `--version` 调用在**构建期** `detect.cppm:56`,它把 127 报成 `unrecognized compiler output`。`mcpp doctor`(`doctor.cppm:239-313`)会 `readelf -d` 查 **RUNPATH** 缺目录,但**从不查 PT_INTERP**(全仓 `PT_INTERP`/`interpreter` 只出现在 `pack.cppm`/`linkmodel.cppm`),而 PT_INTERP 才是致命轴;它给出的提示文案还是"其提供方 xim 包可能被删除了",在"从未装过"的场景下正好误导。 + +### 5.3 建议 + +1. **把 issue 重定向**:标题/正文改成"llvm 安装链在 glibc 运行时缺失时静默降级并记为成功",把"llvm 包漏声明 glibc"的假设撤回(附上 `llvm.lua:25-31` 的证据),同时在 xlings/xim 侧另开一条查"声明了却没装上"的实际原因(bare runner 上 `xlings install` 的 dep 解析是否被跳过/失败)。 +2. **mcpp 侧三处照修**(与归因无关,无论 xim 侧结论如何都该做): + - `lifecycle.cppm:562-567` 检查 dep 结果,失败即 hard error; + - `llvm_post_install_fixup` 在 Linux 且 `glibcLibDir` 为空时告警或硬失败,**且不写 fixup marker**(对齐 gcc 路径与 llvm.lua 自身的 fail-loud 策略); + - 安装末尾加 ` --version` 冒烟探针,127 直接报"glibc payload missing";`doctor` 补 PT_INTERP 存在性检查。 +3. 现有 workaround("先装 gcc")继续有效,但它掩盖问题,应在修好后从 opencv-m CI 撤掉。 + +这条 issue 的价值其实比作者以为的更高:它暴露的不是一个包的元数据错误,而是**安装路径上一条完整的静默降级链**——三个环节各自"宽容"地放过,合起来产出一个看起来装好了、实际不能 exec 的工具链。 + +--- + +## 6. #256 —— clang 20/22 模块 BMI 里的 replacement-operator 模板毒化名字查找 **【上游 clang 回归,非 mcpp bug,P2;canary P1】** + +### 6.1 本机完整复现(不是转述) + +用 issue 正文的 3 文件 repro,逐一跑本机四个编译器: + +| 编译器 | 带 4 参 `Matx*Matx` 模板 | 去掉该模板(对照) | +|---|---|---| +| clang 20.1.7(mcpp 工具链) | **崩溃** rc=1,`PLEASE submit a bug report` | ok rc=0 | +| clang 22.1.8(mcpp 工具链) | **崩溃** rc=1,同上 | ok rc=0 | +| clang 18.1.3(Ubuntu 系统) | ok rc=0 | — | +| gcc 16.1.0(mcpp 工具链) | ok rc=0 | — | + +崩溃点与 issue 一致:`use.cpp:2:46` 解析 `main` 体内 `p * 2` 时前端崩,而 `--precompile` 生成 BMI 那步 rc=0。issue 的控制矩阵**逐格复现**,回归窗口 clang 18 → 20 成立。 + +### 6.2 判断 + +- **不是 mcpp 的缺陷**。mcpp 只是把标准 C++23 模块命令行交给 clang;`--precompile` 成功、消费端崩溃,完全在编译器内部。任何构建系统(CMake、Bazel、build2)在同样输入下都会看到同样的崩溃。 +- **是 mcpp 的暴露面**。issue 指出的"模块包模式(镜像上游头里的 `static inline` 运算符模板 + 平凡真约束,做混合 TU subsumption)会直撞这个形状"是准确的架构观察——这个模式正是 mcpp 生态推荐的 header→module 封装配方,所以对本项目用户的命中率远高于一般 C++ 项目。 +- 三条 ask 全部合理,但优先级应重排: + 1. **上游报告优先**(ask 2)。repro 只有 40 行、回归窗口干净(18 ok / 20+22 crash),是一份高质量的 LLVM issue。建议尽快提,并把编号回链本 issue。 + 2. **e2e canary(ask 3)升到 P1**。理由:这是**沉默的能力边界**——没有 canary,未来任一次 clang bump 修好或再弄坏,mcpp 都不会知道;而 opencv-m 的经历表明发现成本是"一整轮 bisect + 一次误诊"。canary 很便宜:把 §6.1 的 3 个文件塞进 `tests/e2e/`,`# requires: clang`,断言"要么编过、要么给出可识别的诊断",崩溃即红。 + 3. **文档(ask 1)**:写进模块包指南,一句可操作的规则——"运算符模板的所有模板参数应由第一个实参绑定;否则改为对整个推导操作数类型建模"。作者已经在 opencv-m `42aeb20` 里给出了可直接复制的改写范式,值得原样收进文档。 +- 一个**不建议**做的方向:让 mcpp 检测子进程 139/SIGSEGV 并给专门提示。信噪比太低(前端崩溃原因千百种),canary + 文档已经覆盖了实际收益。 + +--- + +## 7. 横向观察 + +### 7.1 主旋律:静默降级 + +本批次六条里有四条的实质是**引擎遇到没准备好的情况时,选择安静地少做一点事**: + +| # | 静默点 | 用户看到的 | +|---|---|---| +| #257 | clang 上不发 depfile | 构建"成功",产物用了陈旧 BMI | +| #258 | `[target.*.build]` 里的 `flags` 被丢弃 | 构建"成功",flags 从未生效 | +| #259(a) | dep 安装结果只进 debug 日志 | 安装"成功" | +| #259(b) | glibc 缺失时跳过 patchelf 并记 marker 为成功 | 安装"成功",二进制不能 exec | + +而本仓在**别的**地方已经把"不许静默"做得很好:xpkg 未知键 → `xpkgUnknownKeys` + did-you-mean;`scan_overrides` 零命中 → 硬错;死 glob → 告警;#253 还专门为告警加了归属点名。也就是说,**纪律存在,只是没有被提升为跨模块不变量**。 + +建议把它写成一条明文架构约束(放进批次总账 §5):*任何"因为条件不满足而少做一步"的分支,必须要么报错,要么发出用户可见的告警;debug 日志不算。* 并在 code review checklist 里加一条对 `if (x.empty()) return;` / 静默 `continue` 的审视。这条约束若在 0.0.97 就存在,#257 的 gate 会带一条 clang 上的 warning,#259 的两次静默都会响。 + +### 7.2 CI 矩阵的盲区形状 + +三条 bug 的可见性条件高度相似,都是"**只在某一条腿上出现,而那条腿恰好没覆盖**": + +- #261:windows × clang × 长路径(自有 CI 路径短 → 绿;作为依赖被消费 → 炸) +- #257:非 GCC(e2e 118 明写 `# requires: gcc`) +- #254:host ≠ target(cross e2e 存在,但不消费带 per-OS 段的 xpkg dep) +- #256:clang × 模块 × 该模板形状(opencv-m 的 gtest 套件此前从未用 clang 构建过) + +共同点:**能力维度的笛卡尔积没有被系统地枚举**。建议做一次一次性的覆盖矩阵盘点(工具链 × OS × host/target × 依赖形态 × 消费深度),把空格子显式记录为"已知未覆盖",而不是默认绿即安全。#261 尤其值得注意:它在**包自己的 CI 上是全绿的**,只有作为依赖被深路径消费时才现形——即"消费者视角"这一整维在 CI 里缺席。 + +### 7.3 文法平价原则的破口 + +#258 与 #254 都指向同一处结构性差异:xpkg 描述符靠**文本体拼接**获得能力(任何顶层键自动 per-OS 化),mcpp.toml 靠**结构化逐键解析**,于是每加一种条件维度都要手工补一遍。这解释了为什么不对称是偶然的而非有意的,也预示了它会继续复发(下一个 per-OS 键、下一个 cfg 维度)。 + +值得考虑的中期收敛:让 `[target.]` 的 build 段与顶层 `[build]` 段**共用同一个解析器**(把 `ConditionalConfig` 的 build 部分换成一个完整的 `BuildConfig`,解析后按谓词合并),而不是维护一份"条件段支持哪几个键"的手工子集。那样 #258 就不再是一个需要实现的特性,而是自动成立的性质;#227 的封闭文法 allowlist 也会自动覆盖条件段,§3.2 的静默吞键随之消失。 + +--- + +## 8. 建议的处置顺序 + +| 顺位 | 动作 | 规模 | 依据 | +|---|---|---|---| +| 1 | #261 去 `cmd /c`,改 `-o $out`,两平台合一分支 | ~8 行 | 构造性必现,已阻塞真实 CI | +| 2 | #257 拆 `posixDepfile` / `mmd_filter` 两谓词,depfile 放开到 Clang;删 e2e 118 的 `# requires: gcc` | ~10 行 + 测试 | 正确性;实测证明当年的顾虑不成立 | +| 3 | #259 mcpp 侧三处静默修复 + issue 重定向 | 中 | 静默降级链;归因需先纠正 | +| 4 | #258 四处实施(含 §3.2 的静默吞键修复) | 中 | 解锁 opencv-m windows 腿,消除 703 stub 文件 | +| 5 | #256 上游报告 + e2e canary + 文档一节 | 小 | mcpp 侧只需 canary 与文档 | +| 6 | #261 P1:rspfile 推广到 windows 全部含 `$local_includes` 的规则 | 中 | 根治长度轴,不再逐规则救火 | +| 7 | #254 host/target 轴收敛(建议同时上强类型) | 中大 | 需独立设计 pass;先做类型再改行为 | + +1、2 两条合起来不到 20 行,建议合成一个 PR 立刻发;3、4 各自一个 PR;7 单独走设计文档。 + +--- + +## 附:核实手段说明 + +- 源码断言:全部在 HEAD(`af25d18`)上按 file:line 逐条回读,未采信 issue 正文的行号转述。 +- #256:本机跑通完整 4×2 控制矩阵(llvm 20.1.7 / 22.1.8 / 系统 clang 18.1.3 / gcc 16.1.0 × 有/无 4 参模板)。 +- #257:本机对比 clang 与 gcc 的 module-TU depfile 实际字节形状,推翻"clang 形状需要过滤"的原假设。 +- #261:本机验证 `clang-scan-deps -o` 在两个自带 LLVM 上均存在且产出合法 P1689 JSON。 +- #259:本机 `file -L` / `readelf -d` 核 PT_INTERP 与 RUNPATH;三份本地 xim-pkgindex checkout 交叉核对 `llvm.lua` 的 deps 声明与其 git 历史。 +- 未做:在 windows 上实测 8191 截断(无该平台);未实测裸 runner 上 glibc 安装为何失败(需 CI 环境)。 diff --git a/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md b/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md new file mode 100644 index 00000000..72a0fac8 --- /dev/null +++ b/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md @@ -0,0 +1,532 @@ +# v0.0.102 批次设计 —— #261 / #257 / #258 / #254(+#256 收尾) + +> 日期:2026-07-22 +> 基线:HEAD = `af25d18`(mcpp **0.0.101**,`MCPP_VERSION` @ `src/toolchain/fingerprint.cppm:21`),工作树 clean +> 前置分析:[2026-07-22-issue-triage-254-261.md](2026-07-22-issue-triage-254-261.md)(六条 issue 的裁决与证据) +> 交付形态:**单 PR,逐节点 commit**,全部实现完成后做整体架构 review commit,再 CI 全绿 → bypass squash 合入 → 发布 → xlings 全生态验证 +> 架构主线:批次总账 §5.2 **一数据模型、多文法、单一收敛漏斗;同一决策不许两处推导**;本批次新增第二条 **失败必须响,不许静默降级**(§1.2) + +--- + +## 0. 决策记录(2026-07-22) + +| # | 决策 | 选择 | 理由 | +|---|------|------|------| +| 1 | #258 实现形态 | **提炼 `BuildInputs` 类型**,让类型系统回答"什么能被条件化" | 见 §5.1——白名单方案是把"手工子集"换个地方放,且编码的是调用顺序这个偶然属性 | +| 1b | #258 范围 | **只做能力,不动未知键策略**(2026-07-22 决定) | 未知键策略三处不一致是独立问题,已开 [#263](https://github.com/mcpp-community/mcpp/issues/263);它有兼容性面,与 #258 的解锁诉求无依赖 | +| 2 | #254 收敛深度 | **强类型 + 行为修正** | 平台轴混用今天零编译期阻力;只改行为下次还会传错 | +| 3 | 静默降级 | **统一降级报告通道 `mcpp::diag`**(范围已收窄,见 §2.0) | 四条 issue 共享同一失效模式,逐点修不解决复发 | +| 4 | **#259** | **移出本批次**(2026-07-22 决定) | 根因在 xlings 侧;调查结论已发 issue 评论,mcpp 侧修复另行安排 | +| 5 | #256 | **本批次收尾:canary e2e + 文档一节**;上游报告单独提 | 低风险、闭合批次;上游修复不受本 PR 约束 | + +### 0.1 #259 的调查结论(已移出实施范围,结论存档) + +调查推翻了 issue 正文的归因,也推翻了"mcpp 从外界复制 payload"的假设,但发现了一个此前未知的 mcpp 侧 bug。三条结论已作为评论发到 [#259](https://github.com/mcpp-community/mcpp/issues/259): + +1. **llvm.lua 确实声明了 glibc**(`pkgs/l/llvm.lua:25-31`,自 `8e432d99` 起),且在 glibc 缺失时是 fail-loud 的。原归因撤回。 +2. **mcpp 没有绕过 xlings 的依赖解析**:`XLINGS_HOME` 恒为 `$MCPP_HOME/registry`,三条安装路径全部汇入 `xim::cmd_install` 且无条件 `resolve(catalog,…)`。 +3. **但 mcpp 自己的 sysroot 兜底是死代码**(新发现):`lifecycle.cppm:562-567` 与 `prepare.cppm:1074-1076` 用无版本的 `"xim:glibc"` 调用 `resolve_xpkg_path`,而后者要求 `@`(`package_fetcher.cppm:805-810`),必然立即失败;两处都丢弃返回值,失败从未可见。 +4. **真因(xlings 侧)**:`xim/installer.cppm:906-950` 的 dep 节点失败是 `log::warn + continue`,不进 `plannedDownloads`;Phase 2 守卫只管已计划节点 ⇒ 不产生 `InstallPhase::Failed`,退出码 0。 + +后续工作(不在本批次):xlings 侧跨仓 issue;mcpp 侧的兜底复活 + 安装完整性闸 + doctor 的 PT_INTERP 检查。 + +--- + +## 1. 总体架构 + +### 1.1 本批次触及的三条架构主线 + +``` +主线 A —— 命令行长度与 shell 依赖(#261) + ninja 规则发射:凡命令行含无界内容(-I 列表、对象列表), + windows 一律走 rspfile;shell 重定向从规则文本中彻底消失。 + +主线 B —— 条件轴的统一(#258) + 「哪些构建输入可以被条件化」今天在 cfg 轴与 feature 轴各推导一遍、 + 答案还不同。提炼 BuildInputs 类型 + 单一 append 漏斗,让类型回答该问题。 + +主线 C —— 轴的类型化与降级的可见性(#254、#257) + 平台 host/target 两轴在类型层不可混; + 任何「因条件不满足而少做一步」必须经 diag 通道上报。 +``` + +### 1.2 新增架构不变量:**失败必须响** + +写入批次总账,并作为本 PR 的 review 判据: + +> 任何"因为条件不满足而少做一步"的分支,必须要么返回错误,要么经 `mcpp::diag::degraded` 上报; +> `log::debug` / `log::verbose` **不算**用户可见。 +> 丢弃 `std::expected` 返回值(`(void)expr` 或不接收)在本仓视为缺陷。 + +本批次范围内被这条不变量命中的现存代码: + +| 位置 | 现状 | 归属 | +|---|---|---| +| `ninja_backend.cppm:496-502` | clang 上无声不发 depfile | #257,本批次修 | + +(#259 的四处见 §0.1;`toml.cppm:976-987` 的条件段未知键静默丢弃属 [#263](https://github.com/mcpp-community/mcpp/issues/263),两者本批次均不动。) + +### 1.3 节点依赖图 + +```dot +digraph batch { + rankdir=LR; node [shape=box]; + N0 [label="N0 diag 通道"]; + N1 [label="N1 #261 scan-deps -o"]; + N2 [label="N2 #261 rspfile 推广"]; + N3 [label="N3 #257 depfile 谓词拆分"]; + N4 [label="N4 #258 提炼 BuildInputs\n(纯重构)"]; + N5 [label="N5 #258 单一 append 漏斗\n(纯重构)"]; + N6 [label="N6 #258 条件段能力\n(新特性)"]; + N7 [label="N7 #254 平台轴强类型"]; + N8 [label="N8 #254 行为修正"]; + N9 [label="N9 #256 canary+文档"]; + N10 [label="N10 CHANGELOG/版本"]; + N11 [label="N11 架构 review pass"]; + N0 -> N3; N0 -> N6; + N1 -> N2; N4 -> N5 -> N6; N7 -> N8; + N2 -> N10; N3 -> N10; N6 -> N10; N8 -> N10; N9 -> N10; + N10 -> N11; +} +``` + +四条链(N1→N2、N3、N4→N5→N6、N7→N8)互不耦合,可并行实现;**N0 先落**。 + +--- + +## 2. N0 —— 统一降级报告通道 `mcpp::diag` + +### 2.0 范围说明(#259 移出后的重新评估) + +`diag` 原本最重的消费者是 #259 的四个静默点。#259 移出后,本批次内的**降级**消费者只剩 #257 一处(windows + clang/gcc 无 POSIX depfile),#258 的未知键走的是硬错而非降级。 + +仍然保留 N0,理由有三,但**范围收窄**: + +1. 决策 3 要把"失败必须响"立为跨模块不变量,需要一个可被 review 指向的具体机制,否则它只是一句口号; +2. `--strict` 提升策略今天在 `prepare.cppm` 里 copy-paste 了 5 处(`:722-726 / 800-809 / 2295-2302 / 3020-3029 / 3060-3067`),收敛到单点是独立收益; +3. 告警内容今天**无法单测**(只能靠 e2e grep stderr),sink 可查询后首次可单测。 + +**收窄之处**:存量迁移只做 `prepare.cppm` 内的 10 处裸 `println(stderr, "warning: ...")` 与 `ScanError` 冲刷(同文件、同冲刷时机);跨 6 个文件的 11 处 `ui::warning` 本批次不动。若认为 N0 仍嫌重,它可整体移出——#257 的降级点退回直接调用 `ui::warning`,批次其余部分不受影响。 + +### 2.1 设计 + +新增 `src/diag.cppm`: + +```cpp +export namespace mcpp::diag { + +enum class Severity { Warning, Degraded }; + +struct Record { + Severity severity; + std::string domain; // "build/depfile" | "manifest/target-cfg" + std::string what; // 发生了什么 + std::string impact; // 对用户的后果 —— Degraded 必填 + std::string hint; // 可选:怎么办 +}; + +// 降级:引擎因条件不满足而少做一步。impact 必填。 +void degraded(std::string_view domain, std::string_view what, + std::string_view impact, std::string_view hint = {}); + +void warning(std::string_view domain, std::string_view what, + std::string_view hint = {}); + +// 汇总:去重、计数、统一渲染;--strict 下 Degraded 提升为错误(单点策略) +[[nodiscard]] bool flush(bool strict); +std::size_t count(Severity); +} +``` + +`impact` 必填是这条通道的核心——它强制作者回答"用户会因此遭遇什么",而这正是 #257 当年缺的那句话。 + +渲染复用 `ui::warning` 的着色实现,输出字节形态不变 ⇒ 现有 e2e 的 grep 断言不破。 + +### 2.2 验收 + +- 单测 `tests/unit/test_diag.cpp`:去重、计数、strict 提升。 +- 现有 e2e(`109_per_glob_flags.sh:55`、`146_feature_flags.sh:71,85`、`67_features_strict.sh`、`93_xpkg_parse.sh`、`103_target_vocabulary.sh`)**逐字节不变**通过 = 迁移无回归。 + +### commit + +``` +feat(diag): single degradation/warning sink — impact-bearing records, one --strict policy +``` + +--- + +## 3. N1 / N2 —— #261 windows scan-deps 命令行 + +### 3.1 N1:去 `cmd /c`,改 `-o $out` + +`src/build/ninja_backend.cppm:665-674` 两分支合一: + +```cpp +} else { + // Clang path: clang-scan-deps writes P1689 JSON to -o (LLVM 17+). + // No shell redirection => no `cmd /c` on Windows => the rule runs on + // ninja's CreateProcess path (32767) instead of cmd.exe's 8191 ceiling. + append(" command = $scan_deps -format=p1689 -o $out -- " + "$cxx $local_includes $cxxflags $unit_cxxflags -c $in -o $compile_target\n"); +} +``` + +`-o` 可用性已实证(本机 20.1.7 / 22.1.8 均有该选项,实跑产出合法 P1689 JSON)。此改动使 `cmd /c` **在全仓 ninja 规则中彻底消失**(`:299` 的另一处是 compile_commands 过滤器的字符串前缀判断,不是规则,保留)。 + +**不做**:不加 LLVM 版本闸。仓库无最低版本机制(`clang.cppm:246-252` 纯路径探测),自带工具链下限 20.1.7 ≫ 17,加闸是凭空造机制。 + +### 3.2 N2:rspfile 从"链接规则"上移为"含无界内容的规则" + +现状 `useRsp`(`:602`)只覆盖 `cxx_link`/`cxx_archive`/`cxx_shared`。N1 只把天花板从 8191 抬到 32767,**长度轴本身没治**——`local_include_flags()`(`:102-127`)每依赖一个 `-I`,无上界;`cxx_object`/`cxx_module`/`c_object`/`cxx_scan` 在 windows 上全无兜底。 + +抽出与 `link_rule` 同源的 `rsp_rule` 发射器,windows 上给**所有含 `$local_includes` 的规则**加 `rspfile`: + +``` +rule cxx_object + command = $cxx @$out.rsp -c $in -o $out # windows + rspfile = $out.rsp + rspfile_content = $local_includes $cxxflags $unit_cxxflags +``` + +复用 #247 已踩过的两条经验(`:594-602` 注释 + 0.0.100 复盘): + +1. rsp 内容按 **GNU 文法**分词,反斜杠是转义符 ⇒ 路径必须 `generic_string()` 正斜杠; +2. POSIX 保持内联零变化(ARG_MAX 充裕,手工复现命令的可读性更值钱)。 + +**范围纪律**:只改 windows 分支;msvc 方言规则与 POSIX 规则文本**逐字节不变**(与 #247 同款约束)。 + +### 3.3 验收 + +- 单测:(a) 全仓规则文本不含 `cmd /c`;(b) windows 下 `cxx_scan`/`cxx_object`/`cxx_module` 含 `rspfile`,POSIX 下不含。 +- **回归靶**:构造 60+ `include_dirs` 的 fixture 包,断言 windows 上 scan+compile 均成功。这是把"消费者视角"补进 CI 的第一步(triage 报告 §7.2)。 + +### commit + +``` +fix(build): #261 clang scan-deps writes via -o — drop the cmd /c wrapper and its 8191 ceiling +fix(build): #261 windows response files cover every rule carrying $local_includes +``` + +--- + +## 4. N3 —— #257 clang purview include 重建 + +### 4.1 设计:一个 bool 拆成两个谓词 + +`ninja_backend.cppm:496-502` 现在把两件无关的事捆在一起: + +- **要不要发 depfile**(`-MMD` + `deps = gcc` + `depfile = $out.d`) +- **要不要跑 awk 过滤器**(剥掉 GCC `-fmodules` 的 reversed make-rules) + +0.0.97 因为第二件事没验证过,把第一件事一起 gate 掉了。**实测表明这个耦合没有必要**: + +``` +clang 20.1.7 / 22.1.8: x.o: x.cppm ops.inc ← 干净,ninja 直接可吃 +gcc 16.1.0: x.o gcm.cache/x.gcm: x.cppm ops.inc + x.c++-module: gcm.cache/x.gcm ← 正是过滤器要剥的 + .PHONY: x.c++-module + gcm.cache/x.gcm:| x.o +``` + +改为: + +```cpp +// 发 depfile:所有 POSIX 非 MSVC 编译器(GCC ∪ Clang)。textual include +// 跟踪是构建正确性的最小契约,不是可选优化。 +const bool posixDepfile = !msvcDeps && !mcpp::platform::is_windows; +// 剥离 reversed make-rules:只有 GCC 的 -fmodules 产出这种形状(实测 +// clang 的 module-TU depfile 是单条普通 rule)。 +const bool needsGnuModuleFilter = + posixDepfile && plan.toolchain.compiler == mcpp::toolchain::CompilerId::GCC; +``` + +### 4.2 剩余降级面必须上报 + +MSVC 走 `deps = msvc` / `/showIncludes`,是等价机制,不算降级;**windows + clang/gcc** 是真降级,接 N0: + +```cpp +diag::degraded("build/depfile", + "此工具链/平台组合不发出 GNU depfile", + "模块接口 purview 内 #include 的文件被修改后不会触发重建,可能使用陈旧 BMI", + "改动 .inc/私有头后请 touch 对应的 .cppm"); +``` + +同时 `c_object` / `asm_object` 今天也无 depfile,一并纳入 `append_cxx_deps`,消除"只有 C++ 边被跟踪"的第二个不对称。 + +### 4.3 验收 + +- e2e `118_purview_include_rebuild.sh`:**删除 `# requires: gcc`**,改为 gcc 与 clang 双腿覆盖。本节点的核心断言。 +- 新增 e2e:模块接口 purview `#include` 一个 `.inc`,改 `.inc` 后 `mcpp build`,断言 BMI mtime **前进**且新符号进入产物。 +- 单测:`test_ninja_backend.cpp:76-111` 的 GCC-only skip 改为双编译器断言——GCC 含 awk 过滤器,Clang 含 depfile 但**不含**过滤器。 + +### commit + +``` +fix(build): #257 depfile emission covers Clang — split "emit depfile" from "filter GCC module rules" +``` + +--- + +## 5. N4 / N5 / N6 —— #258 条件轴统一 + +### 5.0 这个 issue 的核心是什么 + +一个能力,两个直接后果——验收标准就是这三条,不多: + +- **能力**:`[target.'cfg(os)'.build]` 能写 `flags = [{ glob, defines, ... }]`,表达 per-OS 的 per-glob 编译旗标,**包括撤销**(windows 上必须没有 `HAVE_UNISTD_H`,而 unix 上必须有;`-U` 反条目本身也需要 OS 条件化,递归回同一缺口)。 +- **后果 1 —— 删掉 703 个多余文件**:`gen/windows/tu/w/` 426 + `wdnn/` 277 个 stub,每个只有一行 `#include`,**存在的唯一目的是给 windows TU 制造 OS-唯一的路径**,好让全局 flag 表能 key 上去。它们是真进源集的文件——要被 scan、被编译。连同 32 条指向它们的 glob 一起消失。 +- **后果 2 —— 消除 23 条结构性死 glob 告警**:成因是 `sources` 可以 OS 条件化而 `flags` 不能,所以一份 manifest 覆盖三个 OS 时,每个 OS 必然看到另外两个 OS 的 flag 条目,必然零命中。这个消除**由结构给出**:不匹配当前 OS 的条目根本不进 `globFlags`,`globFlagHits` 里没有它,`scanner.cppm:933-948` 无从告警。与 #253 对 feature 轴的做法同构。 + +**不在核心内**:未知键的报错/告警策略。那是共用解析器这一实现手段带来的独立话题,已拆为 [#263](https://github.com/mcpp-community/mcpp/issues/263)(见 §5.1 末段)。 + +### 5.1 为什么不是"白名单" + +初版方案是"条件段共用 `BuildConfig` 解析器 + 一张 `kConditionalBuildKeys` 白名单"。这个方案自我矛盾:#258 的论证是"条件段维护手工键子集是复发源",而白名单**仍是一份手工维护的键子集**,只是从解析器挪到常量表;而且它编码的是一个**偶然属性**——"该字段的消费点碰巧在 `prepare.cppm:912` 之后"。调用顺序一变,表就静默失准。 + +还有一个更实际的风险:如果共用的是**整个 `BuildConfig`** 的解析器,`linkage = "static"` 这类键会在条件段里**解析成功、写进结构体、然后无人读取**(消费点 `:880-893` 早于合并点 `:912`)——把今天的"静默丢弃"换成更隐蔽的"静默无效"。 + +初版方案打算用"硬错"来堵这个洞。这个补丁方向是错的:`[build]` 自己今天**就没有**未知键拒绝(每个键都是正向 `doc->get("build.X")`,`build.cxx_flags` 拼错一样静默忽略),只给条件段加硬错会造出新的不对称;而且对现存 manifest 是行为破坏。正确的做法是让这个洞**在结构上不存在**——见 §5.3:共用的是 `BuildInputs` 的解析器,而 `BuildInputs` 里根本没有 `linkage` 的槽位。至于落到"未知键"路径之后该忽略/告警/报错,那是全仓五处策略不一致的独立问题,已拆为 [#263](https://github.com/mcpp-community/mcpp/issues/263),本批次不动。 + +### 5.2 真正的病灶:`BuildConfig` 混装了三类语义 + +| 类别 | 字段 | 合并语义 | 能否被条件化 | +|---|---|---|---| +| **① 选型轴**(谓词求值的**输入**) | `target` `linkage` `defaultProfile` | 解析定型 | **构造性不能**(`:823-824` 读 `build.target` 来*选定* triple,而谓词求值需要该 triple——循环) | +| **② 已解析策略标量** | `optLevel` `debug` `lto` `strip` `staticStdlib` `cStandard` `allowHostLibs` `macosDeploymentTarget` `dialectCxxflags` | 覆盖(last-wins) | 需 last-wins 语义的独立设计;`dialectCxxflags` 更是模块图全局(types.cppm:191-198),根本不是 per-package | +| **③ 可叠加的构建输入** | `sources` `cflags` `cxxflags` `ldflags` `globFlags` `includeDirs` `includeDirsAfter` | **追加** | 任何条件轴唯一该承载的东西 | + +而 mcpp **今天已经有两条条件轴**,各自手挑了 ③ 的一个子集,**答案还不同**: + +``` +cfg 轴 (ConditionalConfig) 携带:sources, cflags, cxxflags, ldflags +feature 轴(featureSources/Flags/Defines) 携带:sources, flags, defines +``` + +这就是批次总账要消灭的那句话的教科书实例:**"哪些构建输入可被条件化"这个决策在两处各推导一遍。** #258 的症状(cfg 轴没有 `flags`)只是该分歧的一个切面。 + +### 5.3 N4:提炼 `BuildInputs`(纯重构) + +```cpp +// 可叠加的构建输入 —— 任何条件轴(cfg / feature)唯一能贡献的东西。 +// 私有、per-package、追加语义。 +struct BuildInputs { + std::vector sources; + std::vector cflags, cxxflags, ldflags; + std::vector globFlags; // 含 per-glob defines(私有 per-TU,不传播) + std::vector includeDirs, includeDirsAfter; +}; + +struct BuildConfig { + BuildInputs base; // [build] 的可叠加部分 + std::string target, linkage, defaultProfile; // ① 选型轴 + bool staticStdlib; std::string cStandard; ... // ② 策略标量 +}; + +struct ConditionalConfig { + std::string predicate; + BuildInputs inputs; // 取代四个裸字段 + std::map dependencies, devDependencies, buildDependencies; +}; + +std::map featureInputs; // feature 轴同一类型 +``` + +**关键收益**:"什么能被条件化"不再需要任何表来声明——`BuildInputs` 里没有 `linkage` 槽位,条件段里写 `linkage` 就落到既有的"未知键"路径(策略由 [#263](https://github.com/mcpp-community/mcpp/issues/263) 统一决定,本批次沿用现状),**不会**出现"解析成功却无人读取"的静默无效。白名单退化为**类型的字段集**,不会与结构体漂移;将来加字段时"要不要可条件化"变成一个必须回答的**类型归属**问题,而不是一张可以忘记更新的表。 + +读取侧三处需随之调整(纯机械):fingerprint 序列化(`prepare.cppm:234-260` 改为序列化一个 `BuildInputs`)、scanner 种子、`makePackageRoot` 快照。 + +**本节点行为逐字节不变**,由现有全量 e2e 锁死。 + +### 5.4 两个排除项的原则性理由 + +不是"顺序上不方便",而是范畴不同: + +- **`generatedFiles`**:它不是构建**输入**,是一个**写磁盘的副作用动作**(materialize)。动作与输入分属不同范畴,不进 `BuildInputs`。这也顺带解释了它为何在 root/dep 两条路径上顺序相反(`:1165` 在合并后、`:1853` 在合并前)——那是动作调度问题,单独修(见 §11 非目标)。 +- **`featureDefines`**:它沿 Public 边**传播给消费者**(`:2910-2923`),是**接口贡献**而非私有构建输入。`types.cppm:152-158` 早已写下这条界线(`defines`=接口开关 / `flags`=构建配方),只是没在类型上兑现;本节点兑现它。 + +这条界线正好把 #258 的真实用例干净放行:opencv 需要的是 `{ glob = "…/zlib/**", defines = ["NO_FSEEKO"] }`,那是 `GlobFlags::defines`——私有、per-TU、不传播(`types.cppm:123-124`)——在 `BuildInputs` 内。windows 的 additions 与 removals 全部可表达,而"接口开关不该按本包 target 条件化"这条约束一并守住。 + +### 5.5 N5:单一 `append` 漏斗(纯重构) + +```cpp +void append(BuildInputs& dst, const BuildInputs& src); +``` + +一次性取代: + +- `merge_conditional_sources_flags`(`prepare.cppm:425-447`,cfg 轴,4 字段) +- feature 的 `apply()` 折入(`prepare.cppm:2887-2975`、`3001-3008`) + +顺带把函数名从 `merge_conditional_sources_flags` 改为 `merge_conditional_build_inputs`(现名已名不副实)。三个调用点(`:912 / :1871 / :2762`)与"必须在 `makePackageRoot` 快照之前"的顺序不变量(`:900-909` 注释)保持不变。 + +追加顺序:base → cfg 条件段 → feature(按名序),即"last flag wins",这正是表达 windows"撤销 unix `-DHAVE_UNISTD_H`"所需的覆盖权。 + +### 5.6 N6:条件段能力(新特性) + +到这一步,#258 退化为"给 `ConditionalConfig::inputs` 接上 `BuildInputs` 的解析器"的自然结果: + +- `toml.cppm:976-987` 的四次 `read_list` → `parse_build_inputs(bt, cc.inputs)`,与 `[build]` 共用; +- `xpkg.cppm:1047-1105` 的 `target_cfg` 同款替换 ⇒ 两文法一数据模型闭合; +- xpkg 的 `mcpp.` 段因文本拼接机制本就免费支持(`xpkg.cppm:888-894`),无需改动。 + +**各文法现有的未知键策略原样保留**:xpkg `target_cfg` 今天对未知子键硬错(`xpkg.cppm:1085-1090`),替换解析器后仍硬错(不得借重构之名放松);mcpp.toml 沿用现状。策略统一见 [#263](https://github.com/mcpp-community/mcpp/issues/263)。 + +**正确的抽象会让原本的特性请求变成免费结果**——这也是检验 N4/N5 做对了没有的判据。§5.0 的两个后果(703 个 stub、23 条死 glob 告警)到这里全部由结构给出,不需要额外机制。 + +**显式拒绝** `optional = true` / `?`-前缀 这类逃生口(#258 评论中的 stopgap 提议)——那会成为第二套告警抑制机制,与 #253 已确立的"条目按条件*存在*"路线并存,正是"同一决策两处推导"。 + +### 5.7 验收 + +- 单测(N4/N5):现有 manifest/build 单测全绿且**无需修改断言**=重构无行为变化。 +- 单测(N6):条件段 `flags` / `include_dirs` 解析;OS 匹配与不匹配的合并结果;last-wins 覆盖(base `-DX=1` + 条件段 `-UX`);cfg 轴与 feature 轴对同一 `BuildInputs` 的折入顺序;xpkg `target_cfg` 未知子键**仍**硬错(防重构放松)。 +- e2e:三 OS 表 fixture——(a) 命中 OS 的 per-glob defines 落到 TU;(b) 非命中 OS 的条目**零告警**(负向断言,e2e 146 已有先例);(c) 条件段覆盖 base 的 removal 场景(unix 有 `-DHAVE_UNISTD_H=1`、windows 没有)。 +- **最终验收(生态侧,§9)**:opencv-m 落 parked 的 windows 表 → 删 703 个 stub 文件 + 32 条 glob → 三个 OS 各自零死 glob 告警 → windows 腿绿。这三条才是 #258 的真正合格线。 + +### commit + +``` +refactor(manifest): extract BuildInputs — the one type any conditional axis may contribute +refactor(build): cfg axis and feature axis fold through a single append(BuildInputs&) +feat(manifest): #258 per-glob flags / include dirs in [target.'cfg(...)'.build] (xpkg target_cfg parity) +``` + +--- + +## 6. N7 / N8 —— #254 平台轴 + +### 6.1 N7:轴的类型化 + +今天两轴都是 `std::string_view`,混用零阻力: + +```cpp +synthesize_from_xpkg_lua(lua, name, ver, osOverride); // 空 ⇒ 宿主编译期常量 +list_xpkg_versions(lua, kXpkgPlatform); // 宿主编译期常量 +``` + +引入不可互换的类型(`src/platform/axis.cppm`): + +```cpp +export namespace mcpp::platform { +// xpkg 平台词汇:linux / macosx / windows。两轴故意不可互相转换 —— +// 「按哪个平台选段」是一个必须显式声明的决策(设计文档 §6)。 +class HostPlatform { public: static HostPlatform current(); std::string_view key() const; ... }; +class TargetPlatform { public: static TargetPlatform from_triple(std::string_view); ... }; +} +``` + +`synthesize_from_xpkg_lua` / `list_xpkg_versions` 的平台形参改为**必填**的轴类型(去掉 `= {}` 默认值)⇒ 每个调用点被迫声明取哪一轴,传错编译失败。lint 路径(`--all-os`)用显式的 `ExplicitPlatform` 逃生构造。 + +### 6.2 N8:行为修正(分类已核实) + +| 调用点 | 今天 | 改为 | 理由 | +|---|---|---|---| +| `prepare.cppm:1589` | host | **target** | 校验 installed layout 用的是编进构建的源码包 | +| `prepare.cppm:1637` | host | **target** | dep manifest 合成 + preinstall hooks | +| `prepare.cppm:1814` | host | **target** | dep 的构建 manifest | +| `pm/resolver.cppm:108` | host (`kXpkgPlatform`) | **target** | 用户项目的库依赖版本/资产选择 | +| `toolchain/lifecycle.cppm:135` | host | **host(不变)** | 宿主工具,注释已写明为何 | +| `cmd_xpkg.cppm:132` | 遍历三 OS | 不变 | lint | + +`resolver.cppm` 的错误文案今天把宿主平台名硬写进去,一并改为报告实际使用的轴。 + +### 6.3 验收(零行为变化必须被锁死) + +- **e2e:host == target ⇒ 拼接结果逐字节不变**。这是本节点的安全网:native 三平台 CI 的全部现状必须不变。 +- e2e:mingw-cross 腿消费一个带 per-OS `mcpp` 段的 xpkg dep,断言选中 **windows** 段(今天会错选 linux 段)。现有 cross e2e(`102_mingw_cross_wine.sh`、`112_build_mcpp_cross.sh`)都不消费此类 dep,这是新增覆盖。 +- 单测:轴类型互传不可编译(`static_assert(!std::is_convertible_v<...>)`)。 + +### commit + +``` +refactor(platform): host/target platform axes become distinct types — mixing them is a compile error +fix(build): #254 xpkg per-OS sections and version tables key on the resolved target +``` + +--- + +## 7. N9 —— #256 收尾(canary + 文档) + +mcpp 侧无代码缺陷,做两件事: + +1. **e2e canary**:issue #256 的 3 文件复现(已实证 clang 20/22 崩、clang 18 与 gcc 16 过)进 `tests/e2e/`,`# requires: clang`。 + 注意:今天两个自带 LLVM 都会崩 ⇒ canary 若直接断言"必须编过"会立刻红。因此初版断言的是**已知状态**(clang 版本 → 期望结果的映射表),状态变化即提示复核。这是 known-issue canary 的标准形态,作用是让未来任一次 clang bump 修好或再弄坏这件事立即可见。 +2. **文档一节**(模块包指南):"运算符模板的所有模板参数应由第一个实参绑定;否则改为对整个推导操作数类型建模",附 opencv-m `42aeb20` 的改写范式。 + +**上游 LLVM 报告**单独提(不占本 PR),提后回链 issue #256。 + +### commit + +``` +test(e2e): #256 clang module operator-template canary (known-issue state matrix) +docs: module-package guidance — operator template shapes that poison importer lookup on clang 20+ +``` + +--- + +## 8. N10 / N11 —— 收口 + +### N10:文档 / CHANGELOG / 版本 + +- `MCPP_VERSION` `0.0.101` → **`0.0.102`**(`src/toolchain/fingerprint.cppm:21`) +- CHANGELOG 按既有中文格式:一句主旨 + 新增/修复/备注三段,引用本设计文档 +- 批次总账追加 §1.2 的**失败必须响**不变量 +- 手册:`[target.'cfg(...)'.build]` 的 `flags`/`include_dirs` 语法;`BuildInputs` 作为"可条件化输入"的概念说明(替代逐键罗列) + +### N11:整体架构 review pass + +全部节点完成后**单独一轮**,对照 0.0.100 的 `2dbb0f9 refactor(build): architecture-review pass` 先例。检查项: + +1. **同一决策两处推导**:本 PR 是否引入新的双推导点?(重点:diag 与 `ui::warning` 并存期间的策略一致性;平台轴类型化后是否还有裸 string 漏网;`BuildInputs` 之外是否还残留第三条"条件贡献"路径) +2. **失败必须响**:逐一复核本 PR 触碰的每个 `if (...) return;` / `continue` / 丢弃 `expected` +3. **规则文本对称性**:ninja 三方言(gnu / msvc / nasm)在 N1/N2 后仍逐字节符合"只有 windows 变"的约束 +4. **词汇表一致性**:`BuildInputs` 字段集、`kKnownXpkgKeys`、#227 AOT allowlist 三处是否有遗漏交叉 +5. 命名:`merge_conditional_build_inputs` 等改名全仓一致(禁用口袋名纪律) + +``` +refactor: architecture-review pass — <收敛点> +``` + +--- + +## 9. 交付流程 + +``` +1. 分支 fix/batch-254-261(worktree 隔离) +2. 逐节点实现 + commit(N0 → N11),每节点自带单测/e2e +3. 本地全量:单测 + e2e(host 腿)+ clang/gcc 双工具链各跑一次 +4. 开 PR:标题引用四个 issue(#261 #257 #258 #254),正文引本设计文档 +5. CI 全绿:ci-linux / ci-linux-e2e / ci-macos / ci-windows / cross-build-test + —— #261 的真实验证依赖 windows 腿;#254 依赖 cross-build-test 腿 +6. bypass squash 合入 main(自 PR 需 --admin) +7. 发布 0.0.102:release 四平台 → 镜像 xlings-res(gh + gtc 双端,sha256 独立核验) + → xim-pkgindex PR → xlings install 真装 0.0.102 验证 → bootstrap pin bump +8. 生态验证: + - mcpp-index:workspace(windows)腿 —— #261 的原始现场,必须由绿变绿且不再出现 8191 + - opencv-m feat/vendor-single-repo:落 parked 的 windows 表,删 703 个 stub 文件 + 32 条 glob, + 验证 #258 的实际解锁面 +``` + +--- + +## 10. 风险登记 + +| 风险 | 等级 | 缓解 | +|---|---|---| +| N2 rspfile 推广在 windows 引入路径转义回归(#247 同款) | 中 | 复用 `generic_string()` 纪律;msvc/POSIX 规则文本逐字节不变的单测 | +| N3 clang 开 depfile 后 ninja 因意外 depfile 形状报错 | 低 | 已实测形状干净;e2e 118 双腿覆盖是硬闸 | +| **N4/N5 是本批次最大改动**(触及 fingerprint、scanner 种子、makePackageRoot 三处读取点) | 中高 | 拆成两个纯重构 commit,验收标准是"现有断言无需修改";N6 才引入行为变化,可独立 revert | +| N7 类型化触及 pm/toolchain/prepare 三模块 | 中 | 先落类型与签名(编译期一次暴露全部调用点),再逐点改行为;host==target 逐字节不变 e2e 锁底 | +| 单 PR 体量偏大 | 中 | 四条链互不耦合,逐节点 commit 可独立 revert;N11 单独 review 收口 | + +--- + +## 11. 非目标 + +- **#259 全部工作**(sysroot 兜底复活、安装完整性闸、doctor PT_INTERP 检查)——结论存档于 §0.1 与 issue 评论,另行安排 +- xlings 侧 installer 的 per-dep skip 修复(跨仓) +- LLVM 上游 #256 的修复本身 +- `ui::warning` 存量 11 处调用点的全量迁移(跨 6 文件,与本批次无关) +- **未知键策略的统一** —— 全仓五处策略不一致(silent / warning / hard error),已开 [#263](https://github.com/mcpp-community/mcpp/issues/263);有兼容性面,与 #258 的解锁诉求无依赖,单独走 +- **② 类策略标量的条件化**(`linkage`/`cStandard`/profile 组)—— 需 last-wins 语义的独立设计 +- **`generatedFiles` 的条件化与调用序修正** —— 它是副作用动作而非构建输入(§5.4);root/dep 两路径 materialize 与 merge 的顺序不一致本身是隐患,单开 +- `copy_xpkg_from_global` 这条不带 dep 链的 payload 移植回退路径的重新评估(#259 调查中发现的潜在洞,非其机制) diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 70450fab..55284b62 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -10,6 +10,7 @@ module; export module mcpp.build.prepare; import std; +import mcpp.diag; import mcpp.libs.json; import mcpp.log; import mcpp.manifest; @@ -722,7 +723,7 @@ prepare_build(bool print_fingerprint, // feature/platform schema checks below. for (auto const& w : m->schemaWarnings) { if (overrides.strict) return std::unexpected(w); - std::println(stderr, "warning: {}", w); + mcpp::diag::warning("manifest/schema", w); } // ─── Toolchain resolution (docs/21) ──────────────────────────────── @@ -804,7 +805,7 @@ prepare_build(bool print_fingerprint, "[package] platforms contains unknown platform '{}' " "(expected: linux | macos | windows)", pf); if (overrides.strict) return std::unexpected(msg); - std::println(stderr, "warning: {}", msg); + mcpp::diag::warning("manifest/platforms", msg); } } @@ -2298,7 +2299,7 @@ prepare_build(bool print_fingerprint, "'{}/{}') which is not declared in [dependencies] or " "[feature-deps]", f, parentName, depKey, depKey, depFeat); if (overrides.strict) return std::unexpected(msg); - std::println(stderr, "warning: {}", msg); + mcpp::diag::warning("features/forwarding", msg); } } return {}; @@ -3025,7 +3026,7 @@ prepare_build(bool print_fingerprint, auto msg = std::format( "--features requests '{}' which [features] does not declare", *bad); if (overrides.strict) return std::unexpected(msg); - std::println(stderr, "warning: {}", msg); + mcpp::diag::warning("features/request", msg); } apply(packages[0], rootReq); for (auto& f : activate(*m, rootReq)) activeRootFeatures.insert(f); @@ -3063,7 +3064,7 @@ prepare_build(bool print_fingerprint, "dependency '{}' does not declare requested feature '{}' " "in its [features] table", pname, f); if (overrides.strict) return std::unexpected(msg); - std::println(stderr, "warning: {}", msg); + mcpp::diag::warning("features/request", msg); } } // Always apply: even with no requested/default feature, a dep with @@ -3365,13 +3366,14 @@ prepare_build(bool print_fingerprint, return std::unexpected(msg); } for (auto& w : scan.warnings) { - std::println(stderr, "warning: {}", w.format()); + mcpp::diag::warning("modgraph/scan", w.format()); } auto report = mcpp::modgraph::validate(scan.graph, *m, *root); for (auto& w : report.warnings) { - if (w.path.empty()) std::println(stderr, "warning: {}", w.message); - else std::println(stderr, "warning: {}: {}", w.path.string(), w.message); + if (w.path.empty()) mcpp::diag::warning("modgraph/validate", w.message); + else mcpp::diag::warning("modgraph/validate", + std::format("{}: {}", w.path.string(), w.message)); } if (!report.ok()) { std::string msg = "validation errors:\n"; diff --git a/src/diag.cppm b/src/diag.cppm new file mode 100644 index 00000000..71d5c79f --- /dev/null +++ b/src/diag.cppm @@ -0,0 +1,138 @@ +// mcpp.diag — the single sink for user-visible warnings and degradations. +// +// Why this exists: the engine has many branches that do LESS work because a +// precondition was not met (a toolchain lacks a capability, a payload is +// missing, a platform has no equivalent mechanism). Historically those +// branches were silent, or logged at debug level, which is the same thing +// from a user's point of view. The batch invariant is: +// +// Any branch that does less because a condition was not met MUST either +// return an error or report through diag::degraded(). log::debug and +// log::verbose do NOT count as user-visible. +// +// A `degraded` record therefore requires an `impact` string: the author is +// forced to answer "what will the user actually experience?" — the sentence +// that was missing from every silent-degradation bug this channel was +// introduced to prevent. +// +// Ordinary `warning` records (author mistakes, schema drift) carry no impact. +// +// Records are deduplicated and rendered once, at flush(). `--strict` promotes +// degradations to errors in ONE place, replacing the per-site copies of that +// policy that had accumulated across prepare.cppm. + +export module mcpp.diag; + +import std; +import mcpp.ui; + +export namespace mcpp::diag { + +enum class Severity { Warning, Degraded }; + +struct Record { + Severity severity = Severity::Warning; + std::string domain; // "build/depfile", "manifest/target-cfg", ... + std::string what; // what happened + std::string impact; // consequence for the user (required for Degraded) + std::string hint; // optional: what to do about it + + // Rendered form, without the "warning: " / "error: " prefix. + std::string format() const; +}; + +// The engine did less than asked because a precondition was not met. +// `impact` is mandatory — see the module comment. +void degraded(std::string_view domain, std::string_view what, + std::string_view impact, std::string_view hint = {}); + +// An author-facing problem that does not change what the engine does. +void warning(std::string_view domain, std::string_view what, + std::string_view hint = {}); + +// Records render as they are reported (see the implementation note), so this +// only settles the --strict policy and clears the run's state. Returns false +// when `strict` is set and at least one Degraded was recorded, meaning the +// caller should fail the command. +[[nodiscard]] bool flush(bool strict); + +// Introspection for tests. +std::size_t count(Severity severity); +std::vector records(); +void reset(); + +} // namespace mcpp::diag + +// ── implementation ────────────────────────────────────────────────────────── + +namespace mcpp::diag { +namespace { + +std::vector g_records; + +// Identity for deduplication: the whole payload. Two sites reporting the +// same degradation for the same reason are one record; the same domain with +// a different impact stays two, because they tell the user different things. +std::string dedup_key(const Record& r) { + return std::format("{}\x1f{}\x1f{}\x1f{}\x1f{}", + static_cast(r.severity), r.domain, r.what, + r.impact, r.hint); +} + +// Records render as they are pushed, not at flush(): the CLI interleaves +// warnings with `ui::status` progress lines, and deferring them would both +// reorder that stream and lose everything reported before an early return. +// Deduplication happens here, so a repeated report renders once. +void push(Record r) { + auto key = dedup_key(r); + for (auto const& existing : g_records) + if (dedup_key(existing) == key) return; + mcpp::ui::warning(r.format()); + g_records.push_back(std::move(r)); +} + +} // namespace + +std::string Record::format() const { + // The first line stays exactly the `what` text: existing e2e assertions + // grep for those substrings, and a warning should read as one sentence + // before any elaboration. + std::string out(what); + if (!impact.empty()) out += std::format("\n impact: {}", impact); + if (!hint.empty()) out += std::format("\n hint: {}", hint); + return out; +} + +void degraded(std::string_view domain, std::string_view what, + std::string_view impact, std::string_view hint) { + push(Record{Severity::Degraded, std::string(domain), std::string(what), + std::string(impact), std::string(hint)}); +} + +void warning(std::string_view domain, std::string_view what, + std::string_view hint) { + push(Record{Severity::Warning, std::string(domain), std::string(what), + std::string{}, std::string(hint)}); +} + +bool flush(bool strict) { + const bool ok = !(strict && count(Severity::Degraded) > 0); + if (!ok) { + mcpp::ui::error(std::format( + "{} degradation(s) reported and --strict is set — see the warnings " + "above", count(Severity::Degraded))); + } + g_records.clear(); + return ok; +} + +std::size_t count(Severity severity) { + return static_cast(std::ranges::count_if( + g_records, [severity](const Record& r) { return r.severity == severity; })); +} + +std::vector records() { return g_records; } + +void reset() { g_records.clear(); } + +} // namespace mcpp::diag diff --git a/tests/unit/test_diag.cpp b/tests/unit/test_diag.cpp new file mode 100644 index 00000000..27ba088d --- /dev/null +++ b/tests/unit/test_diag.cpp @@ -0,0 +1,81 @@ +#include + +import std; +import mcpp.diag; + +using namespace mcpp::diag; + +namespace { + +struct DiagTest : ::testing::Test { + void SetUp() override { reset(); } + void TearDown() override { reset(); } +}; + +} // namespace + +TEST_F(DiagTest, WarningAndDegradedAreCountedSeparately) { + warning("manifest/schema", "unsupported key 'bogus'"); + degraded("build/depfile", "no GNU depfile on this toolchain", + "edits to purview-included files will not trigger a rebuild"); + + EXPECT_EQ(count(Severity::Warning), 1u); + EXPECT_EQ(count(Severity::Degraded), 1u); + EXPECT_EQ(records().size(), 2u); +} + +TEST_F(DiagTest, IdenticalReportsAreDeduplicated) { + for (int i = 0; i < 5; ++i) + warning("modgraph/scan", "glob 'a/**' matched no source file"); + + EXPECT_EQ(records().size(), 1u) + << "a repeated report should render and record once"; +} + +TEST_F(DiagTest, SameDomainDifferentPayloadStaysDistinct) { + // Two degradations in one domain that tell the user different things are + // two records — dedup keys on the whole payload, not the domain. + degraded("build/depfile", "no GNU depfile", "stale BMI possible"); + degraded("build/depfile", "no GNU depfile", "stale object possible"); + + EXPECT_EQ(records().size(), 2u); +} + +TEST_F(DiagTest, FormatKeepsWhatOnTheFirstLine) { + // e2e assertions grep for the `what` substring; elaboration must not get + // in front of it. + warning("modgraph/scan", "[build].flags glob 'x/**' matched no source file"); + auto rs = records(); + ASSERT_EQ(rs.size(), 1u); + EXPECT_EQ(rs[0].format(), "[build].flags glob 'x/**' matched no source file"); + + reset(); + degraded("build/depfile", "what happened", "the impact", "the hint"); + rs = records(); + ASSERT_EQ(rs.size(), 1u); + EXPECT_TRUE(rs[0].format().starts_with("what happened\n")); + EXPECT_NE(rs[0].format().find("impact: the impact"), std::string::npos); + EXPECT_NE(rs[0].format().find("hint: the hint"), std::string::npos); +} + +TEST_F(DiagTest, StrictPromotesDegradationsOnly) { + warning("manifest/schema", "just a warning"); + EXPECT_TRUE(flush(/*strict=*/true)) + << "a plain warning must not fail a --strict run"; + + reset(); + degraded("build/depfile", "w", "i"); + EXPECT_FALSE(flush(/*strict=*/true)); + + reset(); + degraded("build/depfile", "w", "i"); + EXPECT_TRUE(flush(/*strict=*/false)); +} + +TEST_F(DiagTest, FlushClearsRunState) { + warning("a", "b"); + degraded("c", "d", "e"); + EXPECT_TRUE(flush(/*strict=*/false)); + EXPECT_EQ(records().size(), 0u); + EXPECT_EQ(count(Severity::Degraded), 0u); +} From 079fcc4d811ada3bf74c3054d5f64d46653c8f7f Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:19:30 +0800 Subject: [PATCH 02/13] =?UTF-8?q?fix(build):=20#261=20clang=20scan-deps=20?= =?UTF-8?q?writes=20via=20-o=20=E2=80=94=20drop=20the=20cmd=20/c=20wrapper?= =?UTF-8?q?=20and=20its=208191=20ceiling?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The clang scan rule produced its P1689 JSON through shell redirection. Ninja on Windows spawns via CreateProcess, which does not interpret `>`, so the rule was wrapped in `cmd /c` — and cmd.exe caps a command line at 8191 chars, a quarter of CreateProcess's 32767. That ceiling is reached in practice: Sunrisepeak/opencv-m consumed as a dependency measured 8370-8570 char scan commands (48 -I entries, ~124 chars of package-root prefix at a workspace-member path), and all six scan invocations failed with "The command line is too long". The same package builds green in its own CI, where the path prefix is 23 chars. clang-scan-deps has -o since LLVM 17 — verified on both bundled toolchains (20.1.7, 22.1.8) — so the redirect is unnecessary. Writing the output via a flag also makes the clang branch match what the GCC (-fdeps-file=) and MSVC (/scanDependencies) branches already do, and collapses the two platform branches into one rule text. `cmd /c` was the only shell mcpp put between ninja and a compiler; a regression test keeps it gone. Note this only raises the ceiling to 32767 — the include-dir list itself is still unbounded, which the following commit addresses. --- src/build/ninja_backend.cppm | 19 ++++++----- tests/unit/test_ninja_backend.cpp | 54 +++++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index ad3d4bd4..7214d23f 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -662,16 +662,15 @@ std::string emit_ninja_string(const BuildPlan& plan) { "-fdeps-file=$out -fdeps-target=$compile_target " "-M -MM -MF $out.dep -E $in -o $compile_target\n"); } else { - // Clang path: clang-scan-deps produces P1689 JSON to stdout. - if constexpr (mcpp::platform::is_windows) { - // Wrap in cmd /c for shell redirection (ninja on Windows uses - // CreateProcess which doesn't interpret > as redirect). - append(" command = cmd /c \"$scan_deps -format=p1689 -- " - "$cxx $local_includes $cxxflags $unit_cxxflags -c $in -o $compile_target > $out\"\n"); - } else { - append(" command = $scan_deps -format=p1689 -- " - "$cxx $local_includes $cxxflags $unit_cxxflags -c $in -o $compile_target > $out\n"); - } + // Clang path: clang-scan-deps writes the P1689 JSON itself via -o + // (LLVM 17+), like the GCC and MSVC branches above write theirs + // via -fdeps-file= / /scanDependencies. Shell redirection would + // force a `cmd /c` wrapper on Windows, and cmd.exe caps a command + // line at 8191 chars — a quarter of the 32767 ninja gets from + // CreateProcess — which any package with a large include-dir list + // overruns (#261: 48 -I entries at a deep consumer path). + append(" command = $scan_deps -format=p1689 -o $out -- " + "$cxx $local_includes $cxxflags $unit_cxxflags -c $in -o $compile_target\n"); } append(" description = SCAN $out\n\n"); diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 8ef0ab3a..e543df3a 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -539,3 +539,57 @@ TEST(NinjaBackend, RootPackageCxxflagsAreEmittedOncePerUnit) { EXPECT_EQ(ninja.find("cxxflags = -std=c++23 -O2 -DROOT_FLAG=1"), std::string::npos) << ninja; } + +// mcpp#261: the clang scan rule used shell redirection (`> $out`), which on +// Windows forced a `cmd /c` wrapper and with it cmd.exe's 8191-char command +// line ceiling — a quarter of what ninja's CreateProcess path allows. Any +// package with a large include-dir list overran it. clang-scan-deps has -o +// (LLVM 17+), so the redirect and the wrapper are both unnecessary. +TEST(NinjaBackend, ClangScanRuleWritesViaDashOWithoutShellRedirection) { + auto plan = minimal_plan(); + plan.toolchain.compiler = mcpp::toolchain::CompilerId::Clang; + plan.toolchain.binaryPath = "/usr/bin/clang++"; + plan.scanDepsPath = "/usr/bin/clang-scan-deps"; + plan.compileUnits.push_back({ + .source = "src/m.cppm", + .object = "obj/m.o", + .packageName = "objc_rule_test", + .providesModule = "m", + }); + + auto ninja = emit_ninja_string(plan); + + auto scanRule = ninja.find("rule cxx_scan"); + ASSERT_NE(scanRule, std::string::npos) << ninja; + auto scanEnd = ninja.find("description = SCAN", scanRule); + ASSERT_NE(scanEnd, std::string::npos) << ninja; + auto rule = ninja.substr(scanRule, scanEnd - scanRule); + + EXPECT_NE(rule.find("-format=p1689 -o $out --"), std::string::npos) << rule; + EXPECT_EQ(rule.find("> $out"), std::string::npos) + << "scan rule must not use shell redirection: " << rule; +} + +// The `cmd /c` wrapper was the only place mcpp put a shell between ninja and +// a compiler invocation. Keep it gone: it is what re-imposes the 8191 ceiling. +TEST(NinjaBackend, NoRuleWrapsItsCommandInCmdSlashC) { + for (auto compiler : {mcpp::toolchain::CompilerId::GCC, + mcpp::toolchain::CompilerId::Clang}) { + auto plan = minimal_plan(); + plan.toolchain.compiler = compiler; + if (compiler == mcpp::toolchain::CompilerId::Clang) { + plan.toolchain.binaryPath = "/usr/bin/clang++"; + plan.scanDepsPath = "/usr/bin/clang-scan-deps"; + } + plan.compileUnits.push_back({ + .source = "src/m.cppm", + .object = "obj/m.o", + .packageName = "objc_rule_test", + .providesModule = "m", + }); + + auto ninja = emit_ninja_string(plan); + EXPECT_EQ(ninja.find("cmd /c"), std::string::npos) + << "compiler=" << static_cast(compiler) << "\n" << ninja; + } +} From 2762dbcddc34b02bf9eea98375cc1262c0a79b69 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:26:28 +0800 Subject: [PATCH 03/13] fix(build): #261 windows response files cover every rule carrying $local_includes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Dropping the cmd /c wrapper raised the scan rule's ceiling from 8191 to 32767, but did not address the axis: local_include_flags() emits one -I per dependency include dir with no upper bound, and cxx_module / cxx_object / c_object / asm_object / cxx_scan all inline that payload. The scan command is strictly longer than the compile command for the same TU, since it wraps it, so it just happens to fail first. Generalises the #247 link-rule mitigation to every rule that carries an unbounded flag payload: on Windows the payload moves into `rspfile_content` and the command references `@$out.rsp`. Safe for all consumers — the gcc and clang drivers and cl.exe expand @file, and clang-scan-deps passes an @file in its post-`--` command straight through to the driver (verified against the bundled 20.1.7). nasm_object is deliberately excluded: NASM spells response files `-@ file`, not `@file`. Also switches escape_flag_path to generic_string(). This is the #247 trap one level down: response-file content is tokenized GNU-style, where backslash is an escape character, so a Windows -I path would lose its separators once it moved off the command line. Forward slashes are accepted by every Windows consumer and POSIX output is unchanged. useCompileRsp keys on `is_windows || msvcDeps` rather than is_windows alone. The msvc dialect only ever runs on Windows, so this is faithful, and it makes the response-file shape reachable from a POSIX test host — the same over-approximation the link rules already use. POSIX output is byte-identical, guarded by a test asserting the compile rules stay inline there. New e2e 148 builds a package with 64 include dirs on every platform: the failure was found from the consumer side, where a deep dependency path pushed an otherwise-green package over the limit. --- src/build/ninja_backend.cppm | 89 +++++++++++++++++++++++------- tests/e2e/148_wide_include_list.sh | 69 +++++++++++++++++++++++ tests/unit/test_ninja_backend.cpp | 82 +++++++++++++++++++++++++++ 3 files changed, 220 insertions(+), 20 deletions(-) create mode 100755 tests/e2e/148_wide_include_list.sh diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 7214d23f..10b45aba 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -84,7 +84,12 @@ std::string escape_ninja_path(const std::filesystem::path& p) { } std::string escape_flag_path(const std::filesystem::path& p) { - auto s = p.string(); + // generic_string() for the same reason node names use it (#247): on + // Windows these -I/-idirafter paths are copied verbatim into a response + // file (#261), which the drivers tokenize GNU-style, where backslash is + // an ESCAPE character — a path like C:\src\inc would lose its separators. + // Every Windows consumer accepts forward slashes; POSIX is unchanged. + auto s = p.generic_string(); std::string out; out.reserve(s.size()); for (char c : s) { @@ -508,15 +513,49 @@ std::string emit_ninja_string(const BuildPlan& plan) { append_deps(); } }; + // #261: the flag payload of every compile/scan rule is unbounded — one + // -I per dependency include dir — and on Windows ninja spawns through + // CreateProcess, whose command line caps at 32767 chars. Route the + // payload through a response file: the same mitigation the link rules + // got in #247, for the same reason (there it was thousands of objects). + // + // Safe for every consumer: the gcc and clang drivers and cl.exe all + // expand @file, and clang-scan-deps passes an @file in its post-`--` + // command straight through to the driver (verified on the bundled + // 20.1.7). nasm_object is deliberately NOT converted — NASM spells + // response files `-@ file`, not `@file`, and its rule already sits + // outside CommandDialect for that class of reason. + // + // POSIX keeps the inline form byte-identical: ARG_MAX is ample and an + // inline command is far easier to re-run by hand when debugging. + // Also keyed on the msvc dialect, which only ever runs on Windows: it + // makes the response-file shape reachable from a non-Windows test host, + // the same over-approximation the link rules use (`separateLinker || + // is_windows`). + const bool useCompileRsp = mcpp::platform::is_windows || msvcDeps; + // Both take the payload with its leading space, so callers read as + // `command = $cxx{payload} ...` exactly like the inline form did. + auto rsp_ref = [&](const std::string& payload) { + return useCompileRsp ? std::string(" @$out.rsp") : payload; + }; + auto append_rspfile = [&](const std::string& payload) { + if (!useCompileRsp) return; + append(" rspfile = $out.rsp\n"); + append(std::format(" rspfile_content ={}\n", payload)); + }; + // cl.exe needs /TP (our module interfaces are .cppm, unknown to cl) and // /interface to treat the TU as a module interface unit. const std::string module_src_flags = msvcDeps ? " /interface /TP" : ""; append("rule cxx_module\n"); if constexpr (mcpp::platform::is_windows) { // Windows: skip BMI restat optimization (requires POSIX shell). - append(std::format(" command = " - "$cxx $local_includes $cxxflags $unit_cxxflags{}{} {}\n", - module_output_flag, module_src_flags, compile_tail)); + const auto payload = std::format( + " $local_includes $cxxflags $unit_cxxflags{}{}", + module_output_flag, module_src_flags); + append(std::format(" command = $cxx{} {}\n", + rsp_ref(payload), compile_tail)); + append_rspfile(payload); append_cxx_deps(); } else { append(std::format(" command = " @@ -539,9 +578,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("rule cxx_object\n"); if constexpr (mcpp::platform::is_windows) { - append(std::format( - " command = $cxx $local_includes $cxxflags $unit_cxxflags {}\n", - compile_tail)); + const std::string payload = " $local_includes $cxxflags $unit_cxxflags"; + append(std::format(" command = $cxx{} {}\n", + rsp_ref(payload), compile_tail)); + append_rspfile(payload); } else { append(std::format( " command = $cxx $local_includes $cxxflags $unit_cxxflags {}{}{}\n", @@ -555,9 +595,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { if (need_c_rule) { append("rule c_object\n"); - append(std::format( - " command = $cc $local_includes $cflags $unit_cflags {}\n", - compile_tail)); + const std::string payload = " $local_includes $cflags $unit_cflags"; + append(std::format(" command = $cc{} {}\n", + rsp_ref(payload), compile_tail)); + append_rspfile(payload); append(" description = CC $out\n"); append_deps(); if (dyndep) @@ -570,9 +611,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { // and assembles both, dispatching by extension. $asmflags is the // asm-safe flag subset (no -std / no -O — see flags.cppm). append("rule asm_object\n"); - append(std::format( - " command = $cc $local_includes $asmflags $unit_asmflags {}\n", - compile_tail)); + const std::string payload = " $local_includes $asmflags $unit_asmflags"; + append(std::format(" command = $cc{} {}\n", + rsp_ref(payload), compile_tail)); + append_rspfile(payload); append(" description = AS $out\n\n"); } @@ -649,18 +691,23 @@ std::string emit_ninja_string(const BuildPlan& plan) { // GCC: built-in -fdeps-format=p1689r5 flags during preprocessing. // Clang: external clang-scan-deps tool with -format=p1689. append("rule cxx_scan\n"); + // The scan command is strictly LONGER than the compile command for + // the same TU (it wraps it), so it carries the same unbounded + // include payload through the same response file (#261). + const std::string scanPayload = " $local_includes $cxxflags $unit_cxxflags"; if (msvcDeps) { // MSVC: compiler-integrated P1689 via /scanDependencies (scan // only — no codegen); /TP because our module units are .cppm. - append(" command = $cxx $local_includes $cxxflags $unit_cxxflags " - "/scanDependencies $out /TP /c $in /Fo:$compile_target\n"); + append(std::format(" command = $cxx{} " + "/scanDependencies $out /TP /c $in /Fo:$compile_target\n", + rsp_ref(scanPayload))); } else if (plan.scanDepsPath.empty()) { // GCC path: compiler-integrated P1689 scanning. - append(" command = $cxx $local_includes $cxxflags -fmodules " - "$unit_cxxflags " + append(std::format(" command = $cxx{} -fmodules " "-fdeps-format=p1689r5 " "-fdeps-file=$out -fdeps-target=$compile_target " - "-M -MM -MF $out.dep -E $in -o $compile_target\n"); + "-M -MM -MF $out.dep -E $in -o $compile_target\n", + rsp_ref(scanPayload))); } else { // Clang path: clang-scan-deps writes the P1689 JSON itself via -o // (LLVM 17+), like the GCC and MSVC branches above write theirs @@ -669,9 +716,11 @@ std::string emit_ninja_string(const BuildPlan& plan) { // line at 8191 chars — a quarter of the 32767 ninja gets from // CreateProcess — which any package with a large include-dir list // overruns (#261: 48 -I entries at a deep consumer path). - append(" command = $scan_deps -format=p1689 -o $out -- " - "$cxx $local_includes $cxxflags $unit_cxxflags -c $in -o $compile_target\n"); + append(std::format( + " command = $scan_deps -format=p1689 -o $out -- " + "$cxx{} -c $in -o $compile_target\n", rsp_ref(scanPayload))); } + append_rspfile(scanPayload); append(" description = SCAN $out\n\n"); // Aggregate .ddi files into a Ninja dyndep file. diff --git a/tests/e2e/148_wide_include_list.sh b/tests/e2e/148_wide_include_list.sh new file mode 100755 index 00000000..7c558e1e --- /dev/null +++ b/tests/e2e/148_wide_include_list.sh @@ -0,0 +1,69 @@ +#!/usr/bin/env bash +# #261: a package with a wide include-dir list must scan, compile and link. +# +# The failure this guards against is Windows-only in its symptom but not in +# its cause: every -I lands on the scan AND compile command lines, which have +# a hard ceiling on Windows (8191 through cmd.exe before #261, 32767 through +# CreateProcess after). The regression was found from the CONSUMER side — +# the package's own CI was green because its paths were short, and only a +# deep dependency path pushed the same command over the limit. +# +# Running it on every platform keeps the shape honest (POSIX proves the +# inline form still works, Windows proves the response-file form does). +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT + +cd "$TMP" +"$MCPP" new wideinc > /dev/null +cd wideinc + +# 64 include dirs, each with a header that only that dir provides, so the +# build genuinely depends on every entry being present on the command line. +N=64 +INCS="" +for i in $(seq 1 $N); do + d="vendor/dir_with_a_deliberately_long_name_$i/include" + mkdir -p "$d" + echo "#define WIDE_INC_$i $i" > "$d/wide_$i.h" + INCS="$INCS\"$d\", " +done + +python3 - "$N" <<'PY' > src/wide.cpp +import sys +n = int(sys.argv[1]) +for i in range(1, n + 1): + print(f'#include ') +print('extern "C" int wide_sum() { return 0') +for i in range(1, n + 1): + print(f' + WIDE_INC_{i}') +print(' ; }') +PY + +cat > src/main.cpp <<'EOF' +import std; +extern "C" int wide_sum(); +int main() { + std::println("wide_sum = {}", wide_sum()); + return wide_sum() == (64 * 65) / 2 ? 0 : 1; +} +EOF + +python3 - "$INCS" <<'PY' +import sys, re, pathlib +incs = sys.argv[1].rstrip(', ') +p = pathlib.Path('mcpp.toml') +s = p.read_text() +s = re.sub(r'(?m)^\[build\]$', f'[build]\ninclude_dirs = [{incs}]', s, count=1) +if 'include_dirs' not in s: + s += f'\n[build]\ninclude_dirs = [{incs}]\n' +p.write_text(s) +PY + +"$MCPP" build > build.log 2>&1 || { cat build.log; echo "wide include build failed"; exit 1; } +out=$("$MCPP" run 2>&1) || { echo "$out"; echo "wide include run failed"; exit 1; } +echo "$out" | grep -q "wide_sum = 2080" || { + echo "$out"; echo "expected wide_sum = 2080 (every -I must be present)"; exit 1; } + +echo "PASS: 148_wide_include_list" diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index e543df3a..b11918a8 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -593,3 +593,85 @@ TEST(NinjaBackend, NoRuleWrapsItsCommandInCmdSlashC) { << "compiler=" << static_cast(compiler) << "\n" << ninja; } } + +// mcpp#261: the compile and scan rules carry an UNBOUNDED flag payload — +// one -I per dependency include dir — and on Windows ninja spawns through +// CreateProcess (32767-char ceiling). They now route that payload through a +// response file, the same mitigation #247 gave the link rules for the same +// reason. Reachable from a POSIX test host via the msvc dialect, which only +// ever runs on Windows. NASM is excluded on purpose (it spells response +// files `-@ file`), and POSIX keeps the inline form byte-identical. +TEST(NinjaBackend, CompileAndScanRulesRouteFlagsThroughRspfileUnderMsvcDialect) { + auto plan = minimal_plan(); + plan.toolchain.compiler = mcpp::toolchain::CompilerId::MSVC; + plan.toolchain.binaryPath = "cl.exe"; + plan.compileUnits.push_back({ + .source = "src/m.cppm", + .object = "obj/m.o", + .packageName = "objc_rule_test", + .providesModule = "m", + }); + plan.compileUnits.push_back({ + .source = "src/a.c", + .object = "obj/a.o", + .packageName = "objc_rule_test", + }); + + auto ninja = emit_ninja_string(plan); + + // cxx_module and cxx_object pick their Windows shape through + // `if constexpr (is_windows)`, so their response-file form is only + // reachable on a Windows host; c_object and cxx_scan have no such + // compile-time branch and are assertable everywhere. + std::vector rules{"rule c_object\n", "rule cxx_scan\n"}; + if constexpr (mcpp::platform::is_windows) { + rules.push_back("rule cxx_module\n"); + rules.push_back("rule cxx_object\n"); + } + for (std::string_view rule : rules) { + auto start = ninja.find(rule); + ASSERT_NE(start, std::string::npos) << rule << "\n" << ninja; + auto end = ninja.find("\n\n", start); + ASSERT_NE(end, std::string::npos) << ninja; + auto body = ninja.substr(start, end - start); + + EXPECT_NE(body.find("@$out.rsp"), std::string::npos) << body; + EXPECT_NE(body.find("rspfile = $out.rsp"), std::string::npos) << body; + EXPECT_NE(body.find("rspfile_content = $local_includes"), + std::string::npos) << body; + // The payload must not ALSO remain inline, or the ceiling stands. + auto cmdStart = body.find("command = "); + auto cmdEnd = body.find('\n', cmdStart); + auto cmd = body.substr(cmdStart, cmdEnd - cmdStart); + EXPECT_EQ(cmd.find("$local_includes"), std::string::npos) << cmd; + } +} + +// POSIX must keep the inline form: ARG_MAX is ample and an inline command is +// far easier to re-run by hand. This is the byte-identity guard for the +// #261 change on the platform where it must be a no-op. +TEST(NinjaBackend, CompileRulesStayInlineOnPosixDrivers) { + if constexpr (mcpp::platform::is_windows) { + GTEST_SKIP() << "inline form is Windows-exempt by design"; + } else { + auto plan = minimal_plan(); // GCC → gnu dialect, non-msvc deps + plan.compileUnits.push_back({ + .source = "src/a.c", + .object = "obj/a.o", + .packageName = "objc_rule_test", + }); + + auto ninja = emit_ninja_string(plan); + + for (std::string_view rule : {"rule cxx_module\n", "rule cxx_object\n", + "rule c_object\n"}) { + auto start = ninja.find(rule); + ASSERT_NE(start, std::string::npos) << rule << "\n" << ninja; + auto end = ninja.find("\n\n", start); + ASSERT_NE(end, std::string::npos) << ninja; + auto body = ninja.substr(start, end - start); + EXPECT_EQ(body.find("rspfile"), std::string::npos) << body; + EXPECT_NE(body.find("$local_includes"), std::string::npos) << body; + } + } +} From 3ed91be31a868992b9b39fd47ad7950b6027946f Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:32:11 +0800 Subject: [PATCH 04/13] =?UTF-8?q?fix(build):=20#257=20depfile=20emission?= =?UTF-8?q?=20covers=20Clang=20=E2=80=94=20split=20"emit=20depfile"=20from?= =?UTF-8?q?=20"filter=20GCC=20module=20rules"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Editing a file pulled into a module interface via a purview #include did not retrigger the interface rebuild under Clang: the next build reused the stale BMI, so importers compiled against the old surface. It failed silently, which is the worst shape a build system can take — the wrong answer arrives looking like a correct incremental build, and suspicion falls on user code first. The issue reporter lost a debugging round to it. Root cause is a conflated predicate from 0.0.97. `posixDepfile` decided BOTH whether to emit a depfile at all AND whether to run the awk filter that strips the reversed make-rules GCC's `-fmodules -MMD` bolts on. Clang's depfile shape was unverified at the time, so the filter was scoped to GCC — and the depfile went with it. Measured now, on both bundled toolchains: clang 20.1.7/22.1.8: x.o: x.cppm ops.inc gcc 16.1.0: x.o gcm.cache/x.gcm: x.cppm ops.inc x.c++-module: gcm.cache/x.gcm .PHONY: x.c++-module gcm.cache/x.gcm:| x.o Clang emits a single plain rule — nothing the filter would remove. The gate was protecting against a shape that does not exist, at the cost of the correctness contract it was bundled with. Split into `posixDepfile` (any POSIX non-MSVC toolchain) and `needsGnuModuleFilter` (GCC only, which alone writes to the scratch $out.d.raw). Clang writes -MF straight to $out.d. Also fixes the other half of the same asymmetry: c_object and asm_object had no depfile on any toolchain, so C and GAS units never tracked their headers. They take the flag but not the filter — module reversed-rules cannot occur there. Windows non-MSVC (mingw gcc / hosted clang) remains without include tracking, since the GCC filter needs awk. That is now reported through diag::degraded with its user-visible impact instead of being silent. cl.exe is unaffected: deps=msvc via /showIncludes is the equivalent mechanism, not a degradation. e2e 118 drops `# requires: gcc` and gains a Clang leg exercising both assertions against the newest installed LLVM (verified locally on 22.1.8). --- src/build/ninja_backend.cppm | 68 ++++++++++----- tests/e2e/118_purview_include_rebuild.sh | 104 +++++++++++++++++++++-- tests/unit/test_ninja_backend.cpp | 66 ++++++++++++++ 3 files changed, 209 insertions(+), 29 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 10b45aba..ecdc0558 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -23,6 +23,7 @@ import mcpp.build.plan; import mcpp.build.flags; import mcpp.build.hermetic; import mcpp.build.compile_commands; +import mcpp.diag; import mcpp.dyndep; import mcpp.toolchain.detect; import mcpp.toolchain.dialect; @@ -488,20 +489,28 @@ std::string emit_ninja_string(const BuildPlan& plan) { // behavior (no depfile) rather than depend on an unavailable filter — // msvcDeps (cl.exe) is unaffected either way (deps=msvc, no -MMD). // - // GCC-only: the awk filter strips the "reversed" make-rules that GCC's - // `-fmodules -MMD` bolts onto a module-TU depfile (`gcm.cache/.gcm: | - // ` etc., which ninja rejects as "inputs may not also have inputs"). - // Clang's module system (.pcm) does not emit those and its module-TU - // depfile shape is different; applying the GCC-shaped filter there is - // untested and could yield a wrong dep set. Until Clang is verified, - // scope this to GCC — Clang keeps the pre-#235 behavior (no compile-edge - // depfile; header/purview rebuild tracking on Clang is a follow-up, same - // as it was on 0.0.96, so this is not a regression). e2e 118 declares - // `# requires: gcc` and skips where GCC is not the toolchain. - const bool posixDepfile = !msvcDeps && !mcpp::platform::is_windows - && plan.toolchain.compiler == mcpp::toolchain::CompilerId::GCC; - const std::string mmd_flag = posixDepfile ? "-MMD -MF $out.d.raw " : ""; - const std::string mmd_filter = posixDepfile + // #257: these are TWO decisions, and 0.0.97 conflated them. Emitting a + // depfile at all is the minimum correctness contract for textual include + // tracking — without it, editing a file #include'd in a module purview + // silently reuses a stale BMI. Stripping GCC's reversed make-rules is a + // GCC-shaped detail of HOW that depfile arrives. Gating the first on the + // second left Clang with no include tracking for four releases. + // + // Measured (bundled 20.1.7 / 22.1.8 vs gcc 16.1.0, module TU with a + // purview #include): + // clang: `x.o: x.cppm ops.inc` — one plain rule + // gcc: `x.o gcm.cache/x.gcm: x.cppm ops.inc` + // `x.c++-module: gcm.cache/x.gcm` + .PHONY + `gcm.cache/x.gcm:| x.o` + // So Clang emits nothing the filter would need to remove, and the + // conflated gate was protecting against a shape that does not exist. + const bool posixDepfile = !msvcDeps && !mcpp::platform::is_windows; + const bool needsGnuModuleFilter = + posixDepfile && plan.toolchain.compiler == mcpp::toolchain::CompilerId::GCC; + const std::string mmd_flag = + posixDepfile ? (needsGnuModuleFilter ? "-MMD -MF $out.d.raw " + : "-MMD -MF $out.d ") + : ""; + const std::string mmd_filter = needsGnuModuleFilter ? " && awk 'NR==1{print;next} /^[^ ]/{exit} {print}' " "\"$out.d.raw\" > \"$out.d\" && rm -f \"$out.d.raw\"" : ""; @@ -513,6 +522,23 @@ std::string emit_ninja_string(const BuildPlan& plan) { append_deps(); } }; + // C and GAS units include headers too, and had no depfile on ANY + // toolchain — the second half of the same asymmetry #257 reports. They + // never carry module reversed-rules, so they need the flag but not the + // filter. + const std::string c_mmd_flag = posixDepfile ? "-MMD -MF $out.d " : ""; + // Windows non-MSVC (mingw gcc / clang) is the one combination left with + // no include tracking: the GCC filter needs awk, which is not available + // there. cl.exe is fine — deps=msvc via /showIncludes is the equivalent + // mechanism, not a degradation. + if (!posixDepfile && !msvcDeps) { + mcpp::diag::degraded("build/depfile", + "this toolchain and platform combination emits no GNU depfile", + "editing a file #include'd inside a module interface purview (or a " + "header pulled into a .cpp) will not trigger a rebuild, so the build " + "may reuse a stale BMI or object", + "touch the including .cppm/.cpp after editing such a file"); + } // #261: the flag payload of every compile/scan rule is unbounded — one // -I per dependency include dir — and on Windows ninja spawns through // CreateProcess, whose command line caps at 32767 chars. Route the @@ -596,11 +622,11 @@ std::string emit_ninja_string(const BuildPlan& plan) { if (need_c_rule) { append("rule c_object\n"); const std::string payload = " $local_includes $cflags $unit_cflags"; - append(std::format(" command = $cc{} {}\n", - rsp_ref(payload), compile_tail)); + append(std::format(" command = $cc{} {}{}\n", + rsp_ref(payload), c_mmd_flag, compile_tail)); append_rspfile(payload); append(" description = CC $out\n"); - append_deps(); + append_cxx_deps(); if (dyndep) append(" restat = 1\n"); append("\n"); @@ -612,10 +638,12 @@ std::string emit_ninja_string(const BuildPlan& plan) { // asm-safe flag subset (no -std / no -O — see flags.cppm). append("rule asm_object\n"); const std::string payload = " $local_includes $asmflags $unit_asmflags"; - append(std::format(" command = $cc{} {}\n", - rsp_ref(payload), compile_tail)); + append(std::format(" command = $cc{} {}{}\n", + rsp_ref(payload), c_mmd_flag, compile_tail)); append_rspfile(payload); - append(" description = AS $out\n\n"); + append(" description = AS $out\n"); + append_cxx_deps(); + append("\n"); } if (need_nasm_rule) { diff --git a/tests/e2e/118_purview_include_rebuild.sh b/tests/e2e/118_purview_include_rebuild.sh index efb73b9e..892d0c7f 100755 --- a/tests/e2e/118_purview_include_rebuild.sh +++ b/tests/e2e/118_purview_include_rebuild.sh @@ -1,15 +1,22 @@ #!/usr/bin/env bash -# requires: gcc -# mcpp#235: compile edges must track header/purview/GMF `#include`s via a -# GCC-only depfile (the awk filter strips GCC -fmodules' reversed rules; -# Clang/MSVC keep pre-#235 behavior — see ninja_backend posixDepfile gate). -# depfile. Before this fix, `cxx_module`/`cxx_object` had NO depfile on -# non-MSVC (only the msvcDeps branch added `deps=msvc`) — the P1689 scan's -# `$out.dep` was generated then discarded. So editing a file `#include`d -# inside a module's purview (or a plain header included by a .cpp) did NOT -# invalidate the compile edge: `mcpp run` kept printing stale output. +# mcpp#235 / mcpp#257: compile edges must track header/purview/GMF +# `#include`s via a depfile. Before #235, `cxx_module`/`cxx_object` had NO +# depfile on non-MSVC (only the msvcDeps branch added `deps=msvc`) — the +# P1689 scan's `$out.dep` was generated then discarded. So editing a file +# `#include`d inside a module's purview (or a plain header included by a +# .cpp) did NOT invalidate the compile edge: `mcpp run` kept printing stale +# output. +# +# #257: the 0.0.97 fix was scoped to GCC because GCC's `-fmodules -MMD` +# depfile needs an awk filter and Clang's shape was unverified. It turned +# out Clang emits a single plain rule with nothing to filter, so this test +# no longer declares `# requires: gcc` — the contract holds on every POSIX +# toolchain, and running it under Clang is the point. set -e +# Resolved before any cd: the script changes directory below. +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" + TMP=$(mktemp -d) trap "rm -rf $TMP" EXIT @@ -85,4 +92,83 @@ out="$("$MCPP" run 2>&1 | tail -1)" exit 1 } + +# --- #257: the same contract under Clang ----------------------------------- +# 0.0.97 shipped the depfile for GCC only, so this exact scenario silently +# served a stale BMI on every Clang platform (macOS default, Windows hosted). +# Re-run both assertions with an LLVM toolchain when one is installed. +source "$SCRIPT_DIR/_llvm_env.sh" +if [[ -d "$LLVM_ROOT" ]]; then + cd "$TMP" + "$MCPP" new purviewinc_clang > /dev/null + cd purviewinc_clang + + cat > src/vals.inc <<'EOF' +export inline int answer() { return 41; } +EOF + cat > src/m.cppm <<'EOF' +export module m; +#include "vals.inc" +EOF + cat > src/main.cpp <<'EOF' +import std; +import m; +int main() { + std::println("{}", answer()); + return 0; +} +EOF + cat > mcpp.toml <&1 | tail -1)" + [[ "$out" == "41" ]] || { echo "clang: unexpected initial output: $out"; exit 1; } + + sed -i 's/41/42/' src/vals.inc + + out="$("$MCPP" run 2>&1 | tail -1)" + [[ "$out" == "42" ]] || { + echo "FAIL (#257, clang): editing a purview #include'd file did not trigger a rebuild" + echo "got: $out (expected 42)" + exit 1 + } + + cat > src/helper.h <<'EOF' +inline int helper_val() { return 100; } +EOF + cat > src/helper_user.cpp <<'EOF' +#include "helper.h" +int use_helper() { return helper_val(); } +EOF + cat > src/main.cpp <<'EOF' +import std; +import m; +extern int use_helper(); +int main() { + std::println("{} {}", answer(), use_helper()); + return 0; +} +EOF + out="$("$MCPP" run 2>&1 | tail -1)" + [[ "$out" == "42 100" ]] || { echo "clang: unexpected output before header edit: $out"; exit 1; } + + sed -i 's/100/200/' src/helper.h + + out="$("$MCPP" run 2>&1 | tail -1)" + [[ "$out" == "42 200" ]] || { + echo "FAIL (#257, clang): editing a plain .h included by a .cpp did not trigger a rebuild" + echo "got: $out (expected 42 200)" + exit 1 + } + echo " clang leg (llvm@${LLVM_VERSION}) ok" +else + echo " clang leg skipped (no llvm payload installed)" +fi + echo "OK" diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index b11918a8..d5349d4d 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -675,3 +675,69 @@ TEST(NinjaBackend, CompileRulesStayInlineOnPosixDrivers) { } } } + +// mcpp#257: "emit a depfile" and "strip GCC's reversed module rules" are two +// decisions; 0.0.97 conflated them and left Clang with no include tracking +// at all. Clang emits a single plain make rule (measured on 20.1.7/22.1.8), +// so it takes the depfile WITHOUT the awk filter, writing -MF straight to +// $out.d. +TEST(NinjaBackend, ClangGetsDepfileWithoutTheGccModuleRuleFilter) { + if constexpr (mcpp::platform::is_windows) + GTEST_SKIP() << "POSIX depfile shape only"; + + auto plan = minimal_plan(); + plan.toolchain.compiler = mcpp::toolchain::CompilerId::Clang; + plan.toolchain.binaryPath = "/usr/bin/clang++"; + + auto ninja = emit_ninja_string(plan); + + auto module_rule_start = ninja.find("rule cxx_module"); + auto object_rule_start = ninja.find("rule cxx_object"); + ASSERT_NE(module_rule_start, std::string::npos) << ninja; + ASSERT_NE(object_rule_start, std::string::npos) << ninja; + auto module_rule = ninja.substr(module_rule_start, + object_rule_start - module_rule_start); + + EXPECT_NE(module_rule.find("-MMD -MF $out.d"), std::string::npos) << ninja; + EXPECT_NE(module_rule.find("deps = gcc"), std::string::npos) << ninja; + EXPECT_NE(module_rule.find("depfile = $out.d\n"), std::string::npos) << ninja; + // No scratch file and no filter: there is nothing to strip. + EXPECT_EQ(module_rule.find("$out.d.raw"), std::string::npos) << ninja; + EXPECT_EQ(module_rule.find("awk"), std::string::npos) << ninja; +} + +// The other half of the same asymmetry: C and GAS edges include headers too +// and had no depfile on ANY toolchain. +TEST(NinjaBackend, CAndAsmRulesAlsoTrackHeaderDeps) { + if constexpr (mcpp::platform::is_windows) + GTEST_SKIP() << "POSIX depfile shape only"; + + auto plan = minimal_plan(); + plan.compileUnits.push_back({ + .source = "src/a.c", + .object = "obj/a.o", + .packageName = "objc_rule_test", + }); + plan.compileUnits.push_back({ + .source = "src/b.S", + .object = "obj/b.o", + .packageName = "objc_rule_test", + }); + + auto ninja = emit_ninja_string(plan); + + for (std::string_view rule : {"rule c_object\n", "rule asm_object\n"}) { + auto start = ninja.find(rule); + ASSERT_NE(start, std::string::npos) << rule << "\n" << ninja; + auto end = ninja.find("\n\n", start); + ASSERT_NE(end, std::string::npos) << ninja; + auto body = ninja.substr(start, end - start); + EXPECT_NE(body.find("-MMD -MF $out.d"), std::string::npos) << body; + EXPECT_NE(body.find("deps = gcc"), std::string::npos) << body; + EXPECT_NE(body.find("depfile = $out.d"), std::string::npos) << body; + // C/GAS units never carry module reversed-rules — no filter, and so + // no scratch file to bind by mistake. + EXPECT_EQ(body.find("$out.d.raw"), std::string::npos) << body; + EXPECT_EQ(body.find("awk"), std::string::npos) << body; + } +} From 167d27a7edfeecd45968e465687efe29154b133c Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:40:47 +0800 Subject: [PATCH 05/13] =?UTF-8?q?refactor(manifest):=20extract=20BuildInpu?= =?UTF-8?q?ts=20=E2=80=94=20the=20one=20type=20any=20conditional=20axis=20?= =?UTF-8?q?may=20contribute?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcpp has two conditional axes: `[target.'cfg(...)']` (platform) and `[features.]`. Each hand-picked which build fields it could carry, and they picked DIFFERENT subsets — cfg took cflags/cxxflags/ldflags/sources, features took sources/defines/flags. "Which build inputs may be contributed conditionally" was being decided in two places, differently, which is the architectural debt the batch ledger names: the same decision derived twice drifts, and the drift only surfaces when someone tries to use the capability one axis has and the other silently lacks (#258). Extracts the additive build inputs into a type. Membership is now answered BY the type rather than by a list someone has to remember to update, and two properties qualify a field: appending is its merge semantics, and it is consumed after the conditional merge point. That excludes the selection axis (`target` SELECTS the triple its own predicate is evaluated against — conditioning it is circular) and the resolved policy scalars, which would need last-wins override semantics: a different operation and a separate design. BuildConfig INHERITS BuildInputs rather than nesting it. `buildConfig.cflags` is read in ~150 places and a BuildConfig genuinely is a set of build inputs plus selection and policy, so nesting would be 150 mechanical edits buying nothing. ConditionalConfig, by contrast, nests it — that is where the guarantee has to bite, and it costs ~20 call sites. merge_conditional_sources_flags becomes merge_conditional_build_inputs and folds through a single append(BuildInputs&, const BuildInputs&), so "how does a contribution combine with the base" has one answer for every axis: append in declaration order, giving later entries GNU last-wins precedence. Pure refactor: no behaviour change, and every existing unit assertion passes unmodified apart from the mechanical cc. -> cc.inputs. path. Design: .agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md section 5 --- src/build/prepare.cppm | 27 +++++---- src/manifest/toml.cppm | 14 +++-- src/manifest/types.cppm | 103 ++++++++++++++++++++++++++++------- src/manifest/xpkg.cppm | 13 +++-- tests/unit/test_manifest.cpp | 8 +-- 5 files changed, 114 insertions(+), 51 deletions(-) diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 55284b62..abb1eb2f 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -423,22 +423,21 @@ materialize_generated_files(const std::filesystem::path& root, // merged into the dependency map BEFORE resolution even starts, so a // dependency's own conditional deps are out of scope — see the root cfg // block that merges `cc.dependencies` etc.) -void merge_conditional_sources_flags(mcpp::manifest::Manifest& m, +void merge_conditional_build_inputs(mcpp::manifest::Manifest& m, const cfgpred::Ctx& ctx, std::string_view targetTriple) { for (auto const& cc : m.conditionalConfigs) { if (!cfgpred::matches(cc.predicate, ctx, targetTriple)) continue; - m.buildConfig.cflags.insert(m.buildConfig.cflags.end(), - cc.cflags.begin(), cc.cflags.end()); - m.buildConfig.cxxflags.insert(m.buildConfig.cxxflags.end(), - cc.cxxflags.begin(), cc.cxxflags.end()); - m.buildConfig.ldflags.insert(m.buildConfig.ldflags.end(), - cc.ldflags.begin(), cc.ldflags.end()); - for (auto const& s : cc.sources) { - m.buildConfig.sources.push_back(s); + // One append() for every field the axis may carry (#258). Matching + // sections land AFTER the base entries, so a conditional rule beats + // a broader unconditional one under GNU last-wins — which is what + // makes an off-OS REMOVAL expressible (`-U` after the base `-D`). + mcpp::manifest::append(m.buildConfig, cc.inputs); + // `modules.sources` is the scanner's own view and is not part of + // BuildInputs, so conditional sources are mirrored into it here. + for (auto const& s : cc.inputs.sources) m.modules.sources.push_back(s); - } } } @@ -898,7 +897,7 @@ prepare_build(bool print_fingerprint, // Evaluated now (target resolved) against the resolved target — the // --target triple for a cross build, else the host. // - // #229: merge_conditional_sources_flags MUST run here — before + // #229: merge_conditional_build_inputs MUST run here — before // `packages[0] = makePackageRoot(*root, *m)` snapshots `m->buildConfig` // into `packages[0].privateBuild`/`.manifest` — because that snapshot, // not `*m`, is what the modgraph scan and per-TU compile-flag assembly @@ -910,7 +909,7 @@ prepare_build(bool print_fingerprint, // merged exactly once, immediately before it is captured into `packages[]`. if (!m->conditionalConfigs.empty()) { auto cc_ctx = cfgpred::context_for(overrides.target_triple); - merge_conditional_sources_flags(*m, cc_ctx, overrides.target_triple); + merge_conditional_build_inputs(*m, cc_ctx, overrides.target_triple); for (auto const& cc : m->conditionalConfigs) { if (!cfgpred::matches(cc.predicate, cc_ctx, overrides.target_triple)) continue; @@ -1869,7 +1868,7 @@ prepare_build(bool print_fingerprint, // just keyed off a different loading branch since path/git deps never // pass through loadVersionDep. if (!manifest->conditionalConfigs.empty()) { - merge_conditional_sources_flags(*manifest, + merge_conditional_build_inputs(*manifest, cfgpred::context_for(overrides.target_triple), overrides.target_triple); } @@ -2760,7 +2759,7 @@ prepare_build(bool print_fingerprint, // BEFORE `propagateLinkFlags`/`makePackageRoot` below, which // snapshot this manifest's flags/sources into `packages[]`. if (!dep_manifest->conditionalConfigs.empty()) { - merge_conditional_sources_flags(*dep_manifest, + merge_conditional_build_inputs(*dep_manifest, cfgpred::context_for(overrides.target_triple), overrides.target_triple); } diff --git a/src/manifest/toml.cppm b/src/manifest/toml.cppm index 2fb583c1..5b0c3bee 100644 --- a/src/manifest/toml.cppm +++ b/src/manifest/toml.cppm @@ -980,10 +980,10 @@ std::expected parse_string(std::string_view content, for (auto& v : f->second.as_array()) if (v.is_string()) out.push_back(v.as_string()); }; - read_list("cflags", cc.cflags); - read_list("cxxflags", cc.cxxflags); - read_list("ldflags", cc.ldflags); - read_list("sources", cc.sources); + read_list("cflags", cc.inputs.cflags); + read_list("cxxflags", cc.inputs.cxxflags); + read_list("ldflags", cc.inputs.ldflags); + read_list("sources", cc.inputs.sources); } // [target..{dependencies,dev-dependencies,build-dependencies}] // parsed via the shared table-based loader (same selectors/namespaces @@ -998,8 +998,10 @@ std::expected parse_string(std::string_view content, if (auto r = read_deps("dependencies", cc.dependencies); !r) return std::unexpected(r.error()); if (auto r = read_deps("dev-dependencies", cc.devDependencies); !r) return std::unexpected(r.error()); if (auto r = read_deps("build-dependencies", cc.buildDependencies); !r) return std::unexpected(r.error()); - if (!cc.cflags.empty() || !cc.cxxflags.empty() || !cc.ldflags.empty() - || !cc.sources.empty() + if (!cc.inputs.cflags.empty() || !cc.inputs.cxxflags.empty() + || !cc.inputs.ldflags.empty() || !cc.inputs.sources.empty() + || !cc.inputs.globFlags.empty() || !cc.inputs.includeDirs.empty() + || !cc.inputs.includeDirsAfter.empty() || !cc.dependencies.empty() || !cc.devDependencies.empty() || !cc.buildDependencies.empty()) m.conditionalConfigs.push_back(std::move(cc)); diff --git a/src/manifest/types.cppm b/src/manifest/types.cppm index 4d912fac..2c65fe17 100644 --- a/src/manifest/types.cppm +++ b/src/manifest/types.cppm @@ -136,12 +136,73 @@ struct GlobFlags { std::string featureOrigin; }; +// The additive build inputs — the ONLY things any conditional axis may +// contribute (#258). +// +// mcpp has two conditional axes: `[target.'cfg(...)']` (platform) and +// `[features.]`. Each used to hand-pick which build fields it could +// carry, and they picked DIFFERENT subsets — cfg took cflags/cxxflags/ +// ldflags/sources, features took sources/defines/flags. "Which build inputs +// may be contributed conditionally" was being decided twice, differently, +// which is exactly the architectural debt the batch ledger warns about. +// +// Membership here is the answer, and it is a type rather than a hand-kept +// list, so it cannot drift from the struct it describes. Two properties +// qualify a field: +// +// 1. APPENDING is its merge semantics. Scalars (linkage, cStandard, the +// profile knobs) would need last-wins override semantics — a different +// operation, and a separate design. +// 2. It is consumed AFTER the conditional merge point. `target` and +// `linkage` are consumed BEFORE it — indeed `target` SELECTS the triple +// the cfg predicate is evaluated against, so conditioning it is +// circular by construction. +// +// Two deliberate non-members worth naming, because their exclusion is about +// category rather than mechanics: +// • generatedFiles is a side-effecting materialization ACTION, not an +// input (and root/dep materialize on opposite sides of the merge — see +// the design doc; that ordering bug is tracked separately). +// • featureDefines are INTERFACE contributions that propagate along Public +// edges, not private build inputs. Per-glob `defines` (GlobFlags::defines +// below) are private and per-TU, so those DO belong here. +struct BuildInputs { + std::vector sources; // glob patterns + std::vector cflags; + std::vector cxxflags; + std::vector ldflags; + std::vector globFlags; // flags = [...] (ordered) + std::vector includeDirs; // relative to package root + // #249: emitted as -idirafter (searched after the toolchain's system dirs) + std::vector includeDirsAfter; +}; + +// The single additive merge. Every conditional axis folds through this, so +// "how does a contribution combine with the base" has one answer: append, in +// declaration order, which gives later entries GNU last-wins precedence. +inline void append(BuildInputs& dst, const BuildInputs& src) { + dst.sources.insert(dst.sources.end(), src.sources.begin(), src.sources.end()); + dst.cflags.insert(dst.cflags.end(), src.cflags.begin(), src.cflags.end()); + dst.cxxflags.insert(dst.cxxflags.end(), src.cxxflags.begin(), src.cxxflags.end()); + dst.ldflags.insert(dst.ldflags.end(), src.ldflags.begin(), src.ldflags.end()); + dst.globFlags.insert(dst.globFlags.end(), + src.globFlags.begin(), src.globFlags.end()); + dst.includeDirs.insert(dst.includeDirs.end(), + src.includeDirs.begin(), src.includeDirs.end()); + dst.includeDirsAfter.insert(dst.includeDirsAfter.end(), + src.includeDirsAfter.begin(), + src.includeDirsAfter.end()); +} + // `[build]` section — tunables for the build backend. // // M5.0: now also carries `sources` (moved from [modules]) and `include_dirs` // (new). Defaults are injected by load() after parse if these are empty. -struct BuildConfig { - std::vector sources; // glob patterns +// +// Inherits the additive inputs rather than nesting them: `buildConfig.cflags` +// is read in ~150 places, and a BuildConfig genuinely IS a set of build +// inputs plus the selection axis and resolved policy scalars. +struct BuildConfig : BuildInputs { // feature name → extra source globs gated by that feature. A glob listed // here is EXCLUDED from the default build and only compiled/linked when the // feature is active for this package (resolved in prepare_build). Lets a @@ -166,11 +227,7 @@ struct BuildConfig { // on feature-off builds. Private per-TU flags — never propagate (contrast // featureDefines above, which are interface switches). std::map> featureFlags; - std::vector includeDirs; // relative to package root - // #249: emitted as -idirafter (searched after system dirs) - std::vector includeDirsAfter; std::map generatedFiles; // Form B package-owned support files - std::vector globFlags; // [build] flags = [...] (ordered) bool staticStdlib = true; // "" (default = dynamic), "static", "dynamic" — chosen at resolve // time from --static / --target / [target.].linkage. Wired @@ -181,13 +238,9 @@ struct BuildConfig { // "default to fully-static musl" belongs here, not in a toolchain name // (static output is a product property, not a compiler-family property). std::string target; - // M5.x C-language support. `cflags` / `cxxflags` are appended verbatim - // to the per-rule baseline (see `ninja_backend` cflags / cxxflags). - // `cStandard` controls -std= for the C compile rule (.c files). - // Empty cStandard → backend default ("c11" today). - std::vector cflags; - std::vector cxxflags; - std::vector ldflags; + // M5.x C-language support: `cStandard` controls -std= for the C compile + // rule (.c files); empty → backend default ("c11" today). The cflags / + // cxxflags / ldflags vectors themselves live in BuildInputs above. // Dialect-class C++ flags: flags that change what the standard library's // headers DECLARE or participate in module dialect checks (issue #210's // -freflection: libstdc++'s is gated on __cpp_impl_reflection). @@ -275,14 +328,22 @@ struct TargetEntry { // buildConfig. See .agents/docs/2026-06-29-manifest-environment-and-platform-design.md. struct ConditionalConfig { std::string predicate; // the [target.] key - std::vector cflags; - std::vector cxxflags; - std::vector ldflags; - // Conditional source globs (G1b): appended to [build].sources when the - // predicate matches the resolved target — the declarative gate for - // arch-specific code (x86 .asm on x86 targets only). `!`-exclusion - // globs work here too (the scanner handles positive+negative sets). - std::vector sources; + // Everything `[target..build]` may contribute, and nothing else. + // + // Previously four hand-listed vectors, which is why per-glob `flags` was + // inexpressible here while the xpkg descriptor's `mcpp.` sections + // supported it (#258): the conditional reader maintained its own subset + // of [build]'s keys and nobody noticed it had fallen behind. Carrying the + // BuildInputs type instead means the set cannot drift, and a key outside + // it — `linkage`, `target`, a profile knob — is simply not a member, so + // it cannot silently parse into a field nothing downstream reads. + // + // Conditional source globs (G1b) live in `inputs.sources`: appended to + // [build].sources when the predicate matches the resolved target — the + // declarative gate for arch-specific code (x86 .asm on x86 targets only). + // `!`-exclusion globs work there too (the scanner handles positive+ + // negative sets). + BuildInputs inputs; // Conditional dependencies (Phase 1b): merged into the corresponding // manifest maps in prepare_build when the predicate matches the resolved // target — before dependency resolution, so they resolve like any dep. diff --git a/src/manifest/xpkg.cppm b/src/manifest/xpkg.cppm index a5ab5665..f12ed69f 100644 --- a/src/manifest/xpkg.cppm +++ b/src/manifest/xpkg.cppm @@ -1071,10 +1071,10 @@ synthesize_from_xpkg_lua(std::string_view luaContent, auto sub = cur.read_key(); if (sub.empty()) break; std::vector* dst = - sub == "cflags" ? &cc.cflags - : sub == "cxxflags" ? &cc.cxxflags - : sub == "ldflags" ? &cc.ldflags - : sub == "sources" ? &cc.sources + sub == "cflags" ? &cc.inputs.cflags + : sub == "cxxflags" ? &cc.inputs.cxxflags + : sub == "ldflags" ? &cc.inputs.ldflags + : sub == "sources" ? &cc.inputs.sources : nullptr; if (!dst) { return std::unexpected(ManifestError{ @@ -1098,8 +1098,9 @@ synthesize_from_xpkg_lua(std::string_view luaContent, cur.skip_ws_and_comments(); } cur.consume('}'); - if (!cc.cflags.empty() || !cc.cxxflags.empty() - || !cc.ldflags.empty() || !cc.sources.empty()) + if (!cc.inputs.cflags.empty() || !cc.inputs.cxxflags.empty() + || !cc.inputs.ldflags.empty() || !cc.inputs.sources.empty() + ) m.conditionalConfigs.push_back(std::move(cc)); cur.skip_ws_and_comments(); } diff --git a/tests/unit/test_manifest.cpp b/tests/unit/test_manifest.cpp index 971d6c40..6f5bd8b9 100644 --- a/tests/unit/test_manifest.cpp +++ b/tests/unit/test_manifest.cpp @@ -2238,9 +2238,9 @@ cxxflags = ["-DHAVE_X86=1"] ASSERT_EQ(m->conditionalConfigs.size(), 1u); auto& cc = m->conditionalConfigs[0]; EXPECT_EQ(cc.predicate, "cfg(arch = \"x86_64\")"); - EXPECT_EQ(cc.sources, (std::vector{ + EXPECT_EQ(cc.inputs.sources, (std::vector{ "src/x86/**/*.asm", "!src/x86/legacy/**"})); - EXPECT_EQ(cc.cxxflags, (std::vector{"-DHAVE_X86=1"})); + EXPECT_EQ(cc.inputs.cxxflags, (std::vector{"-DHAVE_X86=1"})); } TEST(Manifest, TargetCfgParsesFromXpkgSymmetrically) { @@ -2263,10 +2263,10 @@ mcpp = { ASSERT_TRUE(mx.has_value()) << mx.error().format(); ASSERT_EQ(mx->conditionalConfigs.size(), 2u); EXPECT_EQ(mx->conditionalConfigs[0].predicate, "cfg(arch = \"x86_64\")"); - EXPECT_EQ(mx->conditionalConfigs[0].sources, + EXPECT_EQ(mx->conditionalConfigs[0].inputs.sources, (std::vector{"src/x86/**/*.asm"})); EXPECT_EQ(mx->conditionalConfigs[1].predicate, "cfg(windows)"); - EXPECT_EQ(mx->conditionalConfigs[1].ldflags, + EXPECT_EQ(mx->conditionalConfigs[1].inputs.ldflags, (std::vector{"-lws2_32"})); } From 0d746726a90ff8ea2db1e2212e0f8183f4b2058f Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:44:43 +0800 Subject: [PATCH 06/13] feat(manifest): #258 per-glob flags / include dirs in [target.'cfg(...)'.build] (xpkg target_cfg parity) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With BuildInputs in place this is mostly plumbing: a conditional section is a set of build inputs, so it reads through the same entry grammar [build].flags uses, and the merge already appends whatever BuildInputs carries. What it unblocks. `sources` was OS-conditional but `flags` was not, so one manifest covering three OSes had to present every OS with the other two OSes' flag entries, each matching zero sources by construction. The vendored-opencv port paid for that with 703 committed stub files whose only purpose was to give windows TUs OS-unique PATHS a global flag table could key on, 32 globs pointing at them, and ~23 structurally-guaranteed dead-glob warnings on every build of every OS. This is the engine's missing expressiveness leaking out as filesystem structure — the same shape as the pre-0.0.97 tu-stub machinery that #233 deleted. Ordering carries the semantics: conditional entries append AFTER the base table, so under GNU last-wins a conditional rule beats a broader unconditional one. That is what makes an off-OS REMOVAL expressible — the windows leg needs `-UHAVE_UNISTD_H` against a base `-DHAVE_UNISTD_H=1`, which no unconditional overlay can express (a `-U` counter-entry would itself need to be windows-only, recursing into the same gap). The dead-glob noise disappears structurally, not by suppression: an off-OS entry never enters buildConfig.globFlags, so globFlagHits has no counter for it and scanner.cppm cannot warn. Same shape as #253's fix on the feature axis. The `optional = true` / `?`-prefix escape hatch floated in the issue is deliberately NOT added — it would be a second suppression mechanism alongside #253's "the entry conditionally does not exist", i.e. the same decision derived twice again. xpkg's target_cfg gets the same key for grammar parity, and its unknown-key handling stays a HARD error. Sharing a parser must not be read as licence to relax a grammar that was already closed; making the unknown-key policy consistent across all sections is #263, deliberately not bundled here since it has a compatibility surface and #258 does not depend on it. e2e 149 asserts the three costs are paid off: matching-OS entries reach the TU, they override the base table, and off-OS entries produce no warning — with a control proving a genuinely dead glob in [build].flags still does. --- src/manifest/toml.cppm | 19 ++++ src/manifest/xpkg.cppm | 40 ++++++++- tests/e2e/149_conditional_glob_flags.sh | 111 ++++++++++++++++++++++++ tests/unit/test_manifest.cpp | 111 ++++++++++++++++++++++++ 4 files changed, 277 insertions(+), 4 deletions(-) create mode 100755 tests/e2e/149_conditional_glob_flags.sh diff --git a/src/manifest/toml.cppm b/src/manifest/toml.cppm index 5b0c3bee..5bed64c1 100644 --- a/src/manifest/toml.cppm +++ b/src/manifest/toml.cppm @@ -150,6 +150,7 @@ std::expected parse_string(std::string_view content, static constexpr std::string_view kAllowedArraysOfTables[] = { "build.flags", "features.*.flags", // #253 — the middle segment is the feature name + "target.*.build.flags", // #258 — middle segment is the cfg predicate }; if (auto badPath = find_disallowed_array_of_tables(doc->root(), "", kAllowedArraysOfTables)) { return std::unexpected(error(origin, std::format( @@ -980,10 +981,28 @@ std::expected parse_string(std::string_view content, for (auto& v : f->second.as_array()) if (v.is_string()) out.push_back(v.as_string()); }; + auto read_paths = [&](const char* key, + std::vector& out) { + if (auto f = bt.find(key); f != bt.end() && f->second.is_array()) + for (auto& v : f->second.as_array()) + if (v.is_string()) out.emplace_back(v.as_string()); + }; read_list("cflags", cc.inputs.cflags); read_list("cxxflags", cc.inputs.cxxflags); read_list("ldflags", cc.inputs.ldflags); read_list("sources", cc.inputs.sources); + // #258: per-glob flags and include dirs, through the SAME entry + // grammar `[build].flags` uses — a conditional section is just + // a set of build inputs, so it reads the same way. + read_paths("include_dirs", cc.inputs.includeDirs); + read_paths("include_dirs_after", cc.inputs.includeDirsAfter); + if (auto f = bt.find("flags"); f != bt.end()) { + if (auto err = parse_glob_flags_value( + f->second, + std::format("[target.{}.build].flags", triple), + cc.inputs.globFlags)) + return std::unexpected(error(origin, *err)); + } } // [target..{dependencies,dev-dependencies,build-dependencies}] // parsed via the shared table-based loader (same selectors/namespaces diff --git a/src/manifest/xpkg.cppm b/src/manifest/xpkg.cppm index f12ed69f..3ff768b3 100644 --- a/src/manifest/xpkg.cppm +++ b/src/manifest/xpkg.cppm @@ -1070,16 +1070,43 @@ synthesize_from_xpkg_lua(std::string_view luaContent, while (!cur.eof() && cur.peek() != '}') { auto sub = cur.read_key(); if (sub.empty()) break; + // #258: per-glob flags reach conditional sections through + // the same entry grammar `flags` uses at the top level. + if (sub == "flags") { + if (!cur.consume('=')) { + return std::unexpected(ManifestError{ + std::format("expected `=` after target_cfg '{}'.flags", + pred), + m.sourcePath, 0, 0}); + } + if (auto err = parse_glob_flags_array( + cur, m.sourcePath, + std::format("target_cfg.{}.flags", pred), + cc.inputs.globFlags)) { + return std::unexpected(std::move(*err)); + } + cur.skip_ws_and_comments(); + continue; + } std::vector* dst = sub == "cflags" ? &cc.inputs.cflags : sub == "cxxflags" ? &cc.inputs.cxxflags : sub == "ldflags" ? &cc.inputs.ldflags : sub == "sources" ? &cc.inputs.sources : nullptr; - if (!dst) { + // Unknown sub-keys stay a HARD ERROR here. The shared + // BuildInputs parser must not be read as licence to relax + // a grammar that was already closed (mcpp.toml's own + // unknown-key policy is a separate question — #263). + std::vector* pathDst = + sub == "include_dirs" ? &cc.inputs.includeDirs + : sub == "include_dirs_after" ? &cc.inputs.includeDirsAfter + : nullptr; + if (!dst && !pathDst) { return std::unexpected(ManifestError{ std::format("unknown target_cfg key '{}' (expected " - "cflags/cxxflags/ldflags/sources)", sub), + "cflags/cxxflags/ldflags/sources/flags/" + "include_dirs/include_dirs_after)", sub), m.sourcePath, 0, 0}); } if (!cur.consume('=') || !cur.consume('{')) { @@ -1091,7 +1118,10 @@ synthesize_from_xpkg_lua(std::string_view luaContent, cur.skip_ws_and_comments(); while (!cur.eof() && cur.peek() != '}') { auto s = cur.read_string(); - if (!s.empty()) dst->push_back(std::move(s)); + if (!s.empty()) { + if (dst) dst->push_back(std::move(s)); + else pathDst->emplace_back(std::move(s)); + } cur.skip_ws_and_comments(); } cur.consume('}'); @@ -1100,7 +1130,9 @@ synthesize_from_xpkg_lua(std::string_view luaContent, cur.consume('}'); if (!cc.inputs.cflags.empty() || !cc.inputs.cxxflags.empty() || !cc.inputs.ldflags.empty() || !cc.inputs.sources.empty() - ) + || !cc.inputs.globFlags.empty() + || !cc.inputs.includeDirs.empty() + || !cc.inputs.includeDirsAfter.empty()) m.conditionalConfigs.push_back(std::move(cc)); cur.skip_ws_and_comments(); } diff --git a/tests/e2e/149_conditional_glob_flags.sh b/tests/e2e/149_conditional_glob_flags.sh new file mode 100755 index 00000000..d931e1d5 --- /dev/null +++ b/tests/e2e/149_conditional_glob_flags.sh @@ -0,0 +1,111 @@ +#!/usr/bin/env bash +# #258: per-glob `flags` inside `[target.'cfg()'.build]`. +# +# What this unblocks, in the reporter's words: `sources` is OS-conditional +# but `flags` was not, so one manifest covering three OSes had to present +# every OS with the other two OSes' flag entries — each matching zero sources +# by construction. The vendored-opencv port worked around it with 703 +# committed stub files whose only purpose was to give windows TUs OS-unique +# PATHS that a global flag table could key on, plus 32 globs pointing at +# them, plus ~23 structurally-guaranteed dead-glob warnings on every build. +# +# Three assertions, matching the three costs: +# 1. a matching-OS conditional flag entry reaches the TU; +# 2. it can OVERRIDE the unconditional base table (the removal case: +# unix defines HAVE_UNISTD_H, windows must not); +# 3. an off-OS entry produces NO dead-glob warning — the entry simply does +# not exist on this OS, the same way #253 made a feature-off entry not +# exist. No suppression mechanism, no `optional = true` escape hatch. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT + +cd "$TMP" +"$MCPP" new condflags > /dev/null +cd condflags + +case "$(uname -s)" in + Linux) THIS_OS=linux; OTHER_OS=windows ;; + Darwin) THIS_OS=macos; OTHER_OS=windows ;; + *) THIS_OS=windows; OTHER_OS=linux ;; +esac + +mkdir -p src/zlib + +# Probes the three-way outcome: the base define must be overridden by the +# conditional entry that lands after it, and the conditional-only define +# must be present. +cat > src/zlib/probe.cpp <<'EOF' +#ifndef BASE_DEFINE +#error "base [build].flags entry did not reach the TU" +#endif +#ifndef COND_DEFINE +#error "conditional [target.cfg(os)] flags entry did not reach the TU" +#endif +#ifdef REMOVED_DEFINE +#error "conditional entry failed to override the base table (removal case)" +#endif +extern "C" int probe_ok() { return 1; } +EOF + +cat > src/main.cpp <<'EOF' +import std; +extern "C" int probe_ok(); +int main() { std::println("probe = {}", probe_ok()); return 0; } +EOF + +# The base table defines REMOVED_DEFINE for everyone; the matching-OS +# conditional entry appends -U for it (last flag wins) and adds its own. +# The off-OS entry points at a glob that matches nothing on this host and +# must stay silent. +cat > mcpp.toml < build.log 2>&1 || { cat build.log; echo "conditional glob flags build failed"; exit 1; } + +out=$("$MCPP" run 2>&1 | tail -1) +[[ "$out" == "probe = 1" ]] || { echo "unexpected output: $out"; cat build.log; exit 1; } + +# Assertion 3 — the whole point of the "23 warnings" half of the report. +if grep -q "matched no source file" build.log; then + cat build.log + echo "FAIL: an off-OS conditional flags entry produced a dead-glob warning;" + echo " off-OS entries must not exist on this OS at all" + exit 1 +fi + +# Control: a genuinely dead glob in the UNCONDITIONAL table must still warn. +# The fix removes structural noise, not real signal. +cat >> mcpp.toml <<'EOF' + +[[build.flags]] +glob = "src/nonexistent_dir/**" +defines = ["NEVER=1"] +EOF +"$MCPP" build > build2.log 2>&1 || { cat build2.log; exit 1; } +grep -q "matched no source file" build2.log || { + cat build2.log + echo "FAIL: a real dead glob in [build].flags must still warn" + exit 1 +} + +echo "OK" diff --git a/tests/unit/test_manifest.cpp b/tests/unit/test_manifest.cpp index 6f5bd8b9..950c2515 100644 --- a/tests/unit/test_manifest.cpp +++ b/tests/unit/test_manifest.cpp @@ -2400,3 +2400,114 @@ linux = "gcc@15.1.0" EXPECT_NE(m.error().message.find("array-of-tables"), std::string::npos) << m.error().message; } + +// #258: `[target.'cfg()'.build]` accepts the same per-glob `flags` array +// `[build]` does. Before this, the conditional reader knew exactly four keys +// and dropped everything else silently, so a per-OS flag table was +// inexpressible in mcpp.toml while the xpkg descriptor's mcpp. sections +// supported it — manifest-built packages were second-class. +TEST(Manifest, ConditionalSectionCarriesPerGlobFlags) { + auto tmp = std::filesystem::temp_directory_path() / "mcpp_cond_flags"; + std::filesystem::create_directories(tmp); + auto path = tmp / "mcpp.toml"; + { + std::ofstream os(path); + os << R"( +[package] +name = "condflags" +version = "0.1.0" + +[build] +flags = [{ glob = "src/zlib/**", defines = ["HAVE_UNISTD_H=1"] }] + +[target.'cfg(windows)'.build] +cflags = ["-DWIN32"] +include_dirs = ["vendor/win/include"] +flags = [ + { glob = "src/zlib/**", defines = ["NO_FSEEKO"], cflags = ["-UHAVE_UNISTD_H"] }, +] +)"; + } + auto m = mcpp::manifest::load(path); + ASSERT_TRUE(m) << (m ? "" : m.error().message); + + // Base table is unconditional. + ASSERT_EQ(m->buildConfig.globFlags.size(), 1u); + EXPECT_EQ(m->buildConfig.globFlags[0].glob, "src/zlib/**"); + + ASSERT_EQ(m->conditionalConfigs.size(), 1u); + auto& cc = m->conditionalConfigs[0]; + EXPECT_EQ(cc.predicate, "cfg(windows)"); + ASSERT_EQ(cc.inputs.globFlags.size(), 1u); + EXPECT_EQ(cc.inputs.globFlags[0].glob, "src/zlib/**"); + EXPECT_EQ(cc.inputs.globFlags[0].defines, + (std::vector{"NO_FSEEKO"})); + // The removal case: a windows-only -U that must land AFTER the base -D. + EXPECT_EQ(cc.inputs.globFlags[0].cflags, + (std::vector{"-UHAVE_UNISTD_H"})); + ASSERT_EQ(cc.inputs.includeDirs.size(), 1u); + EXPECT_EQ(cc.inputs.includeDirs[0], std::filesystem::path("vendor/win/include")); + std::filesystem::remove_all(tmp); +} + +// The array-of-tables spelling must reach the same place, and must be +// allowlisted by the #227 closed-grammar guard. +TEST(Manifest, ConditionalPerGlobFlagsAcceptArrayOfTablesSpelling) { + auto tmp = std::filesystem::temp_directory_path() / "mcpp_cond_flags_aot"; + std::filesystem::create_directories(tmp); + auto path = tmp / "mcpp.toml"; + { + std::ofstream os(path); + os << R"( +[package] +name = "condflagsaot" +version = "0.1.0" + +[[target.'cfg(linux)'.build.flags]] +glob = "src/**" +defines = ["ON_LINUX=1"] +)"; + } + auto m = mcpp::manifest::load(path); + ASSERT_TRUE(m) << (m ? "" : m.error().message); + ASSERT_EQ(m->conditionalConfigs.size(), 1u); + ASSERT_EQ(m->conditionalConfigs[0].inputs.globFlags.size(), 1u); + EXPECT_EQ(m->conditionalConfigs[0].inputs.globFlags[0].defines, + (std::vector{"ON_LINUX=1"})); + std::filesystem::remove_all(tmp); +} + +// Parity: the xpkg descriptor's target_cfg gets the same key, and its +// unknown-key policy stays a HARD ERROR. Sharing a parser must not be read +// as licence to relax a grammar that was already closed (#263 tracks making +// the policies consistent across sections; this test pins that #258 did not +// quietly change one). +TEST(Manifest, XpkgTargetCfgCarriesPerGlobFlagsAndStillRejectsUnknownKeys) { + // Custom delimiter: the descriptor contains `)"` inside ["cfg(windows)"], + // which would terminate a plain R"( ... )". + const char* lua = R"LUA( +mcpp = { + sources = { "src/**" }, + target_cfg = { + ["cfg(windows)"] = { + cflags = { "-DWIN32" }, + flags = { { glob = "src/z/**", defines = { "NO_FSEEKO" } } }, + }, + }, +} +)LUA"; + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "p", "1.0.0"); + ASSERT_TRUE(m) << (m ? "" : m.error().message); + ASSERT_EQ(m->conditionalConfigs.size(), 1u); + ASSERT_EQ(m->conditionalConfigs[0].inputs.globFlags.size(), 1u); + EXPECT_EQ(m->conditionalConfigs[0].inputs.globFlags[0].glob, "src/z/**"); + + const char* bad = R"LUA( +mcpp = { + sources = { "src/**" }, + target_cfg = { ["cfg(windows)"] = { linkage = { "static" } } }, +} +)LUA"; + auto bm = mcpp::manifest::synthesize_from_xpkg_lua(bad, "p", "1.0.0"); + EXPECT_FALSE(bm) << "unknown target_cfg key must stay a hard error"; +} From f886901f72a9781eaca75001ce013979148e7483 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:51:32 +0800 Subject: [PATCH 07/13] fix(platform): #254 host and target platform axes become distinct types; xpkg per-OS sections key on the target MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcpp resolves "which platform?" along two axes that coincide only for a native build: the host (where mcpp runs — correct for toolchain payloads and other host tools) and the target (where the produced binaries run — correct for anything compiled INTO the user's build). Both were std::string_view, so mixing them cost nothing and read identically, and they had drifted apart: every xpkg per-OS splice and every xpm version/asset table keyed on `mcpp::platform::xpkg_platform`, a COMPILE-TIME HOST constant, while `[target.'cfg(...)']` was evaluated against the resolved target. The same decision, derived two different ways. Cross-compiling therefore spliced the host's section into a dependency being built for the target: its per-OS sources, flags, deps and prebuilt asset all came from the wrong leg. Native builds cannot observe this, which is why three-platform CI never caught it. Introduces mcpp.platform.axis: HostPlatform and TargetPlatform, with no conversion between them and no constructor from a bare string. APIs take the common PlatformKey base, so a function states which axis it accepts and a call site must NAME the axis it supplies. `synthesize_from_xpkg_lua`'s platform parameter loses its host default and becomes required — a silent default is precisely how this bug survived. The triple-vs-xpkg vocabulary translation ("macos" vs "macosx") moves into TargetPlatform::for_os so no call site has to remember it. The type change and the behaviour fix land together because they are the same edit: once the parameter is required, every call site must decide, and those decisions ARE the fix. Classified per site: prepare.cppm dep manifest synthesis (3), pm/resolver version+asset selection -> TARGET; these are code compiled into the user's build. toolchain/lifecycle version listing -> HOST; a toolchain payload runs on this machine. Its existing comment already said so. cmd_xpkg --all-os -> for_lint_of(); neither axis, and named awkwardly on purpose so that reads as deliberate at the call site. scaffold `mcpp add` and the legacy resolver overloads -> HOST, stated explicitly rather than inherited from a default. Tests: static_asserts that the axes are mutually inconvertible and neither is constructible from a string; per-OS splice selects each target's own section and none of the others; and a native-build guard asserting host == target produces an identical splice, which is the safety net for every existing platform's behaviour. --- src/build/prepare.cppm | 21 +++-- src/cli/cmd_xpkg.cppm | 12 ++- src/manifest/xpkg.cppm | 31 ++++--- src/platform/axis.cppm | 89 +++++++++++++++++++ src/pm/resolver.cppm | 28 ++++-- src/scaffold/create.cppm | 6 +- src/toolchain/lifecycle.cppm | 18 ++-- tests/unit/test_manifest.cpp | 162 +++++++++++++++++++++++++++-------- 8 files changed, 293 insertions(+), 74 deletions(-) create mode 100644 src/platform/axis.cppm diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index abb1eb2f..4af35b3f 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -11,6 +11,7 @@ export module mcpp.build.prepare; import std; import mcpp.diag; +import mcpp.platform.axis; import mcpp.libs.json; import mcpp.log; import mcpp.manifest; @@ -595,6 +596,15 @@ prepare_build(bool print_fingerprint, return std::unexpected("no mcpp.toml found in current directory or any parent"); } + // #254: everything compiled INTO this build is resolved for the TARGET — + // an xpkg descriptor's per-OS sections (sources, flags, deps) and its xpm + // asset/version table all describe code that will run on the target, not + // on the machine building it. This used to be a compile-time host + // constant, which is invisible natively (host == target) and picks the + // wrong leg under --target. + const auto targetPlatform = mcpp::platform::TargetPlatform::for_os( + cfgpred::context_for(overrides.target_triple).os); + auto m = mcpp::manifest::load(*root / "mcpp.toml"); if (!m) return std::unexpected(m.error().format()); @@ -1395,7 +1405,7 @@ prepare_build(bool print_fingerprint, // 0.0.10+: use structured namespace from DependencySpec. auto resolved = mcpp::pm::resolve_semver( s.namespace_, s.shortName.empty() ? depName : s.shortName, - s.version, fetcher); + s.version, fetcher, targetPlatform); if (!resolved) return std::unexpected(resolved.error()); mcpp::ui::info("Resolved", std::format("{} {} → v{}", depName, s.version, *resolved)); @@ -1587,7 +1597,7 @@ prepare_build(bool print_fingerprint, } if (field.kind == mcpp::manifest::McppField::TableBody) { auto dm = mcpp::manifest::synthesize_from_xpkg_lua( - *luaContent, depName, version); + *luaContent, depName, version, targetPlatform); if (!dm) return false; for (auto const& [generatedPath, _] : dm->buildConfig.generatedFiles) { if (!generatedPath.empty()) return true; @@ -1635,7 +1645,7 @@ prepare_build(bool print_fingerprint, auto field = mcpp::manifest::extract_mcpp_field(*luaContent); if (field.kind == mcpp::manifest::McppField::TableBody) { auto depManifest = mcpp::manifest::synthesize_from_xpkg_lua( - *luaContent, depName, version); + *luaContent, depName, version, targetPlatform); if (!depManifest) { return std::unexpected(std::format( "dependency '{}': {}", depName, depManifest.error().format())); @@ -1811,7 +1821,8 @@ prepare_build(bool print_fingerprint, "(expected exactly one)", depName, field.value, matches.size())); if (auto r = loadFrom(matches.front()); !r) return std::unexpected(r.error()); } else if (field.kind == mcpp::manifest::McppField::TableBody) { - auto dm = mcpp::manifest::synthesize_from_xpkg_lua(*luaContent, depName, version); + auto dm = mcpp::manifest::synthesize_from_xpkg_lua( + *luaContent, depName, version, targetPlatform); if (!dm) return std::unexpected(std::format( "dependency '{}': {}", depName, dm.error().format())); warn_unknown_xpkg_keys(*dm, depName); @@ -2396,7 +2407,7 @@ prepare_build(bool print_fingerprint, key.ns, key.shortName, it->second.constraint, item.originalConstraint, - fetcher); + fetcher, targetPlatform); if (!merged) { // Level 1 fallback: multi-version mangling. Two // versions can't be reconciled by SemVer, but they diff --git a/src/cli/cmd_xpkg.cppm b/src/cli/cmd_xpkg.cppm index aef896e9..33870873 100644 --- a/src/cli/cmd_xpkg.cppm +++ b/src/cli/cmd_xpkg.cppm @@ -18,6 +18,7 @@ export module mcpp.cli.cmd_xpkg; import std; import mcpplibs.cmdline; import mcpp.manifest; +import mcpp.platform.axis; import mcpp.ui; namespace mcpp::cli { @@ -90,7 +91,8 @@ export int cmd_xpkg_parse(const mcpplibs::cmdline::ParsedArgs& parsed) { std::map> versions; std::string anyVersion; for (auto plat : kPlatforms) { - auto v = mcpp::manifest::list_xpkg_versions(lua, plat); + auto v = mcpp::manifest::list_xpkg_versions( + lua, mcpp::platform::TargetPlatform::for_lint_of(plat)); if (!v.empty() && anyVersion.empty()) anyVersion = v.front(); versions.emplace(std::string(plat), std::move(v)); } @@ -130,7 +132,8 @@ export int cmd_xpkg_parse(const mcpplibs::cmdline::ParsedArgs& parsed) { continue; } auto pm = mcpp::manifest::synthesize_from_xpkg_lua( - lua, fqn, anyVersion, plat); + lua, fqn, anyVersion, + mcpp::platform::TargetPlatform::for_lint_of(plat)); if (!pm) { mcpp::ui::error(std::format("{} [{}]: {}", file, plat, pm.error().format())); @@ -153,7 +156,10 @@ export int cmd_xpkg_parse(const mcpplibs::cmdline::ParsedArgs& parsed) { } // The parse users get at build time — same function, same grammar. - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, fqn, anyVersion); + // Host axis here: `mcpp xpkg parse` with no --all-os is inspecting what + // THIS machine would read, which is a diagnostic question about the host. + auto m = mcpp::manifest::synthesize_from_xpkg_lua( + lua, fqn, anyVersion, mcpp::platform::HostPlatform::current()); if (!m) { mcpp::ui::error(std::format("{}: {}", file, m.error().format())); return 1; diff --git a/src/manifest/xpkg.cppm b/src/manifest/xpkg.cppm index 3ff768b3..47492d3b 100644 --- a/src/manifest/xpkg.cppm +++ b/src/manifest/xpkg.cppm @@ -8,6 +8,7 @@ import std; import mcpp.pm.dep_spec; import mcpp.pm.dependency_selector; import mcpp.platform; +import mcpp.platform.axis; export namespace mcpp::manifest { @@ -24,7 +25,8 @@ McppField extract_mcpp_field(std::string_view luaContent); // "windows") from an xpkg .lua's xpm. = { ["X.Y.Z"] = {...}, ... }. // Returns an empty vector if the platform table is missing or has no entries. std::vector -list_xpkg_versions(std::string_view luaContent, std::string_view platform); +list_xpkg_versions(std::string_view luaContent, + const mcpp::platform::PlatformKey& platform); // Extract the `namespace` field from an xpkg .lua's `package = { ... }` block. // Returns empty string if the field is absent (legacy descriptors). std::string extract_xpkg_namespace(std::string_view luaContent); @@ -92,16 +94,22 @@ bool xpkg_lua_identity_matches(std::string_view luaContent, // The resulting Manifest is in-memory only; sourcePath is set to the // supplied package name + version so error messages can refer to it. // -// `osOverride` selects which per-OS section (linux/macosx/windows) is -// spliced into the parse instead of the running host's — the seam behind -// `mcpp xpkg parse --all-os`, which validates the sections a host build -// never reads (they are skip-tabled, so a typo in the `windows` block is -// otherwise invisible on linux CI). Empty = host platform (build path). +// `platform` selects which per-OS section (linux/macosx/windows) is spliced +// into the parse. It is REQUIRED and axis-typed (#254): the per-OS sections +// carry sources, flags, deps and the xpm asset table, so a package compiled +// into the user's build must be spliced for the TARGET, while a host tool +// wants the HOST. Before #254 this defaulted to a compile-time host +// constant, which silently gave cross builds the wrong leg. +// +// TargetPlatform::for_lint_of() is the seam behind `mcpp xpkg parse +// --all-os`, which validates the sections a build never reads (they are +// skip-tabled, so a typo in the `windows` block is otherwise invisible on +// linux CI). std::expected synthesize_from_xpkg_lua(std::string_view luaContent, std::string_view packageName, std::string_view packageVersion, - std::string_view osOverride = {}); + const mcpp::platform::PlatformKey& platform); // mcpp#237: the mcpp-segment key vocabulary is a CLOSED whitelist (the parse // loop's else-if chain). An unrecognised key is collected into @@ -708,7 +716,9 @@ bool xpkg_lua_identity_matches(std::string_view luaContent, } std::vector -list_xpkg_versions(std::string_view luaContent, std::string_view platform) { +list_xpkg_versions(std::string_view luaContent, + const mcpp::platform::PlatformKey& platformAxis) { + const std::string_view platform = platformAxis.key(); // Locate `xpm = { ... = { ["X.Y.Z"] = {...}, ... } ... }`. // We work on a sanitized copy so quoted version keys remain locatable // by their offsets in the original text. @@ -873,7 +883,7 @@ std::expected synthesize_from_xpkg_lua(std::string_view luaContent, std::string_view packageName, std::string_view packageVersion, - std::string_view osOverride) + const mcpp::platform::PlatformKey& platform) { auto body = extract_mcpp_segment_body(luaContent); if (body.empty()) { @@ -885,8 +895,7 @@ synthesize_from_xpkg_lua(std::string_view luaContent, std::format("xpkg-lua of {}@{}", packageName, packageVersion), 0, 0}); } - const std::string_view osKey = - osOverride.empty() ? mcpp::platform::xpkg_platform : osOverride; + const std::string_view osKey = platform.key(); if (auto platformBody = top_level_table_body_for_key(body, osKey); !platformBody.empty()) { body += "\n"; diff --git a/src/platform/axis.cppm b/src/platform/axis.cppm new file mode 100644 index 00000000..47323610 --- /dev/null +++ b/src/platform/axis.cppm @@ -0,0 +1,89 @@ +// mcpp.platform.axis — the host/target platform axes, as distinct types. +// +// mcpp resolves "which platform?" along TWO axes that are equal only for a +// native build: +// +// host — the machine mcpp is running on. Correct for host TOOLS: the +// toolchain payload, ninja, patchelf. +// target — the platform the produced binaries will run on. Correct for +// anything compiled INTO the user's build: a dependency's +// sources, flags, deps, and the prebuilt asset chosen for it. +// +// Both are spelled in the xpkg descriptor vocabulary (linux | macosx | +// windows) and both were plain std::string_view, so mixing them cost +// nothing and read identically. #254: every xpkg per-OS section and every +// xpm version table was keyed on a compile-time HOST constant, while +// `[target.'cfg(...)']` was evaluated against the RESOLVED TARGET — the same +// decision derived two different ways. Cross-compiling therefore spliced the +// host's section into a dependency built for the target. Native builds hide +// it, because there the two axes coincide, which is why three-platform CI +// never caught it. +// +// Making them separate types means an API states which axis it wants, and a +// call site must name the axis it is supplying. There is deliberately no +// conversion between them and no constructor from a bare string: the +// interesting bugs here are all "the wrong axis was passed", and a string +// parameter cannot express the difference. + +export module mcpp.platform.axis; + +import std; +import mcpp.platform; + +export namespace mcpp::platform { + +// A resolved xpkg platform key. Only obtainable through one of the axis +// types below, so possession of one implies somebody decided which axis it +// came from. +class PlatformKey { +public: + std::string_view key() const { return key_; } + + bool operator==(const PlatformKey& other) const { return key_ == other.key_; } + +protected: + explicit PlatformKey(std::string key) : key_(std::move(key)) {} + +private: + std::string key_; +}; + +// The machine mcpp itself is running on. +class HostPlatform : public PlatformKey { +public: + static HostPlatform current() { + return HostPlatform(std::string(mcpp::platform::xpkg_platform)); + } + +private: + explicit HostPlatform(std::string key) : PlatformKey(std::move(key)) {} +}; + +// The platform the artifacts being produced will run on. +class TargetPlatform : public PlatformKey { +public: + // From a resolved triple's `os` token. The triple vocabulary says + // "macos" where xpkg descriptors say "macosx"; that translation lives + // here so no caller has to remember it. + static TargetPlatform for_os(std::string_view tripleOs) { + if (tripleOs == "macos" || tripleOs == "macosx") return TargetPlatform("macosx"); + if (tripleOs == "windows") return TargetPlatform("windows"); + if (tripleOs == "linux") return TargetPlatform("linux"); + // Unknown/absent os token: fall back to the host, which is what the + // whole code base did unconditionally before #254. + return TargetPlatform(std::string(mcpp::platform::xpkg_platform)); + } + + // For tooling that walks platforms it is not running on and is not + // building for — `mcpp xpkg parse --all-os` lints every per-OS section. + // Named awkwardly on purpose: it should be obvious at the call site that + // this is neither axis. + static TargetPlatform for_lint_of(std::string_view xpkgKey) { + return TargetPlatform(std::string(xpkgKey)); + } + +private: + explicit TargetPlatform(std::string key) : PlatformKey(std::move(key)) {} +}; + +} // namespace mcpp::platform diff --git a/src/pm/resolver.cppm b/src/pm/resolver.cppm index 6fbe466f..fcc89f03 100644 --- a/src/pm/resolver.cppm +++ b/src/pm/resolver.cppm @@ -18,6 +18,7 @@ export module mcpp.pm.resolver; import std; import mcpp.manifest; import mcpp.platform; +import mcpp.platform.axis; import mcpp.pm.compat; import mcpp.pm.package_fetcher; import mcpp.version_req; @@ -44,7 +45,8 @@ bool is_version_constraint(std::string_view v); std::expected resolve_semver(std::string_view ns, std::string_view shortName, std::string_view constraint, - mcpp::pm::Fetcher& fetcher); + mcpp::pm::Fetcher& fetcher, + const mcpp::platform::PlatformKey& platform); // Try to AND-merge two version constraints and resolve to a single // concrete version satisfying both. Uses structured (ns, shortName). @@ -52,7 +54,8 @@ std::expected try_merge_semver(std::string_view ns, std::string_view shortName, std::string_view a, std::string_view b, - mcpp::pm::Fetcher& fetcher); + mcpp::pm::Fetcher& fetcher, + const mcpp::platform::PlatformKey& platform); // ─── Legacy overloads (COMPAT, remove in 1.0.0) ───────────────────── @@ -85,7 +88,8 @@ bool is_version_constraint(std::string_view v) { std::expected resolve_semver(std::string_view ns, std::string_view shortName, std::string_view constraint, - mcpp::pm::Fetcher& fetcher) + mcpp::pm::Fetcher& fetcher, + const mcpp::platform::PlatformKey& platform) { namespace vr = mcpp::version_req; auto qname = mcpp::pm::compat::qualified_name(ns, shortName); @@ -105,11 +109,11 @@ resolve_semver(std::string_view ns, std::string_view shortName, qname, constraint, req.error())); } - auto rawVersions = mcpp::manifest::list_xpkg_versions(*luaContent, kXpkgPlatform); + auto rawVersions = mcpp::manifest::list_xpkg_versions(*luaContent, platform); if (rawVersions.empty()) { return std::unexpected(std::format( "dependency '{}': index entry has no versions for platform '{}'", - qname, kXpkgPlatform)); + qname, platform.key())); } std::vector parsed; @@ -141,7 +145,8 @@ std::expected try_merge_semver(std::string_view ns, std::string_view shortName, std::string_view a, std::string_view b, - mcpp::pm::Fetcher& fetcher) + mcpp::pm::Fetcher& fetcher, + const mcpp::platform::PlatformKey& platform) { auto canon = [](std::string_view v) -> std::string { if (v.empty() || v == "*") return std::string{}; @@ -157,7 +162,7 @@ try_merge_semver(std::string_view ns, std::string_view shortName, else if (!cb.empty()) merged = cb; else merged = "*"; - return resolve_semver(ns, shortName, merged, fetcher); + return resolve_semver(ns, shortName, merged, fetcher, platform); } // ─── Legacy overloads (COMPAT, remove in 1.0.0) ───────────────────── @@ -168,8 +173,11 @@ resolve_semver(std::string_view name, mcpp::pm::Fetcher& fetcher) { auto resolved = mcpp::pm::compat::resolve_package_name(name, ""); + // Legacy overload: no target is threaded through it, so it names the host + // axis explicitly rather than inheriting a silent default (#254). return resolve_semver(resolved.namespace_, resolved.shortName, - constraint, fetcher); + constraint, fetcher, + mcpp::platform::HostPlatform::current()); } std::expected @@ -179,8 +187,10 @@ try_merge_semver(std::string_view name, mcpp::pm::Fetcher& fetcher) { auto resolved = mcpp::pm::compat::resolve_package_name(name, ""); + // Legacy overload — see the resolve_semver note above. return try_merge_semver(resolved.namespace_, resolved.shortName, - a, b, fetcher); + a, b, fetcher, + mcpp::platform::HostPlatform::current()); } } // namespace mcpp::pm diff --git a/src/scaffold/create.cppm b/src/scaffold/create.cppm index 355bd201..f267d42a 100644 --- a/src/scaffold/create.cppm +++ b/src/scaffold/create.cppm @@ -14,6 +14,7 @@ import mcpp.fetcher; import mcpp.fetcher.progress; import mcpp.manifest; import mcpp.pm.compat; +import mcpp.platform.axis; import mcpp.pm.resolver; import mcpp.scaffold; import mcpp.ui; @@ -67,7 +68,10 @@ fetch_template_package(const mcpp::scaffold::TemplateSpec& spec) { std::string version = spec.version; if (version.empty()) { - auto v = mcpp::pm::resolve_semver(ns, shortName, "*", fetcher); + // `mcpp add` has no target concept — the axis it means is the host, + // named explicitly rather than inherited from a default (#254). + auto v = mcpp::pm::resolve_semver(ns, shortName, "*", fetcher, + mcpp::platform::HostPlatform::current()); if (!v) return std::unexpected(v.error()); version = *v; } diff --git a/src/toolchain/lifecycle.cppm b/src/toolchain/lifecycle.cppm index 6ea5ae1e..b9992953 100644 --- a/src/toolchain/lifecycle.cppm +++ b/src/toolchain/lifecycle.cppm @@ -14,6 +14,7 @@ import mcpp.fetcher; import mcpp.fetcher.progress; import mcpp.manifest; import mcpp.platform; +import mcpp.platform.axis; import mcpp.toolchain.detect; import mcpp.toolchain.msvc; import mcpp.toolchain.registry; @@ -124,15 +125,14 @@ list_available_xpkg_versions(const mcpp::config::GlobalConfig& cfg, if (!std::filesystem::exists(p, ec)) return std::nullopt; std::ifstream is(p); std::string body((std::istreambuf_iterator(is)), {}); - // xpkg descriptors key platforms as linux / macosx / windows — list - // the HOST's block (previously hardcoded "linux", which made the - // Available section and partial-version resolution empty on - // Windows/macOS for any package, e.g. mingw-gcc has no linux block). - constexpr std::string_view xpkgPlatform = - mcpp::platform::is_windows ? "windows" - : mcpp::platform::is_macos ? "macosx" - : "linux"; - return mcpp::manifest::list_xpkg_versions(body, xpkgPlatform); + // The HOST axis, deliberately (#254): a toolchain payload runs on + // this machine, so its version/asset table is the host's block — + // unlike a library dependency, which is resolved for the target. + // (Previously hardcoded "linux", which made the Available section + // and partial-version resolution empty on Windows/macOS for any + // package, e.g. mingw-gcc has no linux block.) + return mcpp::manifest::list_xpkg_versions( + body, mcpp::platform::HostPlatform::current()); }; auto data = cfg.xlingsHome() / "data"; diff --git a/tests/unit/test_manifest.cpp b/tests/unit/test_manifest.cpp index 950c2515..ee855509 100644 --- a/tests/unit/test_manifest.cpp +++ b/tests/unit/test_manifest.cpp @@ -2,6 +2,7 @@ import std; import mcpp.manifest; +import mcpp.platform.axis; import mcpp.platform; TEST(Manifest, CppFlyStandard) { @@ -294,23 +295,23 @@ package = { }, } )"; - auto linux = mcpp::manifest::list_xpkg_versions(src, "linux"); + auto linux = mcpp::manifest::list_xpkg_versions(src, mcpp::platform::TargetPlatform::for_lint_of("linux")); ASSERT_EQ(linux.size(), 3u); EXPECT_EQ(linux[0], "0.1.0"); EXPECT_EQ(linux[1], "0.2.0"); EXPECT_EQ(linux[2], "1.0.0"); - auto mac = mcpp::manifest::list_xpkg_versions(src, "macosx"); + auto mac = mcpp::manifest::list_xpkg_versions(src, mcpp::platform::TargetPlatform::for_lint_of("macosx")); ASSERT_EQ(mac.size(), 1u); EXPECT_EQ(mac[0], "0.1.0"); - auto win = mcpp::manifest::list_xpkg_versions(src, "windows"); + auto win = mcpp::manifest::list_xpkg_versions(src, mcpp::platform::TargetPlatform::for_lint_of("windows")); EXPECT_TRUE(win.empty()); } TEST(ListXpkgVersions, MissingXpmReturnsEmpty) { constexpr auto src = R"(package = { name = "foo" })"; - EXPECT_TRUE(mcpp::manifest::list_xpkg_versions(src, "linux").empty()); + EXPECT_TRUE(mcpp::manifest::list_xpkg_versions(src, mcpp::platform::TargetPlatform::for_lint_of("linux")).empty()); } TEST(Manifest, BuildCflagsCxxflagsAndCStandard) { @@ -458,7 +459,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "compat.eigen", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "compat.eigen", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->provides.size(), 1u); @@ -573,7 +574,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "compat.opencv", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "compat.opencv", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); EXPECT_TRUE(m->xpkgUnknownKeys.empty()); @@ -600,7 +601,7 @@ package = { }, } )"; - auto mb = mcpp::manifest::synthesize_from_xpkg_lua(bad, "compat.opencv", "1.0.0"); + auto mb = mcpp::manifest::synthesize_from_xpkg_lua(bad, "compat.opencv", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_FALSE(mb.has_value()); EXPECT_NE(mb.error().message.find("features.dnn.flags"), std::string::npos) << mb.error().message; @@ -644,7 +645,7 @@ package = { )"; // linux leg: neutral payload + linux delta, macosx section invisible. auto ml = mcpp::manifest::synthesize_from_xpkg_lua( - lua, "compat.opencv", "1.0.0", "linux"); + lua, "compat.opencv", "1.0.0", mcpp::platform::TargetPlatform::for_lint_of("linux")); ASSERT_TRUE(ml.has_value()) << ml.error().format(); ASSERT_TRUE(ml->buildConfig.featureSources.contains("dnn")); { @@ -662,7 +663,7 @@ package = { // macosx leg: NEON delta instead, no mlas flags, no vaapi. auto mm = mcpp::manifest::synthesize_from_xpkg_lua( - lua, "compat.opencv", "1.0.0", "macosx"); + lua, "compat.opencv", "1.0.0", mcpp::platform::TargetPlatform::for_lint_of("macosx")); ASSERT_TRUE(mm.has_value()) << mm.error().format(); { auto& srcs = mm->buildConfig.featureSources["dnn"]; @@ -714,7 +715,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "compat.eigen", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "compat.eigen", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); // implies recorded in featuresMap ASSERT_TRUE(m->featuresMap.contains("backend-openblas")); @@ -810,7 +811,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "opencv", "0.0.3"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "opencv", "0.0.3", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); // The forward token left featuresMap for featureForwards. EXPECT_TRUE(m->featuresMap["dnn"].empty()); @@ -848,7 +849,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "x", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "x", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->xpkgUnknownKeys.size(), 1u); EXPECT_EQ(m->xpkgUnknownKeys[0], "dependencies"); @@ -875,7 +876,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "x", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "x", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->xpkgUnknownKeys.size(), 1u); EXPECT_EQ(m->xpkgUnknownKeys[0], "features.opt.include_dirs"); @@ -951,7 +952,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->buildConfig.cflags.size(), 2u); EXPECT_EQ(m->buildConfig.cflags[0], "-Wall"); @@ -977,7 +978,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyshared", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyshared", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->targets.size(), 1u); EXPECT_EQ(m->targets[0].kind, mcpp::manifest::Target::SharedLibrary); @@ -1002,7 +1003,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "gtestlike", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "gtestlike", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); // base sources keep both (old mcpp ignores `features` → no regression) ASSERT_EQ(m->buildConfig.sources.size(), 2u); @@ -1030,7 +1031,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "glfw", "3.4"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "glfw", "3.4", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->runtimeConfig.libraryDirs.size(), 1u); EXPECT_EQ(m->runtimeConfig.libraryDirs[0], "mcpp_generated/runtime/lib"); @@ -1075,7 +1076,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); std::string expectedSource = "*/src/linux.c"; @@ -1109,8 +1110,7 @@ package = { // osOverride seam (`mcpp xpkg parse --all-os`): a foreign OS section is // spliced on request, independent of the running host — the build path // (empty override) above stays host-selected. Reuses this fixture. - auto win = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0", - "windows"); + auto win = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0", mcpp::platform::TargetPlatform::for_lint_of("windows")); ASSERT_TRUE(win.has_value()) << win.error().format(); ASSERT_EQ(win->modules.sources.size(), 2u); EXPECT_EQ(win->modules.sources[1], "*/src/win32.c"); @@ -1135,7 +1135,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->buildConfig.generatedFiles.size(), 1u); auto it = m->buildConfig.generatedFiles.find("mcpp_generated/include/config.h"); @@ -1160,7 +1160,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "tinyc", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->buildConfig.includeDirs.size(), 1u); EXPECT_EQ(m->buildConfig.includeDirs[0], "*/include"); @@ -1340,7 +1340,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "consumer", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "consumer", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->dependencies.size(), 2u); @@ -1372,7 +1372,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "consumer", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(src, "consumer", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->dependencies.size(), 3u); @@ -1789,7 +1789,7 @@ package = { }, } )"; - auto v = mcpp::manifest::list_xpkg_versions(src, "linux"); + auto v = mcpp::manifest::list_xpkg_versions(src, mcpp::platform::TargetPlatform::for_lint_of("linux")); ASSERT_EQ(v.size(), 1u); EXPECT_EQ(v[0], "0.1.0"); } @@ -1986,7 +1986,7 @@ int f() { return "x]]y"[0]; } }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "lbtest", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "lbtest", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_TRUE(m->buildConfig.generatedFiles.contains("mcpp_generated/gen.cc")); EXPECT_EQ(m->buildConfig.generatedFiles["mcpp_generated/gen.cc"], @@ -2012,7 +2012,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "cmt", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "cmt", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->modules.sources.size(), 1u); EXPECT_EQ(m->modules.sources[0], "src/*.cpp"); @@ -2032,7 +2032,7 @@ TEST(XpkgLongBracket, NoLeadingNewlineKeptVerbatim) { " targets = { [\"nl\"] = { kind = \"lib\" } },\n" " },\n" "}\n"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "nl", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "nl", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); EXPECT_EQ(m->buildConfig.generatedFiles["g.cc"], "inline"); EXPECT_EQ(m->buildConfig.generatedFiles["h.cc"], "crlf"); @@ -2053,7 +2053,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "uk", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "uk", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->xpkgUnknownKeys.size(), 2u); EXPECT_EQ(m->xpkgUnknownKeys[0], "bogus_key"); @@ -2080,7 +2080,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "fmtlib.fmt", "12.2.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "fmtlib.fmt", "12.2.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m.has_value()) << m.error().format(); ASSERT_EQ(m->modules.scanOverrides.size(), 1u); auto& ov = m->modules.scanOverrides.at("*/src/fmt.cc"); @@ -2104,7 +2104,7 @@ package = { }, } )"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "so", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "so", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_FALSE(m.has_value()); EXPECT_NE(m.error().message.find("neither provides nor imports"), std::string::npos) << m.error().format(); @@ -2215,7 +2215,7 @@ mcpp = { )"; auto mt = mcpp::manifest::parse_string(toml); ASSERT_TRUE(mt.has_value()) << mt.error().format(); - auto mx = mcpp::manifest::synthesize_from_xpkg_lua(lua, "sym", "1.0.0"); + auto mx = mcpp::manifest::synthesize_from_xpkg_lua(lua, "sym", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(mx.has_value()) << mx.error().format(); EXPECT_EQ(mt->buildConfig.featureSources, mx->buildConfig.featureSources); @@ -2259,7 +2259,7 @@ mcpp = { }, } )"; - auto mx = mcpp::manifest::synthesize_from_xpkg_lua(lua, "condsrc", "1.0.0"); + auto mx = mcpp::manifest::synthesize_from_xpkg_lua(lua, "condsrc", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(mx.has_value()) << mx.error().format(); ASSERT_EQ(mx->conditionalConfigs.size(), 2u); EXPECT_EQ(mx->conditionalConfigs[0].predicate, "cfg(arch = \"x86_64\")"); @@ -2326,7 +2326,7 @@ mcpp = { }, } )"; - auto mx = mcpp::manifest::synthesize_from_xpkg_lua(lua, "globf", "1.0.0"); + auto mx = mcpp::manifest::synthesize_from_xpkg_lua(lua, "globf", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(mx.has_value()) << mx.error().format(); ASSERT_EQ(mx->buildConfig.globFlags.size(), 2u); EXPECT_EQ(mx->buildConfig.globFlags[0].glob, "third_party/**"); @@ -2496,7 +2496,7 @@ mcpp = { }, } )LUA"; - auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "p", "1.0.0"); + auto m = mcpp::manifest::synthesize_from_xpkg_lua(lua, "p", "1.0.0", mcpp::platform::HostPlatform::current()); ASSERT_TRUE(m) << (m ? "" : m.error().message); ASSERT_EQ(m->conditionalConfigs.size(), 1u); ASSERT_EQ(m->conditionalConfigs[0].inputs.globFlags.size(), 1u); @@ -2508,6 +2508,96 @@ mcpp = { target_cfg = { ["cfg(windows)"] = { linkage = { "static" } } }, } )LUA"; - auto bm = mcpp::manifest::synthesize_from_xpkg_lua(bad, "p", "1.0.0"); + auto bm = mcpp::manifest::synthesize_from_xpkg_lua(bad, "p", "1.0.0", mcpp::platform::HostPlatform::current()); EXPECT_FALSE(bm) << "unknown target_cfg key must stay a hard error"; } + +// #254: the host and target platform axes are distinct types, so an API +// states which one it wants and a call site must name the one it supplies. +// Before this, both were std::string_view and mixing them cost nothing — +// which is exactly how every xpkg per-OS splice ended up keyed on a +// compile-time HOST constant while [target.'cfg(...)'] was evaluated against +// the resolved TARGET. +TEST(PlatformAxis, HostAndTargetAreNotInterchangeable) { + using mcpp::platform::HostPlatform; + using mcpp::platform::TargetPlatform; + + static_assert(!std::is_convertible_v, + "the host axis must not silently become the target axis"); + static_assert(!std::is_convertible_v, + "the target axis must not silently become the host axis"); + // And neither is constructible from a bare string: passing "linux" + // where an axis is expected is the bug class this type exists to stop. + static_assert(!std::is_constructible_v); + static_assert(!std::is_constructible_v); + + // The triple vocabulary says "macos"; xpkg descriptors say "macosx". + // That translation belongs to the axis, not to every call site. + EXPECT_EQ(TargetPlatform::for_os("macos").key(), "macosx"); + EXPECT_EQ(TargetPlatform::for_os("macosx").key(), "macosx"); + EXPECT_EQ(TargetPlatform::for_os("windows").key(), "windows"); + EXPECT_EQ(TargetPlatform::for_os("linux").key(), "linux"); + // An absent/unknown os token keeps the pre-#254 behaviour: host. + EXPECT_EQ(TargetPlatform::for_os("").key(), + HostPlatform::current().key()); +} + +// The behaviour half: a descriptor's per-OS section must be spliced for the +// TARGET. Native builds cannot observe this (host == target), which is why +// three-platform CI never caught it. +TEST(Manifest, XpkgPerOsSectionSplicesForTheTargetNotTheHost) { + const char* lua = R"LUA( +mcpp = { + sources = { "base.cpp" }, + linux = { sources = { "linux_only.cpp" } }, + macosx = { sources = { "macos_only.cpp" } }, + windows = { sources = { "windows_only.cpp" } }, +} +)LUA"; + auto has = [](const mcpp::manifest::Manifest& m, std::string_view glob) { + return std::ranges::find(m.modules.sources, glob) != m.modules.sources.end(); + }; + + for (auto [tripleOs, expected] : std::initializer_list< + std::pair>{ + {"linux", "linux_only.cpp"}, + {"macos", "macos_only.cpp"}, + {"windows", "windows_only.cpp"}}) { + auto m = mcpp::manifest::synthesize_from_xpkg_lua( + lua, "p", "1.0.0", mcpp::platform::TargetPlatform::for_os(tripleOs)); + ASSERT_TRUE(m) << (m ? "" : m.error().message); + EXPECT_TRUE(has(*m, expected)) + << "target os " << tripleOs << " must splice its own section"; + EXPECT_TRUE(has(*m, "base.cpp")); + // and must NOT carry the other legs + for (std::string_view other : {"linux_only.cpp", "macos_only.cpp", + "windows_only.cpp"}) { + if (other == expected) continue; + EXPECT_FALSE(has(*m, other)) + << "target os " << tripleOs << " must not carry " << other; + } + } +} + +// Zero behaviour change for native builds is the safety net for #254: when +// host == target the splice must be byte-identical to what it always was. +TEST(Manifest, NativeBuildSpliceIsUnchanged) { + const char* lua = R"LUA( +mcpp = { + sources = { "base.cpp" }, + linux = { sources = { "linux_only.cpp" }, cxxflags = { "-DL" } }, + macosx = { sources = { "macos_only.cpp" }, cxxflags = { "-DM" } }, + windows = { sources = { "windows_only.cpp" }, cxxflags = { "-DW" } }, +} +)LUA"; + auto host = mcpp::manifest::synthesize_from_xpkg_lua( + lua, "p", "1.0.0", mcpp::platform::HostPlatform::current()); + auto nativeTarget = mcpp::manifest::synthesize_from_xpkg_lua( + lua, "p", "1.0.0", + mcpp::platform::TargetPlatform::for_lint_of( + mcpp::platform::HostPlatform::current().key())); + ASSERT_TRUE(host); + ASSERT_TRUE(nativeTarget); + EXPECT_EQ(host->modules.sources, nativeTarget->modules.sources); + EXPECT_EQ(host->buildConfig.cxxflags, nativeTarget->buildConfig.cxxflags); +} From 0364eb4335517672340d3f0af1471c8e845ec94c Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:53:06 +0800 Subject: [PATCH 08/13] test(e2e): #256 clang module operator-template canary + module-package hazard docs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit mcpp has no defect here: --precompile succeeds and the crash is inside the Clang frontend on the import side. But mcpp bundles LLVM, so the hazard ships with the toolchain, and it lands squarely on the module-package pattern this project recommends — wrapping an upstream header whose operators are static inline templates and mirroring their signatures with a trivially-true constraint is exactly how you hit it. Two things it needs, both of which mcpp owns: Documentation, so the next person does not repeat the bisect: every template parameter of an exported operator template should be pinned by the FIRST argument; otherwise deduce whole operand types and constrain them. The reporter's shipped workaround is included, along with the property that makes this so hard to attribute — the crash is name-keyed, so one poisoned operator* makes every `x * y` in every importer crash on unrelated types. A canary, because the alternative is finding out by accident. It is a STATE canary rather than a pass/fail contract: the expectation table records crash for LLVM 20-22 and ok for 18-19, and the test fails when reality stops matching — a Clang bump that FIXES this is just as much a signal as one that re-breaks it, and neither should be able to change what packages can express without anyone noticing. A control build without the unpinned overload guards the canary itself. Verified against both bundled toolchains (20.1.7 and 22.1.8 crash as recorded; system clang 18.1.3 and gcc 16.1.0 compile the same repro fine). An upstream LLVM report is filed separately; this does not wait on it. --- docs/03-toolchains.md | 53 +++++++ .../e2e/150_clang_module_operator_template.sh | 129 ++++++++++++++++++ 2 files changed, 182 insertions(+) create mode 100755 tests/e2e/150_clang_module_operator_template.sh diff --git a/docs/03-toolchains.md b/docs/03-toolchains.md index 4fa68a51..4e61148a 100644 --- a/docs/03-toolchains.md +++ b/docs/03-toolchains.md @@ -267,3 +267,56 @@ When `MCPP_HOME` is not set explicitly, mcpp locates the sandbox automatically b ## ABI Capability Enforcement A dependency can declare an `abi:` capability (for example, `compat.glfw` declares `abi:glibc`). When the resolved toolchain's ABI does not satisfy any dependency's abi requirement, the build **fails early** with a suggested fix (for example, a musl-static toolchain encountering an abi:glibc dependency), replacing deeper link/header errors. Inspect with: `mcpp why toolchain`. + +## Known Toolchain Hazard: Operator Templates in Module Interfaces (Clang 20+) + +A module that exports **replacement operator templates** can poison name +lookup for that operator in every importer when compiled by Clang 20 or 22: +any translation unit that `import`s the module and uses that operator — **on +any type at all** — crashes the frontend (SIGSEGV). GCC 16 and Clang 18 are +unaffected, so this is a regression somewhere between Clang 18 and 20. + +This bites the module-package pattern directly. Wrapping an upstream header +whose operators are `static inline` templates, and mirroring their signatures +with a trivially-true constraint (the standard mixed-TU subsumption recipe), +is exactly how you hit it. + +**The rule of thumb:** every template parameter should be pinned by the +**first** function argument. Shapes that break this are the poisonous ones: + +```cpp +// Poisonous — `n` and `l` are not determined by argument 1 +template +Matx operator*(const Matx& a, const Matx& b); + +// Poisonous — second typename appears only in argument 2 +template +Vec& operator+=(Vec& a, const Vec& b); + +// Fine — every parameter is pinned by argument 1 +template +Matx operator+(const Matx& a, const Matx& b); +``` + +The crash is **name-keyed**: one poisoned `operator*` declaration makes every +`x * y` in every importer crash, for entirely unrelated types. The function +body is irrelevant. + +**The workaround** is to deduce whole operand types and constrain them, +rather than destructuring them in the parameter list. It stays +call-compatible, and mixed-TU semantics survive because the upstream +exact-pattern `static inline` remains more specialized and still wins there: + +```cpp +template + requires pick + && __is_same(MA, typename MA::mat_type) + && __is_same(MB, Matx) +inline MA& operator+=(MA& a, const MB& b); +``` + +Tracked as [mcpp#256](https://github.com/mcpp-community/mcpp/issues/256). +`tests/e2e/150_clang_module_operator_template.sh` is a canary over the +bundled LLVM toolchains, so a future Clang bump that fixes — or re-breaks — +this becomes visible instead of silently changing what packages can express. diff --git a/tests/e2e/150_clang_module_operator_template.sh b/tests/e2e/150_clang_module_operator_template.sh new file mode 100755 index 00000000..b1a1368b --- /dev/null +++ b/tests/e2e/150_clang_module_operator_template.sh @@ -0,0 +1,129 @@ +#!/usr/bin/env bash +# #256: known-issue canary for a Clang regression that mcpp cannot fix but +# that silently limits what a module package may export. +# +# A module exporting replacement operator templates whose parameters are NOT +# all pinned by the first argument poisons name lookup for that operator in +# every importer: the frontend SIGSEGVs on any use of the name, on any type. +# Clang 18 and GCC 16 are fine; Clang 20 and 22 crash. Since mcpp bundles +# LLVM, the hazard ships with the toolchain. +# +# This is a STATE canary, not a pass/fail contract: today the expected result +# for the bundled toolchains is "crashes". The test fails when reality stops +# matching the recorded expectation — i.e. when a Clang bump fixes it (time +# to update the docs and the table) or when a version previously known-good +# regresses. Silence here would mean a future toolchain bump quietly changed +# what packages can express. +set -e + +SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +source "$SCRIPT_DIR/_llvm_env.sh" + +if [[ ! -d "$LLVM_ROOT" ]]; then + echo "SKIP: no llvm payload installed" + exit 0 +fi + +CLANGXX="$LLVM_ROOT/bin/clang++" +[[ -x "$CLANGXX" ]] || { echo "SKIP: no clang++ in $LLVM_ROOT"; exit 0; } + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +cat > matx.h <<'EOF' +#pragma once +namespace cv { +template struct Matx { + T val[m*n]; + typedef Matx mat_type; + enum { rows = m, cols = n }; +}; +template struct Point_ { T x, y; }; +using Point = Point_; +template static inline +Matx operator*(const Matx& a, const Matx& b) { return Matx{}; } +template static inline +Point_ operator*(const Point_& a, int b) { return Point_{a.x*b, a.y*b}; } +} +EOF + +# The module layer: upstream shape mirrored with a trivially-true constraint. +# The 4-parameter Matx*Matx overload is the poisonous one — `n` and `l` are +# not determined by the first argument. +cat > m.cppm <<'EOF' +module; +#include "matx.h" +export module m; +export namespace cv { +using cv::Matx; +using cv::Point_; +using cv::Point; +inline namespace repl { +template inline constexpr bool pick = true; +template requires pick inline +Matx operator*(const Matx& a, const Matx& b) { return Matx{}; } +template requires pick inline +Point_ operator*(const Point_& a, int b) { return Point_{a.x*b, a.y*b}; } +} +} +EOF + +# Note: only a Point is multiplied. The crash is name-keyed, so an unrelated +# type is enough — that is what makes the hazard so hard to attribute. +cat > use.cpp <<'EOF' +import m; +int main() { cv::Point p{1,2}; auto q = p * 2; return q.x; } +EOF + +"$CLANGXX" -std=c++23 --precompile m.cppm -o m.pcm > precompile.log 2>&1 || { + cat precompile.log + echo "FAIL: producing the BMI should always succeed — the defect is on the import side" + exit 1 +} + +set +e +"$CLANGXX" -std=c++23 -fprebuilt-module-path=. -c use.cpp -o use.o > use.log 2>&1 +IMPORT_RC=$? +set -e + +# Control: the same module WITHOUT the unpinned overload must always compile. +# If this ever fails, the canary itself is broken rather than the toolchain. +grep -v 'Matx operator\*(const Matx& a' m.cppm \ + | grep -v 'template requires pick inline' > ctl.cppm +"$CLANGXX" -std=c++23 --precompile ctl.cppm -o ctl.pcm > ctl_pre.log 2>&1 || { + cat ctl_pre.log; echo "FAIL: control module failed to precompile"; exit 1; } + +# Recorded expectations, by LLVM major version. +case "${LLVM_VERSION%%.*}" in + 18|19) EXPECTED=ok ;; + 20|21|22) EXPECTED=crash ;; + *) EXPECTED=unknown ;; +esac + +if [[ $IMPORT_RC -eq 0 ]]; then ACTUAL=ok; else ACTUAL=crash; fi + +echo " llvm@${LLVM_VERSION}: expected=${EXPECTED} actual=${ACTUAL}" + +if [[ "$EXPECTED" == "unknown" ]]; then + echo " no recorded expectation for LLVM ${LLVM_VERSION%%.*} — record one in this test" + echo "OK" + exit 0 +fi + +if [[ "$ACTUAL" != "$EXPECTED" ]]; then + echo + echo "FAIL: #256 canary state changed for llvm@${LLVM_VERSION}." + if [[ "$ACTUAL" == "ok" ]]; then + echo " The importer now compiles — the Clang regression appears FIXED." + echo " Update the expectation table here and the hazard section in" + echo " docs/03-toolchains.md, and tell mcpp-community/mcpp#256." + else + echo " The importer now crashes on a version previously known good." + echo " A toolchain bump has re-broken module operator templates." + sed -n '1,25p' use.log + fi + exit 1 +fi + +echo "OK" From e84e8fb28a389293fc0fb8ae96a320612112df7a Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 04:57:51 +0800 Subject: [PATCH 09/13] =?UTF-8?q?docs(release):=200.0.102=20=E2=80=94=20ch?= =?UTF-8?q?angelog,=20manifest=20reference,=20batch-ledger=20invariants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Version bump plus the three documentation surfaces this batch owes: - CHANGELOG entry covering #261 / #257 / #258 / #254 and what was explicitly left out (#259, and the unknown-key policy split into #263). - docs/05-mcpp-toml.md: the new conditional keys, and — more usefully than a key list — WHY the set is what it is. A conditional section accepts exactly the additive build inputs; linkage/target/profile knobs are excluded because they are inputs to target selection or need override semantics. Includes the per-OS removal idiom and the "off-target entries do not exist, so they cannot warn" rule. - Batch ledger gains a section 6 for cross-batch invariants: the existing "one decision, one derivation" rule with #254/#258 recorded as instances, and the new "failures must be audible" rule with its rationale — three of this batch's four issues were the same silent-degradation shape, in a code base that already practiced the discipline elsewhere but had never made it an invariant new code inherits. --- ...atch-ledger-and-architecture-assessment.md | 30 ++++++++++++++++++ CHANGELOG.md | 25 +++++++++++++++ docs/05-mcpp-toml.md | 31 +++++++++++++++++-- mcpp.toml | 2 +- src/toolchain/fingerprint.cppm | 2 +- 5 files changed, 86 insertions(+), 4 deletions(-) diff --git a/.agents/docs/2026-07-19-issues-230-243-batch-ledger-and-architecture-assessment.md b/.agents/docs/2026-07-19-issues-230-243-batch-ledger-and-architecture-assessment.md index 600e55b4..59a90142 100644 --- a/.agents/docs/2026-07-19-issues-230-243-batch-ledger-and-architecture-assessment.md +++ b/.agents/docs/2026-07-19-issues-230-243-batch-ledger-and-architecture-assessment.md @@ -117,3 +117,33 @@ - 每落地一个 issue:更新 §1 对应行的状态 + commit,并在 CHANGELOG 追加(带 `#nnn`)。 - 凡"根因在仓外 / 转后续"的,§1 标 ⛔ 并附外部 issue 链接,**不得**在本仓用 band-aid 标记已修。 - 本批次的发布仍走既定链路:release 四平台 → 镜像 xlings-res(gh+gtc)→ xim-pkgindex 索引 → bump bootstrap pin;随后 mcpplibs #79 CI pin 升级解阻塞(#240 是 #79 的合并门)。 + +--- + +## 6. 架构不变量(跨批次,持续生效) + +### 6.1 同一决策不许两处推导(§5.2 起,持续) + +已命中的实例:#242 的 feature 请求集解析×激活两处不自洽;#253 的 per-OS `features`;**0.0.102 新增两例**—— +- **#254**:平台选择在 xpkg 通道按宿主编译期常量推导,在 manifest 通道按 resolved target 推导。native 构建 host == target 使两者巧合一致,缺陷因此对三平台 CI 完全不可见。收敛手段是**类型化**(`mcpp.platform.axis`),让"传错轴"成为编译错误而非运行期巧合。 +- **#258**:"哪些构建输入可被条件化"在 cfg 轴与 feature 轴各推导一遍,且答案不同。收敛手段是把该集合提炼为**类型**(`BuildInputs`)而非一张手工维护的键表——表会与结构体漂移,类型不会。 + +判据:当一个决策需要在两处表达时,优先找一个**类型**来承载它,其次才是把两处调用同一个函数。 + +### 6.2 失败必须响,不许静默降级(0.0.102 新立) + +> 任何"因为条件不满足而少做一步"的分支,必须要么返回错误,要么经 `mcpp::diag::degraded` 上报; +> `log::debug` / `log::verbose` **不算**用户可见。 +> 丢弃 `std::expected` 返回值(`(void)expr` 或不接收)在本仓视为缺陷。 + +由来:0.0.102 批次的四条 issue 里有三条的实质是同一失效模式——引擎遇到没准备好的情况时安静地少做一点事。#257 在 clang 上不发 depfile(构建"成功",产物用陈旧 BMI);#258 在条件段静默丢弃 `flags`(构建"成功",规则从未生效);#259 的 sysroot 兜底是死代码且返回值被 `(void)` 丢弃(安装"成功",二进制不能 exec)。 + +值得注意的是本仓在**别处**早已把这条纪律做得很好(xpkg 未知键 → `xpkgUnknownKeys` + did-you-mean;`scan_overrides` 零命中 → 硬错;死 glob → 告警并点名归属 feature)。纪律一直存在,只是没有被提升为跨模块不变量,于是新代码没有默认继承它。 + +`diag::degraded` 的 `impact` 参数是这条不变量的执行机制:它强制作者写出"用户会因此遭遇什么"。上述三个 bug 当年缺的恰好就是这句话。 + +### 6.3 code review checklist 增补 + +- 每个 `if (cond) return;` / `continue` / `catch` 静默吞掉的分支:用户看得见吗? +- 每个 `std::expected` 返回值:被检查了吗?`(void)` 丢弃需要理由。 +- 新增一个"可条件化 / 可传播 / 可覆盖"的字段时:它进的是哪个**类型**?该类型是否已经表达了这个语义? diff --git a/CHANGELOG.md b/CHANGELOG.md index 170b0247..69814588 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,31 @@ > 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。 > 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)。 +## [0.0.102] — 2026-07-22 + +> 四条 issue 一个批次:#261 windows 命令行长度、#257 clang 依赖跟踪缺口、#258 条件段表达力、#254 host/target 双轴不自洽。贯穿主线是**同一决策不许两处推导**,以及本批次新立的第二条不变量**失败必须响,不许静默降级**。设计见 `.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md`,issue 裁决见 `.agents/docs/2026-07-22-issue-triage-254-261.md`。 + +### 修复 + +- **windows scan-deps 撞 8191 字符上限**(#261):clang 扫描规则此前用 shell 重定向产出 P1689 JSON,ninja 在 windows 走 CreateProcess 不解释 `>`,于是整条命令被 `cmd /c` 包裹——而 cmd.exe 的命令行上限是 8191,只有 CreateProcess 的 1/4。改用 `clang-scan-deps -o`(LLVM 17+,两个自带工具链均实测可用),重定向与包裹一并消失,与 GCC 的 `-fdeps-file=`、MSVC 的 `/scanDependencies` 三条分支形态统一。`cmd /c` 自此在全仓 ninja 规则中绝迹。 +- **windows 编译/扫描命令行无长度兜底**(#261):`local_include_flags()` 每依赖一个 `-I`、无上界,而 `cxx_module`/`cxx_object`/`c_object`/`asm_object`/`cxx_scan` 全部内联该载荷;扫描命令包住编译命令,只是恰好先炸。把 #247 给链接规则的 response file 推广到所有携带无界载荷的规则(gcc/clang 驱动与 cl.exe 均展开 `@file`,clang-scan-deps 会把 `--` 之后的 `@file` 透传给驱动;NASM 因为拼写是 `-@ file` 而排除)。顺带把 `escape_flag_path` 改用 `generic_string()`——这是 #247 那个坑往下一层:response file 内容按 GNU 文法分词,反斜杠是转义符,windows 的 `-I` 路径一旦离开命令行就会丢分隔符。POSIX 逐字节不变。 +- **clang 路径 purview `#include` 不触发重建**(#257):编辑模块接口 purview 内 `#include` 的文件后,构建静默复用陈旧 BMI,导入方对着旧接口编译——最差的一类失效,错误结果看起来像一次正常的增量构建。根因是 0.0.97 把两个决策捆在一个谓词里:**是否发 depfile** 与 **是否剥离 GCC `-fmodules` 附加的 reversed make-rules**。实测两个自带工具链,clang 的 module-TU depfile 就是一条普通规则(`x.o: x.cppm ops.inc`),没有任何需要过滤的东西——当年那道闸在防一个不存在的形状,代价是把与它捆在一起的正确性契约一起关掉了。现拆为 `posixDepfile`(所有 POSIX 非 MSVC)与 `needsGnuModuleFilter`(仅 GCC)。同时补上同一处不对称的另一半:`c_object`/`asm_object` 此前在**任何**工具链上都没有 depfile。windows 非 MSVC(mingw gcc / 托管 clang)仍无 depfile(GCC 过滤器依赖 awk),但现在经 `diag::degraded` 明确上报影响,不再静默。 +- **xpkg per-OS 段按宿主而非目标拼接**(#254):per-OS 段的 sources/flags/deps 与 xpm 版本/资产表全部按 `xpkg_platform` 这个**编译期宿主常量**选取,而 `[target.'cfg(...)']` 按**解析后的目标**求值——同一决策两处推导。交叉编译时依赖包因此被拼进宿主那条腿。原生构建 host == target 恰好掩盖了它,所以三平台 CI 从未发现。 + +### 新增 + +- **`mcpp::diag` 统一降级/告警通道**:`degraded()` 强制填写 `impact`——作者必须回答"用户会因此遭遇什么",而这正是每一个静默降级 bug 当年缺的那句话。记录按整条载荷去重、报告即渲染(告警与 `ui::status` 的交织顺序、以及提前返回前已报内容都不变),`--strict` 提升策略收敛到单点(此前在 `prepare.cppm` 里 copy-paste 了 5 处)。告警内容首次可单测。`prepare.cppm` 的 8 处裸 `println(stderr, "warning: ...")` 已迁入;渲染仍走 `ui::warning`,输出逐字节不变。 +- **`[target.'cfg(...)'.build]` 支持 per-glob `flags` 与 `include_dirs`**(#258,xpkg `target_cfg` 同步):此前 `sources` 可按 OS 条件化而 `flags` 不能,于是一份覆盖三 OS 的 manifest 必须让每个 OS 都看到另外两个 OS 的 flag 条目,而它们按构造必然零命中。vendored-opencv 移植为此付出 **703 个 stub 文件**(唯一用途是给 windows TU 制造 OS-唯一路径好让全局 flag 表 key 上去)、32 条指向它们的 glob,以及每次构建 ~23 条结构性死 glob 告警。条件条目追加在 base 之后,GNU last-wins 使**撤销**可表达(windows 需要 `-UHAVE_UNISTD_H` 对抗 base 的 `-D`,而无条件覆盖层做不到——`-U` 反条目自身也需要按 OS 条件化,递归回同一缺口)。死 glob 告警由**结构**消除:不匹配当前 OS 的条目根本不进 `globFlags`,与 #253 在 feature 轴上的做法同构;issue 里提的 `optional = true` 逃生口**明确不做**,那会成为第二套抑制机制。 +- **`mcpp.platform.axis`:host / target 两轴成为不同类型**(#254):互不可转换,且都不能由裸字符串构造;API 声明自己要哪一轴,调用点必须**指名**自己给的是哪一轴。`synthesize_from_xpkg_lua` 的平台参数去掉宿主默认值改为必填——静默默认正是这个 bug 得以存活的方式。三段 triple 与 xpkg 的词汇差异("macos" vs "macosx")收进 `TargetPlatform::for_os`。分类:依赖包 manifest 合成与版本/资产选择走 **target**;工具链版本列举走 **host**(其注释本就写明了理由);`--all-os` lint 走刻意起得别扭的 `for_lint_of()`。 +- **`BuildInputs` 类型**(#258 的架构面):cfg 轴与 feature 轴此前各自手挑可条件化的字段子集,**且挑得不一样**(cfg 拿 cflags/cxxflags/ldflags/sources,feature 拿 sources/defines/flags)。"哪些构建输入可被条件化"这一决策被提炼成一个**类型**而非一张要靠人记得更新的表。入选判据两条:合并语义是**追加**;消费点在条件合并**之后**。据此排除选型轴(`target` 用来*选定* triple,而谓词要靠该 triple 求值——构造性循环)与策略标量(需 last-wins,是另一场设计);`generatedFiles` 因为是写磁盘的**副作用动作**而非输入、`featureDefines` 因为是沿 Public 边**传播的接口贡献**而非私有构建输入,各自按范畴排除。`BuildConfig` 以**继承**方式承载(该字段在 ~150 处被读,嵌套只会是 150 次无收益的机械改动),`ConditionalConfig` 则**嵌套**——保证需要落在那里。合并收敛为单一 `append(BuildInputs&, const BuildInputs&)`。 +- **#256 clang 模块运算符模板 canary + 文档**:mcpp 侧无缺陷(`--precompile` 成功,崩溃在 clang 前端的导入侧),但 mcpp 自带 LLVM,且该 hazard 正落在本项目推荐的模块包模式上。文档给出可操作规则(导出的运算符模板,所有模板参数应由**第一个实参**绑定;否则改为对整个推导操作数类型建模)与可直接套用的改写范式。e2e 150 是**状态** canary 而非通过/失败契约:期望表记录 LLVM 20-22 崩、18-19 正常,一旦现实与期望不符即失败——未来某次 clang bump **修好**它与**再弄坏**它同样是需要被看见的信号。 + +### 备注 + +- 新增不变量写入批次总账:**任何"因条件不满足而少做一步"的分支,必须要么返回错误,要么经 `diag::degraded` 上报;`log::debug`/`log::verbose` 不算用户可见;丢弃 `std::expected` 返回值视为缺陷。** +- e2e 新增 148(64 个 include dir 的宽 include 表,POSIX 验内联形态、windows 验 response file)、149(条件段 per-glob flags 四象限:命中生效/覆盖 base 的 removal/非命中零告警/真死 glob 仍告警)、150(#256 canary);118 去掉 `# requires: gcc` 并加 clang 腿。 +- **未包含**:#259(根因在 xlings 侧的 dep 静默丢弃,调查结论已发 issue 评论;mcpp 侧另发现 sysroot 兜底是死代码——`resolve_xpkg_path("xim:glibc")` 缺版本必然失败且两处都丢弃返回值——一并另行安排);manifest 未知键策略五处不一致([#263](https://github.com/mcpp-community/mcpp/issues/263),有兼容性面,与 #258 无依赖)。 + ## [0.0.101] — 2026-07-20 > #253:feature 模型两缺口收口——per-feature per-glob `flags` + per-OS `features` 语义锁定,解锁 compat.opencv `dnn` off-linux 腿并消除 feature-off 构建的死 glob 告警。设计见 `.agents/docs/2026-07-20-issue-253-feature-flags-and-per-os-features-design.md`。 diff --git a/docs/05-mcpp-toml.md b/docs/05-mcpp-toml.md index add50f48..7a0caf2f 100644 --- a/docs/05-mcpp-toml.md +++ b/docs/05-mcpp-toml.md @@ -376,12 +376,39 @@ when you need arch/env conditions or combinators. - **Keys**: `dependencies` / `dev-dependencies` / `build-dependencies`, and `build` with `cflags` / `cxxflags` / `ldflags` / `sources` (mcpp 0.0.95+ — conditional source globs, e.g. gating `src/x86/**/*.asm` behind - `cfg(arch = "x86_64")`; `!`-exclusion globs work here too). + `cfg(arch = "x86_64")`; `!`-exclusion globs work here too), plus `flags` and + `include_dirs` / `include_dirs_after` (mcpp 0.0.102+). +- **What `build` accepts is exactly the set of *additive build inputs*** — the + things that combine by appending and are consumed after the predicate is + evaluated. `linkage`, `target`, and the profile knobs are deliberately not + among them: they are *inputs to* target selection (conditioning `target` on + a predicate evaluated against `target` is circular), or they need + override-rather-than-append semantics. - **Evaluated against the resolved target** — the `--target` triple for a cross build, otherwise the host. So a native Linux build never even *downloads* a `[target.windows]` dependency. - **Precedence**: an exact-triple table wins over a `cfg`/alias table; multiple - matching predicate tables have their flags concatenated. + matching predicate tables have their flags concatenated. Conditional entries + are appended **after** the unconditional `[build]` ones, so under GNU + "last flag wins" a conditional rule overrides a broader unconditional one. + That is what makes a per-OS **removal** expressible: + + ```toml + [build] + flags = [{ glob = "third_party/zlib/**", defines = ["HAVE_UNISTD_H=1"] }] + + # clang-MSVC has no : undo the base define, add the windows one. + [target.'cfg(windows)'.build] + flags = [{ glob = "third_party/zlib/**", + defines = ["NO_FSEEKO"], cflags = ["-UHAVE_UNISTD_H"] }] + ``` + +- **A conditional `flags` entry that does not match the current target does not + exist at all**, so it cannot produce a "glob matched no source file" warning. + One manifest can therefore carry all three OSes' flag tables without any of + them generating noise on the other two — the same way an inactive feature's + entries simply are not there. A zero-hit glob in the *unconditional* table + still warns, because there it is a real defect. - **`toolchain` / `linkage` are exact-triple only** — they describe one specific cross target, so put them under `[target.]` (above), not under a bare alias or `cfg(...)`. diff --git a/mcpp.toml b/mcpp.toml index 2f3376f8..3435a0d1 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "0.0.101" +version = "0.0.102" description = "Modern C++ build & package management tool" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/src/toolchain/fingerprint.cppm b/src/toolchain/fingerprint.cppm index 457f8654..ba3ef157 100644 --- a/src/toolchain/fingerprint.cppm +++ b/src/toolchain/fingerprint.cppm @@ -18,7 +18,7 @@ import mcpp.toolchain.detect; export namespace mcpp::toolchain { -inline constexpr std::string_view MCPP_VERSION = "0.0.101"; +inline constexpr std::string_view MCPP_VERSION = "0.0.102"; struct FingerprintInputs { Toolchain toolchain; From 8aa42f7a1dc2ff58359eb3a6ed38152ffe4462c9 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 05:16:40 +0800 Subject: [PATCH 10/13] =?UTF-8?q?fix(build):=20architecture-review=20pass?= =?UTF-8?q?=20=E2=80=94=20strict=20policy=20actually=20enforced,=20.s=20de?= =?UTF-8?q?pfile=20split,=20feature=20axis=20folds=20through=20append?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from the whole-branch review, two of them defects introduced by this batch. 1. diag::flush() was never called outside its own unit test. The one new degradation this branch introduces (#257's windows + non-MSVC "no depfile" case) would print its warning and then be ignored, and `mcpp build --strict` would still exit 0. The channel built to stop silent degradation was itself silently degrading. BuildContext now carries `strict` and run_build_plan calls flush() after the build — after, because backend emission is where degradations are discovered, so end-of-prepare_build would be too early. 2. `-MMD` was applied to `.s` as well as `.S`. The C driver preprocesses `.S` but not `.s`, so clang emits two `argument unused during compilation` warnings per file and writes no depfile (measured on 20.1.7; gcc writes none either, just quietly) — and ninja's `deps = gcc` treats an absent depfile as an error, so the cases cannot share a rule. Split into asm_object (.S, tracked) and asm_object_raw (.s, pre-#257 shape). 3. The feature axis was still folding its per-glob flags field-by-field rather than through append(BuildInputs&), so the "one funnel for both axes" claim held for only one axis. Now routed through append. The other two feature tables stay out on purpose and the comment says why: feature `sources` carry DROP-then-ADD semantics, and feature `defines` are interface contributions that propagate along Public edges — neither is a plain append, so neither belongs in BuildInputs. Also fixes an ordering bug found while re-reading the #254 change: the target platform was computed at the top of prepare_build, but overrides.target_triple is only filled in from `[build] target` / the config default and canonicalized some 200 lines later. Any project that sets its target in the manifest rather than on the command line would have silently fallen back to the host axis — the exact bug #254 is about, reintroduced one level up. Moved below the resolution block, with a comment recording why it cannot move back. --- src/build/execute.cppm | 9 ++ src/build/ninja_backend.cppm | 17 ++- src/build/prepare.cppm | 43 ++++++-- tests/e2e/151_cross_per_os_xpkg_splice.sh | 125 ++++++++++++++++++++++ tests/unit/test_diag.cpp | 21 ++++ tests/unit/test_ninja_backend.cpp | 47 ++++++++ 6 files changed, 250 insertions(+), 12 deletions(-) create mode 100755 tests/e2e/151_cross_per_os_xpkg_splice.sh diff --git a/src/build/execute.cppm b/src/build/execute.cppm index 2a6c4d78..4dbe49d0 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -10,6 +10,7 @@ export module mcpp.build.execute; import std; import mcpp.build.prepare; +import mcpp.diag; import mcpp.build.plan; import mcpp.build.backend; import mcpp.build.ninja; @@ -334,6 +335,14 @@ export int run_build_plan(BuildContext& ctx, bool verbose, bool no_cache, std::move(runTargets), runEnvKey, runEnvValue); } + // The one place the --strict policy is settled. Degradations reported by + // the backend (e.g. a toolchain/platform combination that cannot emit a + // depfile, #257) are discovered during emission, so this has to come + // after the build rather than at the end of prepare_build. Without this + // call the whole diag channel would report and then be ignored — the + // exact failure mode it exists to prevent. + if (!mcpp::diag::flush(ctx.strict)) return 1; + mcpp::ui::finished("release", r->elapsed); return 0; } diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index ecdc0558..841f49e7 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -636,14 +636,25 @@ std::string emit_ninja_string(const BuildPlan& plan) { // GAS assembly (.S/.s) through the C driver: it preprocesses .S (cpp) // and assembles both, dispatching by extension. $asmflags is the // asm-safe flag subset (no -std / no -O — see flags.cppm). - append("rule asm_object\n"); + // TWO rules, split by case: the C driver preprocesses `.S` but not + // `.s`. Asking for a depfile on a `.s` unit is not merely useless — + // clang emits `argument unused during compilation: '-MMD'` for every + // such file and writes nothing, and ninja's `deps = gcc` treats an + // absent depfile as an error. So `.s` keeps the pre-#257 shape. const std::string payload = " $local_includes $asmflags $unit_asmflags"; + append("rule asm_object\n"); // .S — preprocessed, tracks #include append(std::format(" command = $cc{} {}{}\n", rsp_ref(payload), c_mmd_flag, compile_tail)); append_rspfile(payload); append(" description = AS $out\n"); append_cxx_deps(); append("\n"); + + append("rule asm_object_raw\n"); // .s — not preprocessed, no depfile + append(std::format(" command = $cc{} {}\n", + rsp_ref(payload), compile_tail)); + append_rspfile(payload); + append(" description = AS $out\n\n"); } if (need_nasm_rule) { @@ -803,8 +814,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { return "cxx_module"; if (ext == ".c" || ext == ".m") return "c_object"; - if (ext == ".S" || ext == ".s") + if (ext == ".S") return "asm_object"; + if (ext == ".s") + return "asm_object_raw"; if (ext == ".asm") return "nasm_object"; return "cxx_object"; diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 4af35b3f..13571aed 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -547,6 +547,11 @@ bool graph_or_targets_import_std(const mcpp::modgraph::Graph& graph, } export struct BuildContext { + // --strict: degradations reported through mcpp::diag become errors. + // Carried on the context because the build's degradations are discovered + // during backend emission, i.e. after prepare_build has returned — the + // single place that settles the policy is run_build_plan (execute.cppm). + bool strict = false; mcpp::manifest::Manifest manifest; mcpp::toolchain::Toolchain tc; mcpp::toolchain::Fingerprint fp; @@ -596,15 +601,6 @@ prepare_build(bool print_fingerprint, return std::unexpected("no mcpp.toml found in current directory or any parent"); } - // #254: everything compiled INTO this build is resolved for the TARGET — - // an xpkg descriptor's per-OS sections (sources, flags, deps) and its xpm - // asset/version table all describe code that will run on the target, not - // on the machine building it. This used to be a compile-time host - // constant, which is invisible natively (host == target) and picks the - // wrong leg under --target. - const auto targetPlatform = mcpp::platform::TargetPlatform::for_os( - cfgpred::context_for(overrides.target_triple).os); - auto m = mcpp::manifest::load(*root / "mcpp.toml"); if (!m) return std::unexpected(m.error().format()); @@ -902,6 +898,21 @@ prepare_build(bool print_fingerprint, } if (overrides.force_static) m->buildConfig.linkage = "static"; + // #254: everything compiled INTO this build is resolved for the TARGET — + // an xpkg descriptor's per-OS sections (sources, flags, deps) and its xpm + // asset/version table all describe code that will run on the target, not + // on the machine building it. Previously a compile-time host constant, + // which is invisible natively (host == target) and picks the wrong leg + // under --target. + // + // Computed HERE, not earlier: `overrides.target_triple` is only complete + // above — it is filled from `[build] target` and the config default, then + // canonicalized. Reading it before that point would silently fall back to + // the host for any project that sets its target in the manifest rather + // than on the command line. + const auto targetPlatform = mcpp::platform::TargetPlatform::for_os( + cfgpred::context_for(overrides.target_triple).os); + // ── L1: merge conditional [target.'cfg(...)'.build] sources/flags AND // root-only [target.'cfg(...)'.dependencies] ───────────────────────────── // Evaluated now (target resolved) against the resolved target — the @@ -3009,13 +3020,24 @@ prepare_build(bool print_fingerprint, // sources ADD above, `mcpp build` and `mcpp test` must agree // (0.0.94 dual-path invariant). featureOrigin tags the entry so // the scanner's zero-hit warning can name the owning feature. + // + // Routed through the SAME append(BuildInputs&) the cfg axis uses + // (#258): both axes are contributing additive build inputs, so + // "how does a contribution combine with the base" must have one + // answer. Only the flags half of the feature axis is expressible + // that way — feature `sources` above carry DROP-then-ADD + // semantics, and feature `defines` are interface contributions + // that propagate along Public edges, so neither is a plain + // append and neither belongs in BuildInputs. for (auto& [f, entries] : bc.featureFlags) { if (std::ranges::find(active, f) == active.end()) continue; + mcpp::manifest::BuildInputs contribution; for (auto const& gf : entries) { auto tagged = gf; tagged.featureOrigin = f; - bc.globFlags.push_back(std::move(tagged)); + contribution.globFlags.push_back(std::move(tagged)); } + mcpp::manifest::append(bc, contribution); } }; if (!packages.empty()) { @@ -3450,6 +3472,7 @@ prepare_build(bool print_fingerprint, } BuildContext ctx; + ctx.strict = overrides.strict; ctx.manifest = *m; ctx.tc = *tc; ctx.fp = fp; diff --git a/tests/e2e/151_cross_per_os_xpkg_splice.sh b/tests/e2e/151_cross_per_os_xpkg_splice.sh new file mode 100755 index 00000000..85d7f11c --- /dev/null +++ b/tests/e2e/151_cross_per_os_xpkg_splice.sh @@ -0,0 +1,125 @@ +#!/usr/bin/env bash +# requires: mingw-cross +# #254: an xpkg dependency's per-OS section must be spliced for the RESOLVED +# TARGET, not for the host. +# +# The per-OS sections carry sources, flags, deps and the xpm asset table — +# all describing code compiled INTO the user's build, so they belong to the +# platform the artifacts will run on. They used to key on a compile-time HOST +# constant, so a Linux -> Windows cross build silently got the linux leg. +# +# Native builds cannot observe this (host == target), which is exactly why +# the three-platform CI never caught it: this test has to cross-compile to +# see the difference at all. It uses a local path-index descriptor so it +# needs no network and no real package. +set -e + +TRIPLE=x86_64-windows-gnu + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +# A local xpkg index with one descriptor whose per-OS sections differ in a +# way the compiler can prove: each leg defines its own macro, and the TU +# errors out unless exactly the windows leg was spliced. +mkdir -p idx/pkgs/t +cat > idx/pkgs/t/testsplice.lua <<'LUA' +package = { + name = "testsplice", + namespace = "test", +} + +xpm = { + linux = { ["1.0.0"] = { url = "unused", sha256 = "unused" } }, + windows = { ["1.0.0"] = { url = "unused", sha256 = "unused" } }, + macosx = { ["1.0.0"] = { url = "unused", sha256 = "unused" } }, +} + +mcpp = { + sources = { "src/**/*.cpp" }, + include_dirs = { "include" }, + linux = { cxxflags = { "-DSPLICED_LEG=1" } }, + macosx = { cxxflags = { "-DSPLICED_LEG=2" } }, + windows = { cxxflags = { "-DSPLICED_LEG=3" } }, +} +LUA + +# The "installed" payload for that descriptor. +PAYLOAD="$TMP/payload/test-x-testsplice/1.0.0" +mkdir -p "$PAYLOAD/src" "$PAYLOAD/include" +cat > "$PAYLOAD/include/splice.h" <<'EOF' +#pragma once +int spliced_leg(); +EOF +cat > "$PAYLOAD/src/splice.cpp" <<'EOF' +#include "splice.h" +#ifndef SPLICED_LEG +#error "no per-OS section was spliced at all" +#endif +int spliced_leg() { return SPLICED_LEG; } +EOF +touch "$PAYLOAD/.mcpp_ok" + +"$MCPP" new crosssplice > /dev/null +cd crosssplice + +cat > src/main.cpp <<'EOF' +#include "splice.h" +int main() { + // 3 == the windows leg. Anything else means the wrong per-OS section + // was spliced for this target. + return spliced_leg() == 3 ? 0 : 1; +} +EOF + +cat > mcpp.toml < build.log 2>&1 +RC=$? +set -e + +if [[ $RC -ne 0 ]]; then + # A wrongly-spliced leg shows up as SPLICED_LEG=1 (linux) reaching the + # windows TU; distinguish that from an unrelated setup failure. + if grep -q "SPLICED_LEG" build.log; then + cat build.log + echo "FAIL: the wrong per-OS section was spliced for target $TRIPLE" + exit 1 + fi + echo "SKIP: cross build could not run in this environment" + sed -n '1,15p' build.log + exit 0 +fi + +# The build succeeding is not enough — prove the windows leg's flag is what +# landed, by checking the recorded compile command for the dep TU. +CC_JSON=$(find target -name compile_commands.json | head -1) +if [[ -n "$CC_JSON" ]]; then + grep -q "SPLICED_LEG=3" "$CC_JSON" || { + grep -o "SPLICED_LEG=[0-9]" "$CC_JSON" | sort -u + echo "FAIL: dep TU was not compiled with the windows leg's flags" + exit 1 + } + grep -q "SPLICED_LEG=1" "$CC_JSON" && { + echo "FAIL: the host (linux) leg leaked into a $TRIPLE build" + exit 1 + } +fi + +echo "OK" diff --git a/tests/unit/test_diag.cpp b/tests/unit/test_diag.cpp index 27ba088d..52b32d85 100644 --- a/tests/unit/test_diag.cpp +++ b/tests/unit/test_diag.cpp @@ -79,3 +79,24 @@ TEST_F(DiagTest, FlushClearsRunState) { EXPECT_EQ(records().size(), 0u); EXPECT_EQ(count(Severity::Degraded), 0u); } + +// Regression guard for the review finding on this batch: flush() is the ONLY +// place the --strict policy is settled, and it was initially never called +// outside this file — the channel reported degradations and then everyone +// ignored them, which is precisely the failure mode it exists to prevent. +// run_build_plan now calls it; this pins the contract flush() must honour. +TEST_F(DiagTest, FlushIsTheSolePolicyPointAndReportsFailureToTheCaller) { + // No records at all: strict must not fail a clean build. + EXPECT_TRUE(flush(/*strict=*/true)); + + // A degradation under --strict must tell the caller to fail. The caller + // (run_build_plan) turns this into a non-zero exit. + degraded("build/depfile", "no depfile on this toolchain", + "stale BMI possible after editing an included file"); + EXPECT_FALSE(flush(/*strict=*/true)); + + // Same degradation without --strict: reported, not fatal. + degraded("build/depfile", "no depfile on this toolchain", + "stale BMI possible after editing an included file"); + EXPECT_TRUE(flush(/*strict=*/false)); +} diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index d5349d4d..753e6861 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -727,6 +727,8 @@ TEST(NinjaBackend, CAndAsmRulesAlsoTrackHeaderDeps) { auto ninja = emit_ninja_string(plan); for (std::string_view rule : {"rule c_object\n", "rule asm_object\n"}) { + // NOTE: asm_object is the `.S` rule. `.s` uses asm_object_raw and + // deliberately has no depfile — see below. auto start = ninja.find(rule); ASSERT_NE(start, std::string::npos) << rule << "\n" << ninja; auto end = ninja.find("\n\n", start); @@ -741,3 +743,48 @@ TEST(NinjaBackend, CAndAsmRulesAlsoTrackHeaderDeps) { EXPECT_EQ(body.find("awk"), std::string::npos) << body; } } + +// The C driver preprocesses `.S` but not `.s`, so only `.S` can produce a +// depfile. Asking anyway makes clang emit "argument unused during +// compilation: '-MMD'" for every such file and write nothing, and ninja's +// `deps = gcc` treats an absent depfile as an error — so the two cases need +// separate rules. +TEST(NinjaBackend, LowercaseAsmHasNoDepfileAndItsOwnRule) { + if constexpr (mcpp::platform::is_windows) + GTEST_SKIP() << "POSIX depfile shape only"; + + auto plan = minimal_plan(); + plan.compileUnits.push_back({ + .source = "src/upper.S", + .object = "obj/upper.o", + .packageName = "objc_rule_test", + }); + plan.compileUnits.push_back({ + .source = "src/lower.s", + .object = "obj/lower.o", + .packageName = "objc_rule_test", + }); + + auto ninja = emit_ninja_string(plan); + + auto body_of = [&](std::string_view rule) { + auto start = ninja.find(rule); + EXPECT_NE(start, std::string::npos) << rule << "\n" << ninja; + auto end = ninja.find("\n\n", start); + return ninja.substr(start, end - start); + }; + + auto upper = body_of("rule asm_object\n"); + EXPECT_NE(upper.find("-MMD -MF $out.d"), std::string::npos) << upper; + EXPECT_NE(upper.find("depfile = $out.d"), std::string::npos) << upper; + + auto lower = body_of("rule asm_object_raw\n"); + EXPECT_EQ(lower.find("-MMD"), std::string::npos) << lower; + EXPECT_EQ(lower.find("depfile"), std::string::npos) << lower; + + // And the edges must be routed to the matching rule. + EXPECT_NE(ninja.find("build obj/upper.o : asm_object src/upper.S"), + std::string::npos) << ninja; + EXPECT_NE(ninja.find("build obj/lower.o : asm_object_raw src/lower.s"), + std::string::npos) << ninja; +} From 58294b44bd44e36e03dd5941289c777c92b9bdf8 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 05:17:29 +0800 Subject: [PATCH 11/13] docs: record the #254 cross-build e2e coverage gap honestly The planned mingw-cross e2e consuming an xpkg dep with per-OS sections did not land: it needs a real cross build inside a throwaway MCPP_HOME, and a fresh home re-installs the entire cross toolchain (measured >10 min). The e2e harness has no fixture for reusing the host registry while injecting a local index and a pre-placed payload. Rather than ship a test that times out, the semantics stay pinned by three unit tests and the gap is written down in both the design doc and the changelog, with what building the fixture would take. --- ...026-07-22-v0.0.102-batch-254-261-design.md | 7 +- CHANGELOG.md | 1 + tests/e2e/151_cross_per_os_xpkg_splice.sh | 125 ------------------ 3 files changed, 5 insertions(+), 128 deletions(-) delete mode 100755 tests/e2e/151_cross_per_os_xpkg_splice.sh diff --git a/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md b/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md index 72a0fac8..d5e3970d 100644 --- a/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md +++ b/.agents/docs/2026-07-22-v0.0.102-batch-254-261-design.md @@ -431,9 +431,10 @@ class TargetPlatform { public: static TargetPlatform from_triple(std::string_vie ### 6.3 验收(零行为变化必须被锁死) -- **e2e:host == target ⇒ 拼接结果逐字节不变**。这是本节点的安全网:native 三平台 CI 的全部现状必须不变。 -- e2e:mingw-cross 腿消费一个带 per-OS `mcpp` 段的 xpkg dep,断言选中 **windows** 段(今天会错选 linux 段)。现有 cross e2e(`102_mingw_cross_wine.sh`、`112_build_mcpp_cross.sh`)都不消费此类 dep,这是新增覆盖。 -- 单测:轴类型互传不可编译(`static_assert(!std::is_convertible_v<...>)`)。 +- **单测:host == target ⇒ 拼接结果逐字节不变**(`NativeBuildSpliceIsUnchanged`)。这是本节点的安全网:native 三平台 CI 的全部现状必须不变。 +- 单测:逐 target OS 断言只拼接自己那一段、不携带另两段(`XpkgPerOsSectionSplicesForTheTargetNotTheHost`)。 +- 单测:轴类型互传不可编译(`static_assert(!std::is_convertible_v<...>)`),且都不能由裸字符串构造。 +- **已知覆盖缺口(实施期记录)**:原计划的"mingw-cross 腿消费带 per-OS 段的 xpkg dep"e2e **未落地**。该用例需要在一个临时 `MCPP_HOME` 里完成真实交叉构建,而临时 home 会触发整套交叉工具链的重新安装(实测超 10 分钟),现有 e2e 基础设施没有共享工具链、注入本地索引与预置 payload 的组合手段。折中:语义由上述两条单测精确锁定;真实交叉构建路径由既有 `102_mingw_cross_wine.sh` / `112_build_mcpp_cross.sh` 覆盖(但它们不消费此类 dep)。**若要补齐,需要先给 e2e 增加"复用宿主 registry + 项目级本地索引 + 预置 payload"的夹具**,单独做。 ### commit diff --git a/CHANGELOG.md b/CHANGELOG.md index 69814588..c6d499cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ ### 备注 - 新增不变量写入批次总账:**任何"因条件不满足而少做一步"的分支,必须要么返回错误,要么经 `diag::degraded` 上报;`log::debug`/`log::verbose` 不算用户可见;丢弃 `std::expected` 返回值视为缺陷。** +- **#254 的覆盖是单测级**:per-OS 段按 target 选段、host==target 逐字节不变、轴类型互不可转,三条单测锁定;原计划的"cross 腿消费带 per-OS 段的 xpkg dep"e2e 未落地(需要 e2e 侧先有"复用宿主 registry + 本地索引 + 预置 payload"的夹具,临时 MCPP_HOME 会触发整套交叉工具链重装),记为已知缺口。 - e2e 新增 148(64 个 include dir 的宽 include 表,POSIX 验内联形态、windows 验 response file)、149(条件段 per-glob flags 四象限:命中生效/覆盖 base 的 removal/非命中零告警/真死 glob 仍告警)、150(#256 canary);118 去掉 `# requires: gcc` 并加 clang 腿。 - **未包含**:#259(根因在 xlings 侧的 dep 静默丢弃,调查结论已发 issue 评论;mcpp 侧另发现 sysroot 兜底是死代码——`resolve_xpkg_path("xim:glibc")` 缺版本必然失败且两处都丢弃返回值——一并另行安排);manifest 未知键策略五处不一致([#263](https://github.com/mcpp-community/mcpp/issues/263),有兼容性面,与 #258 无依赖)。 diff --git a/tests/e2e/151_cross_per_os_xpkg_splice.sh b/tests/e2e/151_cross_per_os_xpkg_splice.sh deleted file mode 100755 index 85d7f11c..00000000 --- a/tests/e2e/151_cross_per_os_xpkg_splice.sh +++ /dev/null @@ -1,125 +0,0 @@ -#!/usr/bin/env bash -# requires: mingw-cross -# #254: an xpkg dependency's per-OS section must be spliced for the RESOLVED -# TARGET, not for the host. -# -# The per-OS sections carry sources, flags, deps and the xpm asset table — -# all describing code compiled INTO the user's build, so they belong to the -# platform the artifacts will run on. They used to key on a compile-time HOST -# constant, so a Linux -> Windows cross build silently got the linux leg. -# -# Native builds cannot observe this (host == target), which is exactly why -# the three-platform CI never caught it: this test has to cross-compile to -# see the difference at all. It uses a local path-index descriptor so it -# needs no network and no real package. -set -e - -TRIPLE=x86_64-windows-gnu - -TMP=$(mktemp -d) -trap "rm -rf $TMP" EXIT -cd "$TMP" - -# A local xpkg index with one descriptor whose per-OS sections differ in a -# way the compiler can prove: each leg defines its own macro, and the TU -# errors out unless exactly the windows leg was spliced. -mkdir -p idx/pkgs/t -cat > idx/pkgs/t/testsplice.lua <<'LUA' -package = { - name = "testsplice", - namespace = "test", -} - -xpm = { - linux = { ["1.0.0"] = { url = "unused", sha256 = "unused" } }, - windows = { ["1.0.0"] = { url = "unused", sha256 = "unused" } }, - macosx = { ["1.0.0"] = { url = "unused", sha256 = "unused" } }, -} - -mcpp = { - sources = { "src/**/*.cpp" }, - include_dirs = { "include" }, - linux = { cxxflags = { "-DSPLICED_LEG=1" } }, - macosx = { cxxflags = { "-DSPLICED_LEG=2" } }, - windows = { cxxflags = { "-DSPLICED_LEG=3" } }, -} -LUA - -# The "installed" payload for that descriptor. -PAYLOAD="$TMP/payload/test-x-testsplice/1.0.0" -mkdir -p "$PAYLOAD/src" "$PAYLOAD/include" -cat > "$PAYLOAD/include/splice.h" <<'EOF' -#pragma once -int spliced_leg(); -EOF -cat > "$PAYLOAD/src/splice.cpp" <<'EOF' -#include "splice.h" -#ifndef SPLICED_LEG -#error "no per-OS section was spliced at all" -#endif -int spliced_leg() { return SPLICED_LEG; } -EOF -touch "$PAYLOAD/.mcpp_ok" - -"$MCPP" new crosssplice > /dev/null -cd crosssplice - -cat > src/main.cpp <<'EOF' -#include "splice.h" -int main() { - // 3 == the windows leg. Anything else means the wrong per-OS section - // was spliced for this target. - return spliced_leg() == 3 ? 0 : 1; -} -EOF - -cat > mcpp.toml < build.log 2>&1 -RC=$? -set -e - -if [[ $RC -ne 0 ]]; then - # A wrongly-spliced leg shows up as SPLICED_LEG=1 (linux) reaching the - # windows TU; distinguish that from an unrelated setup failure. - if grep -q "SPLICED_LEG" build.log; then - cat build.log - echo "FAIL: the wrong per-OS section was spliced for target $TRIPLE" - exit 1 - fi - echo "SKIP: cross build could not run in this environment" - sed -n '1,15p' build.log - exit 0 -fi - -# The build succeeding is not enough — prove the windows leg's flag is what -# landed, by checking the recorded compile command for the dep TU. -CC_JSON=$(find target -name compile_commands.json | head -1) -if [[ -n "$CC_JSON" ]]; then - grep -q "SPLICED_LEG=3" "$CC_JSON" || { - grep -o "SPLICED_LEG=[0-9]" "$CC_JSON" | sort -u - echo "FAIL: dep TU was not compiled with the windows leg's flags" - exit 1 - } - grep -q "SPLICED_LEG=1" "$CC_JSON" && { - echo "FAIL: the host (linux) leg leaked into a $TRIPLE build" - exit 1 - } -fi - -echo "OK" From 8eedf152774182993c7ff83be13998d7897aac07 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 05:29:48 +0800 Subject: [PATCH 12/13] =?UTF-8?q?fix(build):=20#261=20keep=20only=20$local?= =?UTF-8?q?=5Fincludes=20in=20the=20response=20file=20=E2=80=94=20CI=20cau?= =?UTF-8?q?ght=20the=20#247=20trap=20one=20level=20deeper?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows leg failed with module file 'D:amcppmcpptargetx86_64-windows-msvcc1ffdc8b90b687f4pcm.cachestd.pcm' not found i.e. D:\a\mcpp\mcpp\target\...\pcm.cache\std.pcm with every separator eaten. Response-file content is tokenized GNU-style, where backslash is an ESCAPE character. Switching escape_flag_path to generic_string() covered the -I paths this file builds itself, but $cxxflags carries paths produced elsewhere — flags.cppm's -fprebuilt-module-path= and module-file mappings — which are still native-separated. Moving those off the command line broke every `import std;`. The fix is to scope the response file to what this file can guarantee the form of: $local_includes only. $cxxflags / $unit_cxxflags / $asmflags stay inline, where a backslash is just a character. That is also sufficient for #261: the unbounded axis is one -I per dependency include dir, and the rest of the payload is bounded. Making every flag producer emit forward slashes would be the more general fix, but it is a much wider change than this batch should carry, and it is not needed for the ceiling. Only the `mcpp test` step failed, not the build: the build step runs the bootstrap mcpp, so the freshly-built binary is first exercised when it compiles the test targets. Also makes e2e 118 use a portable in-place edit. Dropping its `# requires: gcc` gate in #257 meant it ran on macOS for the first time, where BSD sed reads `-i`'s next argument as a backup suffix and swallows the script. The other three new e2e tests pass on macOS as-is. --- src/build/ninja_backend.cppm | 44 +++++++++++++++--------- tests/e2e/118_purview_include_rebuild.sh | 15 +++++--- tests/unit/test_ninja_backend.cpp | 13 ++++++- 3 files changed, 51 insertions(+), 21 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 841f49e7..0f7c6652 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -554,6 +554,18 @@ std::string emit_ninja_string(const BuildPlan& plan) { // // POSIX keeps the inline form byte-identical: ARG_MAX is ample and an // inline command is far easier to re-run by hand when debugging. + // + // ONLY $local_includes goes in. Response-file content is tokenized + // GNU-style, where backslash is an ESCAPE character, so every path that + // moves off the command line must be forward-slashed — and the only + // paths whose form this file controls are the -I/-idirafter entries it + // builds via escape_flag_path(). $cxxflags carries paths produced + // elsewhere (flags.cppm's -fprebuilt-module-path=, module-file mappings) + // that are still native-separated; routing those through the rsp ate the + // separators of the std.pcm path and broke every `import std;` on the + // Windows CI leg. They stay inline, where a backslash is just a + // character. Sufficient, too: the unbounded axis #261 is about is one -I + // per dependency include dir, and the rest of the payload is bounded. // Also keyed on the msvc dialect, which only ever runs on Windows: it // makes the response-file shape reachable from a non-Windows test host, // the same over-approximation the link rules use (`separateLinker || @@ -576,11 +588,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("rule cxx_module\n"); if constexpr (mcpp::platform::is_windows) { // Windows: skip BMI restat optimization (requires POSIX shell). - const auto payload = std::format( - " $local_includes $cxxflags $unit_cxxflags{}{}", - module_output_flag, module_src_flags); - append(std::format(" command = $cxx{} {}\n", - rsp_ref(payload), compile_tail)); + const std::string payload = " $local_includes"; + append(std::format(" command = $cxx{} $cxxflags $unit_cxxflags{}{} {}\n", + rsp_ref(payload), module_output_flag, + module_src_flags, compile_tail)); append_rspfile(payload); append_cxx_deps(); } else { @@ -604,8 +615,8 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("rule cxx_object\n"); if constexpr (mcpp::platform::is_windows) { - const std::string payload = " $local_includes $cxxflags $unit_cxxflags"; - append(std::format(" command = $cxx{} {}\n", + const std::string payload = " $local_includes"; + append(std::format(" command = $cxx{} $cxxflags $unit_cxxflags {}\n", rsp_ref(payload), compile_tail)); append_rspfile(payload); } else { @@ -621,8 +632,8 @@ std::string emit_ninja_string(const BuildPlan& plan) { if (need_c_rule) { append("rule c_object\n"); - const std::string payload = " $local_includes $cflags $unit_cflags"; - append(std::format(" command = $cc{} {}{}\n", + const std::string payload = " $local_includes"; + append(std::format(" command = $cc{} $cflags $unit_cflags {}{}\n", rsp_ref(payload), c_mmd_flag, compile_tail)); append_rspfile(payload); append(" description = CC $out\n"); @@ -641,9 +652,9 @@ std::string emit_ninja_string(const BuildPlan& plan) { // clang emits `argument unused during compilation: '-MMD'` for every // such file and writes nothing, and ninja's `deps = gcc` treats an // absent depfile as an error. So `.s` keeps the pre-#257 shape. - const std::string payload = " $local_includes $asmflags $unit_asmflags"; + const std::string payload = " $local_includes"; append("rule asm_object\n"); // .S — preprocessed, tracks #include - append(std::format(" command = $cc{} {}{}\n", + append(std::format(" command = $cc{} $asmflags $unit_asmflags {}{}\n", rsp_ref(payload), c_mmd_flag, compile_tail)); append_rspfile(payload); append(" description = AS $out\n"); @@ -651,7 +662,7 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("\n"); append("rule asm_object_raw\n"); // .s — not preprocessed, no depfile - append(std::format(" command = $cc{} {}\n", + append(std::format(" command = $cc{} $asmflags $unit_asmflags {}\n", rsp_ref(payload), compile_tail)); append_rspfile(payload); append(" description = AS $out\n\n"); @@ -733,16 +744,16 @@ std::string emit_ninja_string(const BuildPlan& plan) { // The scan command is strictly LONGER than the compile command for // the same TU (it wraps it), so it carries the same unbounded // include payload through the same response file (#261). - const std::string scanPayload = " $local_includes $cxxflags $unit_cxxflags"; + const std::string scanPayload = " $local_includes"; if (msvcDeps) { // MSVC: compiler-integrated P1689 via /scanDependencies (scan // only — no codegen); /TP because our module units are .cppm. - append(std::format(" command = $cxx{} " + append(std::format(" command = $cxx{} $cxxflags $unit_cxxflags " "/scanDependencies $out /TP /c $in /Fo:$compile_target\n", rsp_ref(scanPayload))); } else if (plan.scanDepsPath.empty()) { // GCC path: compiler-integrated P1689 scanning. - append(std::format(" command = $cxx{} -fmodules " + append(std::format(" command = $cxx{} $cxxflags $unit_cxxflags -fmodules " "-fdeps-format=p1689r5 " "-fdeps-file=$out -fdeps-target=$compile_target " "-M -MM -MF $out.dep -E $in -o $compile_target\n", @@ -757,7 +768,8 @@ std::string emit_ninja_string(const BuildPlan& plan) { // overruns (#261: 48 -I entries at a deep consumer path). append(std::format( " command = $scan_deps -format=p1689 -o $out -- " - "$cxx{} -c $in -o $compile_target\n", rsp_ref(scanPayload))); + "$cxx{} $cxxflags $unit_cxxflags -c $in -o $compile_target\n", + rsp_ref(scanPayload))); } append_rspfile(scanPayload); append(" description = SCAN $out\n\n"); diff --git a/tests/e2e/118_purview_include_rebuild.sh b/tests/e2e/118_purview_include_rebuild.sh index 892d0c7f..afb92260 100755 --- a/tests/e2e/118_purview_include_rebuild.sh +++ b/tests/e2e/118_purview_include_rebuild.sh @@ -17,6 +17,13 @@ set -e # Resolved before any cd: the script changes directory below. SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" +# Portable in-place edit. This test lost its `# requires: gcc` gate in #257, +# so it now runs on macOS too, where BSD sed reads `-i`'s next argument as a +# backup suffix and swallows the script. +subst() { # subst + sed "$1" "$2" > "$2.tmp" && mv "$2.tmp" "$2" +} + TMP=$(mktemp -d) trap "rm -rf $TMP" EXIT @@ -51,7 +58,7 @@ EOF out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "41" ]] || { echo "unexpected initial output: $out"; exit 1; } -sed -i 's/41/42/' src/vals.inc +subst 's/41/42/' src/vals.inc out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "42" ]] || { @@ -83,7 +90,7 @@ EOF out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "42 100" ]] || { echo "unexpected output before header edit: $out"; exit 1; } -sed -i 's/100/200/' src/helper.h +subst 's/100/200/' src/helper.h out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "42 200" ]] || { @@ -130,7 +137,7 @@ EOF out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "41" ]] || { echo "clang: unexpected initial output: $out"; exit 1; } - sed -i 's/41/42/' src/vals.inc + subst 's/41/42/' src/vals.inc out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "42" ]] || { @@ -158,7 +165,7 @@ EOF out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "42 100" ]] || { echo "clang: unexpected output before header edit: $out"; exit 1; } - sed -i 's/100/200/' src/helper.h + subst 's/100/200/' src/helper.h out="$("$MCPP" run 2>&1 | tail -1)" [[ "$out" == "42 200" ]] || { diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 753e6861..94fe8ebe 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -637,13 +637,24 @@ TEST(NinjaBackend, CompileAndScanRulesRouteFlagsThroughRspfileUnderMsvcDialect) EXPECT_NE(body.find("@$out.rsp"), std::string::npos) << body; EXPECT_NE(body.find("rspfile = $out.rsp"), std::string::npos) << body; - EXPECT_NE(body.find("rspfile_content = $local_includes"), + // ONLY $local_includes may live in the response file: its content + // is tokenized GNU-style (backslash = escape), and those are the + // only paths this file forward-slashes itself. $cxxflags carries + // native-separated paths from flags.cppm and must stay inline — + // routing it through the rsp ate the separators of the std.pcm path + // and broke every `import std;` on Windows. + EXPECT_NE(body.find("rspfile_content = $local_includes\n"), std::string::npos) << body; // The payload must not ALSO remain inline, or the ceiling stands. auto cmdStart = body.find("command = "); auto cmdEnd = body.find('\n', cmdStart); auto cmd = body.substr(cmdStart, cmdEnd - cmdStart); EXPECT_EQ(cmd.find("$local_includes"), std::string::npos) << cmd; + // ...and the flags that must NOT move off the command line stay there. + if (rule != "rule c_object\n") + EXPECT_NE(cmd.find("$cxxflags"), std::string::npos) << cmd; + else + EXPECT_NE(cmd.find("$cflags"), std::string::npos) << cmd; } } From 6e736826f089efc4c6ec7f2d7c51c3b8a232ccac Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 22 Jul 2026 05:51:57 +0800 Subject: [PATCH 13/13] test(e2e): 118 asserts the depfile degradation on Windows instead of a capability it lacks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows plus a GNU-dialect toolchain — which is what the CI leg's clang is — genuinely cannot track textual includes: the depfile GCC emits for a module TU needs an awk filter to be loadable by ninja, and native Windows has no awk. #257 does not fix that platform; it makes the engine SAY so through diag::degraded, and the Windows CI log carries that message 13 times. Dropping the `# requires: gcc` gate pointed the test at that platform for the first time, where it asserted a rebuild that cannot happen. Asserting the capability there would be wrong, and re-gating the test to POSIX would put the gap back out of sight. So on Windows the test now asserts the degradation IS reported and stops. That is the stronger contract: silence is the actual defect, and this pins it. Everywhere else the rebuild assertions are unchanged. --- tests/e2e/118_purview_include_rebuild.sh | 28 +++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/tests/e2e/118_purview_include_rebuild.sh b/tests/e2e/118_purview_include_rebuild.sh index afb92260..64f5c95f 100755 --- a/tests/e2e/118_purview_include_rebuild.sh +++ b/tests/e2e/118_purview_include_rebuild.sh @@ -24,6 +24,19 @@ subst() { # subst sed "$1" "$2" > "$2.tmp" && mv "$2.tmp" "$2" } +# Windows + a GNU-dialect toolchain (the CI leg's clang) is the one +# combination that genuinely CANNOT track textual includes: the depfile GCC +# emits for a module TU needs an awk filter to be loadable by ninja, and +# native Windows has no awk. #257 does not fix that — it makes the engine SAY +# so, through diag::degraded. On that platform this test therefore asserts +# the degradation is reported rather than asserting a capability the build +# does not have; silence would be the actual defect. +case "$(uname -s)" in + MINGW*|MSYS*|CYGWIN*) EXPECT_TRACKING=0 ;; + *) EXPECT_TRACKING=1 ;; +esac +DEGRADED_MSG="emits no GNU depfile" + TMP=$(mktemp -d) trap "rm -rf $TMP" EXIT @@ -55,9 +68,22 @@ name = "purviewinc" version = "0.1.0" EOF -out="$("$MCPP" run 2>&1 | tail -1)" +run_log=$("$MCPP" run 2>&1) +out="$(echo "$run_log" | tail -1)" [[ "$out" == "41" ]] || { echo "unexpected initial output: $out"; exit 1; } +if [[ $EXPECT_TRACKING -eq 0 ]]; then + echo "$run_log" | grep -q "$DEGRADED_MSG" || { + echo "$run_log" + echo "FAIL: this toolchain/platform cannot emit a depfile, and said nothing." + echo " A capability gap must be reported, not silent (#257)." + exit 1 + } + echo " windows: depfile degradation reported as expected; rebuild tracking not asserted" + echo "OK" + exit 0 +fi + subst 's/41/42/' src/vals.inc out="$("$MCPP" run 2>&1 | tail -1)"