fix(modgraph): a directory name the code page cannot spell must not end the build (2026.8.27.2) - #517
Merged
Merged
Conversation
This commit deliberately does NOT contain the one-line fix. It exists so the next one has something to prove. mcpp#516: on Windows `path::string()` converts the native (wide) name through the process ANSI code page and throws std::system_error for a character that code page cannot spell. `is_excluded_walk_dir` narrows a directory name on the FIRST line of the glob walk loop, so it fires before the guard #231 added to `path_matches_glob` one line later — which is why hardening that one could never cover a directory name. Two things here are supposed to be RED on this commit, and green on the next: * ci-windows Scanner.GlobWalkSurvivesNamesTheCodePageCannotSpell — proves the test can fail, rather than passing by accident. * ci-linux check_narrow_conversions.sh, naming scanner.cppm — proves the gate catches the real defect, not just the negative control I fed it while writing it. A test that has never been red does not prove anything, and neither does a gate that has only ever seen a synthetic violation. What this commit does contain: * mcpp::modgraph::try_narrow() — the ONE place a walk-derived path becomes a narrow string. Leaf-safe: modgraph imports only std, and nothing under src/modgraph or src/manifest imports mcpp.ui or mcpp.diag. * note_unnarrowable_path() / take_unnarrowable_paths() — records a skip, deduplicated to the nearest ancestor that CAN be named (never the offending name, which cannot enter a message without throwing the same exception). * cli::run scope guard draining those into mcpp.diag as `degraded` — silently dropping a file is the half-fix this whole change exists to avoid, and diag already requires an `impact` sentence for exactly this situation. * pack/digest.cppm: interface_set_digest folds in u8string(), not string(). Its input is an UNFILTERED recursive_directory_iterator over a published package, and the old spelling also made the digest depend on the host code page — a tree packed on Linux and verified on Windows could disagree about a non-ASCII name while every byte on disk was identical. ASCII names hash identically, so no published digest changes. * scaffold/template.cppm: same, for a third-party template provider's tree. * docs (EN + zh), the contributing invariant, and the CI gate. Refs #516, #230, #231
…2026.8.27.4 The fix the previous commit exists to prove. `is_excluded_walk_dir` compared a directory name against three ASCII literals by first converting it to a narrow string. On Windows that conversion goes through the process ANSI code page and throws for anything the code page cannot spell. The comparison never needed a narrow string: `path::operator==` compares native representations, the three literals are ASCII and convert losslessly, and the decision is byte-for-byte the one the narrow comparison made. Static path constants keep the per-entry cost below what it was (#225 bounded this walk on purpose, and this is its hot path). Evidence from the previous commit, both on real runners: ci-linux failed at `Check no walk-derived path is narrowed directly`, naming src/modgraph/scanner.cppm:238. ci-windows Scanner.GlobWalkSurvivesNamesTheCodePageCannotSpell failed with Actual: it throws std::system_error with description "No mapping for the Unicode character exists in the target multi-byte code page." — word for word what #516 reported, reproduced from `expand_dir_glob(dir, "*")`, which is the exact call the `include_dirs = { "*" }` convention makes. The test did NOT skip, so the runner's ACP is not UTF-8 and the guard is live. 92 other tests passed, so nothing else was disturbed. Also here: * The report drain in cli.cppm is a function-try-block. A destructor is implicitly noexcept, and run() can be left by an exception — main() catches one. A change whose whole subject is "an uncaught exception must not end the build" does not get to introduce a second one in its own reporting path. * xlings pin 2026.8.17.2 → 2026.8.27.4 across all 17 live pin sites (check_version_pins.sh enforces they equal `pinned::kXlingsVersion`). The three remaining 2026.8.17.2 strings in bootstrap-mcpp/action.yml are NOT pins — they are recorded measurements ("Measured on the 2026.8.17.2 assets", "Cache hit for: …xl2026.8.17.2…") and rewriting them would falsify a record of what was actually observed. All four platform assets for v2026.8.27.4 were verified to exist before bumping. * Two comments pointed at files that do not exist (`tools/check-narrow- conversions.sh`, `AGENTS.md` — mcpp keeps its conventions in .agents/skills/). Found in self-review; a reference that cannot be followed is the same defect class as a printed command that cannot be run. Closes #516
Member
Author
「打补丁前必须红」的证据(来自
|
CI on fade10f was 36/36 green, including the Windows leg where the new test ran (`[ OK ] … (40 ms)`, not SKIPPED). Reading the whole diff back afterwards turned up six defects anyway — none of them compile errors, all of them the kind that mislead the next reader: 1. A comment claimed "nothing calls .string() on such a path directly". That is false — p1689.cppm does, with a NARROW-OK rationale. An absolute claim that is not true is worse than the convention it describes; reworded to say it is a convention with a gate, not a guarantee. 2. Two comments pointed at files that do not exist: `tools/check-narrow-conversions.sh` (it is .github/tools/, underscores) and `AGENTS.md` (mcpp keeps its conventions in .agents/skills/ — AGENTS.md is an xlings habit). A reference nobody can follow is the same defect class as a printed command nobody can run. 3. "run() has five exits" — a number I could not cheaply verify (most of the `return`s in that function body belong to lambdas). Replaced with the enumeration that is actually checkable. 4. The docs showed the warning as `'C:\...\test\www'`. The anchor is stored via try_narrow, which returns generic_string(), so the real output uses forward slashes. The documentation was showing output the code does not produce. Fixed both the docs and the code comment that should have said so. 5. The blast-radius statistic was stale AND loosely defined. "128 recipes, 101 with a `*`-leading glob" was measured before mcpp-index moved under me (aa8df60 → 891b2f7); it is 130/103 now. A hard-coded count in a CHANGELOG is wrong the next day, so it now names the commit it was measured on and states the counting rule (and records the false-positive check: every `"*…` string in those recipes really is a glob — `*/include`, `*`, `*/src`, `*/mcpp.toml`). Corrected in the CHANGELOG, the design doc, and the PR body. 6. (in fade10f, listed for completeness) the report drain was an ordinary destructor, i.e. implicitly noexcept, on a path that runs during unwinding. No behaviour change: comments, docs and one CHANGELOG paragraph.
Member
Author
红 → 绿闭环(同一个 runner、同一个测试、同一个 ACP)
自我 review 又抓到 6 处(
|
Sunrisepeak
added a commit
to mcpplibs/mcpp-index
that referenced
this pull request
Aug 27, 2026
…266) MCPP_VERSION 2026.8.27.1 -> 2026.8.27.2. #263 moved this pin seventeen versions forward, to 2026.8.27.1. That is the version PR #260 is failing on, and the failure is not #260's: error: internal: unhandled exception: No mapping for the Unicode character exists in the target multi-byte code page. three times, in `workspace (windows default 0/2)` and `1/2` — one per httplib example. It is mcpp-community/mcpp#516: on Windows `path::string()` converts through the process ANSI code page and throws for a name that code page cannot spell, and `is_excluded_walk_dir` narrowed a directory name on the first line of the glob walk. cpp-httplib ships `test/www/<CJK>Dir/`, and the `include_dirs = { "*" }` convention walks the whole extracted tarball. Fixed in mcpp 2026.8.27.2 (mcpp-community/mcpp#517), released and mirrored on both hosts, with xim-pkgindex's `latest` pointing at it.⚠️ RE-RUNNING #260 WITHOUT THIS BUMP PROVES NOTHING. Its CI would fetch 2026.8.27.1 — the version that still carries the defect — and reproduce the same three errors. That is a re-confirmation of the bug, not a test of the fix. Attribution first: the failing job's log was read and it is #516's message verbatim, not some other Windows problem wearing the same red. Keeping #263's caution in force: a red at this pin can come from the engine change or from what accumulated across the versions this repository never ran against, and a one-sided reading cannot tell them apart.
Sunrisepeak
added a commit
to mcpplibs/mcpp-index
that referenced
this pull request
Aug 27, 2026
* Validate against the engine that fixes what this index tripped over MCPP_VERSION 2026.8.27.1 -> 2026.8.27.2. #263 moved this pin seventeen versions forward, to 2026.8.27.1. That is the version PR #260 is failing on, and the failure is not #260's: error: internal: unhandled exception: No mapping for the Unicode character exists in the target multi-byte code page. three times, in `workspace (windows default 0/2)` and `1/2` — one per httplib example. It is mcpp-community/mcpp#516: on Windows `path::string()` converts through the process ANSI code page and throws for a name that code page cannot spell, and `is_excluded_walk_dir` narrowed a directory name on the first line of the glob walk. cpp-httplib ships `test/www/<CJK>Dir/`, and the `include_dirs = { "*" }` convention walks the whole extracted tarball. Fixed in mcpp 2026.8.27.2 (mcpp-community/mcpp#517), released and mirrored on both hosts, with xim-pkgindex's `latest` pointing at it.⚠️ RE-RUNNING #260 WITHOUT THIS BUMP PROVES NOTHING. Its CI would fetch 2026.8.27.1 — the version that still carries the defect — and reproduce the same three errors. That is a re-confirmation of the bug, not a test of the fix. Attribution first: the failing job's log was read and it is #516's message verbatim, not some other Windows problem wearing the same red. Keeping #263's caution in force: a red at this pin can come from the engine change or from what accumulated across the versions this repository never ran against, and a one-sided reading cannot tell them apart. * Five packages: the routes openkal 0.8's atoms made possible openkal-musl 0.4.0 -> 0.5.0 openkal-macos 0.4.0 -> 0.5.0 openkal-windows 0.2.0 -> 0.3.0 openkal-llvm-runtime 0.2.0 -> 0.3.0 sbase 0.1.1 -> 0.1.2 The specification does not move. openkal 0.8 already carried the atoms; what changed is that the layers above and below now use them --- the socket family upon `openkal.net', datagrams upon `openkal.datagram', `poll' and `select' upon `openkal.timeout', and `fork' composed above `openkal.space'. Reported as mcpplibs/openkal-linux#13 and mcpplibs/openkal-musl#13. `openkal-windows' provides four of the five and declines `openkal.space' in terms: this system starts a NAMED PROGRAM and has no primitive that copies an address space, so a program calling `kal_space_start' fails at the link naming the operation, which is clause 6.1's report. `sbase' had been pinned below 0.4.0 --- a caret on a 0.x version bounds it under the next minor --- so ninety-seven utilities were testing a C library two minors behind the one everything else builds against.⚠️ EVERY HASH WAS FETCHED BACK BEFORE IT WAS WRITTEN HERE. The publisher tags, uploads, and then RETRIEVES the CN asset and compares its bytes; this index records one sha256 for two URLs, and a wrong digit produces a package that resolves, downloads and fails verification on every machine but the one that published it. Both mirrors were then read a second time, independently of the publisher, and all ten fetches matched. --------- Co-authored-by: speak-agent <x.d2learn.org@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #516.
一句话
不是解压问题,解压是对的。 抛异常的是 mcpp 自己,在
src/modgraph/scanner.cppm:238的dir.filename().string()—— 而这是 #230 的同一处漏网:#231加固了三个窄化站点,漏掉了同一个 walk 循环里早一行执行的第四处。根因
MSVC 的
path::string()走WideCharToMultiByte(ACP),遇到当前代码页拼不出的字符就抛std::system_error,what()恰好是 issue 里那句No mapping for the Unicode character exists in the target multi-byte code page.is_excluded_walk_dir()是 walk 循环体的第一行,每个目录条目过一次 —— 所以它比#231加固过的path_matches_glob更早执行。触发条件比看上去宽:
include_dirs = { "*" }会从解压根无界递归遍历整棵上游源码树(在 mcpplibs/mcpp-index
891b2f7上量:130 个 recipe 里 103 个含以*开头的 glob;这个比例随索引增长而变,写下的是那个 commit 的数)。cpp-httplib 带了
test/www/日本語Dir/,于是三个 httplib 测试在 Windows 上一起挂,Linux/macOS 全绿 ——因为那两个平台上
path::string()不做任何编码转换。归属反证
有人会怀疑是 xlings 解压时按 ANSI 写坏了名字。恰恰相反:
ERROR_NO_UNICODE_TRANSLATION的前提是宽名里存在 ACP 拼不出的字符。若盘上是mojibake(UTF-8 字节被 CP1252 逐字节打散),那些字符逐个都在 CP1252 里,mcpp 反而不会抛。
mcpp 抛了,正好证明 xlings 写对了。 xlings 自己的同类隐患另立 openxlings/xlings#571。
改动
不是第四个 try/catch,而是按用途分三档的一条规则:
path比,根本不窄化u8string()—— 各平台同一串字节,不碰代码页mcpp::modgraph::try_narrow(),处理nulloptis_excluded_walk_dir改为静态path常量比较。行为与原窄串比较逐字同解(大小写敏感、ASCII 字面量无损转换),且比原来"每条目一次
std::string构造"更便宜。try_narrow()收敛为唯一入口;path_matches_glob、scan_file的诊断文案、pack/digest.cppm、scaffold/template.cppm全部归并。mcpp.diag的degraded通道 ——它的批次不变式本就要求"因前提不满足而少做事必须给出
impact"。src/modgraph/src/manifest是 leaf 层(全仓没有一条到mcpp.ui/mcpp.diag的import 边),所以 glob 层记录、
cli::run的 scope guard 排空上报。interface_set_digest用.string()折入文件名,输入是对已发布包的未过滤走查 —— 除了会抛,还让"Linux 打包 / Windows 校验"的同一棵树
对非 ASCII 名字给出不同摘要,表现为
does not match what was packaged。改用
u8string();纯 ASCII 名字字节不变,已发布包摘要不变。.github/tools/check_narrow_conversions.sh。Test plan
在 Linux/macOS 上写的任何测试都无法证伪这个缺陷 —— 那两个平台
path::string()不做转换。所以:
Scanner.GlobWalkSurvivesNamesTheCodePageCannotSpell—— Windows 专属,自己探测GetACP(),拼得出就GTEST_SKIP并说明原因(runner 镜像哪天默认 UTF-8 ACP,它不会变成永远绿的装饰品)。用天城文而非日文:日文在 CP932/936/950 上是可拼写的,
在日文/中文开发机上单用它等于零覆盖。名字由显式码位构造,不依赖源文件编码。
断言三件事:不抛、walk 没被截断、跳过被记录。
Scanner.GlobWalkHandlesNonAsciiNames/Glob.UnnarrowablePathsDedupToTheirSpellableAncestor—— 跨平台可跑。
mcpp build46s 通过,mcpp test93 passed / 0 failed。ce86b41不含 P0 修复。它存在的目的是让下一个 commit 有东西可证明:ci-windows应在Scanner.GlobWalkSurvivesNamesTheCodePageCannotSpell失败→ 证明这个测试能失败;
ci-linux应在check_narrow_conversions.sh失败并点名scanner.cppm:238→ 证明这道门能抓到真实缺陷,而不只是抓到我构造的负例。
一个从来没红过的测试不证明任何事,只见过合成负例的门也一样。
范围之外(按 review 决定)
activeCodePage=UTF-8清单)与"项目根目录含非 ACP → 先以『没有源文件』失败"另立 issue。P2 不危险(ASCII 路径下字节不变),但可能不彻底 ——
build.ninja 是另一个进程按它自己的 ACP 读的。
完整分析:
.agents/docs/2026-08-27-issue516-windows-acp-glob-walk-fix.md