From 71b9ed7ddbf2a5310afb2b057ce33adde27ec1cd Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Mon, 10 Aug 2026 05:05:14 +0800 Subject: [PATCH 1/3] fix(pkginfo): an unanswerable dependency query must not look like an absent one (0.0.56) 0.0.55 made explicit dependency store roots the resolver for `dep_install_dir` when the host supplies them. Those roots answer an EXACT, NAMESPACED coordinate and nothing else, on purpose -- a bare `zlib` would have to pick between `compat-x-zlib` and `other-x-zlib`, and guessing is the decoy problem the roots exist to remove. That part was right and is unchanged. What was wrong is that an underspecified query returned nil with no word to the caller, and a recipe cannot tell that from "the dependency is not installed". Measured under xlings 2026.8.10.1: `gcc.lua` and `llvm.lua` both call `pkginfo.dep_install_dir("glibc")` -- bare and unversioned -- while both declare `xim:glibc@>=2.39`. They know the namespace; they just did not pass it. Both got nil and reported "glibc payload not found" on a home where glibc was installed, and gcc could not install on Linux at all. Under 0.0.54 and earlier the roots field did not exist, so the legacy scan ran and the same call resolved. The query is answerable the moment the caller names the dependency the way it declared it: `dep_install_dir("xim:glibc")` goes through `resolved_deps`, the single source this path is built around. So nil stays, and now it explains itself and shows the call that works. Three tests. Two of them are the ones that matter: NamespacedUnversionedQueryUsesResolvedRecord the fix recipes should adopt BareNameUnderExplicitRootsExplainsItself the diagnostic, asserted by its text, not by its absence ExactNamespacedCoordinateStillFailsClosed the 0.0.55 guarantee, intact A first attempt made the roots a preferred source with a fallback to the scan. It fixed gcc and broke three existing tests -- ExplicitRootsRejectBareNameRequests among them -- because the scan is exactly the guessing those tests forbid. The tests were right; keeping them red would have traded a loud failure for a quiet wrong answer. --- mcpp.toml | 2 +- src/lua-stdlib/xim/libxpkg/pkginfo.lua | 38 +++++++- tests/test_executor.cpp | 119 +++++++++++++++++++++++++ 3 files changed, 157 insertions(+), 2 deletions(-) diff --git a/mcpp.toml b/mcpp.toml index b67fd59..1306d77 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "xpkg" -version = "0.0.55" +version = "0.0.56" description = "C++23 reference implementation of the xpkg V2 spec (multi-arch)" license = "Apache-2.0" repo = "https://github.com/openxlings/libxpkg" diff --git a/src/lua-stdlib/xim/libxpkg/pkginfo.lua b/src/lua-stdlib/xim/libxpkg/pkginfo.lua index 95e27a4..1334c9a 100644 --- a/src/lua-stdlib/xim/libxpkg/pkginfo.lua +++ b/src/lua-stdlib/xim/libxpkg/pkginfo.lua @@ -295,7 +295,43 @@ function M.dep_install_dir(dep_name, dep_version) local roots = _RUNTIME and _RUNTIME.dependency_store_roots if type(roots) == "table" then - return _resolve_dep_via_explicit_roots(dep_name, dep_version) + local hit = _resolve_dep_via_explicit_roots(dep_name, dep_version) + if hit then return hit end + + -- Explicit roots answer an EXACT, NAMESPACED coordinate and nothing + -- else, on purpose: a bare `zlib` would have to pick between + -- `compat-x-zlib` and `other-x-zlib`, and guessing is the decoy problem + -- these roots exist to remove. So nil is the right ANSWER here. + -- + -- It was the wrong SILENCE. 0.0.55 returned nil for an underspecified + -- query with no word to the caller, and a recipe cannot tell that from + -- "the dependency is not installed". Measured: `gcc.lua` and + -- `llvm.lua` call `dep_install_dir("glibc")` -- bare, unversioned -- + -- while both declare `xim:glibc@>=2.39`, so they know the namespace and + -- simply did not pass it. Under xlings 2026.8.10.1 they got nil and + -- reported "glibc payload not found" on homes where glibc was + -- installed; gcc could not install on Linux at all. + -- + -- The query is answerable -- through `resolved_deps`, the single + -- source this whole path is built around -- as soon as the caller + -- names the dependency the way it declared it. Say so. + local ns = _parse_namespace(dep_name) + local log = _get_log() + if log and _RUNTIME and _RUNTIME.install_dir then + if not ns then + log.error("dep_install_dir(%s): a bare name cannot be resolved " + .. "against explicit dependency stores -- it does not " + .. "say which namespace. Pass the coordinate as " + .. "declared, e.g. \"ns:%s\".", + tostring(dep_name), tostring(dep_name)) + elseif not _is_exact_store_version(dep_version) then + log.error("dep_install_dir(%s, %s): explicit dependency stores " + .. "need an exact version. Omit the version to use " + .. "the resolved dependency record instead.", + tostring(dep_name), tostring(dep_version)) + end + end + return nil end local result = _resolve_dep_via_scan(dep_name, dep_version) diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index 27738c9..41ac8b2 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -2599,3 +2599,122 @@ TEST(ExecutorTest, SubosEnv_ProbeMustTestTypeBecauseUnknownModulesAreTruthy) { EXPECT_EQ(ops[3].name, "absent.typed.false"); fs::remove_all(dir); } + +// The shape gcc.lua and llvm.lua actually use, and the shape they should use. +// +// Both declare `xim:glibc@>=2.39` and then ask `dep_install_dir("glibc")` -- +// bare and unversioned. 0.0.55 made explicit dependency stores the resolver +// for this path, and they answer only an exact NAMESPACED coordinate, so the +// call started returning nil. Silently: a recipe cannot tell that from "the +// dependency is not installed". Measured under xlings 2026.8.10.1 -- gcc's +// config hook reported "glibc payload not found" on a home where glibc was +// installed, and gcc could not install on Linux. +// +// nil is the right answer for a bare name (guessing between `compat-x-zlib` +// and `other-x-zlib` is the decoy problem these roots remove). Saying nothing +// was not. And the query IS answerable the moment the caller names the +// dependency the way it declared it. +TEST(ExecutorTest, PkgInfo_NamespacedUnversionedQueryUsesResolvedRecord) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-ns-noversion-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path glibcPayload = registryRoot / "xim-x-glibc" / "2.44"; + const fs::path memberPayload = tempDir / "member" / "data" / "xpkgs" / + "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(glibcPayload); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local got = pkginfo.dep_install_dir(\"xim:glibc\")\n" + " assert(got ~= nil, \"namespaced unversioned query resolved to nil\")\n" + " assert(got:find(\"xim-x-glibc\", 1, true) ~= nil, got)\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + ctx.resolved_deps["xim:glibc@>=2.39"] = ResolvedDep { + .spec = "xim:glibc@>=2.39", + .name = "xim:glibc", + .version = "2.44", + .install_dir = glibcPayload.string(), + .libdirs = {}, + .source = "plan-range", + }; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; +} + +// The bare form still returns nil -- and now says why, and what to write +// instead. "Cannot answer" and "not installed" must not look the same. +TEST(ExecutorTest, PkgInfo_BareNameUnderExplicitRootsExplainsItself) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-bare-explains-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path memberPayload = tempDir / "member" / "data" / "xpkgs" / + "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(registryRoot / "xim-x-glibc" / "2.44"); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local got = pkginfo.dep_install_dir(\"glibc\")\n" + " assert(got == nil, \"bare name guessed: \" .. tostring(got))\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_NE(result.output.find("bare name cannot be resolved"), + std::string::npos) + << "a bare-name miss must explain itself:\n" << result.output; + EXPECT_NE(result.output.find("ns:glibc"), std::string::npos) + << "the diagnostic must show the call that would work:\n" + << result.output; +} + +// ...and the guarantee 0.0.55 added is untouched: an EXACT, +// NAMESPACED coordinate that the supplied roots do not contain is a definite +// no. If this fell through to the scan, a same-bare-name decoy elsewhere would +// answer it -- which is the whole thing explicit roots exist to prevent. +TEST(ExecutorTest, PkgInfo_ExactNamespacedCoordinateStillFailsClosed) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-exact-closed-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path memberXpkgs = tempDir / "member" / "data" / "xpkgs"; + const fs::path decoy = memberXpkgs / "other-x-zlib" / "1.3.2"; + const fs::path memberPayload = memberXpkgs / "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(registryRoot); + fs::create_directories(decoy); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local got = pkginfo.dep_install_dir(\"compat:zlib\", \"1.3.2\")\n" + " assert(got == nil, \"exact coordinate fell through to: \" .. tostring(got))\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; +} From 03d44c6d51cdda11b5f5bf68d9f52087cbe4a272 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Mon, 10 Aug 2026 10:42:39 +0800 Subject: [PATCH 2/3] fix(pkginfo): a bare dependency name is not the same thing as an ambiguous one 0.0.55 made explicit dependency store roots the resolver for `dep_install_dir`, and gated the `resolved_deps` bare-name branch on `_is_exact_store_version` as well. That precondition disambiguated NOTHING -- the uniqueness guard was already there and already failed closed -- and its only effect was to reject the unversioned query, which is the shape every recipe in the index writes. Measured against the published recipes (openxlings/xlings#524), replaying each call site with and without the roots field: 6 of 7 returned nil once xlings 2026.8.10.1 started filling `dependency_store_roots` unconditionally. gcc and meson could not install on any cold home; godot silently fell back to the host's GL, which is the exact mcpp#352 failure its dependency exists to prevent. 0.0.56 diagnosed this correctly and only added diagnostics -- same 6 of 7 still nil, now loud. The distinction that matters: ambiguity is a property of the RECORD SET, not of how specific the question was. `resolved_deps` is a closed table of this package's own declared deps, so "is there a second record with this bare name" is decidable here. That is what makes it different from the explicit roots, where refusing to guess IS the right answer -- and that part is unchanged. So the bare branch now matches on the name and lets the uniqueness guard rule, a collision names both providers instead of returning a bare nil, and the version half matches as a RANGE rather than a literal. That last one is xlings#481 again: `>=2.39` compared as a string to the resolver's chosen 2.44 is unequal, and the caller restates the range, never the pick. Nothing is narrowed. Every query 0.0.55/0.0.56 answered is answered the same way, collisions still fail closed, an exact namespaced coordinate absent from the roots is still a definite no, and a record whose payload is not on disk is still a miss with its own message. Clients with no roots field keep the scan. `resolved_dep` gained a second return value naming why it declined, so an ambiguous name no longer also gets "you did not name a namespace" printed under it -- that advice points at the wrong fix when two namespaces answer. Four tests, each verified to fail against the unfixed pkginfo: the bare unversioned query that is #524 itself, range-vs-pick matching, the collision naming both providers without the generic advice, and the absent payload. Refs openxlings/xlings#524 --- src/lua-stdlib/xim/libxpkg/pkginfo.lua | 89 ++++++++--- tests/test_executor.cpp | 202 +++++++++++++++++++++++++ 2 files changed, 270 insertions(+), 21 deletions(-) diff --git a/src/lua-stdlib/xim/libxpkg/pkginfo.lua b/src/lua-stdlib/xim/libxpkg/pkginfo.lua index 1334c9a..7ec823c 100644 --- a/src/lua-stdlib/xim/libxpkg/pkginfo.lua +++ b/src/lua-stdlib/xim/libxpkg/pkginfo.lua @@ -221,6 +221,20 @@ local function _resolve_dep_via_xvm(dep_name, dep_version) return nil end +-- Does an installed version answer what the caller asked for? +-- +-- The version half of a dep is an EXPRESSION -- `2.39`, `>=2.38`, `^1.2` -- and +-- comparing it to a concrete version as a string makes them unequal. That is +-- the same mistake as openxlings/xlings#481, where `xim:glibc@>=2.38` matched +-- no plan node, so nothing got an RPATH and the package installed reporting +-- success. Omitting the version means "whatever the resolver picked", which is +-- the whole reason a resolver record exists. +local function _version_matches_request(have, want) + if want == nil or want == "" then return true end + if have == want then return true end + return _version_satisfies(have, want) +end + -- The resolver's record for a dependency, if this client sends one. -- -- type(), not truthiness: an unknown _RUNTIME field is nil here, but the same @@ -229,21 +243,51 @@ end -- xim.pkgindex.sysroot). -- -- Namespaced requests match their exact spec/canonical identity. Bare requests --- retain compatibility only when an exact requested version selects precisely --- one canonical resolver record; namespace collisions fail closed. +-- are answered when the record set answers them UNIQUELY. +-- +-- Returns the record, or nil plus a reason ("ambiguous") the caller can use to +-- avoid explaining the same failure twice. Extra return values are ignored by +-- every existing caller, so this stays additive. +-- +-- 0.0.55 gated the bare branch on `_is_exact_store_version(dep_version)` as +-- well. That precondition disambiguated NOTHING -- the uniqueness guard below +-- was already there and already failed closed -- and its only effect was to +-- reject the unversioned query, which is the shape every recipe in the index +-- actually writes. Measured (openxlings/xlings#524): 6 of the 7 real +-- `dep_install_dir` call sites returned nil, gcc and meson could not install on +-- any cold home at all, and godot silently fell back to the host's GL. +-- +-- The distinction that matters: ambiguity is a property of the RECORD SET, not +-- of how specific the question was. `resolved_deps` is a closed table of this +-- package's own declared deps, so "is there a second record with this bare +-- name" is decidable here. That is exactly what makes it different from the +-- explicit store roots below, where refusing to guess IS the right answer. function M.resolved_dep(dep_name, dep_version) local t = _RUNTIME and _RUNTIME.resolved_deps if type(t) ~= "table" then return nil end local ns, bare = _parse_namespace(dep_name) if not ns then - if not _is_exact_store_version(dep_version) then return nil end - local candidate = nil + local candidate, candidate_name = nil, nil for spec, rec in pairs(t) do local canonical = rec.name or spec:gsub("@.*", "") local _, record_bare = _parse_namespace(canonical) - if record_bare == bare and rec.version == dep_version then - if candidate then return nil end - candidate = rec + if record_bare == bare + and _version_matches_request(rec.version, dep_version) then + -- Two providers, one bare name: fail closed -- and name them. + -- "not found" would send the reader looking for a missing + -- payload when the payload is there twice. + if candidate then + local log = _get_log() + if log then + log.error("dep_install_dir(%s): ambiguous -- %s and %s " + .. "both provide this name. Pass the " + .. "namespaced coordinate, as declared.", + tostring(dep_name), tostring(candidate_name), + tostring(canonical)) + end + return nil, "ambiguous" + end + candidate, candidate_name = rec, canonical end end return candidate @@ -256,9 +300,11 @@ function M.resolved_dep(dep_name, dep_version) for spec, rec in pairs(t) do local sname = spec:gsub("@.*", "") if sname == dep_name then - if not dep_version or dep_version == "" - or spec == dep_name .. "@" .. dep_version - or rec.version == dep_version then + -- Same range rule as the bare branch. Asking for `>=2.38` when the + -- recipe declared `>=2.39` is a legitimate question with the same + -- answer; string equality called it a miss. + if spec == dep_name .. "@" .. (dep_version or "") + or _version_matches_request(rec.version, dep_version) then return rec end end @@ -278,7 +324,7 @@ end -- with no diagnostic. See -- xlings/.agents/docs/2026-08-05-dependency-resolution-single-source.md function M.dep_install_dir(dep_name, dep_version) - local rec = M.resolved_dep(dep_name, dep_version) + local rec, why = M.resolved_dep(dep_name, dep_version) if rec then if rec.install_dir and rec.install_dir ~= "" and os.isdir(rec.install_dir) then @@ -305,19 +351,20 @@ function M.dep_install_dir(dep_name, dep_version) -- -- It was the wrong SILENCE. 0.0.55 returned nil for an underspecified -- query with no word to the caller, and a recipe cannot tell that from - -- "the dependency is not installed". Measured: `gcc.lua` and - -- `llvm.lua` call `dep_install_dir("glibc")` -- bare, unversioned -- - -- while both declare `xim:glibc@>=2.39`, so they know the namespace and - -- simply did not pass it. Under xlings 2026.8.10.1 they got nil and - -- reported "glibc payload not found" on homes where glibc was - -- installed; gcc could not install on Linux at all. + -- "the dependency is not installed". + -- + -- Reaching here now means `resolved_deps` genuinely could not answer: + -- the dependency was never declared by this package (a transitive dep, + -- or a payload some hook installed itself). A bare name additionally + -- cannot be looked up in the roots, because they key on an exact + -- namespaced coordinate. Say which of the two it is. -- - -- The query is answerable -- through `resolved_deps`, the single - -- source this whole path is built around -- as soon as the caller - -- names the dependency the way it declared it. Say so. + -- `why == "ambiguous"` is the third case, and it already printed a + -- better message naming both providers -- repeating "a bare name + -- cannot be resolved" underneath it would point at the wrong fix. local ns = _parse_namespace(dep_name) local log = _get_log() - if log and _RUNTIME and _RUNTIME.install_dir then + if log and _RUNTIME and _RUNTIME.install_dir and why ~= "ambiguous" then if not ns then log.error("dep_install_dir(%s): a bare name cannot be resolved " .. "against explicit dependency stores -- it does not " diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index 41ac8b2..3446f1a 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -2718,3 +2718,205 @@ TEST(ExecutorTest, PkgInfo_ExactNamespacedCoordinateStillFailsClosed) { auto result = exec->run_hook(HookType::Install, ctx); EXPECT_TRUE(result.success) << result.error << "\n" << result.output; } + +// ── the #524 regression itself ─────────────────────────────────────────── +// +// Everything above tests the case where `resolved_deps` genuinely cannot +// answer. This is the case where it CAN, and 0.0.55 refused anyway. +// +// gcc.lua declares `xim:glibc@>=2.39` and asks `dep_install_dir("glibc")`. +// The record is right there, under a bare name that nothing else in the table +// claims. 0.0.55 rejected it because the QUESTION was underspecified, not +// because the ANSWER was ambiguous -- and the uniqueness guard that decides +// ambiguity was already present and already failing closed. Measured: 6 of +// the 7 real call sites in xim-pkgindex returned nil; gcc and meson could not +// install on any cold home. +TEST(ExecutorTest, PkgInfo_BareUnversionedResolvesWhenTheRecordIsUnique) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-bare-unique-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path glibcPayload = registryRoot / "xim-x-glibc" / "2.44"; + const fs::path memberPayload = tempDir / "member" / "data" / "xpkgs" / + "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(glibcPayload); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local got = pkginfo.dep_install_dir(\"glibc\")\n" + " assert(got ~= nil, \"the unique record did not answer\")\n" + " assert(got:find(\"xim-x-glibc\", 1, true) ~= nil, got)\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + ctx.resolved_deps["xim:glibc@>=2.39"] = ResolvedDep { + .spec = "xim:glibc@>=2.39", + .name = "xim:glibc", + .version = "2.44", + .install_dir = glibcPayload.string(), + .libdirs = {}, + .source = "plan-range", + }; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; +} + +// A caller may restate the version, and what it restates is the RANGE the +// recipe declared -- not the concrete version the resolver picked. Comparing +// those as strings makes them unequal, which is openxlings/xlings#481 again: +// there `xim:glibc@>=2.38` matched no plan node, so nothing got an RPATH and +// the package installed reporting success. +TEST(ExecutorTest, PkgInfo_BareWithRangeMatchesTheResolversConcretePick) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-bare-range-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path glibcPayload = registryRoot / "xim-x-glibc" / "2.44"; + const fs::path memberPayload = tempDir / "member" / "data" / "xpkgs" / + "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(glibcPayload); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local hit = pkginfo.dep_install_dir(\"glibc\", \">=2.39\")\n" + " assert(hit ~= nil, \"a range did not match the resolved 2.44\")\n" + " -- ...and a version the record does NOT satisfy is still a miss.\n" + " local miss = pkginfo.dep_install_dir(\"glibc\", \"2.39\")\n" + " assert(miss == nil, \"wrong exact version matched: \" .. tostring(miss))\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + ctx.resolved_deps["xim:glibc@>=2.39"] = ResolvedDep { + .spec = "xim:glibc@>=2.39", + .name = "xim:glibc", + .version = "2.44", + .install_dir = glibcPayload.string(), + .libdirs = {}, + .source = "plan-range", + }; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; +} + +// Ambiguity is what fails closed -- and it must name both providers. "not +// found" would send the reader looking for a missing payload when the payload +// is there twice. +// +// It must ALSO not repeat the bare-name advice underneath. That message says +// "you did not name a namespace"; here the caller's real problem is that two +// namespaces answer, and stacking the generic advice on top points at the +// wrong fix. +TEST(ExecutorTest, PkgInfo_TwoProvidersOfOneBareNameFailClosedAndNameBoth) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-ambiguous-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path compatZlib = registryRoot / "compat-x-zlib" / "1.3"; + const fs::path otherZlib = registryRoot / "other-x-zlib" / "1.3"; + const fs::path memberPayload = tempDir / "member" / "data" / "xpkgs" / + "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(compatZlib); + fs::create_directories(otherZlib); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local got = pkginfo.dep_install_dir(\"zlib\")\n" + " assert(got == nil, \"ambiguous name guessed: \" .. tostring(got))\n" + " -- naming the namespace resolves it, which is the advice we print\n" + " local ok = pkginfo.dep_install_dir(\"compat:zlib\")\n" + " assert(ok ~= nil and ok:find(\"compat-x-zlib\", 1, true) ~= nil,\n" + " \"namespaced form did not resolve: \" .. tostring(ok))\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + ctx.resolved_deps["compat:zlib@1.3"] = ResolvedDep { + .spec = "compat:zlib@1.3", + .name = "compat:zlib", + .version = "1.3", + .install_dir = compatZlib.string(), + .libdirs = {}, + .source = "plan-exact", + }; + ctx.resolved_deps["other:zlib@1.3"] = ResolvedDep { + .spec = "other:zlib@1.3", + .name = "other:zlib", + .version = "1.3", + .install_dir = otherZlib.string(), + .libdirs = {}, + .source = "plan-exact", + }; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_NE(result.output.find("ambiguous"), std::string::npos) + << "a collision must say so:\n" << result.output; + EXPECT_NE(result.output.find("compat:zlib"), std::string::npos) + << "the diagnostic must name the providers:\n" << result.output; + EXPECT_NE(result.output.find("other:zlib"), std::string::npos) + << "the diagnostic must name the providers:\n" << result.output; + EXPECT_EQ(result.output.find("bare name cannot be resolved"), + std::string::npos) + << "the generic bare-name advice points at the wrong fix here:\n" + << result.output; +} + +// Widening the match must NOT weaken the payload check 0.0.55 added: a record +// whose install_dir is not on disk is a definite no, with its own message. +// Returning the path anyway is how a hook proceeds against a directory that +// does not exist and blames something further downstream. +TEST(ExecutorTest, PkgInfo_UniqueRecordWithNoPayloadOnDiskIsStillAMiss) { + const fs::path tempDir = make_temp_dir("libxpkg-pkginfo-no-payload-"); + const fs::path registryRoot = tempDir / "registry" / "data" / "xpkgs"; + const fs::path memberPayload = tempDir / "member" / "data" / "xpkgs" / + "consumer" / "1.0.0"; + const fs::path pkgPath = tempDir / "consumer.lua"; + fs::create_directories(registryRoot); + fs::create_directories(memberPayload); + + write_text(pkgPath, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"1.0.0\"] = {} } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local got = pkginfo.dep_install_dir(\"glibc\")\n" + " assert(got == nil, \"answered with an absent payload: \" .. tostring(got))\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkgPath); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(memberPayload, "linux"); + ctx.dependency_store_roots = {registryRoot}; + ctx.resolved_deps["xim:glibc@>=2.39"] = ResolvedDep { + .spec = "xim:glibc@>=2.39", + .name = "xim:glibc", + .version = "2.44", + .install_dir = (registryRoot / "xim-x-glibc" / "2.44").string(), + .libdirs = {}, + .source = "plan-range", + }; + + auto result = exec->run_hook(HookType::Install, ctx); + EXPECT_TRUE(result.success) << result.error << "\n" << result.output; + EXPECT_NE(result.output.find("missing payload"), std::string::npos) + << "an absent payload must say so:\n" << result.output; +} From e81acf934fb3a0a355d6445c8abbe4028285cabc Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Mon, 10 Aug 2026 11:16:50 +0800 Subject: [PATCH 3/3] fix(pkginfo): "pass an exact version" was circular advice for an undeclared dep A namespaced coordinate with no version and no resolver record fell into the branch that says explicit dependency stores need an exact version, and offers "omit the version to use the resolved dependency record instead" -- which is what the caller already did. The real problem is that there is no record: the coordinate is not a declared dependency of this package. Measured while building the #524 e2e: `dep_install_dir("xim:never-declared")` from a hook whose deps do not mention it produced advice that, followed literally, changes nothing. Now it names the cause and the two ways out -- declare it, or use tool_payload_dir if the hook installed the payload itself -- and prints what this package actually declares, so the reader can see the mismatch rather than infer it. --- src/lua-stdlib/xim/libxpkg/pkginfo.lua | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/lua-stdlib/xim/libxpkg/pkginfo.lua b/src/lua-stdlib/xim/libxpkg/pkginfo.lua index 7ec823c..2bab65f 100644 --- a/src/lua-stdlib/xim/libxpkg/pkginfo.lua +++ b/src/lua-stdlib/xim/libxpkg/pkginfo.lua @@ -371,6 +371,21 @@ function M.dep_install_dir(dep_name, dep_version) .. "say which namespace. Pass the coordinate as " .. "declared, e.g. \"ns:%s\".", tostring(dep_name), tostring(dep_name)) + elseif dep_version == nil or dep_version == "" then + -- Namespaced, no version, and no record. "Pass an exact + -- version" -- what this used to say -- is circular advice: the + -- caller omitted the version precisely so the resolver's own + -- choice would be used, and the real problem is that there is + -- no record to choose from. Name that instead. + log.error("dep_install_dir(%s): %s is not a declared " + .. "dependency of %s, so the resolver has no record " + .. "of it. Declare it in `deps`, or use " + .. "tool_payload_dir if this hook installed it " + .. "itself. Declared here: %s", + tostring(dep_name), tostring(dep_name), + tostring(M.name() or "this package"), + (#M.deps_list() > 0 + and table.concat(M.deps_list(), ", ") or "")) elseif not _is_exact_store_version(dep_version) then log.error("dep_install_dir(%s, %s): explicit dependency stores " .. "need an exact version. Omit the version to use "