Skip to content

Commit 55ea380

Browse files
authored
fix: payload discovery must skip delegating-package husks (#120) (#121)
* fix: payload discovery must skip delegating-package husks (#120) A delegating index package (xim:linux-headers -> scode:linux-headers) leaves a metadata-only husk (.xim-installed + .xpkg.lua) under its own prefix; find_sibling_package counted .xpkg.lua as content, returned the husk, and probe_payload_paths silently dropped the kernel-header include path -- every glibc toolchain build then died at <linux/limits.h> (e2e 29/31 on a cold sandbox). - payload qualification: dot-prefixed entries are metadata, not content; optional requiredRelPath must exist in the version dir - find_home_tool: search across all index prefixes (was hardcoded xim-x-) with the same qualification rules - probe: require include/linux/limits.h (linux-headers) and include/features.h (glibc fallback); log when no payload qualifies - CI cache hygiene: give ci-linux/ci-windows sandbox caches a '-ci-' lineage disjoint from release's '-release-' caches; a bare restore prefix used to swap in the release-flavored sandbox, which is what exposed this cold-path bug on main * test: use portable env setter (Windows has no setenv/unsetenv)
1 parent 9c95d01 commit 55ea380

6 files changed

Lines changed: 193 additions & 61 deletions

File tree

.github/workflows/ci-linux.yml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,13 @@ jobs:
4040
uses: actions/cache@v4
4141
with:
4242
path: ~/.mcpp
43-
key: mcpp-sandbox-${{ runner.os }}-${{ hashFiles('mcpp.toml', '.xlings.json') }}
43+
# NOTE: the "-ci-" segment keeps this lineage disjoint from
44+
# release.yml's "-release-" caches. A bare "mcpp-sandbox-<os>-"
45+
# restore prefix used to match the release sandbox too, silently
46+
# swapping in a differently-populated registry (issue #120).
47+
key: mcpp-sandbox-${{ runner.os }}-ci-${{ hashFiles('mcpp.toml', '.xlings.json') }}
4448
restore-keys: |
45-
mcpp-sandbox-${{ runner.os }}-
49+
mcpp-sandbox-${{ runner.os }}-ci-
4650
4751
# Cache xlings + its locally installed packages (xim:mcpp etc.).
4852
# Saves the xlings bootstrap roundtrip + the mcpp xpkg download

.github/workflows/ci-windows.yml

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,10 @@ jobs:
2828
uses: actions/cache@v4
2929
with:
3030
path: ~\.mcpp
31-
key: mcpp-sandbox-${{ runner.os }}-${{ hashFiles('mcpp.toml', '.xlings.json') }}
31+
# Disjoint from release.yml's "-release-" cache lineage (issue #120).
32+
key: mcpp-sandbox-${{ runner.os }}-ci-${{ hashFiles('mcpp.toml', '.xlings.json') }}
3233
restore-keys: |
33-
mcpp-sandbox-${{ runner.os }}-
34+
mcpp-sandbox-${{ runner.os }}-ci-
3435
3536
- name: Cache xlings
3637
uses: actions/cache@v4

.github/workflows/release.yml

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,6 @@ jobs:
8080
key: mcpp-sandbox-${{ runner.os }}-release-${{ hashFiles('mcpp.toml', '.xlings.json') }}
8181
restore-keys: |
8282
mcpp-sandbox-${{ runner.os }}-release-
83-
mcpp-sandbox-${{ runner.os }}-
8483
8584
# Cache xlings + xim:mcpp install.
8685
- name: Cache xlings
@@ -466,7 +465,6 @@ jobs:
466465
key: mcpp-sandbox-${{ runner.os }}-release-${{ hashFiles('mcpp.toml', '.xlings.json') }}
467466
restore-keys: |
468467
mcpp-sandbox-${{ runner.os }}-release-
469-
mcpp-sandbox-${{ runner.os }}-
470468
471469
- name: Cache xlings
472470
uses: actions/cache@v4

src/toolchain/probe.cppm

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,7 @@ probe_payload_paths(const std::filesystem::path& compilerBin) {
325325
// its owner home, while the active home may own (or have just installed)
326326
// the sysroot payloads.
327327
auto glibc = paths::find_sibling_tool(compilerBin, "glibc");
328-
if (!glibc) glibc = paths::find_home_tool("glibc");
328+
if (!glibc) glibc = paths::find_home_tool("glibc", "include/features.h");
329329
if (!glibc) return std::nullopt;
330330

331331
// Glibc layout: <root>/include/ + <root>/lib64/ (or lib/).
@@ -344,13 +344,22 @@ probe_payload_paths(const std::filesystem::path& compilerBin) {
344344
pp.glibcLib = glibcLib;
345345

346346
// Find linux kernel headers (optional — search across index prefixes,
347-
// then the active home registry).
348-
auto linuxHeaders = paths::find_sibling_package(compilerBin, "linux-headers");
349-
if (!linuxHeaders) linuxHeaders = paths::find_home_tool("linux-headers");
347+
// then the active home registry). Require the actual payload: a
348+
// delegating index package (xim:linux-headers → scode:linux-headers)
349+
// leaves a metadata-only husk under its own prefix, and the discovery
350+
// must skip it instead of giving up (issue #120: glibc's local_lim.h
351+
// needs <linux/limits.h>, so a silent miss breaks every glibc build).
352+
constexpr std::string_view kLinuxLimits = "include/linux/limits.h";
353+
auto linuxHeaders =
354+
paths::find_sibling_package(compilerBin, "linux-headers", kLinuxLimits);
355+
if (!linuxHeaders)
356+
linuxHeaders = paths::find_home_tool("linux-headers", kLinuxLimits);
350357
if (linuxHeaders) {
351-
auto linuxInclude = *linuxHeaders / "include";
352-
if (std::filesystem::exists(linuxInclude / "linux" / "limits.h"))
353-
pp.linuxInclude = linuxInclude;
358+
pp.linuxInclude = *linuxHeaders / "include";
359+
} else {
360+
mcpp::log::verbose("probe",
361+
"linux-headers payload not found under any index prefix — "
362+
"glibc builds will fail at <linux/limits.h>");
354363
}
355364

356365
mcpp::log::verbose("probe", std::format(

src/xlings.cppm

Lines changed: 65 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -78,9 +78,14 @@ namespace paths {
7878
// Find a sibling package across all index prefixes.
7979
// e.g. find_sibling_package(gcc_bin, "linux-headers") searches for
8080
// xim-x-linux-headers, scode-x-linux-headers, etc.
81+
// Metadata-only dirs (.xim-installed/.xpkg.lua husks left by delegating
82+
// index packages) never qualify; when requiredRelPath is given, only a
83+
// version dir containing it qualifies (the payload may live under a
84+
// different prefix than the husk — issue #120).
8185
std::optional<std::filesystem::path>
8286
find_sibling_package(const std::filesystem::path& compilerBin,
83-
std::string_view packageName);
87+
std::string_view packageName,
88+
std::string_view requiredRelPath = {});
8489

8590
// xpkgs root of the ACTIVE mcpp home ($MCPP_HOME or ~/.mcpp). Payload
8691
// discovery consults this in addition to compiler siblings: an
@@ -89,7 +94,11 @@ namespace paths {
8994
std::optional<std::filesystem::path> active_home_xpkgs();
9095

9196
// Like find_sibling_tool, but anchored at the active home's xpkgs.
92-
std::optional<std::filesystem::path> find_home_tool(std::string_view tool);
97+
// Searches across index prefixes (xim-x-, scode-x-, …) with the same
98+
// husk/requiredRelPath rules as find_sibling_package.
99+
std::optional<std::filesystem::path>
100+
find_home_tool(std::string_view tool,
101+
std::string_view requiredRelPath = {});
93102

94103
// index data root: env.home / "data"
95104
std::filesystem::path index_data(const Env& env);
@@ -543,20 +552,60 @@ std::optional<std::filesystem::path> active_home_xpkgs() {
543552
return xpkgs;
544553
}
545554

546-
std::optional<std::filesystem::path> find_home_tool(std::string_view tool) {
547-
auto xpkgs = active_home_xpkgs();
548-
if (!xpkgs) return std::nullopt;
555+
namespace {
549556

550-
auto root = *xpkgs / std::format("xim-x-{}", tool);
557+
// A version dir qualifies as a payload only if it has real content —
558+
// dot-prefixed entries (.xim-installed, .xpkg.lua) are install metadata,
559+
// and a dir holding nothing else is the husk a delegating index package
560+
// leaves behind (the payload lives under another prefix; issue #120).
561+
// When requiredRelPath is given, the dir must also contain that path.
562+
bool payload_dir_qualifies(const std::filesystem::path& versionDir,
563+
std::string_view requiredRelPath) {
551564
std::error_code ec;
552-
if (!std::filesystem::exists(root, ec)) return std::nullopt;
565+
bool hasContent = false;
566+
for (auto& f : std::filesystem::directory_iterator(versionDir, ec)) {
567+
if (!f.path().filename().string().starts_with(".")) {
568+
hasContent = true;
569+
break;
570+
}
571+
}
572+
if (!hasContent) return false;
573+
if (!requiredRelPath.empty()
574+
&& !std::filesystem::exists(versionDir / requiredRelPath, ec))
575+
return false;
576+
return true;
577+
}
553578

554-
for (auto& v : std::filesystem::directory_iterator(root, ec)) {
555-
if (v.is_directory(ec)) return v.path();
579+
// Scan an xpkgs root across index prefixes (xim-x-, scode-x-, compat-x-, …)
580+
// for the first qualifying version dir of `packageName`.
581+
std::optional<std::filesystem::path>
582+
find_package_in_xpkgs(const std::filesystem::path& xpkgs,
583+
std::string_view packageName,
584+
std::string_view requiredRelPath) {
585+
std::error_code ec;
586+
std::string suffix = std::format("-x-{}", packageName);
587+
for (auto& entry : std::filesystem::directory_iterator(xpkgs, ec)) {
588+
if (!entry.is_directory(ec)) continue;
589+
auto name = entry.path().filename().string();
590+
if (!name.ends_with(suffix)) continue;
591+
for (auto& v : std::filesystem::directory_iterator(entry.path(), ec)) {
592+
if (!v.is_directory(ec)) continue;
593+
if (payload_dir_qualifies(v.path(), requiredRelPath))
594+
return v.path();
595+
}
556596
}
557597
return std::nullopt;
558598
}
559599

600+
} // namespace
601+
602+
std::optional<std::filesystem::path>
603+
find_home_tool(std::string_view tool, std::string_view requiredRelPath) {
604+
auto xpkgs = active_home_xpkgs();
605+
if (!xpkgs) return std::nullopt;
606+
return find_package_in_xpkgs(*xpkgs, tool, requiredRelPath);
607+
}
608+
560609
std::optional<std::filesystem::path>
561610
find_sibling_binary(const std::filesystem::path& compilerBin,
562611
std::string_view tool,
@@ -578,54 +627,22 @@ find_sibling_binary(const std::filesystem::path& compilerBin,
578627

579628
std::optional<std::filesystem::path>
580629
find_sibling_package(const std::filesystem::path& compilerBin,
581-
std::string_view packageName) {
630+
std::string_view packageName,
631+
std::string_view requiredRelPath) {
582632
auto xpkgs = xpkgs_from_compiler(compilerBin);
583633
if (!xpkgs) return std::nullopt;
584634

585635
// Search across index prefixes: xim-x-, scode-x-, compat-x-, etc.
586-
std::error_code ec;
587-
std::string suffix = std::format("-x-{}", packageName);
588-
for (auto& entry : std::filesystem::directory_iterator(*xpkgs, ec)) {
589-
if (!entry.is_directory(ec)) continue;
590-
auto name = entry.path().filename().string();
591-
if (!name.ends_with(suffix)) continue;
592-
// Return the first (highest) version dir that has actual content.
593-
for (auto& v : std::filesystem::directory_iterator(entry.path(), ec)) {
594-
if (!v.is_directory(ec)) continue;
595-
// Skip empty packages (only .xim-installed marker)
596-
bool hasContent = false;
597-
for (auto& f : std::filesystem::directory_iterator(v.path(), ec)) {
598-
if (f.path().filename() != ".xim-installed") {
599-
hasContent = true;
600-
break;
601-
}
602-
}
603-
if (hasContent) return v.path();
604-
}
605-
}
636+
if (auto found = find_package_in_xpkgs(*xpkgs, packageName, requiredRelPath))
637+
return found;
606638

607639
// Also check ~/.xlings/data/xpkgs/ (xlings global home) as fallback.
640+
std::error_code ec;
608641
const char* home = std::getenv("HOME");
609642
if (home) {
610643
auto xlingsXpkgs = std::filesystem::path(home) / ".xlings" / "data" / "xpkgs";
611-
if (xlingsXpkgs != *xpkgs && std::filesystem::exists(xlingsXpkgs, ec)) {
612-
for (auto& entry : std::filesystem::directory_iterator(xlingsXpkgs, ec)) {
613-
if (!entry.is_directory(ec)) continue;
614-
auto name = entry.path().filename().string();
615-
if (!name.ends_with(suffix)) continue;
616-
for (auto& v : std::filesystem::directory_iterator(entry.path(), ec)) {
617-
if (!v.is_directory(ec)) continue;
618-
bool hasContent = false;
619-
for (auto& f : std::filesystem::directory_iterator(v.path(), ec)) {
620-
if (f.path().filename() != ".xim-installed") {
621-
hasContent = true;
622-
break;
623-
}
624-
}
625-
if (hasContent) return v.path();
626-
}
627-
}
628-
}
644+
if (xlingsXpkgs != *xpkgs && std::filesystem::exists(xlingsXpkgs, ec))
645+
return find_package_in_xpkgs(xlingsXpkgs, packageName, requiredRelPath);
629646
}
630647

631648
return std::nullopt;

tests/unit/test_xlings.cpp

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import std;
44
import mcpp.xlings;
5+
import mcpp.platform.env;
56

67
namespace {
78

@@ -143,3 +144,105 @@ TEST(XlingsIndexFreshness, AcceptsOfficialPackageCacheWithCurrentPath) {
143144

144145
std::filesystem::remove_all(home);
145146
}
147+
148+
// ─── Sibling/home payload discovery (issue #120) ─────────────────────
149+
//
150+
// A delegating index package (e.g. xim:linux-headers forwarding to
151+
// scode:linux-headers) leaves a metadata-only husk dir under its own
152+
// prefix (.xim-installed + .xpkg.lua, no payload). Discovery must not
153+
// stop at the husk: the real payload lives under another prefix.
154+
155+
namespace {
156+
157+
void touch(const std::filesystem::path& p, std::string_view content = "x") {
158+
std::filesystem::create_directories(p.parent_path());
159+
std::ofstream(p) << content;
160+
}
161+
162+
} // namespace
163+
164+
TEST(XlingsSiblingPackage, MetadataOnlyHuskIsNotContent) {
165+
auto tmp = make_tempdir("mcpp-husk");
166+
auto xpkgs = tmp / "xpkgs";
167+
auto gccBin = xpkgs / "xim-x-gcc" / "16.1.0" / "bin" / "g++";
168+
touch(gccBin);
169+
170+
// Only a husk exists: .xim-installed + .xpkg.lua, no payload.
171+
auto husk = xpkgs / "xim-x-linux-headers" / "5.11.1";
172+
touch(husk / ".xim-installed");
173+
touch(husk / ".xpkg.lua", "package = {}");
174+
175+
// Isolate from the host's ~/.xlings fallback.
176+
const char* oldHome = std::getenv("HOME");
177+
mcpp::platform::env::set("HOME", tmp.string());
178+
auto found = mcpp::xlings::paths::find_sibling_package(gccBin, "linux-headers");
179+
mcpp::platform::env::set("HOME", oldHome ? oldHome : "");
180+
181+
EXPECT_FALSE(found.has_value());
182+
183+
std::filesystem::remove_all(tmp);
184+
}
185+
186+
TEST(XlingsSiblingPackage, SkipsHuskAndFindsPayloadUnderOtherPrefix) {
187+
auto tmp = make_tempdir("mcpp-husk");
188+
auto xpkgs = tmp / "xpkgs";
189+
auto gccBin = xpkgs / "xim-x-gcc" / "16.1.0" / "bin" / "g++";
190+
touch(gccBin);
191+
192+
auto husk = xpkgs / "xim-x-linux-headers" / "5.11.1";
193+
touch(husk / ".xim-installed");
194+
touch(husk / ".xpkg.lua", "package = {}");
195+
196+
auto real = xpkgs / "scode-x-linux-headers" / "5.11.1";
197+
touch(real / "include" / "linux" / "limits.h");
198+
199+
auto found = mcpp::xlings::paths::find_sibling_package(gccBin, "linux-headers");
200+
ASSERT_TRUE(found.has_value());
201+
EXPECT_EQ(*found, real);
202+
203+
std::filesystem::remove_all(tmp);
204+
}
205+
206+
TEST(XlingsSiblingPackage, RequiredRelPathRejectsContentfulButWrongCandidate) {
207+
auto tmp = make_tempdir("mcpp-husk");
208+
auto xpkgs = tmp / "xpkgs";
209+
auto gccBin = xpkgs / "xim-x-gcc" / "16.1.0" / "bin" / "g++";
210+
touch(gccBin);
211+
212+
// Contentful but missing the payload that matters.
213+
auto stray = xpkgs / "xim-x-linux-headers" / "5.11.1";
214+
touch(stray / "README.md");
215+
216+
auto real = xpkgs / "scode-x-linux-headers" / "5.11.1";
217+
touch(real / "include" / "linux" / "limits.h");
218+
219+
auto found = mcpp::xlings::paths::find_sibling_package(
220+
gccBin, "linux-headers", "include/linux/limits.h");
221+
ASSERT_TRUE(found.has_value());
222+
EXPECT_EQ(*found, real);
223+
224+
std::filesystem::remove_all(tmp);
225+
}
226+
227+
TEST(XlingsHomeTool, FindsPayloadUnderNonXimPrefix) {
228+
auto tmp = make_tempdir("mcpp-husk-home");
229+
auto xpkgs = tmp / "registry" / "data" / "xpkgs";
230+
231+
auto husk = xpkgs / "xim-x-linux-headers" / "5.11.1";
232+
touch(husk / ".xim-installed");
233+
touch(husk / ".xpkg.lua", "package = {}");
234+
235+
auto real = xpkgs / "scode-x-linux-headers" / "5.11.1";
236+
touch(real / "include" / "linux" / "limits.h");
237+
238+
const char* oldMcppHome = std::getenv("MCPP_HOME");
239+
mcpp::platform::env::set("MCPP_HOME", tmp.string());
240+
auto found = mcpp::xlings::paths::find_home_tool(
241+
"linux-headers", "include/linux/limits.h");
242+
mcpp::platform::env::set("MCPP_HOME", oldMcppHome ? oldMcppHome : "");
243+
244+
ASSERT_TRUE(found.has_value());
245+
EXPECT_EQ(*found, real);
246+
247+
std::filesystem::remove_all(tmp);
248+
}

0 commit comments

Comments
 (0)