From a92cc45982abd78d26ef7b0b3843c49de7d851a5 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Thu, 6 Aug 2026 14:45:25 +0800 Subject: [PATCH] fix(pkginfo): install_dir says WHY it has no answer (#487) "cannot get install dir for xim:libcuda-host-link@0.0.1" names an internal state, and it covers two causes that point in opposite directions: - the package is not a dependency of this package on this platform; - it is, but its payload never landed. The first sends the reader to the recipe's platform sections. The second sends them to the dependency install. The old message sent them to neither -- openxlings/xlings#487 read it as a path problem and hypothesised that dependency resolution was not filtering by platform. It is filtered (xlings resolver.cppm: `runtime_deps.find(platform)`), and the recipe was correct too -- ollama declares the CUDA sentinel only under `xpm.linux`. The actual cause was ollama's install hook branching on `is_host("windows")` when the real distinction was linux, so macOS took the linux path and asked for a package macOS never resolves. Fixed separately in the index. `deps_list` is what the resolver produced FOR THIS PLATFORM, so a name missing from it is not "unresolvable" -- it is "not a dependency here". The message now says which of the two it is, names the package asked for, the platform, and what the deps actually are. Two tests, one per branch. The declared-but-absent case asserts the platform wording is ABSENT: describing an incomplete install as a platform mismatch would send the reader to the same wrong place, one door down. The Lua log goes to the process's stdout rather than HookResult::output, so they capture it. --- mcpp.toml | 2 +- src/lua-stdlib/xim/libxpkg/pkginfo.lua | 47 ++++++++++++++- tests/test_executor.cpp | 82 ++++++++++++++++++++++++++ 3 files changed, 128 insertions(+), 3 deletions(-) diff --git a/mcpp.toml b/mcpp.toml index eca2bbc..d4d50a5 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "xpkg" -version = "0.0.52" +version = "0.0.53" 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 1c4f5c7..ddd474f 100644 --- a/src/lua-stdlib/xim/libxpkg/pkginfo.lua +++ b/src/lua-stdlib/xim/libxpkg/pkginfo.lua @@ -255,9 +255,52 @@ function M.install_dir(pkgname, pkgversion) end local dir = M.dep_install_dir(pkgname, pkgversion) if dir then return dir end + + -- Say WHY, not "cannot". + -- + -- "cannot get install dir" names an internal state, and the two causes it + -- covers point in opposite directions: a path/permission problem, and a + -- package that is not a dependency here at all. Measured (openxlings/ + -- xlings#487): a macOS install of ollama reported it for + -- `xim:libcuda-host-link`, a linux-only sentinel that macOS never + -- resolves -- the hook asked for it unconditionally because its branch + -- tested `is_host("windows")` when the real distinction was linux. The + -- message sent the reader looking at paths. + -- + -- `deps_list` is what the resolver produced FOR THIS PLATFORM, so a name + -- missing from it is not "unresolvable" -- it is "not a dependency of + -- this package here". local log = _get_log() - if log then log.error("cannot get install dir for %s@%s", - tostring(pkgname), tostring(pkgversion or "latest")) end + if log then + local want = tostring(pkgname) + local bare = want:gsub("^[%w_-]+:", "") + local declared = false + for _, d in ipairs(M.deps_list()) do + local dn = tostring(d):gsub("@.*$", "") + if dn == want or dn:gsub("^[%w_-]+:", "") == bare then + declared = true + break + end + end + if declared then + log.error("install dir for %s@%s: declared as a dependency of %s " + .. "but no payload is on disk -- the dependency install " + .. "did not complete", + want, tostring(pkgversion or "latest"), + tostring(M.name() or "this package")) + else + log.error("install dir for %s@%s: NOT a dependency of %s on this " + .. "platform (%s). Its deps here are: %s. A package " + .. "declared only under another platform's xpm section " + .. "is never resolved on this one -- guard the hook that " + .. "asks for it with the platform it belongs to.", + want, tostring(pkgversion or "latest"), + tostring(M.name() or "this package"), + tostring(os.host()), + (#M.deps_list() > 0 + and table.concat(M.deps_list(), ", ") or "")) + end + end return nil end diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index 707828d..eec9906 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -582,6 +582,88 @@ TEST(ExecutorTest, HostLinkInterposer_RefusesAnEmptyClosure) { // exactly as successfully as one NEEDing nothing, and the caller reports "no // device" -- a machine without this driver must fail at install, where the // message can say so. +// `install_dir` for a package that is not a dependency here must SAY that. +// +// openxlings/xlings#487: a macOS install of ollama reported "cannot get +// install dir for xim:libcuda-host-link@0.0.1" -- a linux-only sentinel that +// macOS never resolves, asked for by a hook whose branch tested +// `is_host("windows")` when the real distinction was linux. "cannot" names an +// internal state and covers two causes that point opposite ways: a broken +// path, and a package that was never a dependency here. The reader went +// looking at paths. +// +// The hook still returns nil either way -- this is about which of the two the +// message names. +TEST(ExecutorTest, InstallDir_NotADependencyHere_SaysSoRatherThanCannot) { + const fs::path temp_dir = make_temp_dir("libxpkg-installdir-why-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path pkg_path = temp_dir / "consumer.lua"; + fs::create_directories(install_dir); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/d.tar.gz\", sha256 = \"0\" } } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " local d = pkginfo.install_dir(\"xim:linux-only-thing\", \"0.0.1\")\n" + " if d then error(\"expected nil for a package that is not a dep\") end\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(install_dir, "macosx"); + ctx.deps_list = {"xim:something-else@1.0.0"}; + // The Lua `log.error` goes to the process's stdout, not HookResult::output. + testing::internal::CaptureStdout(); + auto r = exec->run_hook(HookType::Install, ctx); + const std::string out = testing::internal::GetCapturedStdout() + r.output; + EXPECT_TRUE(r.success) << "the hook itself is fine; only the message changes"; + EXPECT_NE(out.find("NOT a dependency"), std::string::npos) + << "the message must name the cause, not the internal state. got:\n" << out; + EXPECT_NE(out.find("linux-only-thing"), std::string::npos) + << "the message must name the package asked for. got:\n" << out; + EXPECT_NE(out.find("something-else"), std::string::npos) + << "the message must list what IS a dep here, so the reader can see " + "the platform split. got:\n" << out; + + fs::remove_all(temp_dir); +} + +// A package that IS declared here but has no payload is the OTHER cause, and +// it must not be described as a platform mismatch -- that would send the +// reader to the recipe's platform sections when the dependency install is +// what failed. +TEST(ExecutorTest, InstallDir_DeclaredButAbsent_NamesTheIncompleteInstall) { + const fs::path temp_dir = make_temp_dir("libxpkg-installdir-absent-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path pkg_path = temp_dir / "consumer.lua"; + fs::create_directories(install_dir); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"consumer\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/d.tar.gz\", sha256 = \"0\" } } } }\n" + "local pkginfo = import(\"xim.libxpkg.pkginfo\")\n" + "function install()\n" + " pkginfo.install_dir(\"xim:declared-thing\", \"0.0.1\")\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(install_dir, "linux"); + ctx.deps_list = {"xim:declared-thing@0.0.1"}; + testing::internal::CaptureStdout(); + auto r = exec->run_hook(HookType::Install, ctx); + const std::string out = testing::internal::GetCapturedStdout() + r.output; + EXPECT_TRUE(r.success); + EXPECT_NE(out.find("declared as a dependency"), std::string::npos) + << "got:\n" << out; + EXPECT_EQ(out.find("NOT a dependency"), std::string::npos) + << "a declared-but-absent dep must not be reported as a platform " + "mismatch. got:\n" << out; + + fs::remove_all(temp_dir); +} + TEST(ExecutorTest, HostLinkInterposer_RefusesAMissingVendor) { #ifdef _WIN32 GTEST_SKIP() << "ELF-specific";