Skip to content

fix(toolchain): an installed msvc toolset did not appear in toolchain list - #436

Merged
Sunrisepeak merged 1 commit into
mainfrom
fix/msvc-payload-not-listed
Aug 16, 2026
Merged

fix(toolchain): an installed msvc toolset did not appear in toolchain list#436
Sunrisepeak merged 1 commit into
mainfrom
fix/msvc-payload-not-listed

Conversation

@Sunrisepeak

Copy link
Copy Markdown
Member

Summary

The enumeration asked toolchain_frontend(root / "bin", pkg), got nothing, and continued. 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.

$ mcpp toolchain list        # before
Toolchains:
  *  gcc 16.1.0              (default)
     gcc 15.1.0
     llvm 22.1.8
     llvm 20.1.7             <- the installed msvc 14.44.35207 is simply absent

$ mcpp toolchain list        # after
     msvc 14.44.35207

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 to msvc::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 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.

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.sh step 1b would also have caught it on the next Windows e2e run, now that xim:msvc is 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/26
  • Manual: a payload-shaped fixture under the store now shows up in toolchain list; removed afterwards
  • New 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)

…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
Sunrisepeak merged commit dc6eb34 into main Aug 16, 2026
18 checks passed
@Sunrisepeak
Sunrisepeak deleted the fix/msvc-payload-not-listed branch August 16, 2026 07:29
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>
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants