fix(toolchain): an installed msvc toolset did not appear in toolchain list - #436
Merged
Conversation
…n list`
The enumeration asked `toolchain_frontend(root / "bin", pkg)`, got nothing,
and `continue`d. cl.exe is four levels deeper -- under
VC/Tools/MSVC/<ver>/bin/Host<h>/<arch>/ -- so every msvc payload installed
correctly and was then invisible.
Three places have to know that layout. Install and build knew; the listing
did not, which is what a third inline copy of a rule tends to produce. They
now share `payload_frontend(payloadRoot, pkg, family)`, which delegates to
`msvc::installation_at()` for msvc and to the bin/-shaped lookup for
everything else -- so the three cannot disagree about where a payload keeps
its compiler.
Found by running the RELEASED 2026.8.16.1 binary against a payload-shaped
fixture, not by a test: the unit test I had written pinned
`identify_xim_payload("msvc")`, which was already correct. The identity
mapping and the enumeration are different questions, and only one of them
was being asked.
prepare.cppm's default-toolchain path also resolves through payload->binDir
and is deliberately left alone: the Windows first-run pin is llvm@20.1.7, so
that site is unreachable for msvc.
Regression test asserts both directions -- payload_frontend finds it, and the
`root/bin` question still answers nothing, which is what makes it the wrong
question rather than a broken implementation.
Sunrisepeak
pushed a commit
that referenced
this pull request
Aug 16, 2026
…et that never appeared `toolchain list` asked for `root/bin`; msvc keeps cl.exe four levels deeper. Three places need that layout, two had it. Fixed in #436. Recorded here for how it was found: not by a test. The unit test written for this pinned `identify_xim_payload("msvc")`, which was already correct -- identity mapping and enumeration are different questions and only one was being asked. e2e 239 step 1b would catch it, but only once the package is published and the Windows e2e runs again. "Has a test" and "has been tested" are not the same thing.
Sunrisepeak
added a commit
that referenced
this pull request
Aug 16, 2026
…fault build (#435) * docs: the verification found the packaged toolset could not link a default build Not a planned item -- it turned up while working through the "not covered" list, checking whether the packaged 14.52 could build xrgui. It could not, and neither could it build anything else: the payload set carried only the static CRT, so /MD (the default) failed on an unresolved msvcprt.lib. Fixed in xim-pkgindex#630. Recorded here because of what it says about the acceptance table in §4: "1790 static checks pass" and "windows-test installs it on a real Windows runner" were both true while the toolchain was broken. `installed()` checked cl.exe and std.ixx, and nothing anywhere links with this toolset -- so no gate could have reported it. An "installed" criterion weaker than "usable" reports that the archive unpacked, not that the package works. * docs: and the second one the verification found -- an installed toolset that never appeared `toolchain list` asked for `root/bin`; msvc keeps cl.exe four levels deeper. Three places need that layout, two had it. Fixed in #436. Recorded here for how it was found: not by a test. The unit test written for this pinned `identify_xim_payload("msvc")`, which was already correct -- identity mapping and enumeration are different questions and only one was being asked. e2e 239 step 1b would catch it, but only once the package is published and the Windows e2e runs again. "Has a test" and "has been tested" are not the same thing. --------- Co-authored-by: speak-agent <248744407+speak-agent@users.noreply.github.com>
3 tasks
Sunrisepeak
added a commit
that referenced
this pull request
Aug 16, 2026
Ships #436: an installed msvc toolset was invisible in `toolchain list`. The v2026.8.16.1 tag was cut before that fix, so `latest` currently carries it -- everything else in .1 is fine (install, build, default and remove all work; only the listing row was missing). Co-authored-by: speak-agent <248744407+speak-agent@users.noreply.github.com>
Sunrisepeak
pushed a commit
that referenced
this pull request
Aug 16, 2026
…ution Written from the round that actually got `msvc@<toolset>` working: nine layers of defect, five index publish windows, three tests that could not fail. Every finding is anchored to a file:line, not a preference. The two-axis model (acquisition vs resolution) is right and nothing here proposes replacing it. The findings are all one sentence: that axis exists only for MSVC, and it is only carried half way. Ten findings, including three that are live defects rather than design debt: - doctor.cppm:398 still uses the pre-#436 `toolchain_frontend(root/"bin")` shape, so an installed msvc toolset is visible to `toolchain list` and invisible to `doctor`. One line. - a /MD build with a managed toolset links vcruntime140.dll, which is not an OS component and appears nowhere in src/ -- on a clean Windows box `mcpp build` succeeds and `mcpp run` cannot start. CI hides it by having Visual Studio installed. - `has_usable_msvc()` probes the machine but gates three decisions that also apply to managed toolsets, so a box with a pinned toolset and no VS answers "no MSVC here". And the largest structural gap: `mcpp pack` is ELF-only, and distribution.cppm's contract never reaches pack.cppm at all, so `cxx_runtime` has no enforcer at packaging time on either platform.
Sunrisepeak
pushed a commit
that referenced
this pull request
Aug 16, 2026
With the files moved aside, removal still failed -- this time on the DIRECTORY skeleton, with a sharing violation rather than access denied. That is Windows refusing to delete a directory some process has as its current directory, and mspdbsrv.exe is launched inside the payload, so it is the ordinary tail of a /Zi build with the toolset being removed. Every file was already gone at that point. A toolchain with no files in it is not installed, which is exactly what `remove` promises, so reporting failure was reporting the opposite of what happened. The criterion is now "no regular file remains"; the skeleton is swept by the next lifecycle command. That exposed the matching lie on the other side: `toolchain default` accepted any directory that EXISTS, so a skeleton would have been called an installed toolchain and handed to a build. It now asks `payload_frontend` for a resolvable compiler -- the same rule this whole round has been applying to `installed()`, `find_windows_sdk()` and the index CI. Present is not usable. Also here, from the architecture review: - doctor.cppm used the pre-#436 `toolchain_frontend(root/"bin")` shape, so an installed msvc toolset was visible to `toolchain list` and invisible to `doctor`. Same layout rule, fourth copy. - `msvc_available_here()` joins `has_usable_msvc()`: the latter probes the MACHINE, which is the wrong question on a box with a pinned toolset and no Visual Studio. The three prepare.cppm decisions that gate first-run diversion, offline guidance and the ABI repair now ask the origin-aware one and stop diverting a perfectly good toolchain to mingw. - `vc_redist_dir()` finds the toolset's own vcruntime140.dll/msvcp140.dll and puts it in `linkRuntimeDirs`, which is how `mcpp run` reaches it on PATH. Those DLLs are NOT OS components, so the DEFAULT (/MD) build could link on a machine with only a managed toolset and then fail to start. Five tests: found from the compiler path alone, the redist version is not the tools version (14.44.35112 vs 14.44.35207 -- deriving it finds nothing), newest wins, debug_nonredist is never returned because it may not be redistributed, and a toolset without one is not an error.
Sunrisepeak
added a commit
that referenced
this pull request
Aug 16, 2026
…e removed, and could not fail (#440) * fix(toolchain): the msvc payload's location is known, not inferred error: msvc payload installed at 'C:\Users\...\xpkgs\xim-x-msvc\14.44.35207\VC', but no cl.exe under VC/Tools/MSVC/14.44.35207 Note the `\VC` on the end. `XpkgPayload::root` treats the version directory as the root only when it directly contains bin/ include/ lib/, and otherwise descends into a lone subdirectory (package_fetcher.cppm:1027). An installed msvc payload has exactly one entry -- `VC/` -- so the root came back one level too deep and a perfectly good toolset read as missing. That heuristic is right for the payloads it was written for; it is simply not an answer to "where is this package". The answer is (store, name, version), and all three are known at both call sites: `xim_tool(env, name, version)` gives the version directory outright. `resolve_xpkg_path` still does the installing -- it just stops being asked where. Found on the first e2e run where the install actually succeeded. Every earlier attempt died in the recipe, so this was standing behind three other defects the whole time. The test pins both directions: the version directory resolves, and the `VC` subdirectory does NOT -- an implementation that searched upward from whatever it was handed would pass the first assertion and fail the second. * fix(toolchain): an SDK is headers AND libraries, not headers find_windows_sdk() accepted a root on `Include\<v>\ucrt\corecrt.h` alone. A managed windows-sdk payload missing the MSI that carries kernel32.lib satisfied that, ranked ABOVE the machine's own complete SDK because its version was higher, compiled every translation unit, and then: LINK : fatal error LNK1104: cannot open file 'kernel32.lib' with not one line in the build log mentioning the SDK. This is e2e 239's failure on the managed toolset, and the same shape as the defect has_usable_msvc() was written to prevent -- selecting on a weaker signal than the build actually needs -- so the fix is the one that predicate already models: require both halves. A root now needs `Include\<v>\ucrt\corecrt.h` AND `Lib\<v>\um\<arch>\kernel32.lib`. Partial roots are skipped, so the search falls through to the next one instead of poisoning the build. Three tests, each run against the old gate: headers-only is rejected, a complete root is still accepted (so the check cannot pass by rejecting everything), and a partial root loses to a complete one even when its version is higher -- which is exactly how the payload outranked the system SDK. The first and third fail without the fix. FakeToolset::add_sdk() now builds a complete SDK; add_sdk_headers_only() builds the half-installed one. * fix(toolchain): removing a payload could not survive Windows `toolchain remove` called remove_all and reported whatever it got. On Windows that is "Access is denied", from either of two causes that look identical: - payload files come out of .vsix/.msi carrying the read-only attribute. POSIX only needs the DIRECTORY writable to unlink a child, so this never appears on Linux or macOS -- and every unit test runs there. - a /Zi build leaves mspdbsrv.exe alive for a few seconds INSIDE the payload it is being asked to delete. Handles both rather than betting on one: clear the write bit across the tree and retry, then allow a bounded window (10 x 300ms) for a live process to exit. Bounded because `toolchain remove` must not hang on a directory something holds forever. The error now names the file it is stuck on. "Access is denied" without a path is not something a user can act on. No unit test: on the only platform CI would run one, removing a read-only file succeeds with or without this change, so the test could not fail. e2e 239 on the Windows runner is the gate -- it is where this surfaced, after the SDK fix let the build get far enough to reach the remove step. * fix(toolchain): a failed remove is not a no-op, and must not deepen the damage Two corrections to the removal path, both found by using it. The diagnostic worked -- it named `bin\Hostx64\x64\Microsoft.VisualStudio.Telemetry.dll`, which is neither a read-only file nor mspdbsrv but `vctip.exe`, the background telemetry uploader cl.exe spawns, holding a DLL inside the payload it is being asked to delete. Nothing on this side can delete a file another process holds open, so the real fix is openxlings/xim-pkgindex#637 (stop installing vctip.exe at all). What stays here is the generic fallback and an error someone can act on. 1. The probe no longer deletes. It was finding the stuck file by trying to remove each entry, which makes a diagnostic into a second act of damage. `remove_all` has already deleted everything it could, so the first SURVIVING file is the one that blocked it -- no destruction required. 2. The error now says the payload is INCOMPLETE. `remove_all` deletes what it can before stopping, so a failed remove leaves a toolchain with holes. "remove failed" alone reads as "nothing happened", and the next thing that someone meets is a build error. * test(e2e): 239 could pass or skip, never fail The skip was 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. It hid a real one, on the first Windows run where the package was actually installable: tar: Cannot connect to C: resolve failed tar -xf "C:\Users\...\.payloads\Microsoft.VC...vsix" -C "..." [error] msvc installed but registered none of the programs it declares GNU tar reads `C:` as a hostname. The install ran for 135 seconds, failed, and this script printed PASS. Skip is now decided BEFORE the work, by a positive check for what would make the test impossible (no msvc row in `toolchain list`). Everything after that is a failure, and the output is printed rather than folded into a one-line message. The recipe-side fix is openxlings/xim-pkgindex#632. A skip decided by the shape of a failure is not a skip; it is a way of not looking. * test(e2e): 239 asks for the toolset it needs, and checks the SDK arrived Two follow-ups to making 239 unable to skip its way to green. The skip grepped `toolchain list` for "msvc" -- the FAMILY. During an index publish window the family is listed while this toolset is not yet, so a hard failure would report a timing artifact as a defect. It now greps for $TOOLSET, which is the question the skip actually needs answered. And it now asserts the install reported a Windows SDK. `install` prints `windows sdk: <version> (<root>)`, but this script discarded the output on success -- so a half-installed SDK dependency said nothing here and turned up 100 lines later as LINK : fatal error LNK1104: cannot open file 'kernel32.lib' with nothing in the log naming the SDK. That is exactly how this defect presented today. Asserting it at the step that knows turns "the link failed for an unstated reason" into "the SDK dependency did not install". * fix(toolchain): a payload nothing can delete can still be moved With vctip.exe gone the holder became mspdbcore.dll -- that is mspdbsrv.exe, the PDB server a /Zi build spawns, which outlives cl.exe by tens of seconds and lives INSIDE the payload being removed. Removing a toolset right after building with it is the normal case, not a corner one. Waiting it out is not a fix: any timeout is a guess, and a CLI that hangs on one is worse than the failure. So when the retries are exhausted the payload is RENAMED aside instead. Windows refuses to delete a directory containing an open file and permits renaming one -- the handle keeps working and follows. What `toolchain remove` promises is that the toolchain stops being installed, and after the rename it has. The bytes are swept by the next lifecycle command, by which time nothing holds them; `sweep_parked_payloads` runs before both install and remove. Three tests, all run against the missing fallback. A tree with an unenumerable subdirectory is the portable way to make remove_all fail -- the CAUSE differs per platform (open handle on Windows, permissions here), the contract does not. The third asserts an ordinary payload is still just deleted: a fallback that fires always is not a fallback, and it is the only one of the three that passes without this change. * ci: the xlings-managed msvc toolset gets its own workflow `msvc@system` and `msvc@<toolset>` are different subjects and were sharing a job. Everything MSVC in the main suite (95, 99, 177, 180, 182) tests mcpp against the machine's own Visual Studio -- mcpp's code and nothing else's. 239 tests mcpp against the xlings ECOSYSTEM: index, mirror, payload set, unpack recipe, most of it in another repository moving on its own schedule. Today that difference cost several cycles of reading "your change broke Windows" when what had actually happened was that a package index needed a fix. Three consequences, all of them reasons to split: - a red tick means different things, and mixed together it means neither; - ~380 MB of downloads next to 100+ tests that take seconds each; - the index publish window makes this job flaky in a way the rest is not. Split by CAPABILITY, not by a file list: tests declare `# requires: xlings-msvc`, granted only by MCPP_E2E_XLINGS_MSVC=1, which only ci-windows-msvc-xlings.yml sets. The main suite therefore skips them by construction, and a new test joins the new job by declaring the capability -- there is no second list to drift. Adds E2E_ONLY to run_all.sh so a single-subject workflow can name what it runs. A filter that stops matching would otherwise produce a green tick for running nothing, which looks identical to passing, so the job asserts the glob still selects something. * docs: architecture review of toolchain origins, selection and distribution Written from the round that actually got `msvc@<toolset>` working: nine layers of defect, five index publish windows, three tests that could not fail. Every finding is anchored to a file:line, not a preference. The two-axis model (acquisition vs resolution) is right and nothing here proposes replacing it. The findings are all one sentence: that axis exists only for MSVC, and it is only carried half way. Ten findings, including three that are live defects rather than design debt: - doctor.cppm:398 still uses the pre-#436 `toolchain_frontend(root/"bin")` shape, so an installed msvc toolset is visible to `toolchain list` and invisible to `doctor`. One line. - a /MD build with a managed toolset links vcruntime140.dll, which is not an OS component and appears nowhere in src/ -- on a clean Windows box `mcpp build` succeeds and `mcpp run` cannot start. CI hides it by having Visual Studio installed. - `has_usable_msvc()` probes the machine but gates three decisions that also apply to managed toolsets, so a box with a pinned toolset and no VS answers "no MSVC here". And the largest structural gap: `mcpp pack` is ELF-only, and distribution.cppm's contract never reaches pack.cppm at all, so `cxx_runtime` has no enforcer at packaging time on either platform. * fix(toolchain): move the held FILES aside, not the directory that holds them The park fallback was built on a wrong premise and CI said so. 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 containing one. Renaming the payload root failed with the same "Access is denied", so remove still reported failure and `mspdbcore.dll` was still named as the blocker. What works is the other way round: move every surviving file to a sibling `.trash-*` directory (allowed while open, same volume, the handle follows), then delete the payload tree, which by then holds only directories. If any file cannot be moved, the scratch directory is removed and the whole thing reports failure -- a half-moved payload is worse than one still fully there. Tests: the two that depended on a POSIX "unremovable" fixture are gone, and the reason is worth recording. The only POSIX way to make a file undeletable is to drop write on its parent, and this function's second pass ADDS write back across the tree on purpose -- that is the read-only-payload case it exists to fix. The fixture becomes removable the moment the code under test touches it. That is the function working, not a hole, and a test that cannot fail is the thing this whole round has been removing. e2e 239 on the Windows runner is the gate. What stays is what holds everywhere: the ordinary deletion takes the ordinary route, and the sweep eats `.trash-*` and not the installed toolset beside it. Also corrects the architecture review: its first draft proposed generalising `@system` to gcc/llvm, which is backwards. xlings' principle is to depend on the host as little as possible, and `msvc@system` is a Windows-specific exception rather than an instance of a general capability. The finding stands (one exception, 26 branches); the remedy is to CONTAIN it behind a single resolved Origin -- and to reject `gcc@system` explicitly rather than leave it merely unimplemented. * fix(toolchain): removed means no files left, not an empty tree deleted With the files moved aside, removal still failed -- this time on the DIRECTORY skeleton, with a sharing violation rather than access denied. That is Windows refusing to delete a directory some process has as its current directory, and mspdbsrv.exe is launched inside the payload, so it is the ordinary tail of a /Zi build with the toolset being removed. Every file was already gone at that point. A toolchain with no files in it is not installed, which is exactly what `remove` promises, so reporting failure was reporting the opposite of what happened. The criterion is now "no regular file remains"; the skeleton is swept by the next lifecycle command. That exposed the matching lie on the other side: `toolchain default` accepted any directory that EXISTS, so a skeleton would have been called an installed toolchain and handed to a build. It now asks `payload_frontend` for a resolvable compiler -- the same rule this whole round has been applying to `installed()`, `find_windows_sdk()` and the index CI. Present is not usable. Also here, from the architecture review: - doctor.cppm used the pre-#436 `toolchain_frontend(root/"bin")` shape, so an installed msvc toolset was visible to `toolchain list` and invisible to `doctor`. Same layout rule, fourth copy. - `msvc_available_here()` joins `has_usable_msvc()`: the latter probes the MACHINE, which is the wrong question on a box with a pinned toolset and no Visual Studio. The three prepare.cppm decisions that gate first-run diversion, offline guidance and the ABI repair now ask the origin-aware one and stop diverting a perfectly good toolchain to mingw. - `vc_redist_dir()` finds the toolset's own vcruntime140.dll/msvcp140.dll and puts it in `linkRuntimeDirs`, which is how `mcpp run` reaches it on PATH. Those DLLs are NOT OS components, so the DEFAULT (/MD) build could link on a machine with only a managed toolset and then fail to start. Five tests: found from the compiler path alone, the redist version is not the tools version (14.44.35112 vs 14.44.35207 -- deriving it finds nothing), newest wins, debug_nonredist is never returned because it may not be redistributed, and a toolset without one is not an error. --------- Co-authored-by: speak-agent <248744407+speak-agent@users.noreply.github.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
2 participants
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.
Summary
The enumeration asked
toolchain_frontend(root / "bin", pkg), got nothing, andcontinued.cl.exeis four levels deeper — underVC/Tools/MSVC/<ver>/bin/Host<h>/<arch>/— so every msvc payload installed correctly and was then invisible.The shape of it
Three places have to know that layout. Install and build knew; the listing did not — which is what a third inline copy of a rule tends to produce. They now share
payload_frontend(payloadRoot, pkg, family), delegating tomsvc::installation_at()for msvc and the bin/-shaped lookup for everything else, so the three cannot disagree about where a payload keeps its compiler.prepare.cppm's default-toolchain path also resolves throughpayload->binDirand is deliberately left alone: the Windows first-run pin isllvm@20.1.7, so that site is unreachable for msvc.How it was found
By running the released 2026.8.16.1 binary against a payload-shaped fixture — not by a test. The unit test in #434 pinned
identify_xim_payload("msvc"), which was already correct: the identity mapping and the enumeration are different questions, and only one of them was being asked.tests/e2e/239_msvc_managed_toolset.shstep 1b would also have caught it on the next Windows e2e run, now thatxim:msvcis published — which is the other half of the lesson: the e2e was written at the right unit, it just had not run yet.Test plan
mcpp test test_toolchain_msvc— 26/26toolchain list; removed afterwardspayload_frontendfinds it, and theroot/binquestion still answers nothing (which is what makes it the wrong question rather than a broken implementation)