Skip to content

Commit bd9e2e9

Browse files
committed
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.
1 parent 987b783 commit bd9e2e9

3 files changed

Lines changed: 45 additions & 4 deletions

File tree

src/build/prepare.cppm

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1320,12 +1320,19 @@ prepare_build(bool print_fingerprint,
13201320
// (cl.exe is four levels deeper) and the ELF post-install fixup
13211321
// (there is nothing to patchelf on a PE toolchain).
13221322
if (spec->family == mcpp::toolchain::Family::Msvc) {
1323+
// Not `payload->root`: that field is the fetcher's guess at where
1324+
// the useful tree starts, and it descends into a lone
1325+
// subdirectory when the version dir has no bin/ include/ lib/.
1326+
// An msvc payload's only entry is `VC/`, so the guess lands one
1327+
// level too deep. (store, name, version) is known — use it.
1328+
auto verDir = mcpp::xlings::paths::xim_tool(
1329+
mcpp::config::make_xlings_env(**cfg), pkg.ximName, pkg.ximVersion);
13231330
auto inst = mcpp::toolchain::msvc::installation_at(
1324-
payload->root, pkg.ximVersion);
1331+
verDir, pkg.ximVersion);
13251332
if (!inst) {
13261333
return std::unexpected(std::format(
13271334
"msvc payload at '{}' has no cl.exe under VC/Tools/MSVC/{}",
1328-
payload->root.string(), pkg.ximVersion));
1335+
verDir.string(), pkg.ximVersion));
13291336
}
13301337
explicit_compiler = inst->clPath;
13311338
mcpp::ui::info("Resolved", std::format(

src/toolchain/lifecycle.cppm

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -618,14 +618,26 @@ export int toolchain_install(const mcpp::config::GlobalConfig& cfg,
618618
// VC/Tools/MSVC/<ver>/bin/Hostx64/x64, and there is nothing to
619619
// patchelf on a PE toolchain.
620620
if (spec->family == mcpp::toolchain::Family::Msvc) {
621+
// NOT `payload->root` — that is a GUESS, and it guesses wrong
622+
// here. resolve_xpkg_path calls the version directory the root
623+
// only when it directly contains bin/ include/ lib/; otherwise it
624+
// descends into a lone subdirectory. An installed msvc payload
625+
// has exactly one entry, `VC/`, so the "root" comes back as
626+
// …/14.44.35207/VC and the toolset then looks like it is missing.
627+
//
628+
// The location is not something to infer: it is (store, name,
629+
// version), and all three are known here. resolve_xpkg_path above
630+
// is what INSTALLS; this is what says where.
631+
auto verDir = mcpp::xlings::paths::xim_tool(
632+
mcpp::config::make_xlings_env(cfg), pkg.ximName, pkg.ximVersion);
621633
auto inst = mcpp::toolchain::msvc::installation_at(
622-
payload->root, pkg.ximVersion);
634+
verDir, pkg.ximVersion);
623635
if (!inst) {
624636
mcpp::ui::error(std::format(
625637
"msvc payload installed at '{}', but no cl.exe under "
626638
"VC/Tools/MSVC/{} — the payload is not what this version "
627639
"claims to be",
628-
payload->root.string(), pkg.ximVersion));
640+
verDir.string(), pkg.ximVersion));
629641
return 1;
630642
}
631643
msvc_print_detected(*inst, "Installed");

tests/unit/test_toolchain_msvc.cpp

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,6 +234,28 @@ TEST(MsvcManaged, ResolvesTheDeclaredToolsetAndNotItsNeighbour) {
234234
EXPECT_EQ(newer->toolsVersion, "14.52.36629");
235235
}
236236

237+
TEST(MsvcManaged, TheVersionDirIsTheRootNotItsLoneSubdirectory) {
238+
// A real installed payload has exactly ONE entry: `VC/`. The fetcher's
239+
// `XpkgPayload::root` treats the version directory as the root only when
240+
// it directly contains bin/ include/ lib/, and otherwise descends into a
241+
// lone subdirectory -- so for msvc it hands back `<ver>/VC`, and the
242+
// toolset then looks missing on a payload that installed perfectly.
243+
//
244+
// Pinning both sides here: the version directory resolves, and the `VC`
245+
// subdirectory does NOT. The second half is what makes this a test rather
246+
// than a restatement -- an implementation that searched upward from
247+
// whatever it was given would pass the first and fail this.
248+
FakeToolset t{"verdir"};
249+
t.add_toolset("14.44.35207");
250+
251+
auto ok = msvc::installation_at(t.root, "14.44.35207");
252+
ASSERT_TRUE(ok.has_value()) << "the version directory must be the root";
253+
254+
EXPECT_FALSE(msvc::installation_at(t.root / "VC", "14.44.35207").has_value())
255+
<< "a caller handing in the VC subdir is passing the wrong thing, and "
256+
"must be told so rather than quietly rescued";
257+
}
258+
237259
TEST(MsvcManaged, AbsentToolsetIsNulloptNotASubstitute) {
238260
FakeToolset t{"absent"};
239261
t.add_toolset("14.44.35207");

0 commit comments

Comments
 (0)