diff --git a/.agents/docs/2026-08-16-toolchain-architecture-review.md b/.agents/docs/2026-08-16-toolchain-architecture-review.md new file mode 100644 index 00000000..1a66353e --- /dev/null +++ b/.agents/docs/2026-08-16-toolchain-architecture-review.md @@ -0,0 +1,447 @@ +# 工具链架构 review:两种来源、选择与切换、构建与分发(2026-08-16) + +> 配套 `2026-08-16-msvc-as-a-managed-toolchain.md`(设计)与 +> `2026-08-16-msvc-ecosystem-cross-repo-plan.md`(跨仓库落地)。 +> 那两份讲**已经做了什么**;这份讲**现在的形状哪里还不对,以及怎么更优雅**。 +> +> 依据是把 `msvc@` 真正跑通的这一轮:九层缺陷、五次索引发布窗口、 +> 三处"不可能失败的测试"。下面每一条都指得出具体的 `file:line`, +> 不是风格偏好。 + +--- + +## 0. 结论先行 + +已经做对的那件事,是**把"获取"和"解析"拆成两条正交的轴**: + +| | 获取(哪来的) | 解析(怎么找到 cl.exe) | +|---|---|---| +| `msvc@system` | 探测这台机器 | `installation_from_tools_dir()` | +| `msvc@` | xim 装的 payload | **同一个** `installation_from_tools_dir()` | + +受管 toolset 因此不是第二条代码路径,长不出自己的 bug。**这个模型是对的, +下面所有建议都是把它贯彻得更彻底,没有一条要推翻它。** + +问题集中在一句话:**这条轴只对 MSVC 存在,而且只贯彻了一半。** + +--- + +## 1. `@system` 只对 MSVC 存在 —— 这是**对的**,但它的代价被摊在了 26 处 ⭐ + +### 现状 + +`registry.cppm:428`: + +```cpp +bool is_system_toolchain(const ToolchainSpec& spec) { + return spec.family == Family::Msvc + && (spec.version.empty() || spec.version == "system"); +} +``` + +**族被写进了判据。** + +> **先说清楚:这个限制本身是对的,不要改。** +> xlings 的原则是**能不依赖 host 就不依赖 host** —— 工具链应当由 xim 提供, +> 这样"声明什么就用什么"在每台机器上成立。`gcc@system` / `llvm@system` +> **不该**支持:那是把不确定性请回来。 +> +> `msvc@system` 是 **Windows 平台的特例**,不是通用能力的一个实例: +> Visual Studio 常常已经装在机器上、又不总是能被重新分发,所以必须能用它。 +> +> 我这份 review 的初稿建议"把 `@system` 通用化到所有族",**那是错的**, +> 方向正好反了。下面改成正确的问题。 + +真正的问题不是"为什么只有 MSVC 有",而是:**一个平台特例,凭什么要在 26 个 +地方各写一遍。** + +代价不是"四处特判"这么轻。把两个文件扫一遍:**lifecycle.cppm 里 13 处、 +prepare.cppm 里 13 处** msvc 专有分支,而 gcc / llvm / mingw 三个族在 +lifecycle.cppm 里**一处都没有**(mingw 更是零分支 —— 它整个被表达成一个 +*target*,`x86_64-windows-gnu`,那才是这套设计想要的形状)。 + +其中直接由 `is_system_toolchain` 触发的四处: + +| 位置 | 内容 | +|---|---| +| `lifecycle.cppm:637` | `toolchain install` 拒绝安装 system | +| `lifecycle.cppm:848` | `toolchain default` 持久化 `"msvc@system"` **并清掉 `default_target`**(别的族没有这一步) | +| `lifecycle.cppm:931` | `toolchain remove` 拒绝 | +| `prepare.cppm:1260` | 构建期短路整条 xim 路径 | + +另外三处是同一条规则的**重复拼写**,值得单独记: + +- `xim_tool()` + `installation_at()` + 那段错误信息,在 + `prepare.cppm:1322-1341` 和 `lifecycle.cppm:730-755` **几乎逐字重复**。 + 第三份出现的时候就是下一个 #436。 +- sysroot 依赖规则有两份:`prepare.cppm:1471-1480`(只看 musl) + vs `lifecycle.cppm:694-702`(musl + PE + 宿主),**两者并不等价**, + 而 `lifecycle.cppm:692` 的注释声称它们互相对应。 +- `prepare.cppm:1002-1005` 那段注释说解析是 4 步,实际链路有 **9 个输入** + (manifest → `--toolchain` → 全局 default → Windows 无 VS 的 target 种子 → + `[target.X].toolchain` → 词表 pin → 四个互斥的获取分支 → MSVC ABI 修复门 → + `build.mcpp` 的宿主解析)。注释比代码老,而这段代码是**决定用哪个编译器**的。 + +`prepare.cppm:1257` 那个变量还叫 `tcSpecIsMsvc`,但它其实是 +`is_system_toolchain` 的结果 —— 名字说的是"族",值说的是"来源"。 +**名字和值不是一回事,这正是这一轮所有缺陷的形状。** + +### 建议:**收拢这个特例,而不是推广它** + +保持 `@system` 只有 MSVC 能用(甚至值得在 `parse_toolchain_spec` 里 +**显式拒绝** `gcc@system`,并给一句"xlings 不依赖 host 工具链,用 +`gcc@`"—— 让原则**可执行**,而不是靠没人去写)。 + +要减的是**摊开的代价**,做法是把"来源"变成一个显式的、解析一次的东西: + +```cpp +enum class Origin { Managed, SystemMsvc }; // 只有两种,而且只会有两种 + +struct ResolvedToolchain { + Origin origin; + std::filesystem::path compiler; // payload 里的 cl/gcc,或机器上的 cl + // …… +}; +``` + +`prepare` / `lifecycle` 各自解析一次 `Origin`,后面按它分发,而不是每个子命令 +自己再问一遍 `is_system_toolchain`。 + +配套把三处**重复拼写**消掉(这几处和特例本身无关,纯粹是复制): + +1. `xim_tool()` + `installation_at()` + 错误信息 —— `prepare.cppm:1322-1341` + 与 `lifecycle.cppm:730-755` 几乎逐字重复 → 提一个 + `resolve_managed_msvc(env, pkg)`。 +2. sysroot 依赖规则两份且**不等价**(`prepare.cppm:1471-1480` vs + `lifecycle.cppm:694-702`)→ 合成一个谓词。 +3. `prepare.cppm:1002-1005` 的注释说 4 步、实际 9 个输入 → 要么补齐, + 要么把顺序变成一张数据表,让注释无处可撒谎。 + +**收益**:特例仍然是特例(符合原则),但只在**一处**被认出来; +26 处分支里绝大多数变成"按 Origin 分发"的普通代码。 +`gcc@system` 从"没实现"变成"**明确拒绝并说明理由**"。 + +--- + +## 2. SDK 是"依赖",却被当成"搜索路径" ⭐ 高优先级 + +### 现状 + +编译器拿到了版本轴,**SDK 没有**。 + +`msvc.cppm:589` 的 `find_windows_sdk()` 按顺序扫: +`WindowsSdkDir` → 兄弟 xpkgs → 写死的 `C:\Program Files (x86)\Windows Kits\10`。 + +但对 `msvc@` 来说,SDK 是 recipe 里**声明过的依赖** +(`xim:windows-sdk@10.0.26100`),位置是确定的。确定的东西被拿去搜, +就会出现今天这两个后果: + +1. **半装的 payload 因为版本号更高,排在机器自己那套完整 SDK 前面**, + 每个 TU 都编过,最后炸在 `LNK1104: cannot open file 'kernel32.lib'`, + 而日志里没有一行提到 SDK。(已修:`pick()` 现在两半都要, + 但那是"别选错",不是"直接知道选哪个"。) +2. **manifest 无法钉 SDK 版本**。可以写 `msvc@14.44.35207`, + 不能写"配 10.0.26100 那套 SDK"。两台机器仍可能用不同 SDK 编同一份源码 —— + 正是版本轴当初要解决的那个问题,只是换到了下一层。 + +### 建议 + +对受管 toolset,**绑定**而不是搜索: + +``` +msvc@ → SDK 从同一条 xim 解析拿(和编译器同一个机制) +msvc@system → 维持现在的搜索链(机器上的东西只能靠找) +``` + +再给 manifest 一个可选的显式轴: + +```toml +[toolchain] +windows = "msvc@14.44.35207" +windows_sdk = "10.0.26100" # 可选;省略则用 toolset 声明的那个依赖 +``` + +**收益**:两条来源在**两半**上都对称(编译器 + SDK),而不是只在编译器这一半; +"声明什么就用什么"这句承诺覆盖整个工具链而不是它的一部分。 + +--- + +## 3. `cxx_runtime = "toolchain-coupled"` 对 MSVC 被拒,但工具链**确实带着**运行时 ⭐ 高优先级 + +### 现状 + +`distribution.cppm:432`: + +> `cxx_runtime = "toolchain-coupled"` has no meaning for the MSVC runtime +> (it ships with the OS/redistributable, **not with the toolchain**) + +这句话对 `msvc@system` 是对的,对 `msvc@` 是**错的**。 +受管 payload 里就有: + +``` +VC/Redist/MSVC/14.44.35112/x64/Microsoft.VC143.CRT/ + vcruntime140.dll msvcp140.dll msvcp140_2.dll vcruntime140_threads.dll +``` + +recipe 的 payload 集里明确包含 `Microsoft.VC..CRT.Redist.X64.base.vsix` +(`pkgs/m/msvc.lua:134`)—— **我们下载了它,然后一个字节都没用。** +而 VS 自身安装也带同样的目录,所以两条来源都能支持。 + +于是分发矩阵上出现一个洞: + +| | 自包含 | 宿主耦合 | 工具链耦合 | +|---|---|---|---| +| gcc / libstdc++ | 静态链接 | 用系统 libstdc++ | **拷 libstdc++ 到产物旁** ✅ | +| MSVC | `/MT` ✅ | `/MD` ✅ | **拒绝** ❌ | + +### 建议 + +PE 上实现 toolchain-coupled:把 `Microsoft.VC*.CRT/*.dll` 拷到产物旁边 —— +和 gcc 拷 libstdc++ 是同一件事、同一个语义。 + +**收益**:`/MD` 构建**可分发**,不再要求目标机装 VC++ 运行时; +三行分发契约在两个 ABI 上一致;已经付过的下载有了用处。 + +### 而且这不只是"少个功能",是 `mcpp run` 的一个潜在失败 + +默认是 `/MD`,产物链的是 `vcruntime140.dll` / `msvcp140.dll`。 +这两个**不是** Windows 自带的(自带的是 `ucrtbase.dll`,Win10 起进系统), +它们来自 VC++ 可再发行包。 + +而全仓库搜不到一处 `Redist` / `vcruntime140` / `msvcp140`: + +``` +$ grep -rn "Redist\|vcruntime140\|msvcp140" src/ +（无匹配） +``` + +也就是说:一台**只装了受管 toolset、没装过 VS 也没装过 redist** 的干净 Windows, +`mcpp build` 会成功,`mcpp run` 会以「找不到 vcruntime140.dll」失败。 +今天没暴露,是因为 CI runner 上装着 Visual Studio —— +**又一次「验收环境比目标环境富裕」**,和 §5 是同一个形状。 + +`runtimeLibraryDirs`(Windows 上就是 PATH,`env.cppm:163`)这条通道是现成的, +缺的只是把受管 payload 的 `VC/Redist/.../Microsoft.VC143.CRT` 放进去。 +所以这一条同时修两件事:**运行**(把 DLL 放上 PATH)和 +**分发**(把 DLL 拷到产物旁)。 + +**注意**:debug 的 `vcruntime140d.dll` 在 `debug_nonredist/` 下, +**不可再分发** —— 实现时必须只取 `x64/Microsoft.VC143.CRT/`,不能取 +`debug_nonredist/`。这一条要写进测试。 + +--- + +## 3b. `mcpp doctor` 里还留着 #436 修掉的那个 bug —— 现成的第四份拷贝 🔴 立刻可修 + +`doctor.cppm:398`: + +```cpp +auto bin = mcpp::toolchain::toolchain_frontend( + vEntry.path() / "bin", mcpp::toolchain::to_xim_package(s)); +if (bin.empty()) continue; // ← 装好的 msvc toolset 在这里被丢掉 +``` + +这就是 #436 修的那一条:cl.exe 在 `VC/Tools/MSVC//bin/Host//`, +深四层,`bin/` 下什么都没有 → `continue`。#436 把 +`toolchain list` 换成了 `payload_frontend(root, pkg, family)`, +**但 doctor 没跟着换。** + +后果:同一台机器上 `mcpp toolchain list` 看得见的 msvc toolset, +`mcpp doctor` 看不见 —— 两个都"成功",而且不一致。 + +**修法**:`doctor.cppm:398` 改用 `payload_frontend`。一行。 +这也是 §1 那句"布局规则被抄了第 N 份"的活证据:#436 消掉了第三份, +第四份一直在。 + +--- + +## 3c. `has_usable_msvc()` 把"这台机器有 VS"当成了"这里能用 MSVC" ⭐ 中高优先级 + +`msvc.cppm:660` 只探测机器: + +```cpp +return find_std_module_source().has_value() && find_windows_sdk().has_value(); +``` + +但它被用在三个对**受管 toolset 同样适用**的决策上: + +| 位置 | 决策 | +|---|---| +| `prepare.cppm:1097` | 首次运行时要不要把 Windows 用户导向 mingw | +| `prepare.cppm:1389` | 离线错误文案 | +| `prepare.cppm:1558` | MSVC ABI 修复门 | + +于是:**一台钉了 `msvc@14.44.35207`、但没装 Visual Studio 的机器, +这个判据回答 `false`** —— 明明工具链就在 payload 里躺着。 + +这和 §1 是同一个病:**判据问的是"族/机器",而答案取决于"来源"。** + +**建议**:拆成两个问题 —— `system_msvc_is_usable()`(现在这个)和 +`msvc_is_available(spec)`(system 就探测,受管就看 payload 在不在)。 +调用点各自选一个,而不是共用一个含糊的。 + +--- + +## 3d. Windows 上的"打包分发"是空的 ⭐ 高优先级(范围最大) + +`pack.cppm:637`: + +```cpp +#if defined(_WIN32) + // `mcpp pack` is not yet supported on Windows. +``` + +整个 `mcpp pack` 是 ELF 专用的:`LD_TRACE_LOADED_OBJECTS` / `ldd` / +`patchelf` / `tar`。Windows 上直接返回错误,让人去用 CI 的 zip。 + +而且**两套系统并不通话**: + +- `distribution.cppm` 决定的是**契约**(自包含 / 宿主耦合 / 工具链耦合), + 只影响编译链接**旗标**; +- `pack.cppm` 决定的是**真的拷哪些文件**,它**从不读 Contract**, + 也不认识工具链。 + +所以今天 `cxx_runtime` 这个契约在打包这一步是**没有执行者**的 —— +ELF 上靠 `ldd` 闭包歪打正着,PE 上则完全没有这一步。 + +**建议**(按性价比排序): + +1. **先让 §3 的 toolchain-coupled 落地**(把 VC CRT DLL 拷到产物旁)。 + 这一步不需要 `pack` —— 它属于构建产物布局,而且顺手修掉 §3 那个 + `mcpp run` 在干净机器上的失败。 +2. 再做 PE 版 `pack`:DLL 闭包用 `dumpbin /dependents` 或读 PE 导入表 + (后者无外部依赖,更符合这套代码库的口味),`is_system_lib()` 换成 + Windows 的系统 DLL 白名单(kernel32/user32/ucrtbase/…),打 zip 而不是 tar。 + 已有设计稿:`.agents/docs/2026-05-19-pack-windows-design.md`。 +3. 让 `pack` **读 Contract**:`--mode` 与 `cxx_runtime` 现在是两套词汇, + 讲的是同一件事。至少要在两者矛盾时报出来 + (`pack.cppm:192-229` 已经有这类拒绝的先例,可以照着长)。 + +--- + +## 4. `installed()` 的语义:必须是"这份 recipe 产出的状态" ⭐ 跨仓库规则 + +这一轮九层缺陷里,真正致命的几层全是这一条。 + +### 两条子规则 + +**(a) 它必须是覆盖,不是抽样。** 每条断言对应一个"只有它才提供"的 payload。 +`windows-sdk` 现在是这样做的:`gdi32.lib` 只在 Desktop Libs +(它的 365 个库和 Store Apps Libs 的 116 个**完全不相交**),所以哪个 payload +没到,报错就点名哪一个。 + +**(b) 它必须能表达"不该在什么"。** 版本号不会因为 recipe 改了就变, +所以 `installed()` 是**唯一**能把老机器拉回来的东西。#637 让 recipe 不再安装 +`vctip.exe`,但已经装了的机器一点没变 —— 直到 #639 让 `installed()` +检查它**不在**。 + +> 新增文件靠断言"它在"就能发现,**删掉的文件必须显式说"它不该在"**。 + +### 建议 + +1. 写进 `.agents/skills/xpkg-creater/SKILL.md`(规则 + 这两个反例)。 +2. 加一条 CI lint:`install()` 落 N 个 payload 的 recipe, + `required_files()` 至少要提到 N 个互不相同的路径。 + 便宜,而且正好挡住"覆盖退化成抽样"。 + +--- + +## 5. 不运行被测物的验收步骤,验的是解压 ⭐ 中优先级 + +索引侧 windows-test 做的是「装 → 检查 → 卸」,**全程不编译**。 +而 `vctip.exe` 是 `cl.exe` **运行时**才被拉起的 —— 不跑编译器就没有进程, +卸载自然成功。于是 msvc.lua 连续几个 PR 的 windows-test 全绿, +包括「post-uninstall checks」,而卸载对任何真正构建过东西的人都是坏的。 + +### 建议 + +工具链类包的 windows-test 增加一步:**编译并链接一个真程序**,再卸载。 +至少把包自己 `programs` 里声明的东西各跑一次。 +代价是几十秒,挡住的是"装得上但用不了"这整类缺陷 —— 这一轮里它出现了四次。 + +--- + +## 6. 卸载的健壮性只做在 mcpp 一侧 ⭐ 中优先级 + +`remove_payload_tree()`(`lifecycle.cppm:224`)现在会:清只读位 → 有上限重试 +→ **改名挪走** → 下次生命周期命令清扫。 + +但 `xlings remove msvc` 有**一模一样**的问题:Windows 不让删含打开文件的目录, +而 `/Zi` 构建留下的 `mspdbsrv.exe` 就住在 payload 里。 + +### 建议 + +把 park-and-sweep 下沉到 xlings 的卸载路径。谁装的谁卸,这个能力不该只有 +mcpp 有 —— 否则每个消费者都要自己重写一遍,而写错的方式很多 +(我第一版就是"靠试删来找占用者",诊断变成了第二次破坏)。 + +--- + +## 7. 索引发布窗口没有可观测的"就绪"信号 ⭐ 中优先级 + +今天撞了**五次**。「合入 → 索引 → 装得到」之间有个肉眼不可见的窗口, +而窗口期内的失败信息是 `not found` —— 和"这个包根本不存在"一模一样。 + +最难受的是第 3 次和第 5 次:它们让**已经修好**的缺陷看起来像没修好, +只有对着时间戳才分得清。 + +### 建议 + +索引发布一个**修订号**,消费者能等到 `>= rev`。 +退一步:至少让"刚发布、还没到"与"没有这个包"在报错里能区分开。 +`xlings` 已经有 `run \`xlings update\` if the package was just published` +这句提示 —— 方向是对的,只是 CI 里没有人能照做。 + +--- + +## 8. 已经做对、不要动的部分 + +写下来是为了避免后续 review 把它们"顺手改掉": + +- **两条正交的轴**(§0)。这是整个设计的承重墙。 +- **`msvc_wants_static_crt()` 单一推导**(`dialect.cppm:134`)。 + 三处调用(`flags.cppm:613`、`flags.cppm:851`、`prepare.cppm:5023`) + 传的都是**manifest 原始标量**,所以编译器收到的 `/MT` 和分发表说的 + 自包含不可能不一致。**不要**改成读解析后的 contract —— 那会让每个 + Windows 构建都翻成 `/MT`。 +- **payload 多地址 + 单一 sha256**。镜像不是第二个信任根,是同一份字节的 + 第二个地址。 +- **能力驱动的 e2e 拆分**(`ci-windows-msvc-xlings.yml`)。用能力而不是文件清单 + 分流,新测试靠声明 `# requires: xlings-msvc` 加入,没有第二份清单会走样。 + +--- + +## 9. 建议的落地顺序 + +| # | 项 | 规模 | 影响面 | 依赖 | +|---|---|---|---|---| +| 0 | **§3b doctor 用 `payload_frontend`** | **一行** | mcpp | 无。现成的 bug,先修 | +| 1 | §4 `installed()` 规则 + lint | 小 | xim-pkgindex | 无。最便宜,挡住的最多 | +| 2 | §5 索引 CI 真编译一次 | 小 | xim-pkgindex | 无 | +| 3 | §3 MSVC toolchain-coupled(含 `mcpp run` 的 PATH) | 中 | mcpp | 无。DLL 已在 payload 里 | +| 4 | §3c 拆开 `has_usable_msvc` | 小 | mcpp | 无 | +| 5 | §2 SDK 绑定 + 版本轴 | 中 | mcpp | 无 | +| 6 | §6 park-and-sweep 下沉 | 小 | xlings | 无 | +| 7 | §1 收拢 Origin + 消三处重复(**不**通用化 `@system`) | 大 | mcpp | 建议在 §2/§3c 之后 —— 都动 spec 层 | +| 8 | §3d PE 版 `pack` | 大 | mcpp | 排在 §3 之后:§3 先解决"能跑" | +| 9 | §7 索引修订号 | 大 | xlings + 索引 | 收益最分散 | + +第 0–4 项互不依赖,可以并行,合计不大。§1 和 §3d 是两块真正的工作量, +建议各自单独一轮 —— 尤其 §1 动的是 spec 层,改完之后其余几项的 diff 会更小。 + +⚠️ §1 的方向已修正:**收拢特例,不是推广它**。初稿建议的 `gcc@system` +与 xlings"能不依赖 host 就不依赖 host"的原则相悖,已作废。 + +**如果只做三件**:§3b(一行)、§4(规则,挡住的最多)、§3(让干净 Windows +上 `mcpp run` 真的能跑)。 + +--- + +## 10. 一句话 + +这一轮九层缺陷,没有一层是"写错了"。全都是**验收判据比"能用"弱**: +`installed()` 只查 cl.exe、只查目录;`find_windows_sdk()` 只查头文件; +索引 CI 从不编译;e2e 只能 pass 或 skip。 + +把判据改成"能用"之后,它们一层层自己冒出来了。 +上面七条建议,本质上是同一句话在七个位置的应用。 diff --git a/.github/workflows/ci-windows-msvc-xlings.yml b/.github/workflows/ci-windows-msvc-xlings.yml new file mode 100644 index 00000000..a363850e --- /dev/null +++ b/.github/workflows/ci-windows-msvc-xlings.yml @@ -0,0 +1,98 @@ +name: ci-windows-msvc-xlings + +# mcpp driving the xlings-MANAGED MSVC toolset (`msvc@`) — the whole +# chain, end to end: index → payload → unpack → resolve → build → run → +# remove. +# +# WHY THIS IS NOT IN ci-windows-e2e.yml, which also runs MSVC tests: +# +# Different subject. Everything MSVC in the main suite (95_msvc_system, +# 99_msvc_native_build, 177, 180, 182) tests mcpp against the machine's own +# Visual Studio. That is mcpp's code and nothing else's. This job tests +# mcpp against the xlings ECOSYSTEM — a package index, a mirror, a payload +# set, an unpack recipe — most of which lives in another repository and +# moves on its own schedule. +# +# Different failure meaning. When this job goes red it usually means the +# index moved, not that the pull request broke something. Mixed into the +# main suite that reads as "your change broke Windows", and the honest +# signal (100+ fast tests, all about mcpp) gets buried under one slow test +# about somebody else's package. Keeping them apart keeps both readable. +# +# Different cost. ~380 MB of downloads (xim:msvc + xim:windows-sdk) and a +# real toolchain install, against a suite whose other tests are seconds +# each. +# +# The split is enforced by a capability, not by a file list: the tests carry +# `# requires: xlings-msvc`, granted only by MCPP_E2E_XLINGS_MSVC=1 below. So +# the main suite skips them by construction, and a new test joins this job by +# declaring the capability — there is no second list to keep in sync. +# +# Paired workflows: ci-windows.yml, ci-windows-e2e.yml. + +on: + push: + branches: [ main ] + pull_request: + branches: [ main ] + workflow_dispatch: + +concurrency: + group: ci-${{ github.workflow }}-${{ github.ref }} + cancel-in-progress: true + +jobs: + xlings-msvc: + name: xlings-managed msvc toolset (windows x64, self-host) + runs-on: windows-latest + timeout-minutes: 45 + env: + MCPP_HOME: C:\Users\runneradmin\.mcpp + steps: + - uses: actions/checkout@v4 + - uses: ./.github/actions/bootstrap-mcpp + + - name: Build mcpp from source (self-host) + shell: bash + run: | + export MCPP_VENDORED_XLINGS="$XLINGS_BIN" + "$MCPP" build + # Newest mcpp.exe, not an arbitrary one — `target/` is restored from + # cache and keeps a directory per build fingerprint. Same reasoning + # as ci-windows-e2e.yml, where picking wrong ran the previous + # release's binary. + MCPP_SELF=$(find target -name "mcpp.exe" -path "*/bin/*" -printf "%T@ %p\n" \ + | sort -rn | head -1 | cut -d" " -f2-) + test -n "$MCPP_SELF" || { echo "FAIL: no mcpp.exe"; exit 1; } + MCPP_SELF=$(cd "$(dirname "$MCPP_SELF")" && pwd)/$(basename "$MCPP_SELF") + "$MCPP_SELF" --version + echo "MCPP_SELF=$MCPP_SELF" >> "$GITHUB_ENV" + + - name: xlings-managed msvc e2e + shell: bash + timeout-minutes: 30 + env: + # Grants the `xlings-msvc` capability. Without it these tests skip + # everywhere, which is exactly what the main suite wants. + MCPP_E2E_XLINGS_MSVC: '1' + # Name what this job runs, so the job title and its contents cannot + # drift apart. Widen the glob when a second test joins. + E2E_ONLY: '239_*.sh' + run: | + export MCPP="$MCPP_SELF" + export MCPP_VENDORED_XLINGS="$XLINGS_BIN" + export MCPP_E2E_TOOLCHAIN_MIRROR=GLOBAL + "$MCPP_SELF" self config --mirror GLOBAL + bash tests/e2e/run_all.sh + + # A run that matched nothing is a green tick for having done nothing, + # and it looks exactly like a run that passed. E2E_ONLY is a glob typed + # by hand; if it stops matching, say so here rather than in a report + # nobody reads. + - name: Fail if the filter selected no tests + if: always() + shell: bash + run: | + n=$(ls tests/e2e/239_*.sh 2>/dev/null | wc -l) + test "$n" -gt 0 || { echo "FAIL: E2E_ONLY matched no tests"; exit 1; } + echo "selected $n test(s)" diff --git a/CHANGELOG.md b/CHANGELOG.md index 92b7bb41..268f2034 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,55 @@ > 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。 > 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)。 +## [Unreleased] + +### 修复 + +- **Windows 上 `mcpp toolchain remove ` 报 "Access is denied"。** + + `remove_all` 直接上,不清只读位、不重试、报错也不说是哪个文件。 + payload 是从 .vsix/.msi 解出来的,归档条目带只读属性;POSIX 只要**目录** + 可写就能 unlink 子项,所以这个问题在 Linux/macOS 上根本不出现。 + 另外 `/Zi` 构建之后 mspdbsrv.exe 还会在 payload 里活几秒。 + + 两个原因表现完全一样(都是 "Access is denied"),所以两个都处理: + 先清可写位重试,再给活着的进程一个**有上限**的等待窗口(10 × 300ms)。 + 报错现在会指出卡在哪个文件 —— 光一句 "Access is denied" 没法处理。 + + 这个诊断当场就派上用场了:它点名的是 + `bin\Hostx64\x64\Microsoft.VisualStudio.Telemetry.dll` —— 既不是只读位 + 也不是 mspdbsrv,而是 cl.exe 拉起的后台遥测进程 `vctip.exe` 占着它。 + **占用者不是 mcpp 能删掉的东西**,真正的修复在 payload 那边 + (openxlings/xim-pkgindex#637 不再安装 vctip.exe);这边留下的是通用兜底 + 和那句能读懂的报错。 + + 报错同时会说清楚:**失败的 remove 不是空操作** —— `remove_all` 会先删掉 + 能删的,所以剩下的是一个有洞的工具链,得重来一次而不是接着用。 + + 修完 vctip 之后占用者换成了 `mspdbcore.dll` —— `/Zi` 构建拉起的 + **mspdbsrv.exe**,它构建完还活几十秒,而且就住在正要被删的 payload 里。 + 等它不现实(等多久都是猜),所以改成**挪走**:Windows 拒绝删除含打开文件的 + 目录,但允许**重命名**。`remove` 的承诺是"这个工具链不再装着",改名之后 + 它确实不再装着;剩下的字节在下一条生命周期命令里清扫(那时占用者早退了)。 + +- **半装的 Windows SDK 被当成装好的,链接到最后才炸。** + + `find_windows_sdk()` 认一个根的条件是 `Include\\ucrt\corecrt.h` + 存在 —— 只看头文件。而 SDK 是头文件**和**导入库两半。 + + 托管的 `xim:windows-sdk` payload 少了带 `kernel32.lib` 的那个 MSI 时, + 这个根照样"找到了",而且因为版本号更高,**排在机器自己那套完整 SDK 前面**。 + 于是每个 TU 都编过了,一直到最后一步: + + LINK : fatal error LNK1104: cannot open file 'kernel32.lib' + + 日志里没有任何一行提到 SDK。 + + 现在两半都要:`Include\\ucrt\corecrt.h` 和 + `Lib\\um\\kernel32.lib`。半装的根被跳过,搜索落到下一个, + 本来就能用的构建就能用了 —— 和 `has_usable_msvc()` 坚持"两半都要"是同一条 + 理由,只是这次轮到 SDK 自己。 + ## [2026.8.16.2] — 2026-08-16 ### 修复 diff --git a/docs/03-toolchains.md b/docs/03-toolchains.md index f89469a2..c6be373c 100644 --- a/docs/03-toolchains.md +++ b/docs/03-toolchains.md @@ -310,6 +310,12 @@ pinned toolset in mcpp's store, then `C:\Program Files (x86)\Windows Kits\10`. A missing SDK fails the build with guidance (`mcpp self doctor` reports SDK status). +A root only counts as an SDK when it has **both** halves — `Include\\ucrt\ +corecrt.h` *and* `Lib\\um\\kernel32.lib`. A root with headers and no +import libraries is skipped rather than selected, so a partially unpacked +payload cannot outrank the machine's complete SDK and turn into +`LNK1104: cannot open file 'kernel32.lib'` at the very end of a build. + **CRT model.** `/MD` (host-coupled) by default; `/MT` when either ```toml diff --git a/src/build/prepare.cppm b/src/build/prepare.cppm index 4715df91..2284b0c5 100644 --- a/src/build/prepare.cppm +++ b/src/build/prepare.cppm @@ -1090,11 +1090,22 @@ prepare_build(bool print_fingerprint, // rest: the vocabulary table already maps x86_64-windows-gnu to its pin // (winlibs GCC) and to static linkage, so the toolchain answer stays a // single derivation instead of being spelled out a second time here. + // "Is MSVC usable here" — either origin. Asking `has_usable_msvc()` (which + // probes the machine) would answer "no" on a box that has a pinned + // msvc@ payload and no Visual Studio, and every decision below + // would then divert a perfectly good toolchain to mingw. + auto msvc_usable_either_origin = [&]() -> bool { + auto c = get_cfg(); + if (!c) return mcpp::toolchain::msvc::has_usable_msvc(); + return mcpp::toolchain::msvc::msvc_available_here( + (*c)->xlingsHome() / "data" / "xpkgs"); + }; + bool windowsGnuFirstRun = false; if constexpr (mcpp::platform::is_windows) { if (!tcSpec.has_value() && overrides.target_triple.empty() && m->buildConfig.target.empty() - && !mcpp::toolchain::msvc::has_usable_msvc()) { + && !msvc_usable_either_origin()) { auto cfgW = get_cfg(); if (!cfgW || (*cfgW)->defaultTarget.empty()) { overrides.target_triple = @@ -1320,12 +1331,19 @@ prepare_build(bool print_fingerprint, // (cl.exe is four levels deeper) and the ELF post-install fixup // (there is nothing to patchelf on a PE toolchain). if (spec->family == mcpp::toolchain::Family::Msvc) { + // Not `payload->root`: that field is the fetcher's guess at where + // the useful tree starts, and it descends into a lone + // subdirectory when the version dir has no bin/ include/ lib/. + // An msvc payload's only entry is `VC/`, so the guess lands one + // level too deep. (store, name, version) is known — use it. + auto verDir = mcpp::xlings::paths::xim_tool( + mcpp::config::make_xlings_env(**cfg), pkg.ximName, pkg.ximVersion); auto inst = mcpp::toolchain::msvc::installation_at( - payload->root, pkg.ximVersion); + verDir, pkg.ximVersion); if (!inst) { return std::unexpected(std::format( "msvc payload at '{}' has no cl.exe under VC/Tools/MSVC/{}", - payload->root.string(), pkg.ximVersion)); + verDir.string(), pkg.ximVersion)); } explicit_compiler = inst->clPath; mcpp::ui::info("Resolved", std::format( @@ -1379,7 +1397,7 @@ prepare_build(bool print_fingerprint, // exactly what this machine cannot build. Name the toolchain that // will actually work there instead. if (mcpp::platform::is_windows - && !mcpp::toolchain::msvc::has_usable_msvc()) { + && !msvc_usable_either_origin()) { return std::unexpected(std::format( "no toolchain configured (and no Visual Studio found).\n" " run one of:\n" @@ -1434,7 +1452,7 @@ prepare_build(bool print_fingerprint, if constexpr (mcpp::platform::is_macos) { defaultSpec = std::string(pins::kFirstRunMac); } else if constexpr (mcpp::platform::is_windows) { - // Reaching here means has_usable_msvc() was true — the seed above + // Reaching here means msvc_usable_either_origin() was true — the seed above // diverts the no-Visual-Studio case onto the windows-gnu target // before the target block runs, so it never gets this far. defaultSpec = std::string(pins::kFirstRunWinMsvc); @@ -1548,7 +1566,7 @@ prepare_build(bool print_fingerprint, const bool targetsMsvcAbi = tc->compiler == mcpp::toolchain::CompilerId::MSVC || mcpp::toolchain::is_msvc_target(*tc); - if (targetsMsvcAbi && !mcpp::toolchain::msvc::has_usable_msvc()) { + if (targetsMsvcAbi && !msvc_usable_either_origin()) { // Native cl.exe is ALWAYS a deliberate choice: mcpp never selects // msvc@system on its own — it cannot install one — so the only way it // reaches config.toml is a user typing `mcpp toolchain default msvc`. diff --git a/src/doctor.cppm b/src/doctor.cppm index 7d99841d..ece85bb4 100644 --- a/src/doctor.cppm +++ b/src/doctor.cppm @@ -395,8 +395,15 @@ export int doctor_report() { s.family = id->family; s.version = vEntry.path().filename().string(); s.target = id->target; - auto bin = mcpp::toolchain::toolchain_frontend( - vEntry.path() / "bin", mcpp::toolchain::to_xim_package(s)); + // payload_frontend, not toolchain_frontend(root/"bin"): + // cl.exe is four levels down at + // VC/Tools/MSVC//bin/Host//, so the `bin/` + // lookup finds nothing and `continue` drops every + // installed msvc toolset. That is the defect #436 fixed + // in `toolchain list`; doctor kept its own copy, so the + // two commands disagreed about the same machine. + auto bin = mcpp::toolchain::payload_frontend( + vEntry.path(), mcpp::toolchain::to_xim_package(s), s.family); if (bin.empty()) continue; sawAny = true; diff --git a/src/toolchain/lifecycle.cppm b/src/toolchain/lifecycle.cppm index b3814add..1445418f 100644 --- a/src/toolchain/lifecycle.cppm +++ b/src/toolchain/lifecycle.cppm @@ -208,6 +208,166 @@ void msvc_print_detected(const mcpp::toolchain::msvc::MsvcInstallation& inst, // headers much later. `has_usable_msvc()` exists for exactly this reason; // this is the same judgement applied to the managed origin, where the SDK // arrives as a package dependency and can therefore fail on its own. +// Delete a payload tree, coping with the two things that make a plain +// `remove_all` fail on Windows and on nothing else: +// +// read-only files — payloads are unpacked from .vsix/.msi, and archive +// entries carry the attribute through. POSIX only needs the DIRECTORY +// writable to unlink a child, so this never shows up on Linux or macOS. +// a lingering handle — a build with /Zi leaves mspdbsrv.exe running for a +// few seconds after cl.exe exits, and it lives inside the payload. +// +// Both surface as the same "Access is denied", which is why this handles both +// rather than picking one: clear the attribute, then give a live process a +// bounded moment to go away. Retries are capped and short — `toolchain +// remove` should not hang because something holds the directory forever. +// Whether any FILE is left under `root`. Directories alone are not a +// toolchain: nothing resolves a compiler out of an empty tree. +bool any_regular_file(const std::filesystem::path& root) { + std::error_code ec; + for (auto it = std::filesystem::recursive_directory_iterator( + root, std::filesystem::directory_options::skip_permission_denied, ec); + it != std::filesystem::recursive_directory_iterator{}; it.increment(ec)) { + if (it->is_regular_file(ec)) return true; + } + return false; +} + +export bool remove_payload_tree(const std::filesystem::path& root, + std::error_code& ec) { + std::filesystem::remove_all(root, ec); + if (!ec) return true; + + // Second pass: make everything writable, then try again. + std::error_code ignore; + for (auto it = std::filesystem::recursive_directory_iterator( + root, std::filesystem::directory_options::skip_permission_denied, + ignore); + it != std::filesystem::recursive_directory_iterator{}; it.increment(ignore)) { + std::filesystem::permissions(it->path(), std::filesystem::perms::owner_write, + std::filesystem::perm_options::add, ignore); + } + for (int attempt = 0; attempt < 10; ++attempt) { + ec.clear(); + std::filesystem::remove_all(root, ec); + if (!ec) return true; + std::this_thread::sleep_for(std::chrono::milliseconds{300}); + } + if (!std::filesystem::exists(root, ignore)) return true; + + // Still held. Move the held FILES out instead of waiting on a process we + // do not own. + // + // ⚠️ NOT by renaming the payload directory. Windows lets you rename an + // open FILE — that is how an updater replaces a running .exe — but it + // does NOT let you rename a DIRECTORY that contains one. An earlier + // version of this function renamed `root` and was wrong about exactly + // that: the rename failed with the same "Access is denied", and remove + // still reported failure. What follows is the shape that actually works: + // + // move every surviving file to a sibling `.trash-*` directory (allowed + // even while open, same volume, the handle follows the file) + // then delete the payload tree, which now holds only directories + // + // This is what makes `remove` mean something after a /Zi build with the + // toolset being removed: mspdbsrv.exe lives INSIDE the payload and + // outlives cl.exe by tens of seconds, so "wait for it" is a guess and a + // CLI that hangs on a guess is worse than the failure. The bytes are + // swept by the next lifecycle command, when nothing holds them. + auto trash = root.parent_path() / + std::format(".trash-{}-{}", root.filename().string(), + std::chrono::steady_clock::now() + .time_since_epoch().count()); + std::vector survivors; + for (auto it = std::filesystem::recursive_directory_iterator( + root, std::filesystem::directory_options::skip_permission_denied, + ignore); + it != std::filesystem::recursive_directory_iterator{}; it.increment(ignore)) { + if (it->is_regular_file(ignore)) survivors.push_back(it->path()); + } + if (survivors.empty()) return false; // nothing to move; genuinely stuck + + std::filesystem::create_directories(trash, ignore); + // Flat, index-prefixed: two payload subdirectories can hold the same file + // name, and this directory exists to be deleted, not to be read. + std::size_t moved = 0; + for (std::size_t i = 0; i < survivors.size(); ++i) { + std::error_code ren; + std::filesystem::rename( + survivors[i], + trash / std::format("{}-{}", i, survivors[i].filename().string()), ren); + if (!ren) ++moved; + } + if (moved != survivors.size()) { + // Some file could not even be moved. Leave the rest where they are — + // a half-moved payload is worse than one that is still all there. + std::filesystem::remove_all(trash, ignore); + return false; + } + + ec.clear(); + std::filesystem::remove_all(root, ec); // directories only now + std::filesystem::remove_all(trash, ignore); // usually works; fine if not + if (!ec) return true; + + // The files are gone and an empty directory skeleton would not die. That + // is a sharing violation on a DIRECTORY, which on Windows means some + // process has one of them as its current directory -- mspdbsrv.exe is + // launched inside the payload, so this is the normal tail of a /Zi build. + // + // A toolchain with no files in it is not installed, which is what + // `remove` promises. Reporting failure here would be reporting the + // opposite of what happened, and the skeleton is swept by the next + // lifecycle command once that process exits. + ec.clear(); + return !any_regular_file(root); +} + +// Delete `.trash-*` left behind by a removal that had to park a held payload. +// Takes the directory the payload lived IN (`/xim-x-`), which is +// where park() puts them -- one place, one level, nothing to walk. +// +// Best-effort and run before a lifecycle operation rather than after one: by +// the next command the process that held the bytes is normally gone, so this +// is where they actually get freed. +export void sweep_parked_payloads(const std::filesystem::path& pkgRoot) { + std::error_code ec; + if (!std::filesystem::is_directory(pkgRoot, ec)) return; + for (auto& e : std::filesystem::directory_iterator(pkgRoot, ec)) { + if (!e.is_directory(ec)) continue; + if (e.path().filename().string().starts_with(".trash-")) { + std::filesystem::remove_all(e.path(), ec); + } else if (!any_regular_file(e.path())) { + // A version directory with no files in it is the skeleton a + // removal could not delete because something held a directory + // open. A real install always has files, so this cannot eat one. + std::filesystem::remove_all(e.path(), ec); + } + } +} + +// The first entry that is still there after a failed removal. The error code +// alone says "Access is denied" and not by whom or to what, which is the +// difference between a report someone can act on and one they cannot. +// +// It does not probe by deleting. `remove_all` has already removed everything +// it could, so whatever SURVIVED is exactly what blocked it -- reporting the +// first survivor needs no further destruction. (An earlier version did probe +// by deleting, which turns a diagnostic into a second act of damage on a +// payload the caller may well want to keep and retry.) +export std::optional +first_undeletable(const std::filesystem::path& root) { + std::error_code ec; + if (!std::filesystem::exists(root, ec)) return std::nullopt; + for (auto it = std::filesystem::recursive_directory_iterator( + root, std::filesystem::directory_options::skip_permission_denied, ec); + it != std::filesystem::recursive_directory_iterator{}; it.increment(ec)) { + if (ec) break; + if (it->is_regular_file(ec)) return it->path(); + } + return root; +} + void msvc_warn_if_sdk_missing(const mcpp::toolchain::msvc::MsvcInstallation& inst) { auto roots = mcpp::toolchain::msvc::sibling_sdk_roots(inst.clPath); if (auto sdk = mcpp::toolchain::msvc::find_windows_sdk(roots)) { @@ -600,6 +760,13 @@ export int toolchain_install(const mcpp::config::GlobalConfig& cfg, } mcpp::log::verbose("toolchain", std::format("installing main: {}", pkg.target())); + // A previous `remove` may have parked a held payload beside this one; + // by now whatever held it has exited, so free the bytes before adding + // another few hundred MB. + sweep_parked_payloads( + mcpp::xlings::paths::xim_tool(mcpp::config::make_xlings_env(cfg), + pkg.ximName, pkg.ximVersion) + .parent_path()); auto payload = fetcher.resolve_xpkg_path(pkg.target(), /*autoInstall=*/true, &progress); mcpp::log::verbose("toolchain", std::format("main install result: {}", payload ? ("ok → " + payload->root.string()) : payload.error().message)); @@ -618,14 +785,26 @@ export int toolchain_install(const mcpp::config::GlobalConfig& cfg, // VC/Tools/MSVC//bin/Hostx64/x64, and there is nothing to // patchelf on a PE toolchain. if (spec->family == mcpp::toolchain::Family::Msvc) { + // NOT `payload->root` — that is a GUESS, and it guesses wrong + // here. resolve_xpkg_path calls the version directory the root + // only when it directly contains bin/ include/ lib/; otherwise it + // descends into a lone subdirectory. An installed msvc payload + // has exactly one entry, `VC/`, so the "root" comes back as + // …/14.44.35207/VC and the toolset then looks like it is missing. + // + // The location is not something to infer: it is (store, name, + // version), and all three are known here. resolve_xpkg_path above + // is what INSTALLS; this is what says where. + auto verDir = mcpp::xlings::paths::xim_tool( + mcpp::config::make_xlings_env(cfg), pkg.ximName, pkg.ximVersion); auto inst = mcpp::toolchain::msvc::installation_at( - payload->root, pkg.ximVersion); + verDir, pkg.ximVersion); if (!inst) { mcpp::ui::error(std::format( "msvc payload installed at '{}', but no cl.exe under " "VC/Tools/MSVC/{} — the payload is not what this version " "claims to be", - payload->root.string(), pkg.ximVersion)); + verDir.string(), pkg.ximVersion)); return 1; } msvc_print_detected(*inst, "Installed"); @@ -763,7 +942,14 @@ export int toolchain_set_default(const mcpp::config::GlobalConfig& cfg, } auto installDir = mcpp::xlings::paths::xim_tool(xlEnv, pkg.ximName, pkg.ximVersion); - if (!std::filesystem::exists(installDir)) { + // A RESOLVABLE COMPILER, not a directory that exists. A removal that + // could not delete the last empty directories leaves a skeleton + // behind (see remove_payload_tree), and `exists()` would call that + // skeleton an installed toolchain and then hand it to a build. + // + // Same rule as everywhere else in this round: installed means usable, + // not present. + if (mcpp::toolchain::payload_frontend(installDir, pkg, spec->family).empty()) { // Before "not installed", check whether this is the retired // `msvc@` spelling — otherwise the advice is to // install a toolset that does not exist and never will. @@ -829,9 +1015,27 @@ export int toolchain_remove(const mcpp::config::GlobalConfig& cfg, mcpp::ui::error(std::format("{} is not installed", spec)); return 1; } - std::filesystem::remove_all(installDir, ec); - if (ec) { - mcpp::ui::error(std::format("remove failed: {}", ec.message())); + sweep_parked_payloads(installDir.parent_path()); + if (!remove_payload_tree(installDir, ec)) { + // Say that the payload is now BROKEN, not merely that removal + // failed. `remove_all` deletes what it can before it stops, so a + // failed remove is not a no-op: what is left is a toolchain with + // holes in it, and someone who reads "remove failed" and moves on + // will meet those holes as a build error instead. + mcpp::ui::error(std::format( + "remove failed: {}{}\n" + " The payload is now INCOMPLETE — files were deleted " + "before this one\n" + " blocked the rest. Close whatever holds it and run " + "the same command\n" + " again; do not build with this toolchain until it " + "removes cleanly.", + ec.message(), + first_undeletable(installDir) + .transform([](const std::filesystem::path& p) { + return std::format("\n stuck at: {}", p.string()); + }) + .value_or(std::string{}))); return 1; } mcpp::ui::status("Removed", spec); diff --git a/src/toolchain/msvc.cppm b/src/toolchain/msvc.cppm index a0417e7d..f2506cba 100644 --- a/src/toolchain/msvc.cppm +++ b/src/toolchain/msvc.cppm @@ -173,6 +173,35 @@ sibling_sdk_roots(const std::filesystem::path& clPath); // Always false off Windows: the whole discovery chain is Win32-only. bool has_usable_msvc(); +// Whether MSVC is usable HERE — either origin. `has_usable_msvc()` asks only +// about the machine, which is the wrong question wherever a managed toolset +// would serve just as well: a box with a pinned `msvc@14.44.35207` payload +// and no Visual Studio answers `false` to that one while being perfectly able +// to compile. +// +// `pkgsDir` is mcpp's payload store; a non-empty `xim-x-msvc/` with a +// resolvable cl.exe counts. Off Windows both are false — the whole chain is +// Win32-only. +bool msvc_available_here(const std::filesystem::path& pkgsDir); + +// The redistributable VC runtime that BELONGS TO THIS TOOLSET: +// \Redist\MSVC\\\Microsoft.VC.CRT\ +// vcruntime140.dll msvcp140.dll ... +// +// It matters because the default CRT model is /MD, and those DLLs are NOT +// Windows components — ucrtbase.dll ships with the OS, vcruntime140.dll does +// not. On a machine that has only a managed toolset (no Visual Studio, no +// redistributable installed) a /MD build links fine and then cannot start. +// +// The toolset carries its own copy, so this is the toolchain-coupled runtime +// in exactly the sense libstdc++ is for gcc, and it goes in the same field. +// +// `redistVer` is NOT the tools version (14.44.35112 vs 14.44.35207), so the +// newest directory is chosen rather than derived. `debug_nonredist\` is never +// returned: those DLLs may not be redistributed. +std::filesystem::path vc_redist_dir(const std::filesystem::path& clPath, + std::string_view arch = "x64"); + // Synthesize the environment cl.exe/link.exe need — what vcvars would set, // derived directly from the located VC tools + SDK (no vcvarsall.bat run): // INCLUDE = \include; \Include\\{ucrt,um,shared,winrt} @@ -575,6 +604,17 @@ sibling_sdk_roots(const std::filesystem::path& clPath) { return out; } +// The `Lib\\um\` subdirectory whose absence makes a root unusable +// FOR THIS HOST. Spelled from the host architecture rather than the build's +// target: a cross-compiling link is the caller's business (`build_env_for_cl` +// takes an arch), but a root with no host-arch libs at all is not an SDK this +// machine can link against. The names are the SDK's own. +#if defined(_M_ARM64) || defined(__aarch64__) +constexpr std::string_view sdk_lib_arch = "arm64"; +#else +constexpr std::string_view sdk_lib_arch = "x64"; +#endif + std::optional find_windows_sdk( std::span extraRoots) { // Highest version dir under `root/Include` that actually carries the UCRT @@ -582,17 +622,38 @@ std::optional find_windows_sdk( // (Registry Installed Roots would be marginally more correct — the path // scan covers every real installer layout seen so far and needs no Win32 // API surface.) + // BOTH halves, for the same reason `has_usable_msvc()` asks for both: an + // SDK is headers AND import libraries, and a root carrying only the first + // is a half-installed state that this function used to call "found". + // + // That is not hypothetical. A managed `xim:windows-sdk` payload whose + // ucrt MSI had unpacked but whose um-libs MSI had not left + // `Include//ucrt/corecrt.h` on disk with no `kernel32.lib` anywhere; + // the header check passed, the root was selected over the machine's own + // complete SDK, every translation unit compiled, and the build died at + // LINK : fatal error LNK1104: cannot open file 'kernel32.lib' + // with nothing in the log naming the SDK. Rejecting the partial root + // makes the search fall through to the next one, which is the behaviour + // a user would expect from a probe that reports "not found". + // + // kernel32.lib is the right sentinel: every link needs it, and unlike the + // ucrt libs it is not spread across the SDK's optional pieces. auto pick = [](const std::filesystem::path& root, std::string_view want) -> std::optional { std::error_code ec; auto inc = root / "Include"; if (!std::filesystem::is_directory(inc, ec)) return std::nullopt; + auto usable = [&](const std::filesystem::path& verDir, + const std::string& v) { + return std::filesystem::exists(verDir / "ucrt" / "corecrt.h", ec) + && std::filesystem::exists( + root / "Lib" / v / "um" / sdk_lib_arch / "kernel32.lib", ec); + }; std::string best; for (auto& e : std::filesystem::directory_iterator(inc, ec)) { if (!e.is_directory(ec)) continue; auto v = e.path().filename().string(); - if (!std::filesystem::exists(e.path() / "ucrt" / "corecrt.h", ec)) - continue; + if (!usable(e.path(), v)) continue; if (!want.empty() && v == want) return WindowsSdk{root, v}; if (v > best) best = v; } @@ -636,6 +697,25 @@ bool has_usable_msvc() { #endif } +bool msvc_available_here([[maybe_unused]] const std::filesystem::path& pkgsDir) { +#if defined(_WIN32) + if (has_usable_msvc()) return true; + // A managed toolset is just as usable, and asking the machine about it + // gets the wrong answer. Any installed version whose cl.exe resolves + // counts; `installation_at` is the same resolution install and build use. + std::error_code ec; + auto root = pkgsDir / "xim-x-msvc"; + if (!std::filesystem::is_directory(root, ec)) return false; + for (auto& v : std::filesystem::directory_iterator(root, ec)) { + if (!v.is_directory(ec)) continue; + if (installation_at(v.path(), v.path().filename().string())) return true; + } + return false; +#else + return false; +#endif +} + std::vector build_env_for_cl(const std::filesystem::path& clPath, std::string_view arch, const WindowsSdk& sdk) { @@ -762,6 +842,40 @@ std::vector std_compat_build_commands( ref, crtFlag) }; } +std::filesystem::path vc_redist_dir(const std::filesystem::path& clPath, + std::string_view arch) { + // /Tools/MSVC//bin/Host//cl.exe → up 6 from the arch dir + auto vc = clPath.parent_path(); + for (int i = 0; i < 6 && !vc.empty(); ++i) vc = vc.parent_path(); + std::error_code ec; + auto redist = vc / "Redist" / "MSVC"; + if (!std::filesystem::is_directory(redist, ec)) return {}; + + std::filesystem::path best; + std::string bestVer; + for (auto& v : std::filesystem::directory_iterator(redist, ec)) { + if (!v.is_directory(ec)) continue; + auto archDir = v.path() / std::string(arch); + if (!std::filesystem::is_directory(archDir, ec)) continue; + for (auto& c : std::filesystem::directory_iterator(archDir, ec)) { + if (!c.is_directory(ec)) continue; + auto name = c.path().filename().string(); + // Microsoft.VC143.CRT — the CRT, not CXXAMP/OpenMP, and never + // anything under debug_nonredist (which is not redistributable + // and is a sibling of , not a child, but be explicit). + if (!name.starts_with("Microsoft.VC") || !name.ends_with(".CRT")) + continue; + if (c.path().string().find("debug_nonredist") != std::string::npos) + continue; + if (auto ver = v.path().filename().string(); ver > bestVer) { + bestVer = ver; + best = c.path(); + } + } + } + return best; +} + std::expected enrich_toolchain_from_cl(Toolchain& tc) { auto banner = capture_cl_banner(tc.binaryPath); auto parsed = parse_cl_banner(banner); @@ -809,6 +923,24 @@ std::expected enrich_toolchain_from_cl(Toolchain& tc) { if (auto sdk = find_windows_sdk(extraSdkRoots)) { tc.envOverrides = build_env_for_cl(tc.binaryPath, parsed->second, *sdk); } + + // The toolset's own redistributable CRT, in the same field gcc uses for + // libstdc++ — so `mcpp run` puts it on PATH exactly the way it puts a + // private libstdc++ on LD_LIBRARY_PATH. + // + // Without it, the DEFAULT build (/MD) links vcruntime140.dll and + // msvcp140.dll, which are not OS components, and a machine with only a + // managed toolset cannot start what it just built. That machine is not + // hypothetical — it is any box that installed `msvc@` and never + // had Visual Studio. CI does not see it because the runners have VS. + // + // Safe for the link line: hostflags returns early for MSVC before it + // emits -L, and flags.cppm's runtime-dir block is `supports_rpath`-gated, + // so nothing here reaches cl or link as a flag. + if (auto redist = vc_redist_dir(tc.binaryPath, parsed->second); + !redist.empty()) { + tc.linkRuntimeDirs.push_back(redist); + } return {}; } diff --git a/tests/e2e/239_msvc_managed_toolset.sh b/tests/e2e/239_msvc_managed_toolset.sh index c465086c..d66b7402 100755 --- a/tests/e2e/239_msvc_managed_toolset.sh +++ b/tests/e2e/239_msvc_managed_toolset.sh @@ -1,6 +1,6 @@ #!/usr/bin/env bash -# requires: msvc -# 97_msvc_managed_toolset.sh — `msvc@`: the toolset the manifest +# requires: msvc xlings-msvc +# 239_msvc_managed_toolset.sh — `msvc@`: the toolset the manifest # names is the one that compiles, regardless of what this machine has. # # WHY THIS TEST CANNOT BE SATISFIED BY THE MACHINE'S COMPILER, which is the @@ -13,7 +13,7 @@ # - `msvc@system` on the same machine must still resolve to the SYSTEM cl, # i.e. the two origins do not contaminate each other # -# Network: installs xim:msvc (~85 MB) + xim:windows-sdk (~135 MB). Skips +# Network: installs xim:msvc (~85 MB) + xim:windows-sdk (~291 MB). Skips # cleanly when the index cannot be reached, because an offline runner has # nothing to say about this. set -e @@ -36,20 +36,50 @@ restore() { trap restore EXIT cd "$TMP" -# 1) install it. This runs BEFORE the discoverability check below, so that a -# runner which cannot reach the index skips instead of failing an assertion -# about a list the index would have filled in. +# 0) Decide SKIP here, from a positive check, and never again. +# +# ⚠️ This used to be decided AFTER the install by pattern-matching the +# failure text, and one of the patterns was `*"index"*`. Nearly every mcpp +# command prints "package index" somewhere, so every genuine install +# failure took the skip branch: the test could pass or skip, never fail. +# +# It hid a real one. `tar -xf "C:\...vsix"` fails under GNU tar, which +# reads `C:` as a hostname ("Cannot connect to C: resolve failed"); the +# install ran for 135 seconds, failed, and this script reported PASS. +# +# A skip has to be decided by what is ABSENT before the work starts, not by +# what the failure looked like afterwards. +# Ask for THIS toolset, not the family. Grepping for "msvc" is true during +# an index publish window -- the family is listed, this version is not yet +# -- and a hard failure then reports a timing artifact as a defect. The +# question the skip needs answered is "can this index give me $TOOLSET", +# so that is the question to ask. +if ! "$MCPP" toolchain list 2>&1 | grep -q "$TOOLSET"; then + echo "SKIP: this index does not offer msvc $TOOLSET (publish window? offline?)" + exit 0 +fi + +# 1) install it. Any failure from here on is a FAILURE. rc=0; out=$("$MCPP" toolchain install msvc "$TOOLSET" 2>&1) || rc=$? if [[ $rc -ne 0 ]]; then - case "$out" in - *"index"*|*"network"*|*"resolve"*|*"offline"*|*"connect"*|*"not found"*) - echo "SKIP: xim:msvc@$TOOLSET unreachable: $out"; exit 0 ;; - *) echo "FAIL: install msvc $TOOLSET: $out"; exit 1 ;; - esac + echo "FAIL: install msvc $TOOLSET (rc=$rc):" + echo "$out" + exit 1 fi [[ "$out" == *"$TOOLSET"* ]] \ || { echo "FAIL: install did not report the toolset: $out"; exit 1; } +# 1a) the SDK must have arrived with it. `msvc_warn_if_sdk_missing()` prints +# `windows sdk: ()` on success, and this script used to +# swallow the install output whenever the install exited 0 -- so a payload +# whose SDK dependency was half-installed said nothing here and failed +# ~100 lines later as +# LINK : fatal error LNK1104: cannot open file 'kernel32.lib' +# with no line in the log naming the SDK. Assert at the step that knows. +[[ "$out" == *"windows sdk:"* ]] \ + || { echo "FAIL: toolset installed but no Windows SDK was reported:"; \ + echo "$out"; exit 1; } + # 1b) it must now be LISTED. A toolset that installs but never appears is # indistinguishable from one that did not install, and `toolchain list` is # where a user looks. diff --git a/tests/e2e/run_all.sh b/tests/e2e/run_all.sh index 0ac88e08..8a7dad47 100755 --- a/tests/e2e/run_all.sh +++ b/tests/e2e/run_all.sh @@ -132,6 +132,21 @@ if ls "${MCPP_HOME}/registry/data/xpkgs/xim-x-llvm"/*/share/libc++/v1/std.cppm 2 CAPS+=(import-std-libcxx) fi +# xlings-msvc: mcpp driving the xlings-MANAGED toolset (`msvc@`) -- +# a different subject from `msvc`, which means "this machine has a Visual +# Studio". Opt-in rather than detected, for two reasons: +# +# - it downloads ~380 MB (xim:msvc + xim:windows-sdk) and installs a +# toolchain, so it does not belong in a suite whose other 100+ tests are +# seconds each; +# - its subject is the xlings ECOSYSTEM (index -> payload -> unpack -> +# build -> remove), so it fails for reasons that have nothing to do with +# the change under test, and mixing it in makes every unrelated PR look +# broken when the index moves. +# +# It runs in ci-windows-msvc-xlings.yml, which sets this and nothing else. +[[ "${MCPP_E2E_XLINGS_MSVC:-}" == "1" ]] && CAPS+=(xlings-msvc) + echo "Detected capabilities: ${CAPS[*]:-}" # --------------------------------------------------------------------------- @@ -149,7 +164,7 @@ echo "Detected capabilities: ${CAPS[*]:-}" # sync when adding a capability. KNOWN_CAPS=(elf fresh-sandbox gcc import-std-libcxx macos mingw-cross msvc musl nasm no-msvc pack patchelf python3 scan-deps symlink unix-shell - windows wine) + windows wine xlings-msvc) bad_tokens=0 for tf in "$HERE"/[0-9]*.sh; do @@ -268,7 +283,14 @@ if [[ -n "${E2E_SHARD:-}" ]]; then fi SHARD_POS=0 +# Optional name filter: E2E_ONLY="" runs just the matching tests. +# Used by the workflows that own ONE subject (see ci-windows-msvc-xlings.yml) +# so the job name and the thing it runs cannot drift apart. for test in "$HERE"/[0-9]*.sh; do + if [[ -n "${E2E_ONLY:-}" ]]; then + # shellcheck disable=SC2053 — glob match is the point + [[ "$(basename "$test")" == $E2E_ONLY ]] || continue + fi if (( SHARD_TOTAL > 1 )); then _mine=$(( (SHARD_POS % SHARD_TOTAL) + 1 )) SHARD_POS=$(( SHARD_POS + 1 )) diff --git a/tests/unit/test_toolchain_lifecycle.cpp b/tests/unit/test_toolchain_lifecycle.cpp new file mode 100644 index 00000000..c93cb5b1 --- /dev/null +++ b/tests/unit/test_toolchain_lifecycle.cpp @@ -0,0 +1,112 @@ +#include + +import std; +import mcpp.toolchain.lifecycle; + +using namespace mcpp::toolchain; + +// WHAT IS AND IS NOT TESTED HERE, because the difference is not a gap. +// +// `remove_payload_tree()` falls back to moving held FILES aside so the tree +// can be deleted around them. That path is reachable only where a file can be +// renamed while something holds it open — which is Windows. On POSIX the two +// permissions are the same one: a file you cannot unlink is a file you cannot +// rename either, because both need write on the parent directory. There is no +// POSIX state that models "held open by another process". +// +// A POSIX fixture cannot stand in for it either, and the reason is worth +// keeping: the only POSIX way to make a file undeletable is to drop write on +// its parent directory — and this function's SECOND pass adds write back +// across the tree, on purpose, because that is exactly the read-only-payload +// case it must fix. The fixture therefore becomes removable the moment the +// code under test touches it. That is the function working, not a hole. +// +// So the held-file path is gated by e2e 239 on the Windows runner, and what +// is pinned here is what must hold on every platform: the ordinary deletion +// takes the ordinary route, and the sweep is precise about what it eats. +// Nothing here asserts something this platform cannot express. + +TEST(ToolchainRemove, AnOrdinaryPayloadIsJustDeleted) { + // The common path must not grow a `.trash-` directory: moving files aside + // is the fallback, and a fallback that fires always is not a fallback. + auto dir = std::filesystem::temp_directory_path() + / std::format("mcpp-rm-ok-{}", std::chrono::steady_clock::now() + .time_since_epoch().count()); + std::filesystem::create_directories(dir / "14.44.35207" / "bin"); + std::ofstream{dir / "14.44.35207" / "bin" / "cl.exe"} << "not a compiler"; + + std::error_code ec; + EXPECT_TRUE(remove_payload_tree(dir / "14.44.35207", ec)); + EXPECT_FALSE(std::filesystem::exists(dir / "14.44.35207")); + + bool anyTrash = false; + for (auto& e : std::filesystem::directory_iterator(dir, ec)) + anyTrash |= e.path().filename().string().starts_with(".trash-"); + EXPECT_FALSE(anyTrash) << "a deletable payload was parked instead of deleted"; + + std::filesystem::remove_all(dir, ec); +} + +TEST(ToolchainRemove, TheSweepDeletesParkedPayloadsAndNothingElse) { + // The sweep runs before every install and remove, so it must be precise: + // `.trash-*` goes, an installed version directory beside it stays. + auto pkgRoot = std::filesystem::temp_directory_path() + / std::format("mcpp-sweep-{}", std::chrono::steady_clock::now() + .time_since_epoch().count()); + std::filesystem::create_directories(pkgRoot / ".trash-14.44.35207-1" / "sub"); + std::ofstream{pkgRoot / ".trash-14.44.35207-1" / "sub" / "held.dll"} << "x"; + std::filesystem::create_directories(pkgRoot / "14.44.35207" / "bin"); + std::ofstream{pkgRoot / "14.44.35207" / "bin" / "cl.exe"} << "keep me"; + + sweep_parked_payloads(pkgRoot); + + EXPECT_FALSE(std::filesystem::exists(pkgRoot / ".trash-14.44.35207-1")) + << "parked payload was not swept"; + EXPECT_TRUE(std::filesystem::exists(pkgRoot / "14.44.35207" / "bin" / "cl.exe")) + << "the sweep deleted an installed toolset"; + + std::error_code ec; + std::filesystem::remove_all(pkgRoot, ec); +} + +TEST(ToolchainRemove, ADirectorySkeletonWithNoFilesCountsAsRemoved) { + // Windows can refuse to delete an empty DIRECTORY when a process has it + // as its current directory — mspdbsrv.exe is launched inside the payload, + // so this is the ordinary tail of a /Zi build with the toolset being + // removed. Every file is already gone at that point. + // + // A toolchain with no files in it is not installed, which is exactly what + // `remove` promises. Calling that a failure would report the opposite of + // what happened. + auto dir = std::filesystem::temp_directory_path() + / std::format("mcpp-skel-{}", std::chrono::steady_clock::now() + .time_since_epoch().count()); + std::filesystem::create_directories(dir / "14.44.35207" / "bin" / "Hostx64" / "x64"); + + std::error_code ec; + EXPECT_TRUE(remove_payload_tree(dir / "14.44.35207", ec)) + << "a payload with no files left was reported as still installed"; + + std::filesystem::remove_all(dir, ec); +} + +TEST(ToolchainRemove, TheSweepAlsoClearsAFileLessSkeleton) { + // The deferral half: whatever the removal could not delete goes on the + // next lifecycle command, once the process holding it has exited. + auto pkgRoot = std::filesystem::temp_directory_path() + / std::format("mcpp-skel2-{}", std::chrono::steady_clock::now() + .time_since_epoch().count()); + std::filesystem::create_directories(pkgRoot / "14.44.35207" / "bin" / "x64"); + std::filesystem::create_directories(pkgRoot / "14.52.36629" / "bin"); + std::ofstream{pkgRoot / "14.52.36629" / "bin" / "cl.exe"} << "a real one"; + + sweep_parked_payloads(pkgRoot); + + EXPECT_FALSE(std::filesystem::exists(pkgRoot / "14.44.35207")) + << "the empty skeleton survived the sweep"; + EXPECT_TRUE(std::filesystem::exists(pkgRoot / "14.52.36629" / "bin" / "cl.exe")) + << "the sweep ate an installed toolset"; + + std::error_code ec; + std::filesystem::remove_all(pkgRoot, ec); +} diff --git a/tests/unit/test_toolchain_msvc.cpp b/tests/unit/test_toolchain_msvc.cpp index 4a69dd0c..034a9c0e 100644 --- a/tests/unit/test_toolchain_msvc.cpp +++ b/tests/unit/test_toolchain_msvc.cpp @@ -169,7 +169,20 @@ struct FakeToolset { std::ofstream{tools / "modules" / "std.ixx"} << "export module std;"; } } + // A COMPLETE SDK: headers and import libraries. Both, because + // find_windows_sdk() requires both — a root with only headers is the + // half-installed state `add_sdk_headers_only()` below exists to build. + // Libs for both architectures so the fixture does not care which host it + // is running on. void add_sdk(std::string_view version) { + add_sdk_headers_only(version); + for (auto arch : {"x64", "arm64"}) { + auto lib = root / "Lib" / std::string(version) / "um" / arch; + std::filesystem::create_directories(lib); + std::ofstream{lib / "kernel32.lib"} << "not a library"; + } + } + void add_sdk_headers_only(std::string_view version) { auto inc = root / "Include" / std::string(version) / "ucrt"; std::filesystem::create_directories(inc); std::ofstream{inc / "corecrt.h"} << "#pragma once"; @@ -234,6 +247,28 @@ TEST(MsvcManaged, ResolvesTheDeclaredToolsetAndNotItsNeighbour) { EXPECT_EQ(newer->toolsVersion, "14.52.36629"); } +TEST(MsvcManaged, TheVersionDirIsTheRootNotItsLoneSubdirectory) { + // A real installed payload has exactly ONE entry: `VC/`. The fetcher's + // `XpkgPayload::root` treats the version directory as the root only when + // it directly contains bin/ include/ lib/, and otherwise descends into a + // lone subdirectory -- so for msvc it hands back `/VC`, and the + // toolset then looks missing on a payload that installed perfectly. + // + // Pinning both sides here: the version directory resolves, and the `VC` + // subdirectory does NOT. The second half is what makes this a test rather + // than a restatement -- an implementation that searched upward from + // whatever it was given would pass the first and fail this. + FakeToolset t{"verdir"}; + t.add_toolset("14.44.35207"); + + auto ok = msvc::installation_at(t.root, "14.44.35207"); + ASSERT_TRUE(ok.has_value()) << "the version directory must be the root"; + + EXPECT_FALSE(msvc::installation_at(t.root / "VC", "14.44.35207").has_value()) + << "a caller handing in the VC subdir is passing the wrong thing, and " + "must be told so rather than quietly rescued"; +} + TEST(MsvcManaged, AbsentToolsetIsNulloptNotASubstitute) { FakeToolset t{"absent"}; t.add_toolset("14.44.35207"); @@ -340,6 +375,142 @@ TEST(MsvcSdk, IncompleteSdkRootIsNotAnAnswer) { if (sdk) EXPECT_NE(sdk->root, t.root) << "an SDK-less root was accepted"; } +// ─── the toolset's own redistributable CRT ─────────────────────────────── + +namespace { + +// A toolset laid out the way MSVC actually lays one out, including the part +// that trips a derivation: the Redist version is NOT the tools version. +struct FakeRedist { + std::filesystem::path root, clPath; + + explicit FakeRedist(std::string_view toolsVer, std::string_view redistVer) { + root = std::filesystem::temp_directory_path() + / std::format("mcpp-redist-{}", std::chrono::steady_clock::now() + .time_since_epoch().count()); + auto tools = root / "VC" / "Tools" / "MSVC" / std::string(toolsVer); + auto bin = tools / "bin" / "Hostx64" / "x64"; + std::filesystem::create_directories(bin); + clPath = bin / "cl.exe"; + std::ofstream{clPath} << "not a compiler"; + add(redistVer, "x64", "Microsoft.VC143.CRT", "vcruntime140.dll"); + } + void add(std::string_view ver, std::string_view arch, + std::string_view comp, std::string_view file) { + auto d = root / "VC" / "Redist" / "MSVC" / std::string(ver) + / std::string(arch) / std::string(comp); + std::filesystem::create_directories(d); + std::ofstream{d / std::string(file)} << "dll"; + } + void add_debug(std::string_view ver) { + auto d = root / "VC" / "Redist" / "MSVC" / std::string(ver) + / "debug_nonredist" / "x64" / "Microsoft.VC143.DebugCRT"; + std::filesystem::create_directories(d); + std::ofstream{d / "vcruntime140d.dll"} << "dll"; + } + ~FakeRedist() { std::error_code ec; std::filesystem::remove_all(root, ec); } + FakeRedist(const FakeRedist&) = delete; + FakeRedist& operator=(const FakeRedist&) = delete; +}; + +} // namespace + +TEST(MsvcRedist, FoundFromTheCompilerPathAlone) { + // Nothing configured, no version derived: the compiler's own location + // says which toolset this is, and the toolset carries its runtime. + FakeRedist t{"14.44.35207", "14.44.35112"}; + auto d = msvc::vc_redist_dir(t.clPath, "x64"); + ASSERT_FALSE(d.empty()) << "the toolset's redistributable CRT was not found"; + EXPECT_TRUE(std::filesystem::exists(d / "vcruntime140.dll")); +} + +TEST(MsvcRedist, TheRedistVersionIsNotTheToolsVersion) { + // 14.44.35207 (tools) vs 14.44.35112 (redist) is what MSVC actually + // ships. Deriving one from the other finds nothing — this is why the + // directory is searched rather than composed. + FakeRedist t{"14.44.35207", "14.44.35112"}; + auto d = msvc::vc_redist_dir(t.clPath, "x64"); + ASSERT_FALSE(d.empty()); + EXPECT_NE(d.string().find("14.44.35112"), std::string::npos) + << "did not land in the redist version directory: " << d.string(); +} + +TEST(MsvcRedist, NewestRedistWins) { + FakeRedist t{"14.44.35207", "14.40.00000"}; + t.add("14.44.35112", "x64", "Microsoft.VC143.CRT", "vcruntime140.dll"); + auto d = msvc::vc_redist_dir(t.clPath, "x64"); + ASSERT_FALSE(d.empty()); + EXPECT_NE(d.string().find("14.44.35112"), std::string::npos) + << "older redist won: " << d.string(); +} + +TEST(MsvcRedist, TheDebugCrtIsNeverReturned) { + // debug_nonredist may NOT be redistributed. Returning it would put + // vcruntime140d.dll on PATH and, later, into a shipped artifact. + FakeRedist t{"14.44.35207", "14.44.35112"}; + t.add_debug("14.99.99999"); // newer, and must still lose + auto d = msvc::vc_redist_dir(t.clPath, "x64"); + ASSERT_FALSE(d.empty()); + EXPECT_EQ(d.string().find("debug_nonredist"), std::string::npos) + << "returned the non-redistributable debug CRT: " << d.string(); +} + +TEST(MsvcRedist, AToolsetWithoutARedistIsNotAnError) { + // msvc@system on a machine whose VS install omits the redist component, + // for instance. Empty means "nothing to add to PATH", not a failure. + FakeRedist t{"14.44.35207", "14.44.35112"}; + std::error_code ec; + std::filesystem::remove_all(t.root / "VC" / "Redist", ec); + EXPECT_TRUE(msvc::vc_redist_dir(t.clPath, "x64").empty()); +} + +TEST(MsvcSdk, HeadersWithoutImportLibsIsNotAnAnswer) { + // The half that used to pass. `Include//ucrt/corecrt.h` is there and + // `Lib/` is not, which is exactly what a managed windows-sdk payload + // looked like when its um-libs MSI had not unpacked: every TU compiled + // and the link died with + // LINK : fatal error LNK1104: cannot open file 'kernel32.lib' + // An SDK is headers AND libraries; reporting "found" for half of one puts + // this root ahead of the machine's complete SDK and breaks a build that + // would otherwise have worked. + NoSdkEnv clean; + FakeToolset t{"sdkheadersonly"}; + t.add_sdk_headers_only("10.0.26100.0"); + std::array roots{t.root}; + auto sdk = msvc::find_windows_sdk(roots); + if (sdk) EXPECT_NE(sdk->root, t.root) + << "a headers-only SDK was accepted; the link would fail on kernel32.lib"; +} + +TEST(MsvcSdk, ACompleteRootIsStillAccepted) { + // The other direction, so the check above cannot be satisfied by + // rejecting everything. + NoSdkEnv clean; + FakeToolset t{"sdkcomplete"}; + t.add_sdk("10.0.26100.0"); + std::array roots{t.root}; + auto sdk = msvc::find_windows_sdk(roots); + ASSERT_TRUE(sdk) << "a complete SDK root was rejected"; + EXPECT_EQ(sdk->root, t.root); + EXPECT_EQ(sdk->version, "10.0.26100.0"); +} + +TEST(MsvcSdk, APartialRootYieldsToACompleteOne) { + // Ordering, not just acceptance: given both, the usable one must win even + // though the partial one carries the HIGHER version — which is how the + // managed payload outranked the system SDK in the first place. + NoSdkEnv clean; + FakeToolset partial{"sdkpartialhigh"}; + partial.add_sdk_headers_only("10.0.99999.0"); + FakeToolset complete{"sdkcompletelow"}; + complete.add_sdk("10.0.26100.0"); + std::array roots{partial.root, complete.root}; + auto sdk = msvc::find_windows_sdk(roots); + ASSERT_TRUE(sdk); + EXPECT_EQ(sdk->root, complete.root) + << "the partial root won on version; it cannot link"; +} + TEST(MsvcSdk, DeclaredRootOutranksTheManagedOne) { // Both available. WindowsSdkDir is someone saying which one to use, and // it has to win — the same precedence VSINSTALLDIR has over vswhere.