fix(toolchain): narrow the sweep, and stop spawning cl.exe to ask a yes/no (#444)

Both found by reviewing the 2026.8.16.3 diff after tagging it.

**The sweep could delete another version mid-install.** It walked the whole
family directory and removed any version directory containing no regular
files. But an install populates a version directory over time -- which is why
package_fetcher tracks completeness with a marker file rather than by
existence -- so a DIFFERENT version being extracted right now is briefly
indistinguishable from a leftover skeleton. Two mcpp processes against one
MCPP_HOME is ordinary on a shared or self-hosted runner, and this feature
exists for long-lived installs on exactly those machines.

`.trash-*` is still swept family-wide: that name is only ever written by this
code, so deleting one is safe whoever else is running. The file-less-skeleton
rule now applies to the single version the command names.

**`msvc_available_here()` ran a compiler to answer a yes/no.** It wanted to
know whether a usable toolset exists and called the full `installation_at()`,
which spawns cl.exe for its banner to identify the version. That question is
asked on every build at the MSVC-ABI gate, so a machine with several
installed toolsets paid several subprocess spawns per build -- and those are
precisely the machines this predicate was added for.

`installation_at(..., identifyVersion = false)` skips the banner. The layout
still has exactly ONE implementation; this is a parameter, not a second copy
of the path arithmetic.

The new test was run against the family-wide sweep and fails there.

Co-authored-by: speak-agent <248744407+speak-agent@users.noreply.github.com>

Author: Sunrisepeak

CHANGELOG.md

@@ -3,6 +3,30 @@
 > 本文件追踪 `mcpp-community/mcpp` 公开仓的版本演进。
 > 格式参考 [Keep a Changelog](https://keepachangelog.com/zh-CN/1.1.0/)。
 
+## [Unreleased]
+
+### 修复
+
+- **卸载后的清扫会波及**别的版本**,而那可能正在被另一个进程解压。**
+
+  `sweep_parked_payloads` 原来把整个 family 目录扫一遍,把**任何**没有文件的
+  版本目录删掉。但安装是**逐步**往版本目录里写文件的(所以 package_fetcher
+  用标记文件而不是"目录在"来判断装完没有),于是**另一个正在解压的版本**在那
+  短暂窗口里和"残骨架"长得一模一样 —— 共用一个 `MCPP_HOME` 的机器上
+  (自托管 runner、共享开发机)两个 mcpp 进程同时跑是常态。
+
+  `.trash-*` 照旧全扫(那个名字只有这段代码会写);"没有文件的骨架"这一条
+  收窄成**只扫这条命令点名的那一个版本**。
+
+- **`msvc_available_here()` 每次构建都要为每个已装 payload 起一次 cl.exe。**
+
+  它只想知道"这儿有没有能用的 toolset",却走了完整的 `installation_at()`,
+  那里面会跑一次 cl 拿 banner 来定版本。而这个判据在**每次构建**的 MSVC ABI
+  门上都会被问到 —— 正好是装了多个 toolset 的机器最慢。
+
+  `installation_at(..., identifyVersion=false)` 跳过 banner。**布局仍然只有
+  一份实现** —— 不是再抄一遍路径拼接。
+
 ## [2026.8.16.3] — 2026-08-16
 
 ### 修复

src/toolchain/lifecycle.cppm

@@ -330,20 +330,33 @@ export bool remove_payload_tree(const std::filesystem::path& root,
 // 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) {
+export void sweep_parked_payloads(const std::filesystem::path& pkgRoot,
+                           const std::filesystem::path& skeleton) {
     std::error_code ec;
     if (!std::filesystem::is_directory(pkgRoot, ec)) return;
+
+    // `.trash-*` is a name only this code writes, so deleting any of them is
+    // safe regardless of who else is running.
     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.
+        if (e.is_directory(ec)
+            && e.path().filename().string().starts_with(".trash-"))
             std::filesystem::remove_all(e.path(), ec);
-        }
     }
+
+    // The file-less skeleton, on the other hand, is swept for ONE named
+    // version — the one this command is about — and never for whatever else
+    // happens to be sitting in the family directory.
+    //
+    // "No files in it" is not a safe thing to conclude about someone else's
+    // directory: an install populates a version directory over time (it is
+    // why package_fetcher tracks completeness with a marker file rather than
+    // by existence), so a concurrent `toolchain install` of a DIFFERENT
+    // version is briefly indistinguishable from a skeleton, and sweeping the
+    // whole family would delete it mid-extraction. Two mcpp processes against
+    // one MCPP_HOME is ordinary on a shared or self-hosted runner.
+    if (!skeleton.empty() && std::filesystem::is_directory(skeleton, ec)
+        && !any_regular_file(skeleton))
+        std::filesystem::remove_all(skeleton, ec);
 }
 
 // The first entry that is still there after a failed removal. The error code
@@ -763,10 +776,11 @@ export int toolchain_install(const mcpp::config::GlobalConfig& cfg,
         // 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 vdir = mcpp::xlings::paths::xim_tool(
+                mcpp::config::make_xlings_env(cfg), pkg.ximName, pkg.ximVersion);
+            sweep_parked_payloads(vdir.parent_path(), vdir);
+        }
         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));
@@ -1015,7 +1029,7 @@ export int toolchain_remove(const mcpp::config::GlobalConfig& cfg,
             mcpp::ui::error(std::format("{} is not installed", spec));
             return 1;
         }
-        sweep_parked_payloads(installDir.parent_path());
+        sweep_parked_payloads(installDir.parent_path(), installDir);
         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

src/toolchain/msvc.cppm

@@ -93,8 +93,13 @@ std::optional<MsvcInstallation> detect_installation();
 // anywhere, which is what makes the managed path testable off Windows. The
 // cl banner simply stays unparsed there and `display_version()` falls back
 // to the declared version.
+// `identifyVersion = false` skips running cl.exe for its banner. Use it when
+// the question is only "is there a usable toolset here" -- `msvc_available_here`
+// asks that on every build, and spawning a compiler per installed payload to
+// answer it is a latency regression on exactly the machines that have several.
 std::optional<MsvcInstallation> installation_at(const std::filesystem::path& vsRoot,
-                                                std::string_view toolsVersion);
+                                                std::string_view toolsVersion,
+                                                bool identifyVersion = true);
 
 // Parse a cl.exe banner into (version, arch). Token-based so localized
 // banners work: first "d.d.d[.d]" run is the version, arch is the arm64/x64/
@@ -514,7 +519,8 @@ namespace {
 // on a Linux CI runner.
 std::optional<MsvcInstallation>
 installation_from_tools_dir(const std::filesystem::path& vsRoot,
-                            const std::filesystem::path& tools);
+                            const std::filesystem::path& tools,
+                            bool identifyVersion = true);
 
 // Capture cl.exe's banner. cl prints it (plus a usage complaint) when run
 // bare; the exit status is irrelevant — parse whatever came out.
@@ -526,7 +532,8 @@ std::string capture_cl_banner(const std::filesystem::path& cl) {
 
 std::optional<MsvcInstallation>
 installation_from_tools_dir(const std::filesystem::path& vsRoot,
-                            const std::filesystem::path& tools) {
+                            const std::filesystem::path& tools,
+                            bool identifyVersion) {
     MsvcInstallation inst;
     inst.vsRoot       = vsRoot;
     inst.vsProduct    = product_from_vs_root(vsRoot);
@@ -558,22 +565,25 @@ installation_from_tools_dir(const std::filesystem::path& vsRoot,
     // Version identification: banner is authoritative; tolerate failure
     // (clVersion stays empty and display_version() falls back to the
     // tools-dir version). Off Windows that failure is the normal case.
-    if (auto parsed = parse_cl_banner(capture_cl_banner(inst.clPath))) {
-        inst.clVersion = parsed->first;
-        if (!parsed->second.empty()) inst.arch = parsed->second;
+    if (identifyVersion) {
+        if (auto parsed = parse_cl_banner(capture_cl_banner(inst.clPath))) {
+            inst.clVersion = parsed->first;
+            if (!parsed->second.empty()) inst.arch = parsed->second;
+        }
     }
     return inst;
 }
 
 } // namespace
 
 std::optional<MsvcInstallation> installation_at(const std::filesystem::path& vsRoot,
-                                                std::string_view toolsVersion) {
+                                                std::string_view toolsVersion,
+                                                bool identifyVersion) {
     if (toolsVersion.empty()) return std::nullopt;
     auto tools = vsRoot / "VC" / "Tools" / "MSVC" / std::string(toolsVersion);
     std::error_code ec;
     if (!std::filesystem::is_directory(tools, ec)) return std::nullopt;
-    return installation_from_tools_dir(vsRoot, tools);
+    return installation_from_tools_dir(vsRoot, tools, identifyVersion);
 }
 
 std::optional<MsvcInstallation> detect_installation() {
@@ -708,7 +718,9 @@ bool msvc_available_here([[maybe_unused]] const std::filesystem::path& pkgsDir)
     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;
+        // identifyVersion=false: this asks IF a toolset is here, never which.
+        if (installation_at(v.path(), v.path().filename().string(),
+                            /*identifyVersion=*/false)) return true;
     }
     return false;
 #else

tests/unit/test_toolchain_lifecycle.cpp

@@ -47,6 +47,33 @@ TEST(ToolchainRemove, AnOrdinaryPayloadIsJustDeleted) {
     std::filesystem::remove_all(dir, ec);
 }
 
+TEST(ToolchainRemove, TheSweepDoesNotTouchAnotherVersionThatHasNoFilesYet) {
+    // An install populates a version directory over time, so a DIFFERENT
+    // version being extracted right now is briefly indistinguishable from a
+    // leftover skeleton. Two mcpp processes against one MCPP_HOME is ordinary
+    // on a shared or self-hosted runner, and deleting someone else's
+    // half-extracted toolchain is not a cleanup.
+    //
+    // So the file-less sweep applies to the ONE version this command names.
+    auto pkgRoot = std::filesystem::temp_directory_path()
+                 / std::format("mcpp-sweep3-{}", std::chrono::steady_clock::now()
+                                                     .time_since_epoch().count());
+    auto mine   = pkgRoot / "14.44.35207";       // the one being removed
+    auto theirs = pkgRoot / "14.52.36629";       // someone else, mid-extract
+    std::filesystem::create_directories(mine / "bin");
+    std::filesystem::create_directories(theirs / "VC" / "Tools");
+
+    sweep_parked_payloads(pkgRoot, mine);
+
+    EXPECT_FALSE(std::filesystem::exists(mine)) << "the named skeleton survived";
+    EXPECT_TRUE(std::filesystem::exists(theirs))
+        << "swept a version this command was not about — that is someone "
+           "else's install being extracted";
+
+    std::error_code ec;
+    std::filesystem::remove_all(pkgRoot, 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.
@@ -58,7 +85,7 @@ TEST(ToolchainRemove, TheSweepDeletesParkedPayloadsAndNothingElse) {
     std::filesystem::create_directories(pkgRoot / "14.44.35207" / "bin");
     std::ofstream{pkgRoot / "14.44.35207" / "bin" / "cl.exe"} << "keep me";
 
-    sweep_parked_payloads(pkgRoot);
+    sweep_parked_payloads(pkgRoot, {});
 
     EXPECT_FALSE(std::filesystem::exists(pkgRoot / ".trash-14.44.35207-1"))
         << "parked payload was not swept";
@@ -100,7 +127,7 @@ TEST(ToolchainRemove, TheSweepAlsoClearsAFileLessSkeleton) {
     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);
+    sweep_parked_payloads(pkgRoot, pkgRoot / "14.44.35207");
 
     EXPECT_FALSE(std::filesystem::exists(pkgRoot / "14.44.35207"))
         << "the empty skeleton survived the sweep";