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..2bab65f 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 @@ -295,7 +341,59 @@ 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". + -- + -- 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. + -- + -- `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 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 " + .. "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 " + .. "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..3446f1a 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -2599,3 +2599,324 @@ 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; +} + +// ── 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; +}