From c164c1af1ab2e6b5ab12e04d39b4ecc45357afbb Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 11:27:07 +0800 Subject: [PATCH 1/8] fix(build): stage BMIs through mcpp instead of an in-place shell copy (#311) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows-only symptom, three overlapping defects. 1. `rule cp_bmi` overwrote the destination IN PLACE (`powershell Copy-Item -Force` / `cp -f`). mcpp writes the staged std BMI path into compile_commands.json so clangd can resolve `import std;`, clangd memory-maps that very file, and Windows refuses to replace a file with an open user-mapped section — error 1224, reported as a bare "build failed". Staging now runs through `mcpp stage` (new internal subcommand, same shape as `mcpp dyndep`): skip when the destination is already equivalent, else temp-file + rename, else in-place, with retries, and a failure message that names the file and the likely holder. Never downgraded to a warning — a stale BMI turns into a confusing "module 'std' not found" or a silently mismatched link. The rule is shared with Windows runtime-DLL deployment, which had the same hazard against a program still running from a previous `mcpp run`. 2. `default_cache_root()` was a private copy of the home resolution, unchanged since v0.0.1: no %USERPROFILE% branch, no self-contained detection. On Windows PowerShell (no $HOME) the std BMI cache landed in the *current working directory* as `.mcpp-bmi/` while dep BMIs went to %USERPROFILE%\.mcpp\bmi — two roots for one cache, the second cwd-dependent, which is what made a re-stage a routine event. New leaf module `mcpp.home` is now the single resolver (config, stdmod, prepare's git cache and two more copies in doctor all route through it). 3. The staging rule carried no `restat`, so any re-stage recompiled everything that imports std even when the bytes were identical. Adding `restat = 1` on top of the no-write path fixes that — measured: touching the cache-side BMI now runs the stage edge alone, and the next build is "no work to do". Note for the record: aligning the destination's mtime with the source defeats restat and re-triggers the cascade, so a skipped stage touches no timestamps at all. Also: `FAILED: ` survives the ninja output filter (normalized to `failed: `) — dropping it is why the report couldn't tell a staging failure from a compile error; `mcpp new` ignores `.mcpp/`; `mcpp doctor` points at a leftover `.mcpp-bmi/`. Design + plan: .agents/docs/2026-07-30-issue311-*.md Tests: unit test_home (4), test_build_stage (10), test_ninja_backend (+4), e2e 170 (no-cascade + cache root), e2e 171 (held destination, PowerShell MemoryMappedFile on Windows — reproduces #311 without needing clangd). --- ...ue311-bmi-staging-and-cache-root-design.md | 361 ++++++++++++++++++ ...2026-07-30-issue311-implementation-plan.md | 223 +++++++++++ src/build/execute.cppm | 6 +- src/build/ninja_backend.cppm | 63 ++- src/build/prepare.cppm | 9 +- src/build/stage.cppm | 219 +++++++++++ src/cli.cppm | 11 +- src/cli/cmd_build.cppm | 40 +- src/config.cppm | 59 +-- src/doctor.cppm | 33 +- src/home.cppm | 97 +++++ src/scaffold/create.cppm | 4 +- src/toolchain/stdmod.cppm | 13 +- tests/e2e/170_bmi_staging_no_cascade.sh | 87 +++++ tests/e2e/171_bmi_staging_locked_dest.sh | 101 +++++ tests/unit/test_build_stage.cpp | 181 +++++++++ tests/unit/test_home.cpp | 83 ++++ tests/unit/test_ninja_backend.cpp | 94 +++++ 18 files changed, 1575 insertions(+), 109 deletions(-) create mode 100644 .agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md create mode 100644 .agents/docs/2026-07-30-issue311-implementation-plan.md create mode 100644 src/build/stage.cppm create mode 100644 src/home.cppm create mode 100755 tests/e2e/170_bmi_staging_no_cascade.sh create mode 100755 tests/e2e/171_bmi_staging_locked_dest.sh create mode 100644 tests/unit/test_build_stage.cpp create mode 100644 tests/unit/test_home.cpp diff --git a/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md b/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md new file mode 100644 index 00000000..9cabe078 --- /dev/null +++ b/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md @@ -0,0 +1,361 @@ +# BMI staging 原语 + BMI 缓存根收敛 — 设计 + +日期：2026-07-30 +状态：设计定稿，待实施 +关联：#311（Windows 上 clangd 导致 mcpp build 假失败） +建议目标版本：**2026.7.30.1** + +--- + +## 1. 摘要 + +issue #311 报的症状真实：Windows 上 clangd 把 `pcm.cache/std.pcm` 内存映射住，mcpp 的 +`powershell Copy-Item -Force` 原地覆写同一文件 → Windows error 1224 +（`ERROR_USER_MAPPED_FILE`）→ ninja edge 失败 → `error: build failed`。 + +但这不是一个 bug，而是**三个互相放大的缺陷**叠在同一条链上： + +| 编号 | 缺陷 | 层级 | +|---|---|---| +| **D1** | std BMI staging 用 `Copy-Item -Force` **原地覆写**：非原子、无重试、不判内容等价 | 构建后端 | +| **D2** | `default_cache_root()` 是 home 解析器的第三份拷贝：Windows 无 `USERPROFILE` 分支 ⇒ 缓存根落到 **cwd**，且与 `cfg.bmiCacheDir` 分家 | 路径解析 | +| **D3** | staging edge 无 `restat`⇒ 重新 stage 必级联全量重编；失败诊断无可执行信息 | 构建后端 / 诊断 | + +D2 是**触发器**：它让"内容完全相同但 staging edge 变脏"成为常态，从而把 D1 这个平时藏着的 +脆弱点顶到用户面前；D3 决定了它爆出来时有多难懂、多贵。 + +本设计只做 D1/D2/D3 的收敛，**不做**"clang/MSVC 引用即用、彻底取消拷贝"（见 §7，单独批次）。 + +--- + +## 2. 现状机制（逐段核过的链条） + +### 2.1 mcpp 主动把这个文件交给 clangd + +```cpp +// src/build/flags.cppm:357-362 +if (!traits.stdBmiUsePrefix.empty() && !plan.stdBmiPath.empty()) { + std_module_flag = std::string(traits.stdBmiUsePrefix) // clang: " -fmodule-file=std=" + + escape_path(staged_std_bmi_path(plan)); // 绝对路径，指向 build dir 内的副本 +} +// src/build/flags.cppm:369-382 +prebuilt_module_flag = std::string(traits.bmiSearchPrefix) // " -fprebuilt-module-path=" + + escape_path(plan.outputDir / traits.bmiDir); +``` + +这两个 flag 一起进 `compile_commands.json`（`src/build/compile_commands.cppm:127-170`， +`arguments` 数组直接来自同一个 `flags.cxx`）。`flags.cppm:371-381` 的注释写得很清楚：这里 +刻意用绝对路径，**就是为了让 clangd 能解析** `import std;`。 + +于是：clangd 解析任何 `import std;` 的 TU → 前端 ASTReader 加载该 `.pcm` → LLVM MemoryBuffer +对这种体量（本机实测 GCC 侧 `std.gcm` = 31,466,112 字节；clang 侧同量级）走 mmap → Windows +上即 `CreateFileMapping`，文件带 user-mapped section，且 clangd 会把 AST/preamble 缓存住。 + +### 2.2 mcpp 同时在原地覆写它 + +```cpp +// src/build/ninja_backend.cppm:411-419 +append("rule cp_bmi\n"); +if constexpr (mcpp::platform::is_windows) { + append(" command = powershell -NoProfile -Command \"Copy-Item -Force '$in' -Destination '$out'\"\n"); +} else { + append(" command = mkdir -p $$(dirname $out) && cp -f $in $out\n"); +} +append(" description = STAGE $out\n\n"); +``` + +```cpp +// src/build/ninja_backend.cppm:786-812（四条 staging edge） +build pcm.cache/std.pcm : cp_bmi //pcm.cache/std.pcm +build obj/std.o : cp_bmi //std.o +build pcm.cache/std.compat.pcm : cp_bmi ... | pcm.cache/std.pcm +build obj/std.compat.o : cp_bmi ... +``` + +`Copy-Item -Force` 打开目标写入 ⇒ 目标带 mapped section ⇒ 1224。 + +**POSIX 分支天然免疫**，这也是为什么 CI 全绿：GNU `cp -f` 在目标打不开时会 unlink 重建， +而 POSIX 本身也允许覆写被 mmap 的文件。本机对照实验（scratch 工程，GCC）： + +``` +$ chmod 444 target/x86_64-linux-gnu//gcm.cache/std.gcm +$ touch ~/.mcpp/bmi//gcm.cache/std.gcm # 让缓存侧变新，把 edge 弄脏 +$ mcpp build -v +[1/3] mkdir -p ... && cp -f ~/.mcpp/bmi//gcm.cache/std.gcm gcm.cache/std.gcm ← 成功 +[2/3] g++ ... -c src/main.cpp -o obj/main.o ← 级联重编 +[3/3] g++ obj/main.o obj/std.o -o bin/demo311 +$ ls -l target/.../gcm.cache/std.gcm +-rw-rw-r-- ... 31466112 ← 权限被 unlink-重建抹掉 +``` + +这个实验同时验证了 D3 的级联：staged BMI 是每个 importer 的 implicit input +（`ninja_backend.cppm:964/971`），`cp -f` 不保留 mtime、rule 又没有 `restat = 1`， +所以**只要重新 stage 一次，所有 `import std;` 的 TU 全部重编**——即使字节完全相同。 + +### 2.3 失败如何呈现给用户 + +edge 失败 → ninja 非零 → `execute.cppm:437-448`： + +- `is_stale_ninja_failure()`（`execute.cppm:207-219`）不匹配 → 不走重生成； +- `mcpp::ui::error("build failed")`； +- `filter_ninja_output()`（`ninja_backend.cppm:323-345`）丢掉 `FAILED: ` 行； + `powershell ...` 不匹配任何编译器前缀（`command_prefixes()` 只收 cxx/cc/ar/scan_deps， + `ninja_backend.cppm:278-291`；`read_ninja_command_prefixes()` 同，`execute.cppm:181-205`） + ⇒ **命令行被保留、失败目标被丢掉**。 + +用户最终看到的就是 issue 里贴的那三行：一句 `error: build failed` + 一条 PowerShell 命令 + +一句 Windows 错误。既看不出这是 BMI staging（不是编译错误），也没有任何"该怎么办"。 + +### 2.4 报告中不成立的一处 + +> "编译和链接实际已成功，二进制已生成，只是 post-build copy 失败。" + +冷构建下不成立：staged std BMI 是所有 importer 的 implicit input，copy 失败时它们根本不会 +跑，这是**真·阻塞失败**。这句话只在增量场景下成立——而制造该场景的正是 D2： + +``` +换个 cwd 跑 mcpp build + → default_cache_root() = <新 cwd>/.mcpp-bmi （D2） + → 该目录下无 std BMI → 重新 precompile + → build.ninja 里 staging edge 的 $in 路径变了 + → build dir 仍是 target//（只由 fingerprint 定名，不含缓存根） + → 于是"其它全 up-to-date，只有这条 copy edge 脏了" + → Windows 上它失败，而上一次的二进制还躺在盘上 +``` + +report 是 AI 润色的（作者自己标注了），因果叙述串了，但症状与最终状态自洽。 + +--- + +## 3. D2：缓存根解析为什么会分家 + +```cpp +// src/toolchain/stdmod.cppm:179-187 —— 来自 v0.0.1（git log -L179,187 → 92f1335），一字未改 +std::filesystem::path default_cache_root() { + if (auto* e = std::getenv("MCPP_HOME"); e && *e) return path(e) / "bmi"; + if (auto* e = std::getenv("HOME"); e && *e) return path(e) / ".mcpp" / "bmi"; + return std::filesystem::current_path() / ".mcpp-bmi"; +} +``` + +而正式解析器后来补齐了两件这里没有的事（`src/config.cppm:313-353`）： + +1. **Windows `USERPROFILE`**（`default_mcpp_home()`，:315-318）； +2. **self-contained 探测**：`/bin/mcpp` 形态即认 `` 为 home，并排除 + `target/` 与 `data/xpkgs/` 两种祖先（`home_dir()`，:329-351）。 + +且 `cfg.bmiCacheDir = cfg.mcppHome / "bmi"`（:510）**已经是**这个缓存的规范位置。 +同一逻辑在仓库里一共三份：`config.cppm:313`、`stdmod.cppm:179`、`prepare.cppm:2826` +（git dep 缓存的内联 lambda）。 + +### 3.1 后果 + +| 场景 | dep BMI 缓存（`cfg.bmiCacheDir`） | std BMI 缓存（`default_cache_root()`） | +|---|---|---| +| `MCPP_HOME` 已设 | `$MCPP_HOME/bmi` | `$MCPP_HOME/bmi` ✅ | +| Linux + `xlings install`（xpkgs 形态） | `~/.mcpp/bmi` | `~/.mcpp/bmi` ✅ | +| release tarball 解包（`/bin/mcpp`） | `/bmi` | `~/.mcpp/bmi` ❌ 分家 | +| **Windows PowerShell/cmd（无 `HOME`）** | `%USERPROFILE%\.mcpp\bmi` | **`\.mcpp-bmi`** ❌ | + +issue 日志里出现 `.mcpp-bmi//pcm.cache/std.pcm` 本身就是这个第三兜底分支的铁证—— +纯靠想象编报告的人更可能写成 `~/.mcpp/bmi`。 + +派生问题： + +- **cwd 相关**：从子目录跑 = 另一个缓存根 = 重编一次 std（几十 MB、十几秒）； +- **工程被污染**：`mcpp new` 生成的 `.gitignore` 只有 `target/` + （`src/scaffold/create.cppm:284-287`），所以 `.mcpp-bmi/` 会以 untracked 状态躺在用户 + 工程里。mcpp 自己的仓库靠 `.gitignore` 里手写的 `/.mcpp-bmi/` 掩盖了这个papercut； +- **清理/诊断指错地方**：`mcpp clean --bmi-cache`（`execute.cppm:1130`）、 + `doctor`（`doctor.cppm:155/192`）、`bmi_cache/maintenance.cppm:53/167` 全用 + `default_cache_root()`，而 `config.cppm:219-220` 的重置路径删的是 `cfg.bmiCacheDir` + ——两边可能根本不是同一个目录。 + +### 3.2 为什么名字是 `.mcpp-bmi` 而不是 `.mcpp/bmi` + +没有设计意图，就是 v0.0.1 时随手写的平级名字。工程内 `.mcpp/` 目录**早已是既有约定** +（per-project xlings sandbox，`config.cppm:122/128`），`config.cppm:321` 的同类兜底也写的是 +`current_path() / ".mcpp"`。所以工程内兜底的正确形态是 `/.mcpp/bmi`。 + +--- + +## 4. 设计 + +### S1 — `mcpp stage`：一个可判等、可重试、原子的 staging 原语 + +新增内部子命令（形态对齐既有的 `mcpp dyndep`：`cli.cppm:496-511` + +`cli/cmd_build.cppm:187`，同样"由 ninja 调用、不进 help 主屏"）： + +``` +mcpp stage --output +``` + +语义表（**顺序即优先级**）： + +| 步 | 条件 | 行为 | +|---|---|---| +| 1 | `src` 不存在 | 报错退出 1（这是真 bug，不容忍） | +| 2 | `dst` 存在 且 `size(dst) == size(src)` 且（`--verify content` 时字节亦相等） | **一个字节都不写，也不动任何时间戳**；退出 0 | +| 3 | 否则 | 写 `dst.tmp.`（同目录）→ `rename` 覆盖 `dst` | +| 4 | 步 3 失败 | 退化为原地覆写（`copy_file` overwrite）——赢下"可写但不可删"的持有者形态 | +| 5 | 步 3+4 都失败 | 退避重试 3 次（100/300/900 ms）后仍失败 → 结构化诊断 + 退出 1 | + +关键判据（**为什么步 2 是安全的，而不是"偷懒跳过"**）： + +> build dir 与缓存目录用的是**同一个 fingerprint**（本机可见： +> `target/x86_64-linux-gnu/284e9f...` ↔ `~/.mcpp/bmi/284e9f...`，均来自 +> `prepare.cppm:3612` 传入的 `fp.hex`）。fingerprint 已覆盖编译器身份/版本/target +> triple/stdlib/std 源哈希/标准与方言 flag（`stdmod.cppm:88-112` 的 metadata 同源）。 +> 同 fp ⇒ staged BMI 与缓存 BMI 语义等价 ⇒ 已存在即正确。 + +这不是新发明的判据，**dep BMI 那条路径已经在这么做**了： + +```cpp +// src/bmi_cache.cppm:57-60 +// Copy missing cached files into projectTarget/{bmiDirName,obj}. Existing +// project outputs are left untouched: BMIs may differ byte-for-byte between +// equivalent builds, and overwriting them would dirty downstream modules. +``` + +也就是说：**std staging 是全仓库唯一强制覆写的那一处**，这个不对称本身就是缺陷。 +S1 把它拉回一致。 + +内容校验默认关闭（只比 size）：上面的 fp 判据已经足够强，逐字节比较要多读两遍 31 MB。 +留开关是为了排障（`--verify content` / `MCPP_STAGE_VERIFY=content`）。选逐字节比较而不是 +hash：同样的 I/O 成本，但没有碰撞面，且能提前退出。 + +**为什么步 2 连时间戳都不碰**（这一条实测修正过一次）： + +| 做法 | 下游是否被级联 | 之后 edge 状态 | +|---|---|---| +| 跳过 + 不动任何时间戳 | **否** ✅ | 一次 stage 后即干净（下次 `no work to do`）✅ | +| 跳过 + mtime := `src` 的 mtime | **是** ← 实测重现：`main.cpp` 被重编 | 干净 | +| 跳过 + mtime := now | 是 | 干净 | + +ninja 的 restat 语义是"**mtime 未被命令改变**的输出视为从未需要构建"。只要我们动了 mtime +（哪怕只是对齐到 src），ninja 就认为输出被更新了，级联照旧发生。**任何形式的 mtime 触碰都是 +错的。** + +第一行的两个好处是实测确认的（demo 工程，`touch` 缓存侧 BMI 后连续两次构建）： + +``` +$ touch ~/.mcpp/bmi//gcm.cache/std.gcm && mcpp build -v +[1/3] mcpp stage --output gcm.cache/std.gcm ~/.mcpp/bmi//gcm.cache/std.gcm + Finished release [optimized] in 0.00s ← main.cpp 没有被重编 +$ mcpp build -v +ninja: no work to do. ← edge 也没有一直脏着 +``` + +第二次就干净，是因为 restat 分支下 ninja 会把 build log 里该输出的记录推进到输入的时间， +所以"不动 mtime"既不级联、也不会每次构建都重跑——比设计初稿预期的更好。 + +失败诊断文案（步 5）——必须点名"是什么文件、谁可能持有、怎么办"： + +``` +error: cannot stage BMI into the build directory + file: + from: + os error: 1224 (the requested operation cannot be performed on a file with + a user-mapped section open) +hint: another process has this file memory-mapped or open. The usual holder is + clangd (mcpp writes this exact path into compile_commands.json so clangd + can resolve `import std;`), an editor/IDE indexer, or antivirus. + Close the editor or restart clangd, then re-run `mcpp build`. +``` + +### S2 — 一个 home 解析器：新叶模块 `mcpp.home` + +新建 `src/home.cppm`（`export module mcpp.home;`），只依赖 `std` + `mcpp.platform`： + +```cpp +namespace mcpp::home { +std::filesystem::path root(); // = 现 config.cppm:324 home_dir() 逐字搬迁 +std::filesystem::path bmi_root(); // = root() / "bmi" +} +``` + +- `config.cppm` 的 `home_dir()`/`default_mcpp_home()` 删除，`cfg.mcppHome = mcpp::home::root()`， + `cfg.bmiCacheDir = mcpp::home::bmi_root()`； +- `stdmod.cppm::default_cache_root()` 改为 `return mcpp::home::bmi_root();` + （工程内兜底随之变成 `/.mcpp/bmi`，与 `config.cppm:321` 同形）； +- `prepare.cppm:2826-2832` 的内联 lambda 换成 `mcpp::home::root()`。 + +模块图无环：`mcpp.config` 的 import 闭包是 `{libs.toml, pm.index_spec, xlings→{pm.compat, +platform, log}, platform, log, fallback.{xlings_binary, config_migration, install_integrity}}`， +其中没有任何 `mcpp.toolchain.*`；反向新增 `mcpp.toolchain.stdmod → mcpp.home` 也不成环 +（`mcpp.home` 是叶）。 + +命名说明：不叫 `mcpp.paths` / `mcpp.utils`——按仓库约定（口袋模块名禁用），这个模块只有 +一个职责：解析 MCPP_HOME 及其子目录。 + +### S3 — `restat = 1` + rule 更名 + +staging rule 加 `restat = 1`。这是 S1 步 2"不写字节"能真正省掉级联的前提：ninja 在 edge +跑完后复核输出，mtime 未变则把下游标记为干净。 + +顺手把 rule 名从 `cp_bmi` 改成 `stage_file`：它早就不只搬 BMI 了——Windows 运行期 DLL +部署复用的是同一条 rule（`ninja_backend.cppm:1077-1083`）。**DLL 部署是同一类失败的第二个 +受害者**（往正在运行的进程/调试器已加载的 DLL 上 `Copy-Item -Force` 同样失败），S1 一并治好。 + +另有一处必改：`mcpp = ` 这个 ninja 变量目前只在 `if (dyndep)` 里绑定 +（`ninja_backend.cppm:403-409`）。新 rule 在**所有**配置下都要用它，必须把绑定提到该 +条件之外，否则 GCC 非 dyndep 路径会生成 `$mcpp` 为空的命令。 + +### S4 — 诊断收敛 + +1. `command_prefixes()`（`ninja_backend.cppm:278-291`）加入 `mcpp_exe_path()`， + `read_ninja_command_prefixes()`（`execute.cppm:181-205`）的白名单 key 加 `"mcpp"`： + 这样过滤器会吃掉**被回显的命令行**、保留 mcpp 自己的诊断正文——正好是我们想要的。 +2. `filter_ninja_output()` 对 `FAILED:` 不再一刀切丢弃：保留目标名（改成一行 + `failed: `）。当前把"哪个输出失败了"整行丢掉，是 #311 里那段输出读不懂的 + 直接原因之一。 + +### S5 — 兜底路径的可见性（低成本 papercut） + +1. `mcpp new` 的 `.gitignore` 模板（`scaffold/create.cppm:284-287`）加 `.mcpp/` + ——它同时也是 per-project xlings sandbox 的目录，本来就该忽略； +2. `doctor` 若在 cwd/工程根发现遗留 `.mcpp-bmi/`，打一行提示"legacy BMI cache, safe to + delete"（不自动删）。 + +--- + +## 5. 不采纳 / 需要纠正的方案 + +| 方案 | 判定 | 理由 | +|---|---|---| +| issue 建议 1：copy 前比内容 | **采纳**（= S1 步 2） | 方向正确；实现上先比 size，hash 作为可选 | +| issue 建议 2：copy 失败降级为 warning | **拒绝** | staged BMI 缺失/过期时，后续要么报莫名的 `module 'std' not found`，要么拿旧 BMI 配新 `std.o` 链接。这是把硬失败换成静默错误。只有"已验证等价"时跳过才安全，而那时它是 no-op success，不是 warning | +| issue 建议 3：`Move-Item` + rename 原子替换 | **部分采纳，但不是对症药** | Windows 上删除/改名带 mapped section 的文件同样返回 1224（rename-over 需要目标可删）。它只能挡杀软扫描造成的瞬时 sharing violation(32)。S1 把它放在步 3（先试），并保留步 4/5 | +| 内容寻址的 staged 文件名（`std-.pcm`，永不覆写） | 不采纳 | build dir 已按 fingerprint 隔离，同 fp 下"存在即正确"已足够；再加一层命名只会让产物堆积与 flag 生成复杂化 | +| 完全改为 prepare 阶段进程内 staging、取消 ninja edge | 不采纳（本批次） | 更干净，但会丢掉"staged 文件被手动删除时 ninja 自动补齐"这条鲁棒性，且 `mcpp run/test` 的 cached-ninja 快路径不经 prepare。留作 §7 的一部分 | + +--- + +## 6. 风险与验证 + +| 风险 | 缓解 | +|---|---| +| size 相等但内容不同（S1 步 2 误跳过） | fp 判据 + `--verify=hash` 开关；e2e 用"改一个字节但保持长度"的负例锁住 hash 模式 | +| `restat = 1` 引入意外的"永不重建" | 单测锁 rule 文本；e2e 正例：改 std 源/换 toolchain ⇒ fp 变 ⇒ 新 build dir，不受影响 | +| 收敛缓存根导致老用户一次性重编 std | 只影响 self-contained 与 Windows 两种形态；一次 10–60 s；CHANGELOG 写明，doctor 提示遗留目录 | +| `mcpp` 变量提取出 `if (dyndep)` 后 ninja 文本变化 | `tests/unit/test_ninja_backend.cpp` 断言两种配置（dyndep on/off）下都绑定 | +| Windows 行为无法在 Linux 验证 | e2e 用 `MemoryMappedFile::CreateFromFile` 在 Windows CI 上**不依赖 clangd**地精确复现（见实施计划 P5） | + +必须新增的验证（详见实施计划）： + +- **反级联**（Linux 可跑）：`touch` 缓存侧 BMI → `mcpp build -v` 中**不得**出现 `main.cpp` 的 + 编译行。这条正是今天实测会失败的行为。 +- **锁定目标**（Windows only）：PowerShell 映射住 staged BMI → `mcpp build` 必须**成功** + （走 S1 步 2）；内容确实不同时必须失败并带 hint 文案。 + +--- + +## 7. 后续批次（不在本设计范围） + +**零拷贝：clang/MSVC 直接引用缓存里的 BMI。** +`-fmodule-file=std=` / `/reference std=` 接受任意绝对路径，`std.o` 也可以直接 +进链接行。真正需要文件落在 build dir 的只有 **GCC**（`import std;` 默认按 cwd 相对的 +`gcm.cache/std.gcm` 查找，除非改用 `-fmodule-mapper`）。收益：每个 build dir 少几十 MB +I/O，这类锁冲突结构性消失。代价：build dir 不再自包含（缓存被 GC/`clean --bmi-cache` +时会踩空），且要重新审计 `std.compat` 的 BMI 内记录路径。值得做，但要单独一批 + 四平台 +e2e，不能和本次修复混在一个 PR 里。 diff --git a/.agents/docs/2026-07-30-issue311-implementation-plan.md b/.agents/docs/2026-07-30-issue311-implementation-plan.md new file mode 100644 index 00000000..280f3bcd --- /dev/null +++ b/.agents/docs/2026-07-30-issue311-implementation-plan.md @@ -0,0 +1,223 @@ +# BMI staging 原语 + BMI 缓存根收敛 — 实施计划 + +配套设计：`2026-07-30-issue311-bmi-staging-and-cache-root-design.md` +关联 issue：#311 +建议目标版本：**2026.7.30.1**（常规迭代） + +单 PR 交付。阶段有依赖顺序：**P1 → P2 → P3**（P3 的 ninja 文本依赖 P2 的子命令存在， +P2 的判据依赖 P1 收敛后的缓存根不再随 cwd 漂移）。P4/P5/P6 可与 P3 并行收尾。 + +--- + +## P1 — 单一 home 解析器（`mcpp.home`） + +**新建** `src/home.cppm`： + +```cpp +export module mcpp.home; +import std; +import mcpp.platform; + +export namespace mcpp::home { +std::filesystem::path root(); // MCPP_HOME +std::filesystem::path bmi_root(); // root()/bmi +} +``` + +- `root()` = 把 `src/config.cppm:313-353` 的 `default_mcpp_home()` + `home_dir()` + **逐字搬迁**（含 `USERPROFILE` 分支、self-contained 探测、`target/` 与 `data/xpkgs/` + 两条 disqualify）。两个函数都是纯函数、无副作用，搬迁是安全的。 +- `bmi_root()` = `root() / "bmi"`。 + +**改动点（4 处调用方 + 3 处旧实现）**： + +| 文件 | 位置 | 改动 | +|---|---|---| +| `src/config.cppm` | :313-353 | 删除两个本地函数 | +| `src/config.cppm` | :500 / :510 | `cfg.mcppHome = mcpp::home::root()`；`cfg.bmiCacheDir = mcpp::home::bmi_root()` | +| `src/toolchain/stdmod.cppm` | :179-187 | 整体替换为 `return mcpp::home::bmi_root();`（新增 `import mcpp.home;`） | +| `src/build/prepare.cppm` | :2826-2832 | 内联 lambda → `mcpp::home::root()` | + +**编译系统**：`mcpp.toml` 若显式列源文件需同步；当前是 `src/**` 推导（`mcpp build -v` +里 `Inferred sources`），无需改。 + +**无环性核对**（改前必须自查一次）：`mcpp.home` 只 import `std` + `mcpp.platform`。 +`mcpp.config` 的闭包内无 `mcpp.toolchain.*`，故 `mcpp.toolchain.stdmod → mcpp.home` 不成环。 + +**测试**（`tests/unit/test_config.cpp` 扩写，或新建 `tests/unit/test_home.cpp`）： + +| 用例 | 断言 | +|---|---| +| `MCPP_HOME` 优先 | 设环境变量 → `root()` == 该值；`bmi_root()` == `/bmi` | +| Windows 无 HOME | `is_windows` 下 `USERPROFILE` 生效（用 `if constexpr` 分支或跳过非 Windows） | +| 兜底形态 | 两个变量都不设时，路径以 `.mcpp` 结尾、**不再**以 `.mcpp-bmi` 结尾 | +| 与 config 一致 | `load()` 后 `cfg.bmiCacheDir == mcpp::home::bmi_root()`（同一进程同一环境下必须相等） | + +最后一条是这次的核心不变量，**必须机器校验**——它就是 D2 的回归闸。 + +--- + +## P2 — `mcpp stage` 子命令 + +**新建** `src/build/stage.cppm`（`export module mcpp.build.stage;`），承载纯逻辑： + +```cpp +struct StageResult { bool copied; }; // copied=false ⇒ 判等跳过 +struct StageError { std::string message; }; // 已含 hint 文案 +std::expected stage_file( + const std::filesystem::path& src, + const std::filesystem::path& dst, + bool verify_hash); +``` + +实现按设计 §S1 语义表： + +1. `src` 不存在 → error。 +2. `create_directories(dst.parent_path())`。 +3. `dst` 存在 && `file_size` 相等 && (verify == Size || 逐字节相等) + → **不写字节、不碰时间戳** → `StageOutcome{.copied = false}`。 + （对齐 mtime 会让 `restat` 失效并重新引发级联——实测过，见设计 §S1 的表） +4. 否则：`copy_file(src, dst.tmp.)` → `rename(tmp, dst)`。 +5. rename 失败 → `copy_file(src, dst, overwrite_existing)`。 +6. 4/5 均失败 → 睡 100 / 300 / 900 ms 重试整个 4-5 序列，共 3 轮。 +7. 仍失败 → `StageError`，文案照设计 §S4 的模板（file / from / os error / hint 四段， + hint 必须点名 clangd 与 `compile_commands.json` 的因果）。 +8. 每次退出前清理残留 `dst.tmp.`。 + +`Verify::Content` 由 `MCPP_STAGE_VERIFY=content` 或 `--verify content` 打开；实现是分块 +逐字节比较（同 I/O 成本、无碰撞面、可提前退出），不引入 hash 依赖。 + +**CLI 接线**（照 `dyndep` 的形状）： + +| 文件 | 改动 | +|---|---| +| `src/cli/cmd_build.cppm` | 新增 `export int cmd_stage(const ParsedArgs&)`，就近放在 `cmd_dyndep`（:187）旁 | +| `src/cli.cppm` | :496 附近新增 `.subcommand(cl::App("stage") ...)`，描述以 `(internal: invoked by ninja)` 开头，选项 `--output/-o`、`--verify`；`.action(wrap_rc(cmd_stage))` | +| `src/cli.cppm` | :547-551 的 `known` 白名单加 `"stage"`，**数组长度 22 → 23**（写死的模板实参，漏改即编译失败/静默拒命令） | + +**测试**（新建 `tests/unit/test_build_stage.cpp`）： + +| 用例 | 断言 | +|---|---| +| 目标不存在 | 复制发生，`copied == true`，内容一致 | +| 目标已存在且等长等内容 | `copied == false`，且**目标 mtime 一点没变**（不是"未变成 now"，是完全不变） | +| 等长但内容不同 + `Verify::Content` | `copied == true` | +| 等长但内容不同 + `Verify::Size` | `copied == false`（这是**有意的** fp 判据取舍，注释写清） | +| `src` 缺失 | error，message 含 src 路径 | +| 目标目录不存在 | 自动创建 | +| 只读目标（POSIX `chmod 444`） | 走 rename 分支成功；断言最终内容正确 | +| 错误文案 | 人为构造失败（目标是个目录）→ message 含 `clangd` 与 `hint:` | + +--- + +## P3 — ninja 后端切到新 rule + +**文件**：`src/build/ninja_backend.cppm` + +1. **`mcpp` 变量提取**：把 :404 的 + `append(std::format("mcpp = {}\n", escape_ninja_path(mcpp_exe_path())))` + 移出 `if (dyndep)`（:403），改为无条件绑定；`scan_deps` 仍留在 `if (dyndep)` 内。 +2. **rule 重写**（:411-419），跨平台单一形态、不再分叉 PowerShell/`cp`： + +``` +rule stage_file + command = $mcpp stage --output $out $in + description = STAGE $out + restat = 1 +``` + +3. **rule 更名**：`cp_bmi` → `stage_file`，四处 staging edge（:793/:795/:807/:810）与 + DLL 部署（:1080）同步改名。 +4. `command_prefixes()`（:278-291）追加 `mcpp_exe_path()`。 +5. `filter_ninja_output()`（:323-345）：`FAILED:` 不再整行丢弃，改为归一成 + `failed: ` 保留。 + +**文件**：`src/build/execute.cppm` + +6. `read_ninja_command_prefixes()`（:181-205）白名单 key 加 `"mcpp"`。 + +**测试**（`tests/unit/test_ninja_backend.cpp`）： + +| 用例 | 断言 | +|---|---| +| rule 文本 | 含 `rule stage_file`、`$mcpp stage --output $out $in`、`restat = 1`；**不含** `Copy-Item`、`cp -f` | +| `mcpp` 绑定 | dyndep 开/关两种 plan 下都出现 `mcpp = ` 行 | +| staging edge | 四条 edge 的 rule 名是 `stage_file`；`std.compat` 仍有 `| pcm.cache/std.pcm` order-only 前置 | +| DLL 部署 | `runtimeDeployFiles` 非空时用同一 rule 名 | +| 过滤器 | `filter_ninja_output` 对含 ` stage ...` 的回显行过滤掉、对 `failed:` 与 `hint:` 正文保留 | + +--- + +## P4 — 兜底路径的可见性 + +| 文件 | 改动 | +|---|---| +| `src/scaffold/create.cppm` | :284-287 的 `.gitignore` 模板：`target/` + `.mcpp/` | +| `src/doctor.cppm` | :155/:192 附近：若 cwd 或工程根存在 `.mcpp-bmi/`，输出一行 `legacy BMI cache at — safe to delete`（**不自动删**） | + +`.gitignore` 模板变更需同步 e2e 中断言过 scaffold 产物的用例（`grep -rn "gitignore" tests/e2e` +先扫一遍）。 + +--- + +## P5 — e2e + +**新建 `tests/e2e/170_bmi_staging_no_cascade.sh`**（全平台跑，锁 D3 + S1 步 2）： + +1. `mcpp new` 一个 bin 工程 → `mcpp build`（产出 staged BMI）； +2. 从 `mcpp build -v` 的 STAGE 行或 `build.ninja` 解析出 staging edge 的 `$in` + （**不要**用 `awk '{print $NF}'` 直接切 `rule` 行——本次调查里就踩过，取到的是 `cp_bmi` + 这个字面量；正确做法是匹配 `^build .*: (stage_file|cp_bmi) ` 的行再取最后一个字段）； +3. `touch "$in"` 让 edge 变脏； +4. `mcpp build -v` 断言： + - 退出 0； + - 输出**不含** `src/main.cpp` 的编译行（反级联，今天会失败）； + - staged BMI 的内容与 `$in` 一致。 + +**新建 `tests/e2e/171_bmi_staging_locked_dest.sh`**： + +- **Windows 分支**（`case "$(uname -s)" in *NT*|MINGW*|MSYS*)`）：用 PowerShell 在子进程里 + 映射住 staged BMI，**不依赖 clangd**： + + ```powershell + $f = [System.IO.MemoryMappedFiles.MemoryMappedFile]::CreateFromFile( + $path, [System.IO.FileMode]::Open) + Start-Sleep -Seconds 30 # 持有期覆盖被测构建 + ``` + + 然后 `touch` 缓存侧 BMI（保持内容不变）→ `mcpp build` 必须**成功**（走判等跳过）。 + 这就是 #311 的最小复现，且不需要装 clangd。 +- **POSIX 分支**：`chmod 444` staged BMI + 让缓存侧内容真的不同（改用另一个 fingerprint 的 + BMI 或人为构造一份等长-不同内容的假文件）→ 断言走 rename 分支成功。 +- 负例（两个平台）：把 staged BMI 换成一个**目录**同名占位 → 断言构建失败且 stderr 含 + `hint:` 与 `clangd`。 + +`tests/e2e/run_all.sh` 若是显式清单则登记两个新脚本；编号接 169(上游 169 已被 semver 用例占用)。 + +--- + +## P6 — 收尾 + +1. `CHANGELOG.md`： + - fix(#311)：Windows 上被 clangd 映射的 std BMI 不再让构建失败； + - **behavior change**：BMI 缓存根统一为 `$MCPP_HOME/bmi`（Windows 从 `\.mcpp-bmi`、 + self-contained 安装从 `~/.mcpp/bmi` 迁走）；首次构建会重编一次 std（10–60 s）， + 遗留目录可手动删除。 +2. 版本号：`mcpp.toml` → `2026.7.30.1`（注意 `git status` 里 `mcpp.toml` 已有本地改动， + 提交前先核对那处改动是否该一起进）。 +3. 发布闭环按既有 runbook 走（release → 镜像 xlings-res 双端 → xim-pkgindex → 真装验证 → + bootstrap pin）。**bootstrap pin 与本次发布版本是两组，不要一起 bump**。 +4. 不要在 issue #311 下评论（按本次任务要求）；发布后再回复。 + +--- + +## 自查清单（提交前逐条打勾） + +- [ ] `grep -rn "\.mcpp-bmi" src/` 只剩 doctor 的遗留提示与注释，无路径构造 +- [ ] `grep -rn "getenv(\"HOME\")" src/` 不再出现在 BMI/home 解析路径上 +- [ ] `grep -rn "Copy-Item" src/` 归零 +- [ ] `grep -rn "cp_bmi" src/` 归零 +- [ ] `cli.cppm` 的 `known` 数组长度与元素数一致（22 → 23） +- [ ] `if (dyndep)` 之外能拿到 `$mcpp`（用 GCC 非 dyndep plan 生成一次 build.ninja 目视核对） +- [ ] 单测全绿 + `tests/e2e/170`、`171` 全绿（Linux 本机 + Windows CI） +- [ ] Windows CI 上确认 STAGE 行不再 spawn PowerShell（顺带的启动开销收益） diff --git a/src/build/execute.cppm b/src/build/execute.cppm index b9cd2805..06d92e0c 100644 --- a/src/build/execute.cppm +++ b/src/build/execute.cppm @@ -190,7 +190,11 @@ std::vector read_ninja_command_prefixes(const std::filesystem::path auto key = line.substr(0, eq); while (!key.empty() && std::isspace(static_cast(key.back()))) key.pop_back(); - if (key != "cxx" && key != "cc" && key != "ar" && key != "scan_deps") + // `mcpp` drives the dyndep + stage_file rules; treating it as a command + // prefix filters the echoed command line while keeping the diagnostic + // mcpp itself printed (#311). + if (key != "cxx" && key != "cc" && key != "ar" && key != "scan_deps" + && key != "mcpp") continue; std::string value = line.substr(eq + 1); diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 28bd0959..a33a2a2c 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -67,7 +67,7 @@ std::string escape_ninja_path(const std::filesystem::path& p) { // a response file that gcc/clang/GNU ar tokenize GNU-style, where // backslash is an ESCAPE character — `obj\cli.o` would arrive as // `objcli.o`. Every Windows consumer of these strings (CreateProcess - // path resolution, cl.exe/link.exe, PowerShell Copy-Item, ninja itself) + // path resolution, cl.exe/link.exe, `mcpp stage`, ninja itself) // accepts forward slashes; POSIX output is byte-identical. std::string s = p.generic_string(); std::string out; @@ -288,6 +288,10 @@ std::vector command_prefixes(const CompileFlags& flags, add(flags.ccBinary); add(flags.arBinary); add(plan.scanDepsPath); + // mcpp itself drives the dyndep and stage_file rules; its echoed command + // line is noise, while the message it prints on failure is the diagnostic + // we want to keep. + add(mcpp_exe_path()); return prefixes; } @@ -331,11 +335,26 @@ std::string filter_ninja_output(std::string_view output, auto trimmed = ltrim_copy(line); if (trimmed.starts_with("ninja: Entering directory") || trimmed.starts_with("ninja: build stopped") - || trimmed.starts_with("FAILED:") || is_ninja_progress_line(trimmed) || is_command_line(trimmed, commandPrefixes)) { continue; } + // Keep WHICH output failed. Dropping this line entirely (as we used to) + // is why #311's report couldn't tell a BMI staging failure from a + // compile error. Normalized to lowercase `failed:` so it reads as part + // of mcpp's own diagnostics, and `[code=N]` is dropped as noise. + if (trimmed.starts_with("FAILED:")) { + auto target = ltrim_copy(trimmed.substr(std::string_view("FAILED:").size())); + if (target.starts_with("[code=")) { + if (auto close = target.find(']'); close != std::string::npos) + target = ltrim_copy(target.substr(close + 1)); + } + if (target.empty()) continue; + filtered += "failed: "; + filtered += target; + filtered.push_back('\n'); + continue; + } filtered += line; filtered.push_back('\n'); } @@ -400,23 +419,28 @@ std::string emit_ninja_string(const BuildPlan& plan) { flags.ldBinary.empty() ? std::string("link.exe") : escape_ninja_path(flags.ldBinary))); } + // `$mcpp` is needed by stage_file in EVERY configuration (dyndep or not), + // so the binding cannot live inside the `if (dyndep)` below. + append(std::format("mcpp = {}\n", escape_ninja_path(mcpp_exe_path()))); if (dyndep) { - append(std::format("mcpp = {}\n", escape_ninja_path(mcpp_exe_path()))); if (!plan.scanDepsPath.empty()) { append(std::format("scan_deps = {}\n", escape_ninja_path(plan.scanDepsPath))); } } append("\n"); - append("rule cp_bmi\n"); - if constexpr (mcpp::platform::is_windows) { - // Use PowerShell Copy-Item which handles both forward and back slashes. - // cmd.exe `copy` breaks on forward-slash paths from ninja. - append(" command = powershell -NoProfile -Command \"Copy-Item -Force '$in' -Destination '$out'\"\n"); - } else { - append(" command = mkdir -p $$(dirname $out) && cp -f $in $out\n"); - } - append(" description = STAGE $out\n\n"); + // Staging (cache → build dir) runs through mcpp itself instead of a + // per-platform shell copy: it skips the write when the destination is + // already equivalent, writes out-of-place + renames when it isn't, retries + // transient sharing violations, and fails with a diagnostic that names the + // likely holder. #311: `Copy-Item -Force` overwrote in place, so a std BMI + // that clangd had memory-mapped failed the whole build with error 1224. + // `restat = 1` is what makes the no-write path actually pay off — a skipped + // stage must not dirty every importer of the staged BMI. + append("rule stage_file\n"); + append(" command = $mcpp stage --output $out $in\n"); + append(" description = STAGE $out\n"); + append(" restat = 1\n\n"); // P1: per-file dyndep rule. Converts one .ddi → .dd independently. append(std::format( @@ -790,9 +814,9 @@ std::string emit_ninja_string(const BuildPlan& plan) { bool has_std_artifacts = !plan.stdBmiPath.empty() && !plan.stdObjectPath.empty(); if (has_std_artifacts) { - append(std::format("build {} : cp_bmi {}\n", escape_ninja_path(std_bmi_dst), + append(std::format("build {} : stage_file {}\n", escape_ninja_path(std_bmi_dst), escape_ninja_path(plan.stdBmiPath))); - append(std::format("build {} : cp_bmi {}\n\n", escape_ninja_path(std_o_dst), + append(std::format("build {} : stage_file {}\n\n", escape_ninja_path(std_o_dst), escape_ninja_path(plan.stdObjectPath))); } @@ -804,10 +828,10 @@ std::string emit_ninja_string(const BuildPlan& plan) { if (has_std_compat) { // std.compat.pcm depends on std.pcm — ensure std.pcm is staged first // so clang can resolve the transitive dependency when loading std.compat.pcm. - append(std::format("build {} : cp_bmi {} | {}\n", escape_ninja_path(compat_bmi_dst), + append(std::format("build {} : stage_file {} | {}\n", escape_ninja_path(compat_bmi_dst), escape_ninja_path(plan.stdCompatBmiPath), escape_ninja_path(std_bmi_dst))); - append(std::format("build {} : cp_bmi {}\n\n", escape_ninja_path(compat_o_dst), + append(std::format("build {} : stage_file {}\n\n", escape_ninja_path(compat_o_dst), escape_ninja_path(plan.stdCompatObjectPath))); } @@ -1074,10 +1098,13 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("\n"); // Windows runtime-DLL deployment: one copy edge per staged dep DLL. Emitted - // once (deduped by dest in BuildPlan), reusing the generic cp_bmi copy rule. + // once (deduped by dest in BuildPlan), reusing the generic stage_file rule + // — which also means a DLL still loaded by a running program from a + // previous `mcpp run` gets the skip-if-equivalent treatment instead of a + // hard "cannot copy" failure. // Inert on RPATH platforms where runtimeDeployFiles is empty. for (auto const& d : plan.runtimeDeployFiles) { - append(std::format("build {} : cp_bmi {}\n", + append(std::format("build {} : stage_file {}\n", escape_ninja_path(d.dest), escape_ninja_path(d.source))); } diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index e1b0d926..9a08d907 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.home; import mcpp.platform.axis; import mcpp.libs.json; import mcpp.log; @@ -2823,13 +2824,7 @@ prepare_build(bool print_fingerprint, // them to a commit before forming the cache key; this lets // `mcpp update ` pick up a moved branch without deleting // unrelated git caches. - auto mcppHome = [] { - if (auto* e = std::getenv("MCPP_HOME"); e && *e) - return std::filesystem::path(e); - if (auto* e = std::getenv("HOME"); e && *e) - return std::filesystem::path(e) / ".mcpp"; - return std::filesystem::current_path() / ".mcpp"; - }(); + auto mcppHome = mcpp::home::root(); // single resolver (#311) std::string resolvedGitRev = spec.gitRev; if (spec.gitRefKind == "branch") { auto ref = std::format("refs/heads/{}", spec.gitRev); diff --git a/src/build/stage.cppm b/src/build/stage.cppm new file mode 100644 index 00000000..dee607b4 --- /dev/null +++ b/src/build/stage.cppm @@ -0,0 +1,219 @@ +// mcpp.build.stage — the staging primitive behind the ninja `stage_file` rule. +// +// Staging = publish a file the build cache owns (std BMI, std.o, a dep's +// runtime DLL) into the build directory where the compiler / loader expects +// it. This used to be a shelled-out copy per platform: +// +// POSIX: mkdir -p $(dirname $out) && cp -f $in $out +// Windows: powershell -NoProfile -Command "Copy-Item -Force '$in' -Destination '$out'" +// +// Both overwrite the destination in place, unconditionally. On Windows that is +// a live hazard (#311): mcpp writes the staged std BMI path into +// compile_commands.json so clangd can resolve `import std;`, clangd then +// memory-maps that very file, and an in-place overwrite of a file with an open +// user-mapped section fails with error 1224 — the whole build reports +// "build failed" even when the bytes were already correct. +// +// The rules this module implements: +// +// 1. Never write bytes we don't have to. The destination and the source live +// under the SAME build fingerprint (target// vs +// $MCPP_HOME/bmi/), and the fingerprint already covers compiler +// identity, target triple, stdlib, std module source hash and the +// standard/dialect flags. Same fingerprint + same size ⇒ equivalent ⇒ +// the file is already staged. This mirrors what the dep BMI cache has +// always done (bmi_cache.cppm: "Existing project outputs are left +// untouched"). +// 2. When we do write, write out of place and rename — atomic for readers, +// and it survives a transient sharing violation (antivirus, indexer) +// that an in-place overwrite would lose to. +// 3. Retry a few times, then fail with a diagnostic that names the file and +// the likely holder. Never downgrade a real staging failure to a warning: +// a stale or missing BMI turns into either a confusing +// "module 'std' not found" or a silently mismatched link. +// +// Skipping deliberately does not touch the destination's timestamps AT ALL — +// not even to align them with the source. Measured on the ninja side: any mtime +// bump (to "now" or to the source's mtime) counts as "the command changed this +// output", so `restat = 1` no longer suppresses the downstream rebuild and +// every importer of the staged BMI recompiles for nothing. Leaving the mtime +// alone keeps the edge dirty — ninja re-runs this ~no-op on each build until +// the content actually changes — which is the cheap side of the trade +// (one process vs. recompiling the module graph). + +export module mcpp.build.stage; + +import std; + +export namespace mcpp::build::stage { + +// How hard to look before declaring the destination already-staged. +enum class Verify { + Size, // default: size match under an identical build fingerprint + Content, // byte-for-byte compare (MCPP_STAGE_VERIFY=content / --verify content) +}; + +struct StageOptions { + Verify verify = Verify::Size; + int retries = 3; // attempts after the first + std::chrono::milliseconds backoff{100}; // ×3 per retry: 100/300/900ms +}; + +struct StageOutcome { + bool copied = false; // false = destination was already equivalent +}; + +struct StageError { + std::string message; // multi-line, already carries the `hint:` block +}; + +// Publish `src` at `dst`. See the module comment for the exact semantics. +std::expected stage_file(const std::filesystem::path& src, + const std::filesystem::path& dst, + const StageOptions& opts = {}); + +// Byte-for-byte comparison (exported for tests). False when either file is +// unreadable or the sizes differ. +bool same_content(const std::filesystem::path& a, const std::filesystem::path& b); + +// Parse a --verify / MCPP_STAGE_VERIFY value. Unknown values fall back to Size. +Verify parse_verify(std::string_view value); + +} // namespace mcpp::build::stage + +namespace mcpp::build::stage { + +namespace { + +std::filesystem::path temp_sibling(const std::filesystem::path& dst) { + // Same directory as the destination so the rename stays on one filesystem. + auto stamp = std::chrono::system_clock::now().time_since_epoch().count(); + auto name = dst.filename().string() + std::format(".tmp.{}", stamp); + return dst.parent_path() / name; +} + +// One full write attempt: temp+rename first, in-place overwrite as fallback. +// Returns an empty error_code on success; otherwise the most informative error +// (the in-place one — that's where 1224 / 32 shows up). +std::error_code write_once(const std::filesystem::path& src, + const std::filesystem::path& dst) { + auto tmp = temp_sibling(dst); + std::error_code ec; + std::filesystem::copy_file(src, tmp, std::filesystem::copy_options::overwrite_existing, ec); + if (!ec) { + std::error_code rec; + std::filesystem::rename(tmp, dst, rec); + if (!rec) return {}; + // Renaming over a destination that another process holds mapped fails + // too (the destination must be deletable) — fall through to the + // in-place path, which wins when the holder allows writes but not + // deletes. + std::error_code rmec; + std::filesystem::remove(tmp, rmec); + } else { + std::error_code rmec; + std::filesystem::remove(tmp, rmec); + } + + std::error_code cec; + std::filesystem::copy_file(src, dst, std::filesystem::copy_options::overwrite_existing, cec); + if (!cec) return {}; + return cec; +} + +std::string failure_message(const std::filesystem::path& src, + const std::filesystem::path& dst, + const std::error_code& ec) { + return std::format( + "cannot stage file into the build directory\n" + " file: {}\n" + " from: {}\n" + " os error: {} ({})\n" + "hint: another process has this file memory-mapped, loaded or open.\n" + " The usual holder is clangd (mcpp writes staged BMI paths into\n" + " compile_commands.json so clangd can resolve `import std;`), an\n" + " editor/IDE indexer, antivirus, or — for a .dll — a still-running\n" + " program from a previous `mcpp run`. Close it or restart clangd,\n" + " then re-run the build.", + dst.string(), src.string(), ec.value(), ec.message()); +} + +} // namespace + +bool same_content(const std::filesystem::path& a, const std::filesystem::path& b) { + std::error_code ec1, ec2; + auto sa = std::filesystem::file_size(a, ec1); + auto sb = std::filesystem::file_size(b, ec2); + if (ec1 || ec2 || sa != sb) return false; + + std::ifstream fa(a, std::ios::binary); + std::ifstream fb(b, std::ios::binary); + if (!fa || !fb) return false; + + constexpr std::size_t kChunk = 1u << 16; + std::vector ba(kChunk), bb(kChunk); + while (fa && fb) { + fa.read(ba.data(), static_cast(kChunk)); + fb.read(bb.data(), static_cast(kChunk)); + auto na = fa.gcount(); + auto nb = fb.gcount(); + if (na != nb) return false; + if (na == 0) break; + if (std::memcmp(ba.data(), bb.data(), static_cast(na)) != 0) + return false; + } + return true; +} + +Verify parse_verify(std::string_view value) { + return value == "content" ? Verify::Content : Verify::Size; +} + +std::expected stage_file(const std::filesystem::path& src, + const std::filesystem::path& dst, + const StageOptions& opts) { + std::error_code ec; + if (!std::filesystem::exists(src, ec)) { + return std::unexpected(StageError{ + std::format("staging source does not exist: {}", src.string())}); + } + + if (!dst.parent_path().empty()) { + std::error_code dec; + std::filesystem::create_directories(dst.parent_path(), dec); + if (dec && !std::filesystem::is_directory(dst.parent_path())) { + return std::unexpected(StageError{ + std::format("cannot create '{}': {}", + dst.parent_path().string(), dec.message())}); + } + } + + // Already staged? (see module comment for why size is a sound default) + std::error_code sec; + if (std::filesystem::is_regular_file(dst, sec)) { + std::error_code se1, se2; + auto sizeDst = std::filesystem::file_size(dst, se1); + auto sizeSrc = std::filesystem::file_size(src, se2); + bool equivalent = !se1 && !se2 && sizeDst == sizeSrc + && (opts.verify == Verify::Size || same_content(src, dst)); + if (equivalent) { + // Timestamps left untouched on purpose — see module comment. + return StageOutcome{.copied = false}; + } + } + + auto delay = opts.backoff; + std::error_code last; + for (int attempt = 0; attempt <= opts.retries; ++attempt) { + if (attempt > 0) { + std::this_thread::sleep_for(delay); + delay *= 3; + } + last = write_once(src, dst); + if (!last) return StageOutcome{.copied = true}; + } + + return std::unexpected(StageError{failure_message(src, dst, last)}); +} + +} // namespace mcpp::build::stage diff --git a/src/cli.cppm b/src/cli.cppm index 622b67fc..998edcd7 100644 --- a/src/cli.cppm +++ b/src/cli.cppm @@ -509,6 +509,13 @@ int run(int argc, char** argv) { .option(cl::Option("expect-none") .help("(verification) planner assumed no provides/imports")) .action(wrap_rc(cmd_dyndep))) + .subcommand(cl::App("stage") + .description("(internal: invoked by ninja) Stage a cached artifact into the build dir") + .option(cl::Option("output").short_name('o').takes_value().value_name("PATH") + .help("Destination path inside the build directory")) + .option(cl::Option("verify").takes_value().value_name("MODE") + .help("Already-staged check: size (default) | content")) + .action(wrap_rc(cmd_stage))) ; // The bareword `mcpp help` and `mcpp` (no args) both print the @@ -544,11 +551,11 @@ int run(int argc, char** argv) { { std::string_view first = argv[1]; if (!first.starts_with('-')) { - static constexpr std::array known = { + static constexpr std::array known = { "new", "build", "run", "test", "clean", "add", "remove", "update", "search", "publish", "pack", "emit", "xpkg", "toolchain", "cache", "index", "self", "explain", - "version", "dyndep", "why", "resolve", + "version", "dyndep", "why", "resolve", "stage", }; bool ok = false; for (auto k : known) if (k == first) { ok = true; break; } diff --git a/src/cli/cmd_build.cppm b/src/cli/cmd_build.cppm index 361550e5..f324b5cf 100644 --- a/src/cli/cmd_build.cppm +++ b/src/cli/cmd_build.cppm @@ -1,6 +1,6 @@ // mcpp.cli.cmd_build — CLI parsing + routing for build / run / test / -// clean / dyndep. Implementations live in mcpp.build.prepare, -// mcpp.build.execute and mcpp.dyndep. +// clean / dyndep / stage. Implementations live in mcpp.build.prepare, +// mcpp.build.execute, mcpp.dyndep and mcpp.build.stage. module; #include @@ -12,6 +12,7 @@ import std; import mcpplibs.cmdline; import mcpp.build.prepare; import mcpp.build.execute; +import mcpp.build.stage; import mcpp.dyndep; import mcpp.log; import mcpp.project; @@ -255,4 +256,39 @@ export int cmd_dyndep(const mcpplibs::cmdline::ParsedArgs& parsed) { return os ? 0 : 1; } +// Invoked by ninja during build (stage_file rule): +// mcpp stage --output +// +// Publishes a cache-owned artifact (std BMI, std.o, runtime DLL) into the +// build directory. See mcpp.build.stage for the semantics — in particular why +// an already-equivalent destination is left untouched (#311). +export int cmd_stage(const mcpplibs::cmdline::ParsedArgs& parsed) { + std::filesystem::path outPath = parsed.option_or_empty("output").value(); + if (outPath.empty()) { + std::println(stderr, "error: --output required"); + return 2; + } + if (parsed.positional_count() != 1) { + std::println(stderr, "error: stage requires exactly one source path"); + return 2; + } + + mcpp::build::stage::StageOptions opts; + std::string verify = parsed.option_or_empty("verify").value(); + if (verify.empty()) { + if (const char* e = std::getenv("MCPP_STAGE_VERIFY"); e && *e) + verify = e; + } + if (!verify.empty()) + opts.verify = mcpp::build::stage::parse_verify(verify); + + auto r = mcpp::build::stage::stage_file( + std::filesystem::path{parsed.positional(0)}, outPath, opts); + if (!r) { + std::println(stderr, "error: {}", r.error().message); + return 1; + } + return 0; +} + } // namespace mcpp::cli diff --git a/src/config.cppm b/src/config.cppm index 9236972d..68c57631 100644 --- a/src/config.cppm +++ b/src/config.cppm @@ -20,6 +20,7 @@ module; export module mcpp.config; import std; +import mcpp.home; import mcpp.libs.toml; import mcpp.pm.index_spec; import mcpp.xlings; @@ -287,15 +288,6 @@ namespace t = mcpp::libs::toml; namespace { -// Resolve MCPP_HOME, in priority order: -// 1. $MCPP_HOME env var (explicit override — used by CI / dev / multi-instance) -// 2. /.. — self-contained mode, when mcpp lives at -// /bin/mcpp. Release tarballs and `xlings install mcpp` -// use this layout; the unpacked tree IS the home. Dev builds -// live under target///bin/mcpp, which is the same -// "in a bin/ dir" shape — so we additionally exclude any path -// with a "target" ancestor as mcpp's own dev convention. -// 3. fallback to $HOME/.mcpp. // Right-pad a verb to 12 columns, matching mcpp::ui::verb_padded so the // bootstrap status lines line up under the cyan "Downloading …" lines // produced via the BootstrapProgressCallback. We can't import mcpp.ui @@ -310,49 +302,6 @@ void print_status(std::string_view verb, std::string_view msg) { } } -std::filesystem::path default_mcpp_home() { - // Windows: %USERPROFILE%\.mcpp POSIX: $HOME/.mcpp - if constexpr (mcpp::platform::is_windows) { - if (auto* e = std::getenv("USERPROFILE"); e && *e) - return std::filesystem::path(e) / ".mcpp"; - } - if (auto* e = std::getenv("HOME"); e && *e) - return std::filesystem::path(e) / ".mcpp"; - return std::filesystem::current_path() / ".mcpp"; -} - -std::filesystem::path home_dir() { - // 1. Explicit $MCPP_HOME takes priority (CI, advanced users). - if (auto* e = std::getenv("MCPP_HOME"); e && *e) - return std::filesystem::path(e); - - auto exe = mcpp::platform::fs::self_exe_path(); - if (exe.has_parent_path() && exe.parent_path().filename() == "bin") { - auto candidate = exe.parent_path().parent_path(); - - // Disqualify self-contained mode for two cases: - // a) Dev builds: .../target///bin/ - // b) xlings packages: .../data/xpkgs/xim-x-mcpp//bin/mcpp - // Creating a nested xlings sandbox inside the xpkgs directory - // breaks toolchain installation (nested XLINGS_HOME) and loses - // installed toolchains when the mcpp package version is upgraded. - bool disqualified = false; - for (auto p = candidate; - p.has_parent_path() && p != p.root_path(); - p = p.parent_path()) { - if (p.filename() == "target") { disqualified = true; break; } - if (p.filename() == "xpkgs") { - auto parent = p.parent_path().filename().string(); - if (parent == "data") { disqualified = true; break; } - } - } - if (!disqualified) - return candidate; - } - - return default_mcpp_home(); -} - std::expected run_capture(const std::string& cmd) { return mcpp::xlings::run_capture(cmd); } @@ -496,8 +445,8 @@ std::expected load_or_init( { GlobalConfig cfg; - // 1. Resolve paths - cfg.mcppHome = home_dir(); + // 1. Resolve paths (mcpp.home is the single resolver — #311) + cfg.mcppHome = mcpp::home::root(); cfg.binDir = cfg.mcppHome / "bin"; cfg.registryDir = cfg.mcppHome / "registry"; // xlings lives under registry/, not bin/ — it's a registry tool, @@ -507,7 +456,7 @@ std::expected load_or_init( // making ensure_sandbox_xlings_binary() a no-op. cfg.xlingsBinary = cfg.registryDir / "bin" / (std::string("xlings") + std::string(mcpp::platform::exe_suffix)); - cfg.bmiCacheDir = cfg.mcppHome / "bmi"; + cfg.bmiCacheDir = mcpp::home::bmi_root(); cfg.metaCacheDir = cfg.mcppHome / "cache"; cfg.logDir = cfg.mcppHome / "log"; cfg.configFile = cfg.mcppHome / "config.toml"; diff --git a/src/doctor.cppm b/src/doctor.cppm index 21ed50fc..2e64db30 100644 --- a/src/doctor.cppm +++ b/src/doctor.cppm @@ -16,6 +16,7 @@ import mcpp.build.plan; import mcpp.config; import mcpp.fallback.install_integrity; import mcpp.fetcher.progress; +import mcpp.home; import mcpp.platform; import mcpp.platform.process; import mcpp.toolchain.detect; @@ -131,10 +132,7 @@ export int doctor_report() { mcpp::ui::status("Checking", "mingw (xim:mingw-gcc)"); { - auto pkgs = std::filesystem::path( - std::getenv("MCPP_HOME") ? std::getenv("MCPP_HOME") - : (std::string(std::getenv("USERPROFILE") - ? std::getenv("USERPROFILE") : "") + "\\.mcpp")) + auto pkgs = mcpp::home::root() / "registry" / "data" / "xpkgs" / "xim-x-mingw-gcc"; std::error_code ec; bool any = false; @@ -197,6 +195,17 @@ export int doctor_report() { } else { ok(std::format("BMI cache size = {}", mcpp::bmi_cache::human_bytes(sz))); } + // Pre-#311 builds could park the std BMI cache in the current working + // directory (`.mcpp-bmi/`) whenever neither MCPP_HOME nor HOME resolved — + // the common case on Windows PowerShell. Point at leftovers; never delete. + { + std::error_code lec; + auto legacy = std::filesystem::current_path(lec) / ".mcpp-bmi"; + if (!lec && std::filesystem::is_directory(legacy, lec)) { + warn(std::format("legacy BMI cache at '{}' — no longer used, safe to delete", + legacy.string())); + } + } mcpp::ui::status("Checking", "runtime capabilities"); { @@ -470,18 +479,10 @@ export int self_init(bool force) { // Preserves: bin/mcpp (self-contained mode), config.toml, log/. mcpp::ui::info("Resetting", "mcpp sandbox (registry, caches)"); - // Resolve MCPP_HOME without running bootstrap (which may fail). - std::filesystem::path home; - if (auto* e = std::getenv("MCPP_HOME"); e && *e) - home = e; - else { - const char* userHome = nullptr; -#if defined(_WIN32) - userHome = std::getenv("USERPROFILE"); -#endif - if (!userHome) userHome = std::getenv("HOME"); - if (userHome) home = std::filesystem::path(userHome) / ".mcpp"; - } + // Resolve MCPP_HOME without running bootstrap (which may fail). The + // shared resolver also covers self-contained installs — the local copy + // this replaced would have wiped ~/.mcpp for a `/bin/mcpp` tree. + std::filesystem::path home = mcpp::home::root(); if (!home.empty()) { std::error_code ec; std::filesystem::remove_all(home / "registry", ec); diff --git a/src/home.cppm b/src/home.cppm new file mode 100644 index 00000000..31138431 --- /dev/null +++ b/src/home.cppm @@ -0,0 +1,97 @@ +// mcpp.home — the single resolver for MCPP_HOME and its well-known subdirs. +// +// Every path under the mcpp home must be derived from here. Before #311 this +// logic existed in three places (config.cppm, toolchain/stdmod.cppm and an +// inline lambda in build/prepare.cppm); the copies drifted, and the oldest one +// (stdmod, unchanged since v0.0.1) never learned about Windows' USERPROFILE or +// about self-contained installs. The observable damage: on Windows PowerShell +// (no $HOME) the std BMI cache landed in the *current working directory* as +// `.mcpp-bmi/`, while dep BMIs went to %USERPROFILE%\.mcpp\bmi — two roots for +// one cache, and a cwd-dependent one at that. +// +// This module is deliberately a leaf: it imports std + mcpp.platform only, so +// anything (config, toolchain, build) can depend on it without cycles. + +module; +#include // getenv + +export module mcpp.home; + +import std; +import mcpp.platform; + +export namespace mcpp::home { + +// $MCPP_HOME. See root() below for the resolution order. +std::filesystem::path root(); + +// The cross-project BMI cache root ($MCPP_HOME/bmi). Both the std module +// cache (toolchain/stdmod.cppm) and the dep BMI cache (bmi_cache.cppm, via +// GlobalConfig::bmiCacheDir) live here — they MUST agree. +std::filesystem::path bmi_root(); + +} // namespace mcpp::home + +namespace mcpp::home { + +namespace { + +std::filesystem::path default_home() { + // Windows: %USERPROFILE%\.mcpp POSIX: $HOME/.mcpp + if constexpr (mcpp::platform::is_windows) { + if (auto* e = std::getenv("USERPROFILE"); e && *e) + return std::filesystem::path(e) / ".mcpp"; + } + if (auto* e = std::getenv("HOME"); e && *e) + return std::filesystem::path(e) / ".mcpp"; + return std::filesystem::current_path() / ".mcpp"; +} + +} // namespace + +// Resolve MCPP_HOME, in priority order: +// 1. $MCPP_HOME env var (explicit override — used by CI / dev / multi-instance) +// 2. /.. — self-contained mode, when mcpp lives at +// /bin/mcpp. Release tarballs and `xlings install mcpp` +// use this layout; the unpacked tree IS the home. Dev builds +// live under target///bin/mcpp, which is the same +// "in a bin/ dir" shape — so we additionally exclude any path +// with a "target" ancestor as mcpp's own dev convention. +// 3. fallback to $HOME/.mcpp (%USERPROFILE%\.mcpp on Windows). +std::filesystem::path root() { + // 1. Explicit $MCPP_HOME takes priority (CI, advanced users). + if (auto* e = std::getenv("MCPP_HOME"); e && *e) + return std::filesystem::path(e); + + auto exe = mcpp::platform::fs::self_exe_path(); + if (exe.has_parent_path() && exe.parent_path().filename() == "bin") { + auto candidate = exe.parent_path().parent_path(); + + // Disqualify self-contained mode for two cases: + // a) Dev builds: .../target///bin/ + // b) xlings packages: .../data/xpkgs/xim-x-mcpp//bin/mcpp + // Creating a nested xlings sandbox inside the xpkgs directory + // breaks toolchain installation (nested XLINGS_HOME) and loses + // installed toolchains when the mcpp package version is upgraded. + bool disqualified = false; + for (auto p = candidate; + p.has_parent_path() && p != p.root_path(); + p = p.parent_path()) { + if (p.filename() == "target") { disqualified = true; break; } + if (p.filename() == "xpkgs") { + auto parent = p.parent_path().filename().string(); + if (parent == "data") { disqualified = true; break; } + } + } + if (!disqualified) + return candidate; + } + + return default_home(); +} + +std::filesystem::path bmi_root() { + return root() / "bmi"; +} + +} // namespace mcpp::home diff --git a/src/scaffold/create.cppm b/src/scaffold/create.cppm index 4a026b27..7d1f5208 100644 --- a/src/scaffold/create.cppm +++ b/src/scaffold/create.cppm @@ -288,7 +288,9 @@ int main() { // .gitignore { std::ofstream os(root / ".gitignore"); - os << "target/\n"; + // `.mcpp/` is the per-project xlings sandbox (and, when no MCPP_HOME + // can be resolved, the local BMI cache) — build state, never sources. + os << "target/\n.mcpp/\n"; } std::println("Created {} package '{}' at {}", gui ? "gui" : "bin", name, root.string()); diff --git a/src/toolchain/stdmod.cppm b/src/toolchain/stdmod.cppm index d2251c99..d32596f4 100644 --- a/src/toolchain/stdmod.cppm +++ b/src/toolchain/stdmod.cppm @@ -23,6 +23,7 @@ module; export module mcpp.toolchain.stdmod; import std; +import mcpp.home; import mcpp.libs.json; import mcpp.platform; import mcpp.toolchain.clang; @@ -177,13 +178,11 @@ std::expected run_commands( } // namespace std::filesystem::path default_cache_root() { - if (auto* e = std::getenv("MCPP_HOME"); e && *e) { - return std::filesystem::path(e) / "bmi"; - } - if (auto* e = std::getenv("HOME"); e && *e) { - return std::filesystem::path(e) / ".mcpp" / "bmi"; - } - return std::filesystem::current_path() / ".mcpp-bmi"; + // Single resolver (#311). This used to be a private copy of the home + // resolution that predated Windows' USERPROFILE branch and self-contained + // installs, so the std BMI cache could land in the *current working + // directory* (`.mcpp-bmi/`) while dep BMIs went to $MCPP_HOME/bmi. + return mcpp::home::bmi_root(); } std::expected ensure_built( diff --git a/tests/e2e/170_bmi_staging_no_cascade.sh b/tests/e2e/170_bmi_staging_no_cascade.sh new file mode 100755 index 00000000..7cb54eb3 --- /dev/null +++ b/tests/e2e/170_bmi_staging_no_cascade.sh @@ -0,0 +1,87 @@ +#!/usr/bin/env bash +# requires: +# 170_bmi_staging_no_cascade.sh — mcpp#311: the std BMI staging edge. +# +# Two invariants, both regressions that shipped: +# +# 1. Re-staging an UNCHANGED std BMI must not recompile anything. The old +# `cp -f` / `Copy-Item -Force` rule rewrote the file unconditionally and +# carried no `restat`, so every importer of `import std` rebuilt whenever +# the cache-side BMI got a newer mtime (which happens on any cwd change +# before the cache-root fix below). +# 2. The staging SOURCE must live under $MCPP_HOME/bmi. The std module cache +# used to resolve its root through a private copy of the home logic that +# knew neither %USERPROFILE% nor self-contained installs, so on Windows it +# parked the cache in the current working directory as `.mcpp-bmi/`. +# +# Toolchain-neutral: only needs a package that says `import std;`. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +mkdir -p app/src +cat > app/src/main.cpp <<'EOF' +import std; +int main() { std::println("staged"); return 0; } +EOF +cat > app/mcpp.toml <<'EOF' +[package] +name = "app" +version = "0.1.0" +EOF + +cd app +"$MCPP" build > build1.log 2>&1 || { cat build1.log; echo "FAIL: initial build"; exit 1; } + +NINJA=$(find target -name build.ninja | head -1) +[[ -n "$NINJA" ]] || { echo "FAIL: no build.ninja produced"; exit 1; } + +# Parse the staging edge, NOT the rule declaration: `awk '{print $NF}'` over a +# grep for the rule NAME would return the literal rule name. +EDGE=$(grep -E '^build [^ ]+ : stage_file ' "$NINJA" | head -1) +[[ -n "$EDGE" ]] || { + grep -n "std" "$NINJA" | head -20 + echo "FAIL: no stage_file edge for the std BMI (did import std resolve?)"; exit 1; } +DST=$(echo "$EDGE" | awk '{print $2}') +SRC=$(echo "$EDGE" | awk '{print $NF}') +echo "staging: $SRC -> $DST" + +# ── invariant 2: the cache root is $MCPP_HOME/bmi, never a cwd-local dir ── +HOME_ROOT="${MCPP_HOME:-$HOME/.mcpp}" +case "$SRC" in + "$HOME_ROOT"/bmi/*) ;; + *) echo "FAIL: std BMI cache is '$SRC', expected under '$HOME_ROOT/bmi'"; exit 1 ;; +esac +for leftover in "$TMP/.mcpp-bmi" "$TMP/app/.mcpp-bmi"; do + [[ -d "$leftover" ]] && { echo "FAIL: legacy cache dir created at $leftover"; exit 1; } +done + +# ── invariant 1: a dirty staging edge alone must not cascade ── +# mtime only — never mutate the shared cache's CONTENT from a test. +touch "$SRC" +"$MCPP" build -v > build2.log 2>&1 || { cat build2.log; echo "FAIL: rebuild after re-stage"; exit 1; } + +grep -q "stage --output" build2.log || { + cat build2.log; echo "FAIL: the staging edge did not re-run at all"; exit 1; } +if grep -qE '(-c|/c) .*main\.cpp' build2.log; then + cat build2.log + echo "FAIL: re-staging an unchanged std BMI recompiled main.cpp (cascade)" + exit 1 +fi + +# The staged copy must still be a faithful copy after the no-write path. +# $DST is relative to the build directory (ninja runs with cwd = outputDir). +cmp "$SRC" "$(dirname "$NINJA")/$DST" \ + || { echo "FAIL: staged BMI differs from the cache after staging"; exit 1; } + +# And the edge must not stay dirty forever: ninja's restat bookkeeping settles +# it on the next run. (-v so ninja's own line reaches the log — mcpp only +# forwards captured output on failure otherwise.) +"$MCPP" build -v > build3.log 2>&1 || { cat build3.log; echo "FAIL: third build"; exit 1; } +grep -q "no work to do" build3.log || { + cat build3.log + echo "FAIL: staging edge stayed dirty after a settled re-stage"; exit 1; } + +echo "OK" diff --git a/tests/e2e/171_bmi_staging_locked_dest.sh b/tests/e2e/171_bmi_staging_locked_dest.sh new file mode 100755 index 00000000..20aa478e --- /dev/null +++ b/tests/e2e/171_bmi_staging_locked_dest.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +# requires: +# 171_bmi_staging_locked_dest.sh — mcpp#311: staging must survive a destination +# that another process is holding, and must fail LOUDLY when it genuinely can't. +# +# The reported failure: mcpp writes the staged std BMI path into +# compile_commands.json so clangd can resolve `import std;`, clangd +# memory-maps that file, and the old `Copy-Item -Force` staging step then tried +# to overwrite it in place → Windows error 1224 (a file with a user-mapped +# section open cannot be replaced) → the whole build reported "build failed". +# +# Windows: reproduce it exactly, WITHOUT needing clangd — a background +# PowerShell holds a MemoryMappedFile on the staged BMI. +# POSIX: the closest analogue an ordinary test can create is an unwritable +# destination (chmod 444); the old `cp -f` silently unlinked and rewrote it, +# the new path must not need to write at all. +set -e + +TMP=$(mktemp -d) +trap "rm -rf $TMP" EXIT +cd "$TMP" + +mkdir -p app/src +cat > app/src/main.cpp <<'EOF' +import std; +int main() { std::println("locked"); return 0; } +EOF +cat > app/mcpp.toml <<'EOF' +[package] +name = "app" +version = "0.1.0" +EOF + +cd app +"$MCPP" build > build1.log 2>&1 || { cat build1.log; echo "FAIL: initial build"; exit 1; } + +NINJA=$(find target -name build.ninja | head -1) +EDGE=$(grep -E '^build [^ ]+ : stage_file ' "$NINJA" | head -1) +[[ -n "$EDGE" ]] || { echo "FAIL: no stage_file edge for the std BMI"; exit 1; } +DST="$(dirname "$NINJA")/$(echo "$EDGE" | awk '{print $2}')" +SRC=$(echo "$EDGE" | awk '{print $NF}') +[[ -f "$DST" ]] || { echo "FAIL: staged BMI missing at $DST"; exit 1; } + +# Dirty the staging edge without touching the shared cache's CONTENT. +touch "$SRC" + +HOLDER="" +case "$(uname -s)" in + MINGW* | MSYS* | CYGWIN*) + WINDST="$(cygpath -w "$DST")" + powershell -NoProfile -Command \ + "\$mm = [System.IO.MemoryMappedFiles.MemoryMappedFile]::CreateFromFile('$WINDST', [System.IO.FileMode]::Open); Start-Sleep -Seconds 90" & + HOLDER=$! + # Give the mapping time to exist before the build races it. + sleep 3 + echo "holding a user-mapped section on $WINDST (pid $HOLDER)" + ;; + *) + chmod 444 "$DST" + echo "made $DST read-only" + ;; +esac + +set +e +"$MCPP" build -v > build2.log 2>&1 +rc=$? +set -e +[[ -n "$HOLDER" ]] && { kill "$HOLDER" 2>/dev/null; wait "$HOLDER" 2>/dev/null; } +chmod 644 "$DST" 2>/dev/null || true + +if [[ $rc -ne 0 ]]; then + cat build2.log + echo "FAIL: staging an already-equivalent BMI failed while the destination was held" + exit 1 +fi +grep -q "stage --output" build2.log || { + cat build2.log; echo "FAIL: the staging edge did not run"; exit 1; } +cmp "$SRC" "$DST" || { echo "FAIL: held destination no longer matches the cache"; exit 1; } + +cd "$TMP" + +# ── the loud half: a destination that cannot be produced must fail with an +# actionable diagnostic, never be downgraded to a warning ── +echo payload > src.bin +mkdir -p occupied/child +echo keep > occupied/child/keep.txt +set +e +"$MCPP" stage --output occupied src.bin > stage_err.log 2>&1 +rc=$? +set -e +if [[ $rc -eq 0 ]]; then + cat stage_err.log + echo "FAIL: staging onto an unusable destination reported success"; exit 1 +fi +grep -q "hint:" stage_err.log || { + cat stage_err.log; echo "FAIL: staging failure carried no hint"; exit 1; } +grep -q "clangd" stage_err.log || { + cat stage_err.log; echo "FAIL: hint does not name the usual holder (clangd)"; exit 1; } +[[ -f occupied/child/keep.txt ]] || { echo "FAIL: failed staging clobbered the destination"; exit 1; } + +echo "OK" diff --git a/tests/unit/test_build_stage.cpp b/tests/unit/test_build_stage.cpp new file mode 100644 index 00000000..2a629056 --- /dev/null +++ b/tests/unit/test_build_stage.cpp @@ -0,0 +1,181 @@ +#include + +import std; +import mcpp.build.stage; + +using namespace mcpp::build::stage; + +namespace { + +struct Tmp { + std::filesystem::path path; + Tmp() { + path = std::filesystem::temp_directory_path() + / std::format("mcpp_stage_test_{}", std::random_device{}()); + std::filesystem::create_directories(path); + } + ~Tmp() { + std::error_code ec; + std::filesystem::permissions(path, std::filesystem::perms::owner_all, + std::filesystem::perm_options::add, ec); + std::filesystem::remove_all(path, ec); + } +}; + +void write_file(const std::filesystem::path& p, std::string_view body) { + std::filesystem::create_directories(p.parent_path()); + std::ofstream os(p, std::ios::binary); + os << body; +} + +std::string read_file(const std::filesystem::path& p) { + std::ifstream is(p, std::ios::binary); + return std::string{std::istreambuf_iterator(is), {}}; +} + +// Retries would only slow the failure tests down; the behaviour under test is +// the decision, not the backoff. +StageOptions no_retry(Verify v = Verify::Size) { + return StageOptions{.verify = v, .retries = 0, .backoff = std::chrono::milliseconds{0}}; +} + +} // namespace + +TEST(BuildStage, CopiesWhenDestinationMissing) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + auto dst = tmp.path / "nested" / "dir" / "dst.bin"; + write_file(src, "payload"); + + auto r = stage_file(src, dst, no_retry()); + ASSERT_TRUE(r.has_value()) << (r ? "" : r.error().message); + EXPECT_TRUE(r->copied); + EXPECT_EQ(read_file(dst), "payload"); +} + +// The core of #311: an already-equivalent destination must not be written at +// all — that is what keeps a clangd-mapped std.pcm from failing the build. +TEST(BuildStage, EquivalentDestinationIsNotTouched) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + auto dst = tmp.path / "dst.bin"; + write_file(src, "same-bytes"); + write_file(dst, "same-bytes"); + + // Age the destination so any write (or timestamp bump) is observable. + auto before = std::filesystem::file_time_type::clock::now() - std::chrono::hours{2}; + std::filesystem::last_write_time(dst, before); + auto recorded = std::filesystem::last_write_time(dst); + + auto r = stage_file(src, dst, no_retry()); + ASSERT_TRUE(r.has_value()); + EXPECT_FALSE(r->copied); + // Not "not bumped to now" — not changed at all. Any mtime change defeats + // ninja's `restat = 1` and re-triggers the downstream rebuild cascade. + EXPECT_EQ(std::filesystem::last_write_time(dst), recorded); +} + +// Same size, different bytes: the size-only default deliberately treats this as +// already-staged (destination and source share a build fingerprint, which +// covers compiler identity, stdlib and std source hash). +TEST(BuildStage, SameSizeDifferentBytesIsSkippedUnderSizeVerify) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + auto dst = tmp.path / "dst.bin"; + write_file(src, "AAAA"); + write_file(dst, "BBBB"); + + auto r = stage_file(src, dst, no_retry(Verify::Size)); + ASSERT_TRUE(r.has_value()); + EXPECT_FALSE(r->copied); + EXPECT_EQ(read_file(dst), "BBBB"); +} + +TEST(BuildStage, SameSizeDifferentBytesIsCopiedUnderContentVerify) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + auto dst = tmp.path / "dst.bin"; + write_file(src, "AAAA"); + write_file(dst, "BBBB"); + + auto r = stage_file(src, dst, no_retry(Verify::Content)); + ASSERT_TRUE(r.has_value()); + EXPECT_TRUE(r->copied); + EXPECT_EQ(read_file(dst), "AAAA"); +} + +TEST(BuildStage, DifferentSizeIsAlwaysCopied) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + auto dst = tmp.path / "dst.bin"; + write_file(src, "longer-payload"); + write_file(dst, "short"); + + auto r = stage_file(src, dst, no_retry()); + ASSERT_TRUE(r.has_value()); + EXPECT_TRUE(r->copied); + EXPECT_EQ(read_file(dst), "longer-payload"); +} + +TEST(BuildStage, MissingSourceIsAnError) { + Tmp tmp; + auto r = stage_file(tmp.path / "nope.bin", tmp.path / "dst.bin", no_retry()); + ASSERT_FALSE(r.has_value()); + EXPECT_NE(r.error().message.find("nope.bin"), std::string::npos); +} + +// A read-only destination is the closest POSIX analogue of the Windows holder: +// an in-place overwrite cannot open it, so staging must go through the +// temp-file + rename path. +TEST(BuildStage, ReadOnlyDestinationIsReplacedViaRename) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + auto dst = tmp.path / "dst.bin"; + write_file(src, "new-content"); + write_file(dst, "old"); + std::filesystem::permissions(dst, std::filesystem::perms::owner_read); + + auto r = stage_file(src, dst, no_retry()); + ASSERT_TRUE(r.has_value()) << r.error().message; + EXPECT_TRUE(r->copied); + EXPECT_EQ(read_file(dst), "new-content"); +} + +// When staging genuinely cannot proceed, the failure must name the file and +// carry the actionable hint — never be downgraded to a warning. +TEST(BuildStage, UnwritableDestinationFailsWithActionableHint) { + Tmp tmp; + auto src = tmp.path / "src.bin"; + write_file(src, "payload"); + // A directory in the destination's place: neither rename nor copy can win. + auto dst = tmp.path / "occupied"; + std::filesystem::create_directories(dst / "child"); + write_file(dst / "child" / "keep.txt", "x"); + + auto r = stage_file(src, dst, no_retry()); + ASSERT_FALSE(r.has_value()); + EXPECT_NE(r.error().message.find("hint:"), std::string::npos); + EXPECT_NE(r.error().message.find("clangd"), std::string::npos); + EXPECT_NE(r.error().message.find(dst.string()), std::string::npos); +} + +TEST(BuildStage, SameContentComparesBytesNotJustSize) { + Tmp tmp; + auto a = tmp.path / "a.bin"; + auto b = tmp.path / "b.bin"; + auto c = tmp.path / "c.bin"; + write_file(a, std::string(300000, 'x')); + write_file(b, std::string(300000, 'x')); + write_file(c, std::string(300000, 'x').replace(299999, 1, "y")); + + EXPECT_TRUE(same_content(a, b)); + EXPECT_FALSE(same_content(a, c)); + EXPECT_FALSE(same_content(a, tmp.path / "missing.bin")); +} + +TEST(BuildStage, VerifyModeParsing) { + EXPECT_EQ(parse_verify("content"), Verify::Content); + EXPECT_EQ(parse_verify("size"), Verify::Size); + EXPECT_EQ(parse_verify("nonsense"), Verify::Size); + EXPECT_EQ(parse_verify(""), Verify::Size); +} diff --git a/tests/unit/test_home.cpp b/tests/unit/test_home.cpp new file mode 100644 index 00000000..5bab58d5 --- /dev/null +++ b/tests/unit/test_home.cpp @@ -0,0 +1,83 @@ +#include +#include + +import std; +import mcpp.home; +import mcpp.toolchain.stdmod; + +namespace { + +// Set/restore an environment variable for the duration of a test. +class ScopedEnv { +public: + ScopedEnv(std::string name, const char* value) : name_(std::move(name)) { + if (const char* old = std::getenv(name_.c_str()); old) { + had_ = true; + old_ = old; + } + apply(value); + } + ~ScopedEnv() { apply(had_ ? old_.c_str() : nullptr); } + + ScopedEnv(const ScopedEnv&) = delete; + ScopedEnv& operator=(const ScopedEnv&) = delete; + +private: + void apply(const char* value) { +#if defined(_WIN32) + // _putenv_s with "" removes the variable on Windows. + ::_putenv_s(name_.c_str(), value ? value : ""); +#else + if (value) ::setenv(name_.c_str(), value, 1); + else ::unsetenv(name_.c_str()); +#endif + } + + std::string name_; + bool had_ = false; + std::string old_; +}; + +} // namespace + +TEST(Home, ExplicitMcppHomeWins) { + ScopedEnv home("MCPP_HOME", "/tmp/mcpp-home-test"); + EXPECT_EQ(mcpp::home::root(), std::filesystem::path("/tmp/mcpp-home-test")); + EXPECT_EQ(mcpp::home::bmi_root(), + std::filesystem::path("/tmp/mcpp-home-test") / "bmi"); +} + +TEST(Home, BmiRootIsAlwaysRootSlashBmi) { + ScopedEnv home("MCPP_HOME", "/tmp/mcpp-home-test-2"); + EXPECT_EQ(mcpp::home::bmi_root(), mcpp::home::root() / "bmi"); +} + +// The user-home fallback must land on `.mcpp`, never on the legacy flat +// `.mcpp-bmi` sibling that the pre-#311 std-module resolver produced whenever +// neither MCPP_HOME nor HOME resolved (the default on Windows PowerShell). +TEST(Home, FallbackUsesDotMcppNotLegacyFlatDir) { + ScopedEnv mcppHome("MCPP_HOME", nullptr); + ScopedEnv userHome("HOME", nullptr); +#if defined(_WIN32) + ScopedEnv profile("USERPROFILE", nullptr); +#endif + auto root = mcpp::home::root(); + EXPECT_EQ(root.filename(), std::filesystem::path(".mcpp")); + EXPECT_EQ(mcpp::home::bmi_root().filename(), std::filesystem::path("bmi")); + EXPECT_NE(mcpp::home::bmi_root().filename(), std::filesystem::path(".mcpp-bmi")); +} + +// #311's D2 regression gate: the std module cache and the dep BMI cache must +// resolve to ONE root. `default_cache_root()` used to be a private copy of the +// home resolution that drifted (no USERPROFILE, no self-contained detection), +// so on Windows the std BMI landed in the current working directory. +TEST(Home, StdModuleCacheRootEqualsHomeBmiRoot) { + { + ScopedEnv home("MCPP_HOME", "/tmp/mcpp-home-test-3"); + EXPECT_EQ(mcpp::toolchain::default_cache_root(), mcpp::home::bmi_root()); + } + { + ScopedEnv mcppHome("MCPP_HOME", nullptr); + EXPECT_EQ(mcpp::toolchain::default_cache_root(), mcpp::home::bmi_root()); + } +} diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 94fe8ebe..1791ae8d 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -799,3 +799,97 @@ TEST(NinjaBackend, LowercaseAsmHasNoDepfileAndItsOwnRule) { EXPECT_NE(ninja.find("build obj/lower.o : asm_object_raw src/lower.s"), std::string::npos) << ninja; } + +// ── #311: staging goes through `mcpp stage`, never an in-place shell copy ── + +TEST(NinjaBackend, StageRuleRunsThroughMcppWithRestat) { + auto plan = minimal_plan(); + auto ninja = emit_ninja_string(plan); + + EXPECT_NE(ninja.find("rule stage_file\n"), std::string::npos) << ninja; + EXPECT_NE(ninja.find(" command = $mcpp stage --output $out $in\n"), + std::string::npos) << ninja; + // restat is what keeps a skipped (already-equivalent) stage from dirtying + // every importer of the staged BMI. + auto rulePos = ninja.find("rule stage_file\n"); + auto restatPos = ninja.find(" restat = 1\n", rulePos); + auto nextRulePos = ninja.find("\nrule ", rulePos + 1); + ASSERT_NE(restatPos, std::string::npos) << ninja; + EXPECT_LT(restatPos, nextRulePos) << ninja; + + // The old per-platform in-place copies are gone for good. + EXPECT_EQ(ninja.find("Copy-Item"), std::string::npos) << ninja; + EXPECT_EQ(ninja.find("cp -f $in $out"), std::string::npos) << ninja; + EXPECT_EQ(ninja.find("cp_bmi"), std::string::npos) << ninja; +} + +TEST(NinjaBackend, McppBinaryIsBoundEvenWithoutDyndep) { + // dyndep off ⇒ no scan/collect rules, but stage_file still needs $mcpp. +#if defined(_WIN32) + ::_putenv_s("MCPP_NINJA_DYNDEP", "0"); +#else + ::setenv("MCPP_NINJA_DYNDEP", "0", 1); +#endif + auto plan = minimal_plan(); + auto ninja = emit_ninja_string(plan); +#if defined(_WIN32) + ::_putenv_s("MCPP_NINJA_DYNDEP", ""); +#else + ::unsetenv("MCPP_NINJA_DYNDEP"); +#endif + + EXPECT_EQ(ninja.find("rule cxx_scan"), std::string::npos) << ninja; + EXPECT_NE(ninja.find("\nmcpp = "), std::string::npos) << ninja; +} + +TEST(NinjaBackend, StdArtifactsAndRuntimeDllsUseTheStageRule) { + auto plan = minimal_plan(); + plan.toolchain.compiler = mcpp::toolchain::CompilerId::Clang; + plan.toolchain.binaryPath = "/usr/bin/clang++"; + plan.stdBmiPath = "/cache/bmi/fp/pcm.cache/std.pcm"; + plan.stdObjectPath = "/cache/bmi/fp/std.o"; + plan.stdCompatBmiPath = "/cache/bmi/fp/pcm.cache/std.compat.pcm"; + plan.stdCompatObjectPath = "/cache/bmi/fp/std.compat.o"; + plan.runtimeDeployFiles.push_back({"/pkg/lib/libfoo.dll", "bin/libfoo.dll"}); + + auto ninja = emit_ninja_string(plan); + + EXPECT_NE(ninja.find("build pcm.cache/std.pcm : stage_file " + "/cache/bmi/fp/pcm.cache/std.pcm"), + std::string::npos) << ninja; + EXPECT_NE(ninja.find("build obj/std.o : stage_file /cache/bmi/fp/std.o"), + std::string::npos) << ninja; + // std.compat keeps its order-only dependency on the staged std BMI. + EXPECT_NE(ninja.find("build pcm.cache/std.compat.pcm : stage_file " + "/cache/bmi/fp/pcm.cache/std.compat.pcm | pcm.cache/std.pcm"), + std::string::npos) << ninja; + // Windows DLL deployment shares the rule — and therefore the + // skip-if-equivalent behaviour that keeps a loaded DLL from failing a build. + EXPECT_NE(ninja.find("build bin/libfoo.dll : stage_file /pkg/lib/libfoo.dll"), + std::string::npos) << ninja; +} + +// ── #311: staging failures must stay readable through the output filter ── + +TEST(NinjaBackend, FilterKeepsStagingDiagnosticsAndFailedTarget) { + std::vector prefixes = {"/usr/bin/g++", "/opt/mcpp/bin/mcpp"}; + std::string raw = + "ninja: Entering directory `target/x86_64-linux-gnu/fp'\n" + "[1/3] STAGE pcm.cache/std.pcm\n" + "FAILED: [code=1] pcm.cache/std.pcm\n" + "/opt/mcpp/bin/mcpp stage --output pcm.cache/std.pcm /cache/std.pcm\n" + "error: cannot stage file into the build directory\n" + "hint: another process has this file memory-mapped, loaded or open.\n" + "ninja: build stopped: subcommand failed.\n"; + + auto filtered = filter_ninja_output(raw, prefixes); + + // Which output failed is now preserved (it used to be dropped entirely). + EXPECT_NE(filtered.find("failed: pcm.cache/std.pcm"), std::string::npos) << filtered; + EXPECT_EQ(filtered.find("[code=1]"), std::string::npos) << filtered; + // The diagnostic survives; the echoed command line does not. + EXPECT_NE(filtered.find("error: cannot stage file"), std::string::npos) << filtered; + EXPECT_NE(filtered.find("hint:"), std::string::npos) << filtered; + EXPECT_EQ(filtered.find("stage --output"), std::string::npos) << filtered; + EXPECT_EQ(filtered.find("ninja: Entering directory"), std::string::npos) << filtered; +} From 9d73ccaf6c2226c96755dd466ae15e39ce3fb3b7 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 11:32:37 +0800 Subject: [PATCH 2/8] =?UTF-8?q?chore:=202026.7.30.1=20=E2=80=94=20BMI=20st?= =?UTF-8?q?aging=20+=20cache-root=20convergence=20(#311)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bootstrap pin stays at 2026.7.29.1: it names an already-published mcpp and is bumped separately, after the release exists in xim-pkgindex. --- CHANGELOG.md | 32 ++++++++++++++++++++++++++++++++ mcpp.toml | 2 +- src/toolchain/fingerprint.cppm | 2 +- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 37febf58..e3fad40f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,38 @@ > 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。 > 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)。 +## [2026.7.30.1] — 2026-07-30 + +### 修复 + +- **[#311](https://github.com/mcpp-community/mcpp/issues/311) Windows 上 clangd 映射住 std BMI 会让整个构建报 `build failed`。** mcpp 把 staged std BMI 的路径写进 `compile_commands.json`,好让 clangd 解析 `import std;` —— clangd 于是把这个几十 MB 的文件 mmap 住;而 staging 步骤(`rule cp_bmi`)用 `powershell Copy-Item -Force` **原地覆写同一个文件**,Windows 拒绝替换带 user-mapped section 的文件(error 1224)。也就是说,mcpp 一边把这个路径交给别人读,一边在原地重写它。 + + POSIX 侧看不见:GNU `cp -f` 在目标打不开时会 unlink 重建,POSIX 本身也允许覆写被 mmap 的文件 —— 所以 CI 一直全绿,只有 Windows 用户中招。 + + staging 改为走 mcpp 自己的内部子命令 `mcpp stage`(形态对齐既有的 `mcpp dyndep`):**目标已等价就一个字节都不写**,否则先写同目录临时文件再 rename,再退化为原地覆写,失败按退避重试,最终失败时给出点名文件与可能持有者(clangd / 编辑器索引 / 杀软 / 上一次 `mcpp run` 还在跑的程序)的诊断。**绝不降级为 warning** —— staged BMI 过期或缺失会变成难以归因的 `module 'std' not found`,或者更糟:旧 BMI 配新 `std.o` 静默链接。 + + 「已等价就不写」的判据是 fingerprint:build dir(`target//`)与缓存目录(`$MCPP_HOME/bmi/`)共享同一个 fp,而 fp 已覆盖编译器身份/target triple/stdlib/std 源哈希/标准与方言 flag。dep BMI 缓存一直就是这么做的(`bmi_cache.cppm`: *"Existing project outputs are left untouched"*)—— std staging 是全仓库唯一强制覆写的那一处,这个不对称本身就是缺陷。 + + 同一条 rule 也用于 **Windows 运行期 DLL 部署**,因此「往上一次 `mcpp run` 还加载着的 DLL 上覆写」这个同类失败一并治好。 + +- **重新 stage 一个未变的 std BMI 不再重编整张模块图。** staged BMI 是每个 importer 的 implicit input,而 staging rule 既不保留 mtime 也没有 `restat`,于是缓存侧 BMI 只要 mtime 变新(在下面那个缓存根缺陷下,**换个 cwd 跑就会发生**),所有 `import std` 的 TU 全部重编 —— 即使字节完全相同。 + + 修法是「不写字节」+ `restat = 1`。实测:`touch` 缓存侧 BMI 后,只有 staging edge 重跑,`main.cpp` 不再重编,下一次构建回到 `no work to do`。 + + 一条记录在案的实现约束:**跳过时对 mtime 的任何触碰(包括对齐到 src 的 mtime)都会让 restat 失效并重新引发级联** —— ninja 的 restat 只把「mtime 未被命令改变」的输出视为从未需要构建。所以跳过路径不动任何时间戳。 + +### 变更 + +- **BMI 缓存根统一为 `$MCPP_HOME/bmi`。** `toolchain/stdmod.cppm` 的 `default_cache_root()` 是 home 解析逻辑的一份私有拷贝,自 v0.0.1 起一字未改:**没有 Windows 的 `USERPROFILE` 分支,也没有 self-contained 安装探测**。后果是 Windows PowerShell(不设 `HOME`)下 std BMI 缓存落进**当前工作目录**的 `.mcpp-bmi/`,而 dep BMI 缓存在 `%USERPROFILE%\.mcpp\bmi` —— 一个缓存两个根,其中一个还随 cwd 漂移(从子目录跑就重编一次 std);release tarball 形态的安装在 Linux 上同样分家。 + + 新增叶模块 `mcpp.home` 作为唯一解析器,`config` / `stdmod` / `prepare` 的 git 缓存,以及 `doctor` 里另外两份拷贝(其中 `self init --force` 那份在 self-contained 安装下会去删错的树)全部收敛过来。单测锁死 `default_cache_root() == mcpp::home::bmi_root()`。 + + **升级影响**:Windows 用户与 self-contained 安装的用户首次构建会重新编一次 std 模块(10–60 s);遗留的 `.mcpp-bmi/` 不再使用,`mcpp doctor` 会指出它,可手动删除。 + +- **`FAILED: ` 不再被输出过滤器整行丢掉**,归一成 `failed: ` 保留。#311 的报告读不出「失败的是 BMI staging 而不是编译」,一半原因就是这行被丢了。同时把 mcpp 自身的可执行路径纳入命令行前缀集合,于是被回显的命令行被过滤、mcpp 打印的诊断保留。 + +- **`mcpp new` 生成的 `.gitignore` 加上 `.mcpp/`**(per-project xlings sandbox,以及解析不出 MCPP_HOME 时的本地 BMI 缓存)。 + ## [2026.7.27.1] — 2026-07-27 ### 变更 diff --git a/mcpp.toml b/mcpp.toml index 97a1eca1..b2ca09a4 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,6 +1,6 @@ [package] name = "mcpp" -version = "2026.7.29.2" +version = "2026.7.30.1" 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 b6ab6281..ddbb0405 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 = "2026.7.29.2"; +inline constexpr std::string_view MCPP_VERSION = "2026.7.30.1"; struct FingerprintInputs { Toolchain toolchain; From 8af47ef349990ea1c195830bd5c91a1ba7fc3ffb Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 11:37:22 +0800 Subject: [PATCH 3/8] fix(build): report the staged file's absolute path and drop ninja's trailing pad MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two cosmetics found by driving a real staging failure end-to-end through ninja: the `FAILED: ` line ninja emits carries a trailing space, and `$out` is relative because ninja runs staging with cwd = the build directory — the reader of that diagnostic is the person who has to go unlock the file. --- src/build/ninja_backend.cppm | 3 +++ src/build/stage.cppm | 7 ++++++- tests/unit/test_ninja_backend.cpp | 6 ++++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index a33a2a2c..90006bee 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -349,6 +349,9 @@ std::string filter_ninja_output(std::string_view output, if (auto close = target.find(']'); close != std::string::npos) target = ltrim_copy(target.substr(close + 1)); } + while (!target.empty() + && std::isspace(static_cast(target.back()))) + target.pop_back(); if (target.empty()) continue; filtered += "failed: "; filtered += target; diff --git a/src/build/stage.cppm b/src/build/stage.cppm index dee607b4..602811f1 100644 --- a/src/build/stage.cppm +++ b/src/build/stage.cppm @@ -124,6 +124,11 @@ std::error_code write_once(const std::filesystem::path& src, std::string failure_message(const std::filesystem::path& src, const std::filesystem::path& dst, const std::error_code& ec) { + // ninja runs staging with cwd = the build directory, so `$out` is relative. + // Print it absolute: the reader has to go find (or unlock) this file. + std::error_code aec; + auto shown = std::filesystem::absolute(dst, aec); + if (aec) shown = dst; return std::format( "cannot stage file into the build directory\n" " file: {}\n" @@ -135,7 +140,7 @@ std::string failure_message(const std::filesystem::path& src, " editor/IDE indexer, antivirus, or — for a .dll — a still-running\n" " program from a previous `mcpp run`. Close it or restart clangd,\n" " then re-run the build.", - dst.string(), src.string(), ec.value(), ec.message()); + shown.string(), src.string(), ec.value(), ec.message()); } } // namespace diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 1791ae8d..712a8482 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -876,7 +876,7 @@ TEST(NinjaBackend, FilterKeepsStagingDiagnosticsAndFailedTarget) { std::string raw = "ninja: Entering directory `target/x86_64-linux-gnu/fp'\n" "[1/3] STAGE pcm.cache/std.pcm\n" - "FAILED: [code=1] pcm.cache/std.pcm\n" + "FAILED: [code=1] pcm.cache/std.pcm \n" "/opt/mcpp/bin/mcpp stage --output pcm.cache/std.pcm /cache/std.pcm\n" "error: cannot stage file into the build directory\n" "hint: another process has this file memory-mapped, loaded or open.\n" @@ -885,7 +885,9 @@ TEST(NinjaBackend, FilterKeepsStagingDiagnosticsAndFailedTarget) { auto filtered = filter_ninja_output(raw, prefixes); // Which output failed is now preserved (it used to be dropped entirely). - EXPECT_NE(filtered.find("failed: pcm.cache/std.pcm"), std::string::npos) << filtered; + // ninja pads the target list with a trailing space; the normalized line + // must not inherit it. + EXPECT_NE(filtered.find("failed: pcm.cache/std.pcm\n"), std::string::npos) << filtered; EXPECT_EQ(filtered.find("[code=1]"), std::string::npos) << filtered; // The diagnostic survives; the echoed command line does not. EXPECT_NE(filtered.find("error: cannot stage file"), std::string::npos) << filtered; From 54aede90564c0591b76176efdbc5f1f8f08ee02f Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 11:39:11 +0800 Subject: [PATCH 4/8] test(e2e): make the locked-destination case unable to pass for the wrong reason MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two holes in 171 as first written: a PowerShell holder that failed to start left the build trivially succeeding (silent pass), and `chmod 444` is toothless under root — which the container e2e job runs as. The holder now signals readiness through a sentinel and the test fails if it never appears, and the load-bearing assertion is inode+mtime+size invariance of the destination: 'nothing was written', not 'the write happened to succeed'. --- tests/e2e/171_bmi_staging_locked_dest.sh | 37 ++++++++++++++++++------ 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/tests/e2e/171_bmi_staging_locked_dest.sh b/tests/e2e/171_bmi_staging_locked_dest.sh index 20aa478e..919dfac7 100755 --- a/tests/e2e/171_bmi_staging_locked_dest.sh +++ b/tests/e2e/171_bmi_staging_locked_dest.sh @@ -1,7 +1,7 @@ #!/usr/bin/env bash # requires: # 171_bmi_staging_locked_dest.sh — mcpp#311: staging must survive a destination -# that another process is holding, and must fail LOUDLY when it genuinely can't. +# another process is holding, and must fail LOUDLY when it genuinely can't. # # The reported failure: mcpp writes the staged std BMI path into # compile_commands.json so clangd can resolve `import std;`, clangd @@ -9,11 +9,15 @@ # to overwrite it in place → Windows error 1224 (a file with a user-mapped # section open cannot be replaced) → the whole build reported "build failed". # -# Windows: reproduce it exactly, WITHOUT needing clangd — a background -# PowerShell holds a MemoryMappedFile on the staged BMI. -# POSIX: the closest analogue an ordinary test can create is an unwritable -# destination (chmod 444); the old `cp -f` silently unlinked and rewrote it, -# the new path must not need to write at all. +# Windows: reproduce that exactly and WITHOUT clangd — a background PowerShell +# holds a MemoryMappedFile on the staged BMI. The holder signals readiness +# through a sentinel file, so a PowerShell that never started fails the test +# instead of letting it pass for the wrong reason. +# +# The load-bearing assertion is platform-neutral and root-proof: an equivalent +# destination must not be written AT ALL (same inode, same mtime, same size). +# Permissions alone would not do — root ignores them, and the container e2e job +# runs as root. set -e TMP=$(mktemp -d) @@ -41,6 +45,9 @@ DST="$(dirname "$NINJA")/$(echo "$EDGE" | awk '{print $2}')" SRC=$(echo "$EDGE" | awk '{print $NF}') [[ -f "$DST" ]] || { echo "FAIL: staged BMI missing at $DST"; exit 1; } +identity() { stat -c '%i %Y %s' "$1"; } +BEFORE=$(identity "$DST") + # Dirty the staging edge without touching the shared cache's CONTENT. touch "$SRC" @@ -48,11 +55,17 @@ HOLDER="" case "$(uname -s)" in MINGW* | MSYS* | CYGWIN*) WINDST="$(cygpath -w "$DST")" + READY="$TMP/mapped.flag" + WINREADY="$(cygpath -w "$READY")" powershell -NoProfile -Command \ - "\$mm = [System.IO.MemoryMappedFiles.MemoryMappedFile]::CreateFromFile('$WINDST', [System.IO.FileMode]::Open); Start-Sleep -Seconds 90" & + "\$mm = [System.IO.MemoryMappedFiles.MemoryMappedFile]::CreateFromFile('$WINDST', [System.IO.FileMode]::Open); \ + Set-Content -Path '$WINREADY' -Value 'mapped'; Start-Sleep -Seconds 120" & HOLDER=$! - # Give the mapping time to exist before the build races it. - sleep 3 + for _ in $(seq 1 40); do [[ -f "$READY" ]] && break; sleep 1; done + [[ -f "$READY" ]] || { + kill "$HOLDER" 2>/dev/null || true + echo "FAIL: could not map the staged BMI — the test would not have proven anything" + exit 1; } echo "holding a user-mapped section on $WINDST (pid $HOLDER)" ;; *) @@ -75,6 +88,12 @@ if [[ $rc -ne 0 ]]; then fi grep -q "stage --output" build2.log || { cat build2.log; echo "FAIL: the staging edge did not run"; exit 1; } + +# The real assertion: nothing was written. Not "the write succeeded anyway". +AFTER=$(identity "$DST") +[[ "$BEFORE" == "$AFTER" ]] || { + echo "FAIL: an equivalent destination was rewritten (was '$BEFORE', now '$AFTER')" + exit 1; } cmp "$SRC" "$DST" || { echo "FAIL: held destination no longer matches the cache"; exit 1; } cd "$TMP" From 3d822fe3a8dd189ee75baaac25e453fb0d6a3604 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 11:59:38 +0800 Subject: [PATCH 5/8] fix(build): verify staging by content, and route the last in-place copy through it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Windows CI caught two things the Linux-only self-checks structurally could not. 1. `rule runtime_alias` was a SECOND `Copy-Item -Force`. PE has no soname symlink, so an alias is a copy of a freshly built DLL — the same hazard as BMI staging (a program still running from a previous `mcpp run` holds the old one). Its Windows branch now goes through `$mcpp stage` too; POSIX keeps `ln -s`, where the symlink is semantics and not merely how the file is written. The 'no Copy-Item anywhere' assertion is a no-op on Linux, which is why only the Windows job could find this. 2. Size-only equivalence was unsound for exactly those DLL payloads: PE section padding makes 'genuinely rebuilt, identical size' ordinary, so a stale DLL could survive in the build dir. Content comparison is unconditionally correct — a destination equal to the source needs no write — and it only runs when ninja has already decided the edge is dirty. It is now the default; `--verify size` / MCPP_STAGE_VERIFY=size stays for callers that know the source is fingerprint-scoped, and an unrecognized value falls back to the SAFE mode rather than the fast one. Also from CI: a read-only destination is replaceable on POSIX (rename rewrites the directory entry) but not on Windows, so that test now asserts both outcomes per platform instead of one; the e2e scripts unescape ninja node names (a Windows drive letter arrives as `C$:/Users/...`) and use BSD `stat -f` when GNU `stat -c` is absent (macOS). --- ...ue311-bmi-staging-and-cache-root-design.md | 21 +++++++-- ...2026-07-30-issue311-implementation-plan.md | 2 +- CHANGELOG.md | 4 +- src/build/ninja_backend.cppm | 14 +++++- src/build/stage.cppm | 33 +++++++------ tests/e2e/170_bmi_staging_no_cascade.sh | 16 +++++-- tests/e2e/171_bmi_staging_locked_dest.sh | 17 +++++-- tests/unit/test_build_stage.cpp | 47 ++++++++++++------- tests/unit/test_ninja_backend.cpp | 24 +++++++++- 9 files changed, 128 insertions(+), 50 deletions(-) diff --git a/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md b/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md index 9cabe078..67598643 100644 --- a/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md +++ b/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md @@ -195,7 +195,7 @@ mcpp stage --output | 步 | 条件 | 行为 | |---|---|---| | 1 | `src` 不存在 | 报错退出 1（这是真 bug，不容忍） | -| 2 | `dst` 存在 且 `size(dst) == size(src)` 且（`--verify content` 时字节亦相等） | **一个字节都不写，也不动任何时间戳**；退出 0 | +| 2 | `dst` 存在 且 `size(dst) == size(src)` 且**内容逐字节相等**(默认;`--verify size` 时只比 size) | **一个字节都不写,也不动任何时间戳**;退出 0 | | 3 | 否则 | 写 `dst.tmp.`（同目录）→ `rename` 覆盖 `dst` | | 4 | 步 3 失败 | 退化为原地覆写（`copy_file` overwrite）——赢下"可写但不可删"的持有者形态 | | 5 | 步 3+4 都失败 | 退避重试 3 次（100/300/900 ms）后仍失败 → 结构化诊断 + 退出 1 | @@ -220,9 +220,14 @@ mcpp stage --output 也就是说：**std staging 是全仓库唯一强制覆写的那一处**，这个不对称本身就是缺陷。 S1 把它拉回一致。 -内容校验默认关闭（只比 size）：上面的 fp 判据已经足够强，逐字节比较要多读两遍 31 MB。 -留开关是为了排障（`--verify content` / `MCPP_STAGE_VERIFY=content`）。选逐字节比较而不是 -hash：同样的 I/O 成本，但没有碰撞面，且能提前退出。 +**内容校验是默认,不是开关**(这一条被 Windows CI 纠正过一次)。初稿让 size 相等即视为已 +staged,理由是 fp 判据;但同一条 rule 还搬 **DLL**(`runtimeDeployFiles`、以及 Windows 上的 +`runtime_alias`),而 PE 的节对齐让「真的重建了、大小却一模一样」十分常见 —— size-only 会把 +一个过期的 DLL 留在 build dir 里。逐字节比较则**无条件正确**:内容相同就是不需要写。 + +成本可接受,因为这个判断只在 ninja 已经认定 edge 脏了才会跑(罕见),此时多读两遍 31 MB 换 +的是正确性。`--verify size` / `MCPP_STAGE_VERIFY=size` 保留给「调用方确知源是 fp 作用域」的 +快路径。选逐字节比较而不是 hash:同样的 I/O 成本,但没有碰撞面,且能提前退出。 **为什么步 2 连时间戳都不碰**（这一条实测修正过一次）： @@ -297,6 +302,12 @@ staging rule 加 `restat = 1`。这是 S1 步 2"不写字节"能真正省掉级 部署复用的是同一条 rule（`ninja_backend.cppm:1077-1083`）。**DLL 部署是同一类失败的第二个 受害者**（往正在运行的进程/调试器已加载的 DLL 上 `Copy-Item -Force` 同样失败），S1 一并治好。 +还有**第三个**:`rule runtime_alias`(`ninja_backend.cppm:758-764`)在 Windows 上也是一条 +`Copy-Item -Force` —— PE 没有 soname 符号链接,别名就是刚建出来的 DLL 的副本,同一危险类。 +Windows 分支一并改走 `$mcpp stage`(+ `restat`),**POSIX 分支保持 `ln -s` 不变**:那里符号 +链接是语义而不只是写法(改成拷贝会改变打包产物)。这一处是 Windows CI 抓出来的 —— 初稿的 +「`Copy-Item` 归零」自查项在本地(Linux)恒为真,永远不会失败。 + 另有一处必改：`mcpp = ` 这个 ninja 变量目前只在 `if (dyndep)` 里绑定 （`ninja_backend.cppm:403-409`）。新 rule 在**所有**配置下都要用它，必须把绑定提到该 条件之外，否则 GCC 非 dyndep 路径会生成 `$mcpp` 为空的命令。 @@ -335,7 +346,7 @@ staging rule 加 `restat = 1`。这是 S1 步 2"不写字节"能真正省掉级 | 风险 | 缓解 | |---|---| -| size 相等但内容不同（S1 步 2 误跳过） | fp 判据 + `--verify=hash` 开关；e2e 用"改一个字节但保持长度"的负例锁住 hash 模式 | +| size 相等但内容不同(S1 步 2 误跳过) | **默认逐字节比较**,单测双向锁住(默认必拷贝 / `--verify size` 才跳过) | | `restat = 1` 引入意外的"永不重建" | 单测锁 rule 文本；e2e 正例：改 std 源/换 toolchain ⇒ fp 变 ⇒ 新 build dir，不受影响 | | 收敛缓存根导致老用户一次性重编 std | 只影响 self-contained 与 Windows 两种形态；一次 10–60 s；CHANGELOG 写明，doctor 提示遗留目录 | | `mcpp` 变量提取出 `if (dyndep)` 后 ninja 文本变化 | `tests/unit/test_ninja_backend.cpp` 断言两种配置（dyndep on/off）下都绑定 | diff --git a/.agents/docs/2026-07-30-issue311-implementation-plan.md b/.agents/docs/2026-07-30-issue311-implementation-plan.md index 280f3bcd..e0cd961c 100644 --- a/.agents/docs/2026-07-30-issue311-implementation-plan.md +++ b/.agents/docs/2026-07-30-issue311-implementation-plan.md @@ -84,7 +84,7 @@ std::expected stage_file( hint 必须点名 clangd 与 `compile_commands.json` 的因果）。 8. 每次退出前清理残留 `dst.tmp.`。 -`Verify::Content` 由 `MCPP_STAGE_VERIFY=content` 或 `--verify content` 打开；实现是分块 +`Verify::Content` 是**默认**；`--verify size` / `MCPP_STAGE_VERIFY=size` 才退回只比 size。实现是分块 逐字节比较（同 I/O 成本、无碰撞面、可提前退出），不引入 hash 依赖。 **CLI 接线**（照 `dyndep` 的形状）： diff --git a/CHANGELOG.md b/CHANGELOG.md index e3fad40f..ee0bd855 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,9 +13,9 @@ staging 改为走 mcpp 自己的内部子命令 `mcpp stage`(形态对齐既有的 `mcpp dyndep`):**目标已等价就一个字节都不写**,否则先写同目录临时文件再 rename,再退化为原地覆写,失败按退避重试,最终失败时给出点名文件与可能持有者(clangd / 编辑器索引 / 杀软 / 上一次 `mcpp run` 还在跑的程序)的诊断。**绝不降级为 warning** —— staged BMI 过期或缺失会变成难以归因的 `module 'std' not found`,或者更糟:旧 BMI 配新 `std.o` 静默链接。 - 「已等价就不写」的判据是 fingerprint:build dir(`target//`)与缓存目录(`$MCPP_HOME/bmi/`)共享同一个 fp,而 fp 已覆盖编译器身份/target triple/stdlib/std 源哈希/标准与方言 flag。dep BMI 缓存一直就是这么做的(`bmi_cache.cppm`: *"Existing project outputs are left untouched"*)—— std staging 是全仓库唯一强制覆写的那一处,这个不对称本身就是缺陷。 + 「已等价」由**逐字节比较**判定 —— 无条件正确:内容相同就是不需要写。这个判断只在 ninja 已经认定 edge 脏了才会跑,所以代价可以忽略。(`--verify size` 保留给确知源是 fingerprint 作用域的调用方:build dir 与缓存目录共享同一个 fp,而 fp 已覆盖编译器身份/target triple/stdlib/std 源哈希/标准与方言 flag。但它**不是默认** —— 同一条 rule 还搬 DLL,而 PE 的节对齐让「真的重建了、大小却一样」十分常见。)dep BMI 缓存一直就是「已存在就不动」的(`bmi_cache.cppm`: *"Existing project outputs are left untouched"*)—— std staging 是全仓库唯一强制覆写的那一处,这个不对称本身就是缺陷。 - 同一条 rule 也用于 **Windows 运行期 DLL 部署**,因此「往上一次 `mcpp run` 还加载着的 DLL 上覆写」这个同类失败一并治好。 + 同一条 rule 也用于 **Windows 运行期 DLL 部署**,以及 Windows 上的 `runtime_alias`(PE 没有 soname 符号链接,别名就是刚建出来的 DLL 的副本),因此「往上一次 `mcpp run` 还加载着的 DLL 上覆写」这个同类失败一并治好。POSIX 的 `runtime_alias` 保持符号链接不变 —— 那里符号链接是语义,不只是写法。 - **重新 stage 一个未变的 std BMI 不再重编整张模块图。** staged BMI 是每个 importer 的 implicit input,而 staging rule 既不保留 mtime 也没有 `restat`,于是缓存侧 BMI 只要 mtime 变新(在下面那个缓存根缺陷下,**换个 cwd 跑就会发生**),所有 `import std` 的 TU 全部重编 —— 即使字节完全相同。 diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 90006bee..2ff64e1f 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -757,11 +757,21 @@ std::string emit_ninja_string(const BuildPlan& plan) { append("rule runtime_alias\n"); if constexpr (mcpp::platform::is_windows) { - append(" command = powershell -NoProfile -Command \"Copy-Item -Force '$in' -Destination '$out'\"\n"); + // PE has no soname symlink, so the alias is a copy — and a copy of a + // just-rebuilt DLL is exactly the hazard #311 is about (a program still + // running from a previous `mcpp run` holds the old one). Route it + // through the same staging primitive rather than a second in-place + // PowerShell copy. Content-verified, so a same-size rebuild still + // refreshes the alias. + append(" command = $mcpp stage --output $out $in\n"); } else { append(" command = mkdir -p $$(dirname $out) && rm -f $out && ln -s $$(basename $in) $out\n"); } - append(" description = ALIAS $out\n\n"); + append(" description = ALIAS $out\n"); + if constexpr (mcpp::platform::is_windows) { + append(" restat = 1\n"); + } + append("\n"); if (dyndep) { // Scan rule: produce P1689 .ddi for one TU. diff --git a/src/build/stage.cppm b/src/build/stage.cppm index 602811f1..67fa1280 100644 --- a/src/build/stage.cppm +++ b/src/build/stage.cppm @@ -16,14 +16,17 @@ // // The rules this module implements: // -// 1. Never write bytes we don't have to. The destination and the source live -// under the SAME build fingerprint (target// vs -// $MCPP_HOME/bmi/), and the fingerprint already covers compiler -// identity, target triple, stdlib, std module source hash and the -// standard/dialect flags. Same fingerprint + same size ⇒ equivalent ⇒ -// the file is already staged. This mirrors what the dep BMI cache has -// always done (bmi_cache.cppm: "Existing project outputs are left -// untouched"). +// 1. Never write bytes that are already there. Equivalence is decided by +// COMPARING CONTENT, which is unconditionally correct — a destination +// identical to the source needs no write, whatever the reason. Size alone +// is only a heuristic and is available as `--verify size` for the paths +// where the caller knows better (a std BMI is fingerprint-scoped: the +// cache dir and the build dir share the fp that covers compiler identity, +// target triple, stdlib, std source hash and the dialect flags). It is +// NOT the default, because staging also carries .dll payloads whose PE +// section padding makes "same size, different bytes" ordinary. +// Skipping mirrors what the dep BMI cache has always done +// (bmi_cache.cppm: "Existing project outputs are left untouched"). // 2. When we do write, write out of place and rename — atomic for readers, // and it survives a transient sharing violation (antivirus, indexer) // that an in-place overwrite would lose to. @@ -49,12 +52,12 @@ export namespace mcpp::build::stage { // How hard to look before declaring the destination already-staged. enum class Verify { - Size, // default: size match under an identical build fingerprint - Content, // byte-for-byte compare (MCPP_STAGE_VERIFY=content / --verify content) + Content, // default: byte-for-byte compare — always correct + Size, // size match only (MCPP_STAGE_VERIFY=size / --verify size) }; struct StageOptions { - Verify verify = Verify::Size; + Verify verify = Verify::Content; int retries = 3; // attempts after the first std::chrono::milliseconds backoff{100}; // ×3 per retry: 100/300/900ms }; @@ -76,7 +79,8 @@ std::expected stage_file(const std::filesystem::path& // unreadable or the sizes differ. bool same_content(const std::filesystem::path& a, const std::filesystem::path& b); -// Parse a --verify / MCPP_STAGE_VERIFY value. Unknown values fall back to Size. +// Parse a --verify / MCPP_STAGE_VERIFY value. Unknown values fall back to the +// safe default (Content). Verify parse_verify(std::string_view value); } // namespace mcpp::build::stage @@ -171,7 +175,7 @@ bool same_content(const std::filesystem::path& a, const std::filesystem::path& b } Verify parse_verify(std::string_view value) { - return value == "content" ? Verify::Content : Verify::Size; + return value == "size" ? Verify::Size : Verify::Content; } std::expected stage_file(const std::filesystem::path& src, @@ -193,7 +197,8 @@ std::expected stage_file(const std::filesystem::path& } } - // Already staged? (see module comment for why size is a sound default) + // Already staged? Content by default — size is the caller-opt-in shortcut + // (see module comment). std::error_code sec; if (std::filesystem::is_regular_file(dst, sec)) { std::error_code se1, se2; diff --git a/tests/e2e/170_bmi_staging_no_cascade.sh b/tests/e2e/170_bmi_staging_no_cascade.sh index 7cb54eb3..ba49a522 100755 --- a/tests/e2e/170_bmi_staging_no_cascade.sh +++ b/tests/e2e/170_bmi_staging_no_cascade.sh @@ -17,6 +17,14 @@ # Toolchain-neutral: only needs a package that says `import std;`. set -e +# build.ninja node names are ninja-ESCAPED: on Windows a drive letter arrives as +# `C$:/Users/...`. Unescape before touching the filesystem (the Windows job +# failed on exactly this). +unescape_ninja() { printf '%s' "$1" | sed 's/\$:/:/g; s/\$\$/$/g'; } +# Compare paths across the Windows fork: MCPP_HOME is `C:\Users\...` while ninja +# writes forward slashes. +norm_path() { printf '%s' "$1" | tr '\\' '/'; } + TMP=$(mktemp -d) trap "rm -rf $TMP" EXIT cd "$TMP" @@ -44,13 +52,13 @@ EDGE=$(grep -E '^build [^ ]+ : stage_file ' "$NINJA" | head -1) [[ -n "$EDGE" ]] || { grep -n "std" "$NINJA" | head -20 echo "FAIL: no stage_file edge for the std BMI (did import std resolve?)"; exit 1; } -DST=$(echo "$EDGE" | awk '{print $2}') -SRC=$(echo "$EDGE" | awk '{print $NF}') +DST=$(unescape_ninja "$(echo "$EDGE" | awk '{print $2}')") +SRC=$(unescape_ninja "$(echo "$EDGE" | awk '{print $NF}')") echo "staging: $SRC -> $DST" # ── invariant 2: the cache root is $MCPP_HOME/bmi, never a cwd-local dir ── -HOME_ROOT="${MCPP_HOME:-$HOME/.mcpp}" -case "$SRC" in +HOME_ROOT=$(norm_path "${MCPP_HOME:-$HOME/.mcpp}") +case "$(norm_path "$SRC")" in "$HOME_ROOT"/bmi/*) ;; *) echo "FAIL: std BMI cache is '$SRC', expected under '$HOME_ROOT/bmi'"; exit 1 ;; esac diff --git a/tests/e2e/171_bmi_staging_locked_dest.sh b/tests/e2e/171_bmi_staging_locked_dest.sh index 919dfac7..547bee00 100755 --- a/tests/e2e/171_bmi_staging_locked_dest.sh +++ b/tests/e2e/171_bmi_staging_locked_dest.sh @@ -20,6 +20,14 @@ # runs as root. set -e +# build.ninja node names are ninja-ESCAPED: on Windows a drive letter arrives as +# `C$:/Users/...`. Unescape before touching the filesystem (the Windows job +# failed on exactly this). +unescape_ninja() { printf '%s' "$1" | sed 's/\$:/:/g; s/\$\$/$/g'; } +# Compare paths across the Windows fork: MCPP_HOME is `C:\Users\...` while ninja +# writes forward slashes. +norm_path() { printf '%s' "$1" | tr '\\' '/'; } + TMP=$(mktemp -d) trap "rm -rf $TMP" EXIT cd "$TMP" @@ -41,11 +49,14 @@ cd app NINJA=$(find target -name build.ninja | head -1) EDGE=$(grep -E '^build [^ ]+ : stage_file ' "$NINJA" | head -1) [[ -n "$EDGE" ]] || { echo "FAIL: no stage_file edge for the std BMI"; exit 1; } -DST="$(dirname "$NINJA")/$(echo "$EDGE" | awk '{print $2}')" -SRC=$(echo "$EDGE" | awk '{print $NF}') +DST="$(dirname "$NINJA")/$(unescape_ninja "$(echo "$EDGE" | awk '{print $2}')")" +SRC=$(unescape_ninja "$(echo "$EDGE" | awk '{print $NF}')") [[ -f "$DST" ]] || { echo "FAIL: staged BMI missing at $DST"; exit 1; } -identity() { stat -c '%i %Y %s' "$1"; } +# GNU stat vs BSD/macOS stat (the macOS job failed on `stat -c`). +identity() { + stat -c '%i %Y %s' "$1" 2>/dev/null || stat -f '%i %m %z' "$1" +} BEFORE=$(identity "$DST") # Dirty the staging edge without touching the shared cache's CONTENT. diff --git a/tests/unit/test_build_stage.cpp b/tests/unit/test_build_stage.cpp index 2a629056..192c061b 100644 --- a/tests/unit/test_build_stage.cpp +++ b/tests/unit/test_build_stage.cpp @@ -35,7 +35,7 @@ std::string read_file(const std::filesystem::path& p) { // Retries would only slow the failure tests down; the behaviour under test is // the decision, not the backoff. -StageOptions no_retry(Verify v = Verify::Size) { +StageOptions no_retry(Verify v = Verify::Content) { return StageOptions{.verify = v, .retries = 0, .backoff = std::chrono::milliseconds{0}}; } @@ -75,33 +75,35 @@ TEST(BuildStage, EquivalentDestinationIsNotTouched) { EXPECT_EQ(std::filesystem::last_write_time(dst), recorded); } -// Same size, different bytes: the size-only default deliberately treats this as -// already-staged (destination and source share a build fingerprint, which -// covers compiler identity, stdlib and std source hash). -TEST(BuildStage, SameSizeDifferentBytesIsSkippedUnderSizeVerify) { +// Same size, different bytes must be copied by DEFAULT. Staging also carries +// .dll payloads, where PE section padding makes equal sizes across a real +// rebuild ordinary — so size-only equivalence cannot be the default. +TEST(BuildStage, SameSizeDifferentBytesIsCopiedByDefault) { Tmp tmp; auto src = tmp.path / "src.bin"; auto dst = tmp.path / "dst.bin"; write_file(src, "AAAA"); write_file(dst, "BBBB"); - auto r = stage_file(src, dst, no_retry(Verify::Size)); + auto r = stage_file(src, dst, no_retry()); ASSERT_TRUE(r.has_value()); - EXPECT_FALSE(r->copied); - EXPECT_EQ(read_file(dst), "BBBB"); + EXPECT_TRUE(r->copied); + EXPECT_EQ(read_file(dst), "AAAA"); } -TEST(BuildStage, SameSizeDifferentBytesIsCopiedUnderContentVerify) { +// `--verify size` is the opt-in shortcut for callers that know the source is +// fingerprint-scoped; it accepts the same-size destination as already staged. +TEST(BuildStage, SameSizeDifferentBytesIsSkippedUnderSizeVerify) { Tmp tmp; auto src = tmp.path / "src.bin"; auto dst = tmp.path / "dst.bin"; write_file(src, "AAAA"); write_file(dst, "BBBB"); - auto r = stage_file(src, dst, no_retry(Verify::Content)); + auto r = stage_file(src, dst, no_retry(Verify::Size)); ASSERT_TRUE(r.has_value()); - EXPECT_TRUE(r->copied); - EXPECT_EQ(read_file(dst), "AAAA"); + EXPECT_FALSE(r->copied); + EXPECT_EQ(read_file(dst), "BBBB"); } TEST(BuildStage, DifferentSizeIsAlwaysCopied) { @@ -124,10 +126,12 @@ TEST(BuildStage, MissingSourceIsAnError) { EXPECT_NE(r.error().message.find("nope.bin"), std::string::npos); } -// A read-only destination is the closest POSIX analogue of the Windows holder: -// an in-place overwrite cannot open it, so staging must go through the -// temp-file + rename path. -TEST(BuildStage, ReadOnlyDestinationIsReplacedViaRename) { +// A read-only destination that must actually be replaced: POSIX can do it +// (the directory entry is what gets rewritten, so temp-file + rename wins), +// Windows cannot (replacing a read-only file is denied however you spell it) — +// and there the contract is the loud failure, not a silent skip. Asserted per +// platform rather than skipped, so neither side can rot unnoticed. +TEST(BuildStage, ReadOnlyDestinationOutcomeIsPlatformDefined) { Tmp tmp; auto src = tmp.path / "src.bin"; auto dst = tmp.path / "dst.bin"; @@ -136,9 +140,15 @@ TEST(BuildStage, ReadOnlyDestinationIsReplacedViaRename) { std::filesystem::permissions(dst, std::filesystem::perms::owner_read); auto r = stage_file(src, dst, no_retry()); +#if defined(_WIN32) + ASSERT_FALSE(r.has_value()); + EXPECT_NE(r.error().message.find("hint:"), std::string::npos); + EXPECT_EQ(read_file(dst), "old"); +#else ASSERT_TRUE(r.has_value()) << r.error().message; EXPECT_TRUE(r->copied); EXPECT_EQ(read_file(dst), "new-content"); +#endif } // When staging genuinely cannot proceed, the failure must name the file and @@ -176,6 +186,7 @@ TEST(BuildStage, SameContentComparesBytesNotJustSize) { TEST(BuildStage, VerifyModeParsing) { EXPECT_EQ(parse_verify("content"), Verify::Content); EXPECT_EQ(parse_verify("size"), Verify::Size); - EXPECT_EQ(parse_verify("nonsense"), Verify::Size); - EXPECT_EQ(parse_verify(""), Verify::Size); + // Anything unrecognized must land on the SAFE mode, not the fast one. + EXPECT_EQ(parse_verify("nonsense"), Verify::Content); + EXPECT_EQ(parse_verify(""), Verify::Content); } diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index 712a8482..c683e3f6 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -817,12 +817,34 @@ TEST(NinjaBackend, StageRuleRunsThroughMcppWithRestat) { ASSERT_NE(restatPos, std::string::npos) << ninja; EXPECT_LT(restatPos, nextRulePos) << ninja; - // The old per-platform in-place copies are gone for good. + // The old per-platform in-place copies are gone for good — including the + // second one, in runtime_alias, which the Windows job caught still + // shelling out to PowerShell. EXPECT_EQ(ninja.find("Copy-Item"), std::string::npos) << ninja; EXPECT_EQ(ninja.find("cp -f $in $out"), std::string::npos) << ninja; EXPECT_EQ(ninja.find("cp_bmi"), std::string::npos) << ninja; } +TEST(NinjaBackend, RuntimeAliasCopiesThroughStageOnWindowsAndSymlinksElsewhere) { + auto plan = minimal_plan(); + auto ninja = emit_ninja_string(plan); + auto rulePos = ninja.find("rule runtime_alias\n"); + ASSERT_NE(rulePos, std::string::npos) << ninja; + auto body = ninja.substr(rulePos, ninja.find("\nrule ", rulePos + 1) - rulePos); + + if constexpr (mcpp::platform::is_windows) { + // PE has no soname symlink: the alias is a copy of a freshly built DLL, + // the same hazard class as BMI staging. + EXPECT_NE(body.find("$mcpp stage --output $out $in"), std::string::npos) << body; + EXPECT_NE(body.find("restat = 1"), std::string::npos) << body; + } else { + // POSIX must keep the SYMLINK — turning it into a copy would change + // what gets packaged, not just how it is written. + EXPECT_NE(body.find("ln -s"), std::string::npos) << body; + EXPECT_EQ(body.find("stage --output"), std::string::npos) << body; + } +} + TEST(NinjaBackend, McppBinaryIsBoundEvenWithoutDyndep) { // dyndep off ⇒ no scan/collect rules, but stage_file still needs $mcpp. #if defined(_WIN32) From 5beb9351465e3252102cd94060e2457a05741816 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 12:13:21 +0800 Subject: [PATCH 6/8] test(e2e): don't let `wait` on the killed holder abort 171 (exit 143) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `wait` on a job we just killed returns 143, and it sat as the last command of an `&&` list under `set -e` — so on Windows the script died the moment the mapping was released, before asserting anything about the build it had just run. Probed the shell semantics directly rather than guessing: the old form exits 143 on Linux too, it only never ran there because HOLDER is empty on POSIX. --- tests/e2e/171_bmi_staging_locked_dest.sh | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/e2e/171_bmi_staging_locked_dest.sh b/tests/e2e/171_bmi_staging_locked_dest.sh index 547bee00..d2bd3a97 100755 --- a/tests/e2e/171_bmi_staging_locked_dest.sh +++ b/tests/e2e/171_bmi_staging_locked_dest.sh @@ -89,7 +89,10 @@ set +e "$MCPP" build -v > build2.log 2>&1 rc=$? set -e -[[ -n "$HOLDER" ]] && { kill "$HOLDER" 2>/dev/null; wait "$HOLDER" 2>/dev/null; } +# `wait` on a job we just killed returns 143, and under `set -e` that would +# abort the script right here — with the build's own result never asserted. +# (Exactly how this test failed on Windows the first time it got that far.) +[[ -n "$HOLDER" ]] && { kill "$HOLDER" 2>/dev/null || true; wait "$HOLDER" 2>/dev/null || true; } chmod 644 "$DST" 2>/dev/null || true if [[ $rc -ne 0 ]]; then From ffc29ed14618ad13230dd13f62354571fee942c4 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 12:42:40 +0800 Subject: [PATCH 7/8] fix(env): private-glibc strip was defeated by the composed override MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found while attributing an e2e 156 failure on this branch (NOT caused by the staging change — proven by diffing the generated build.ninja for that exact project: it differs only in an unused rule's text). `process.cppm::merged_environ` strips mcpp's private-glibc payload entries from an INHERITED LD_LIBRARY_PATH, and plan.cppm's comment states that guarantee. But merged_environ takes explicit `extra` overrides verbatim, and `env::prepend_path_list` — which composes exactly such an override for ninja and for run targets — appended the inherited value RAW. So the strip was bypassed precisely in the case it exists for: a nested `mcpp run` → `mcpp test` chain, where a payload tool patched against a different glibc then segfaults inside the dynamic linker before main (bare `__vdso_time`, then SIGSEGV). The predicate moves to mcpp.platform.env, next to path-list composition, since both halves of the guarantee need it; prepend_path_list now sanitizes only the inherited TAIL, so a payload dir the caller passed explicitly — the entry the sandbox binary actually needs — always survives. PATH is untouched. Why it looked like a regression: the two CI runs restored DIFFERENT sandbox caches (…-01baa227… vs …-0e74cc64…), so the payload version set differed and the latent bug only showed on one side. Before this fix, 156 was green only when the poisoned payload version happened to match what the tools expected. --- CHANGELOG.md | 7 +++ src/platform/env.cppm | 52 +++++++++++++++- src/platform/process.cppm | 22 ++----- tests/unit/test_platform_env.cpp | 102 +++++++++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 18 deletions(-) create mode 100644 tests/unit/test_platform_env.cpp diff --git a/CHANGELOG.md b/CHANGELOG.md index ee0bd855..00e19e61 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,13 @@ 一条记录在案的实现约束:**跳过时对 mtime 的任何触碰(包括对齐到 src 的 mtime)都会让 restat 失效并重新引发级联** —— ninja 的 restat 只把「mtime 未被命令改变」的输出视为从未需要构建。所以跳过路径不动任何时间戳。 +- **私有 glibc 的 strip 不再被「组合出的显式覆盖」绕过。** `process.cppm::merged_environ` 会把继承来的 `LD_LIBRARY_PATH` 里的私有 glibc payload 条目剥掉,但**显式 `extra` 覆盖是原样采用的**;而 `env::prepend_path_list` 组合该覆盖时,把继承值(含毒)整段追加了进去 —— 于是 strip 恰好在它唯一有用的场景下失效:嵌套的 `mcpp run` → `mcpp test` 链里,子工具拿到一个**版本不匹配**的 libc.so.6,在动态链接器里 main 之前 SIGSEGV(签名:一行裸 `__vdso_time`)。 + + 修法是把判据下沉到 `mcpp.platform.env`(路径列表组合的所在地),并让 `prepend_path_list` 只清洗**继承来的尾部**:调用方显式传入的 payload 目录必须保留 —— 那正是沙箱二进制需要的那一条。PATH 不受影响。 + + 这个缺陷与 #311 无关,是排查 e2e 156 在本 PR 上失败时找出来的:两次 CI 恢复了**不同的 sandbox 缓存**(`…-01baa227…` vs `…-0e74cc64…`),payload 版本集不同,于是同一个潜伏缺陷只在一侧显形。本改动前该测试的绿灯取决于「毒化的 payload 版本恰好与工具期望的一致」。 + + ### 变更 - **BMI 缓存根统一为 `$MCPP_HOME/bmi`。** `toolchain/stdmod.cppm` 的 `default_cache_root()` 是 home 解析逻辑的一份私有拷贝,自 v0.0.1 起一字未改:**没有 Windows 的 `USERPROFILE` 分支,也没有 self-contained 安装探测**。后果是 Windows PowerShell(不设 `HOME`)下 std BMI 缓存落进**当前工作目录**的 `.mcpp-bmi/`,而 dep BMI 缓存在 `%USERPROFILE%\.mcpp\bmi` —— 一个缓存两个根,其中一个还随 cwd 漂移(从子目录跑就重编一次 std);release tarball 形态的安装在 Linux 上同样分家。 diff --git a/src/platform/env.cppm b/src/platform/env.cppm index ad9069b8..a64210b5 100644 --- a/src/platform/env.cppm +++ b/src/platform/env.cppm @@ -41,6 +41,24 @@ private: std::string path_list_separator(); std::string runtime_library_path_key(); std::string host_tool_runtime_library_path_key(); + +// Drop mcpp's private-glibc payload entries from a loader search path. +// +// An outer `mcpp run`/`mcpp test` points LD_LIBRARY_PATH at the payload so ITS +// child (a sandbox-linked binary) can load. Anything further down the process +// tree that is a HOST binary — /bin/sh, or a payload tool patched against a +// DIFFERENT payload version — then resolves a mismatched libc.so.6 against the +// host ld.so and dies inside the dynamic linker before main (signature: a bare +// `__vdso_time` line, then SIGSEGV). libc and ld.so are version-locked through +// GLIBC_PRIVATE, so this never reproduces when the two happen to match. +// +// Lives here, next to path-list composition, because BOTH consumers need the +// same predicate: process.cppm sanitizes the INHERITED variable for every child, +// and prepend_path_list sanitizes the inherited TAIL it carries into a composed +// override (dirs the caller passed explicitly are always kept — that is the +// entry the sandbox binary actually needs). +std::string strip_private_glibc(std::string_view paths); + std::string prepend_path_list(std::string_view key, std::span dirs); @@ -134,6 +152,25 @@ std::string host_tool_runtime_library_path_key() { #endif } +std::string strip_private_glibc(std::string_view paths) { + auto sep = path_list_separator(); + std::string cleaned; + std::size_t start = 0; + while (start <= paths.size()) { + auto end = paths.find(sep, start); + if (end == std::string_view::npos) end = paths.size(); + auto item = paths.substr(start, end - start); + if (!item.empty() && item.find("/xim-x-glibc/") == std::string_view::npos + && item.find("\\xim-x-glibc\\") == std::string_view::npos) { + if (!cleaned.empty()) cleaned += sep; + cleaned += item; + } + if (end == paths.size()) break; + start = end + sep.size(); + } + return cleaned; +} + std::string prepend_path_list(std::string_view key, std::span dirs) { if (key.empty() || dirs.empty()) return ""; @@ -149,8 +186,19 @@ std::string prepend_path_list(std::string_view key, std::string k(key); if (auto* existing = std::getenv(k.c_str()); existing && *existing) { - value += sep; - value += existing; + // Loader paths only: the inherited value may carry an OUTER mcpp's + // private-glibc entry, and the composed string becomes an EXPLICIT + // override — which process.cppm's merged_environ takes verbatim, + // skipping the sanitation it applies to inherited variables. Without + // this the strip is defeated exactly when it matters (a nested + // `mcpp run` → `mcpp test` chain), and the child tool segfaults in the + // dynamic linker whenever the payload versions differ. + std::string tail = (k == "LD_LIBRARY_PATH" || k == "DYLD_LIBRARY_PATH") + ? strip_private_glibc(existing) : std::string(existing); + if (!tail.empty()) { + value += sep; + value += tail; + } } return value; } diff --git a/src/platform/process.cppm b/src/platform/process.cppm index ea2a8491..48fd420c 100644 --- a/src/platform/process.cppm +++ b/src/platform/process.cppm @@ -188,22 +188,12 @@ char** host_environ() { // `__vdso_time` line). Strip exactly the private-glibc payload entries from // inherited loader paths: user-supplied entries survive, and an `extra` // override (the correct per-child value) always wins over the inherited var. -std::string strip_private_glibc(std::string_view paths) { - std::string cleaned; - std::size_t start = 0; - while (start <= paths.size()) { - auto end = paths.find(':', start); - if (end == std::string_view::npos) end = paths.size(); - auto item = paths.substr(start, end - start); - if (!item.empty() && item.find("/xim-x-glibc/") == std::string_view::npos) { - if (!cleaned.empty()) cleaned += ':'; - cleaned += item; - } - if (end == paths.size()) break; - start = end + 1; - } - return cleaned; -} +// +// The predicate itself lives in mcpp.platform.env, because the OTHER half of +// this guarantee is there: a composed override (dirs + inherited tail) arrives +// here as `extra` and therefore bypasses the sanitation below, so +// prepend_path_list has to sanitize the tail it carries. +using mcpp::platform::env::strip_private_glibc; // Build a child environment block = the current environ with `extra` overrides // applied. Returned vector owns the strings; the caller derives a NUL-terminated diff --git a/tests/unit/test_platform_env.cpp b/tests/unit/test_platform_env.cpp new file mode 100644 index 00000000..572316e9 --- /dev/null +++ b/tests/unit/test_platform_env.cpp @@ -0,0 +1,102 @@ +#include +#include + +import std; +import mcpp.platform.env; + +namespace { + +class ScopedVar { +public: + ScopedVar(std::string name, const char* value) : name_(std::move(name)) { + if (const char* old = std::getenv(name_.c_str()); old) { had_ = true; old_ = old; } + apply(value); + } + ~ScopedVar() { apply(had_ ? old_.c_str() : nullptr); } + ScopedVar(const ScopedVar&) = delete; + ScopedVar& operator=(const ScopedVar&) = delete; +private: + void apply(const char* v) { +#if defined(_WIN32) + ::_putenv_s(name_.c_str(), v ? v : ""); +#else + if (v) ::setenv(name_.c_str(), v, 1); else ::unsetenv(name_.c_str()); +#endif + } + std::string name_; + bool had_ = false; + std::string old_; +}; + +std::string sep() { return mcpp::platform::env::path_list_separator(); } + +std::string join(std::initializer_list parts) { + std::string out; + for (auto p : parts) { + if (!out.empty()) out += sep(); + out += p; + } + return out; +} + +} // namespace + +TEST(PlatformEnv, StripPrivateGlibcDropsOnlyPayloadEntries) { + auto in = join({"/usr/lib", "/home/u/.mcpp/registry/data/xpkgs/xim-x-glibc/2.39/lib64", + "/opt/mine"}); + EXPECT_EQ(mcpp::platform::env::strip_private_glibc(in), + join({"/usr/lib", "/opt/mine"})); +} + +TEST(PlatformEnv, StripPrivateGlibcCanEmptyTheList) { + EXPECT_EQ(mcpp::platform::env::strip_private_glibc( + "/x/xpkgs/xim-x-glibc/2.42/lib64"), ""); + EXPECT_EQ(mcpp::platform::env::strip_private_glibc(""), ""); +} + +// The regression this guards (mcpp#311 investigation): the composed value +// becomes an EXPLICIT override, and process.cppm's merged_environ takes explicit +// overrides verbatim — skipping the sanitation it applies to inherited +// variables. So if the inherited tail carried an outer `mcpp run`'s private +// glibc, it reached the child anyway and a payload tool patched against a +// DIFFERENT glibc segfaulted in the dynamic linker. Sanitizing here is what +// makes process.cppm's guarantee actually hold one hop down. +TEST(PlatformEnv, PrependPathListSanitizesTheInheritedLoaderTail) { + auto inherited = join({"/first", + "/home/u/.mcpp/registry/data/xpkgs/xim-x-glibc/2.39/lib64", + "/last"}); + ScopedVar ld("LD_LIBRARY_PATH", inherited.c_str()); + + std::vector dirs{"/payload/lib"}; + EXPECT_EQ(mcpp::platform::env::prepend_path_list("LD_LIBRARY_PATH", dirs), + join({"/payload/lib", "/first", "/last"})); +} + +// A payload dir the CALLER passed is the entry the sandbox binary needs — it +// must survive even though it matches the same pattern. +TEST(PlatformEnv, PrependPathListKeepsExplicitPayloadDirs) { + ScopedVar ld("LD_LIBRARY_PATH", "/home/u/.mcpp/registry/data/xpkgs/xim-x-glibc/2.39/lib64"); + + std::vector dirs{ + "/home/u/.mcpp/registry/data/xpkgs/xim-x-glibc/2.42/lib64"}; + // Explicit dir kept, inherited (older, mismatched) entry dropped. + EXPECT_EQ(mcpp::platform::env::prepend_path_list("LD_LIBRARY_PATH", dirs), + "/home/u/.mcpp/registry/data/xpkgs/xim-x-glibc/2.42/lib64"); +} + +// PATH is not a loader search path: leave its inherited value alone. +TEST(PlatformEnv, PrependPathListLeavesPathUntouched) { + auto inherited = join({"/bin", "/x/xpkgs/xim-x-glibc/2.39/lib64"}); + ScopedVar path("PATH", inherited.c_str()); + + std::vector dirs{"/tools/bin"}; + EXPECT_EQ(mcpp::platform::env::prepend_path_list("PATH", dirs), + join({"/tools/bin", "/bin", "/x/xpkgs/xim-x-glibc/2.39/lib64"})); +} + +TEST(PlatformEnv, PrependPathListWithNoInheritedValueIsJustTheDirs) { + ScopedVar ld("LD_LIBRARY_PATH", nullptr); + std::vector dirs{"/a", "/b"}; + EXPECT_EQ(mcpp::platform::env::prepend_path_list("LD_LIBRARY_PATH", dirs), + join({"/a", "/b"})); +} From 56ebacee188854db78710ddca89c973fa810bcf0 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 30 Jul 2026 12:59:02 +0800 Subject: [PATCH 8/8] fix(build): verify fingerprint-scoped staging by size, so a read-denying holder can't fail the build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Windows job's own repro exposed the gap: my holder maps the destination with FileShare.None, so `same_content` cannot even OPEN it → equivalence is undecidable → mcpp attempts the write → ERROR_SHARING_VIOLATION(32) → build failed. Which is precisely the #311 outcome, arrived at from the other side. Reading needs an open; SIZE comes from directory metadata and does not. So the verify mode is now per-edge rather than one global setting: std BMI / std.o / std.compat.* --verify size fingerprint-scoped: the cache dir and the build dir share the fp covering compiler identity, triple, stdlib, std source hash and dialect flags ⇒ equal size IS equivalence Windows DLL deploy / runtime_alias content not fp-scoped, and a rebuilt DLL keeps its size (PE sections are page-padded) So #311's actual path — clangd mapping the std BMI — now survives even an EXCLUSIVE lock, while a possibly-stale DLL is still gated byte-for-byte. Carried on a per-edge `$verify` ninja variable; the separating space lives in the rule's command string because ninja trims trailing whitespace in variable values. Verified on Linux with chmod 000 (the closest analogue of a read-denying holder): the fp-scoped edge skips and the build succeeds. Also: bootstrap pin 2026.7.29.1 → 2026.7.29.2. Unrelated to this work and pre-existing on main — the index no longer serves .1 (`available: 2026.7.29.2`), so the aarch64 fresh-install job fails on main too. .2 is released and indexed, and the guard's rule (both pins equal, never newer than the building version) still holds. --- ...ue311-bmi-staging-and-cache-root-design.md | 14 +++++++++ CHANGELOG.md | 2 ++ src/build/ninja_backend.cppm | 30 +++++++++++++++++-- tests/e2e/170_bmi_staging_no_cascade.sh | 2 +- tests/e2e/171_bmi_staging_locked_dest.sh | 9 +++++- tests/unit/test_ninja_backend.cpp | 19 ++++++++---- 6 files changed, 66 insertions(+), 10 deletions(-) diff --git a/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md b/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md index 67598643..d41c3e34 100644 --- a/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md +++ b/.agents/docs/2026-07-30-issue311-bmi-staging-and-cache-root-design.md @@ -229,6 +229,20 @@ staged,理由是 fp 判据;但同一条 rule 还搬 **DLL**(`runtimeDeployFiles` 的是正确性。`--verify size` / `MCPP_STAGE_VERIFY=size` 保留给「调用方确知源是 fp 作用域」的 快路径。选逐字节比较而不是 hash:同样的 I/O 成本,但没有碰撞面,且能提前退出。 +**但 verify 档位是 per-edge 的,不是全局一档**(这一条也是 Windows CI 逼出来的)。判等要读目标 +就必须**打开**它,而持有者可以连读都不给:e2e 171 的 PowerShell 用 `FileShare.None` 映射, +`same_content` 连 open 都失败 → 判不出等价 → 去写 → `ERROR_SHARING_VIOLATION(32)` → 构建挂。 +而 **size 来自目录元数据,不需要 open**,所以: + +| edge | verify | 理由 | +|---|---|---| +| std BMI / std.o / std.compat.* | `--verify size` | fp 作用域 ⇒ 等长即等价;且能在「连读都被拒」的持有下照样跳过 | +| Windows DLL 部署 / `runtime_alias` | 内容(默认) | 源不是 fp 作用域,PE 节对齐让等长-不同内容很常见 | + +也就是说 #311 那条真正的路径(clangd 映射 std BMI)现在**连排他锁都能扛**,而可能过期的 +DLL 仍然逐字节把关。ninja 里用一个 per-edge 变量 `$verify` 承载(注意 ninja 会裁掉变量值的 +尾部空白,所以空格必须留在 rule 的 command 串里)。 + **为什么步 2 连时间戳都不碰**（这一条实测修正过一次）： | 做法 | 下游是否被级联 | 之后 edge 状态 | diff --git a/CHANGELOG.md b/CHANGELOG.md index 00e19e61..b1335bed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,8 @@ 「已等价」由**逐字节比较**判定 —— 无条件正确:内容相同就是不需要写。这个判断只在 ninja 已经认定 edge 脏了才会跑,所以代价可以忽略。(`--verify size` 保留给确知源是 fingerprint 作用域的调用方:build dir 与缓存目录共享同一个 fp,而 fp 已覆盖编译器身份/target triple/stdlib/std 源哈希/标准与方言 flag。但它**不是默认** —— 同一条 rule 还搬 DLL,而 PE 的节对齐让「真的重建了、大小却一样」十分常见。)dep BMI 缓存一直就是「已存在就不动」的(`bmi_cache.cppm`: *"Existing project outputs are left untouched"*)—— std staging 是全仓库唯一强制覆写的那一处,这个不对称本身就是缺陷。 + **verify 档位是 per-edge 的**:std BMI / `std.o` / `std.compat.*` 走 `--verify size`,Windows DLL 部署与 `runtime_alias` 走默认的逐字节比较。原因是判等要读目标就必须 open 它,而持有者可以连读都不给(Windows `FileShare.None` 映射 → open 即 `ERROR_SHARING_VIOLATION`);size 来自目录元数据、不需要 open。于是 #311 那条真实路径(clangd 映射 std BMI)**连排他锁都扛得住**,而可能过期的 DLL 仍逐字节把关 —— 前者靠 fingerprint 作用域保证等长即等价,后者没有这个保证。 + 同一条 rule 也用于 **Windows 运行期 DLL 部署**,以及 Windows 上的 `runtime_alias`(PE 没有 soname 符号链接,别名就是刚建出来的 DLL 的副本),因此「往上一次 `mcpp run` 还加载着的 DLL 上覆写」这个同类失败一并治好。POSIX 的 `runtime_alias` 保持符号链接不变 —— 那里符号链接是语义,不只是写法。 - **重新 stage 一个未变的 std BMI 不再重编整张模块图。** staged BMI 是每个 importer 的 implicit input,而 staging rule 既不保留 mtime 也没有 `restat`,于是缓存侧 BMI 只要 mtime 变新(在下面那个缓存根缺陷下,**换个 cwd 跑就会发生**),所有 `import std` 的 TU 全部重编 —— 即使字节完全相同。 diff --git a/src/build/ninja_backend.cppm b/src/build/ninja_backend.cppm index 2ff64e1f..d0b60a9a 100644 --- a/src/build/ninja_backend.cppm +++ b/src/build/ninja_backend.cppm @@ -440,8 +440,21 @@ std::string emit_ninja_string(const BuildPlan& plan) { // that clangd had memory-mapped failed the whole build with error 1224. // `restat = 1` is what makes the no-write path actually pay off — a skipped // stage must not dirty every importer of the staged BMI. + // $verify is per-edge: empty (⇒ content comparison, the safe default) for + // payloads whose bytes can change under an unchanged name — a rebuilt DLL + // keeps its size, PE sections are page-padded — and `--verify size` for the + // std artifacts, which are fingerprint-scoped: the cache dir and this build + // dir share the fp that covers compiler identity, target triple, stdlib, + // std source hash and the dialect flags, so equal size IS equivalence. + // + // That distinction is not a micro-optimization: reading the destination + // needs to OPEN it, and a holder may deny even that (Windows CI: a + // MemoryMappedFile opened with FileShare.None → ERROR_SHARING_VIOLATION on + // open, so content can't be compared and staging fails). Size comes from + // directory metadata, which stays readable — so the fingerprint-scoped + // edges survive a lock that would otherwise be unsurvivable. append("rule stage_file\n"); - append(" command = $mcpp stage --output $out $in\n"); + append(" command = $mcpp stage $verify --output $out $in\n"); append(" description = STAGE $out\n"); append(" restat = 1\n\n"); @@ -821,6 +834,11 @@ std::string emit_ninja_string(const BuildPlan& plan) { } // Stage prebuilt std artifacts into the compiler-specific BMI cache. + // These four are fingerprint-scoped (see the stage_file rule comment), so + // they carry the size-only equivalence check. + // (ninja trims trailing whitespace in variable values, hence the space +// lives in the rule's command string, not here) + static constexpr std::string_view kFpScopedVerify = " verify = --verify size\n"; auto std_bmi_dst = mcpp::toolchain::staged_std_bmi_path(plan.toolchain, {}); auto std_o_dst = std::filesystem::path("obj") / std::format("std{}", dial.objExt); @@ -829,8 +847,11 @@ std::string emit_ninja_string(const BuildPlan& plan) { if (has_std_artifacts) { append(std::format("build {} : stage_file {}\n", escape_ninja_path(std_bmi_dst), escape_ninja_path(plan.stdBmiPath))); - append(std::format("build {} : stage_file {}\n\n", escape_ninja_path(std_o_dst), + append(std::string(kFpScopedVerify)); + append(std::format("build {} : stage_file {}\n", escape_ninja_path(std_o_dst), escape_ninja_path(plan.stdObjectPath))); + append(std::string(kFpScopedVerify)); + append("\n"); } bool has_std_compat = !plan.stdCompatBmiPath.empty() && !plan.stdCompatObjectPath.empty(); @@ -844,8 +865,11 @@ std::string emit_ninja_string(const BuildPlan& plan) { append(std::format("build {} : stage_file {} | {}\n", escape_ninja_path(compat_bmi_dst), escape_ninja_path(plan.stdCompatBmiPath), escape_ninja_path(std_bmi_dst))); - append(std::format("build {} : stage_file {}\n\n", escape_ninja_path(compat_o_dst), + append(std::string(kFpScopedVerify)); + append(std::format("build {} : stage_file {}\n", escape_ninja_path(compat_o_dst), escape_ninja_path(plan.stdCompatObjectPath))); + append(std::string(kFpScopedVerify)); + append("\n"); } auto bmi_path = [&traits](std::string_view name) { diff --git a/tests/e2e/170_bmi_staging_no_cascade.sh b/tests/e2e/170_bmi_staging_no_cascade.sh index ba49a522..8d365571 100755 --- a/tests/e2e/170_bmi_staging_no_cascade.sh +++ b/tests/e2e/170_bmi_staging_no_cascade.sh @@ -71,7 +71,7 @@ done touch "$SRC" "$MCPP" build -v > build2.log 2>&1 || { cat build2.log; echo "FAIL: rebuild after re-stage"; exit 1; } -grep -q "stage --output" build2.log || { +grep -qE "stage .*--output" build2.log || { cat build2.log; echo "FAIL: the staging edge did not re-run at all"; exit 1; } if grep -qE '(-c|/c) .*main\.cpp' build2.log; then cat build2.log diff --git a/tests/e2e/171_bmi_staging_locked_dest.sh b/tests/e2e/171_bmi_staging_locked_dest.sh index d2bd3a97..6bd7998c 100755 --- a/tests/e2e/171_bmi_staging_locked_dest.sh +++ b/tests/e2e/171_bmi_staging_locked_dest.sh @@ -14,6 +14,13 @@ # through a sentinel file, so a PowerShell that never started fails the test # instead of letting it pass for the wrong reason. # +# Note the holder is STRICTER than clangd: CreateFromFile(path, FileMode.Open) +# takes FileShare.None, so the destination cannot even be OPENED — content +# comparison is impossible. Surviving that is why the std staging edges verify +# by SIZE (fingerprint-scoped ⇒ equal size is equivalence, and size comes from +# directory metadata, which needs no open). Passing here therefore implies the +# real clangd case, which only denies writes. +# # The load-bearing assertion is platform-neutral and root-proof: an equivalent # destination must not be written AT ALL (same inode, same mtime, same size). # Permissions alone would not do — root ignores them, and the container e2e job @@ -100,7 +107,7 @@ if [[ $rc -ne 0 ]]; then echo "FAIL: staging an already-equivalent BMI failed while the destination was held" exit 1 fi -grep -q "stage --output" build2.log || { +grep -qE "stage .*--output" build2.log || { cat build2.log; echo "FAIL: the staging edge did not run"; exit 1; } # The real assertion: nothing was written. Not "the write succeeded anyway". diff --git a/tests/unit/test_ninja_backend.cpp b/tests/unit/test_ninja_backend.cpp index c683e3f6..2649cac8 100644 --- a/tests/unit/test_ninja_backend.cpp +++ b/tests/unit/test_ninja_backend.cpp @@ -807,7 +807,7 @@ TEST(NinjaBackend, StageRuleRunsThroughMcppWithRestat) { auto ninja = emit_ninja_string(plan); EXPECT_NE(ninja.find("rule stage_file\n"), std::string::npos) << ninja; - EXPECT_NE(ninja.find(" command = $mcpp stage --output $out $in\n"), + EXPECT_NE(ninja.find(" command = $mcpp stage $verify --output $out $in\n"), std::string::npos) << ninja; // restat is what keeps a skipped (already-equivalent) stage from dirtying // every importer of the staged BMI. @@ -876,10 +876,15 @@ TEST(NinjaBackend, StdArtifactsAndRuntimeDllsUseTheStageRule) { auto ninja = emit_ninja_string(plan); + // The std artifacts are fingerprint-scoped, so they carry the size-only + // check: it needs no OPEN of the destination, which is what lets staging + // survive a holder that denies reads (Windows FileShare.None). EXPECT_NE(ninja.find("build pcm.cache/std.pcm : stage_file " - "/cache/bmi/fp/pcm.cache/std.pcm"), + "/cache/bmi/fp/pcm.cache/std.pcm\n" + " verify = --verify size\n"), std::string::npos) << ninja; - EXPECT_NE(ninja.find("build obj/std.o : stage_file /cache/bmi/fp/std.o"), + EXPECT_NE(ninja.find("build obj/std.o : stage_file /cache/bmi/fp/std.o\n" + " verify = --verify size\n"), std::string::npos) << ninja; // std.compat keeps its order-only dependency on the staged std BMI. EXPECT_NE(ninja.find("build pcm.cache/std.compat.pcm : stage_file " @@ -887,8 +892,12 @@ TEST(NinjaBackend, StdArtifactsAndRuntimeDllsUseTheStageRule) { std::string::npos) << ninja; // Windows DLL deployment shares the rule — and therefore the // skip-if-equivalent behaviour that keeps a loaded DLL from failing a build. - EXPECT_NE(ninja.find("build bin/libfoo.dll : stage_file /pkg/lib/libfoo.dll"), - std::string::npos) << ninja; + // But NOT the size-only shortcut: a rebuilt DLL keeps its size (PE sections + // are page-padded), so this edge must compare content. + auto dllEdge = ninja.find("build bin/libfoo.dll : stage_file /pkg/lib/libfoo.dll\n"); + ASSERT_NE(dllEdge, std::string::npos) << ninja; + auto afterDll = ninja.substr(dllEdge, ninja.find('\n', dllEdge + 1) - dllEdge + 1); + EXPECT_EQ(afterDll.find("verify"), std::string::npos) << afterDll; } // ── #311: staging failures must stay readable through the output filter ──