feat(elfpatch): the stable farm goes first, and the vendor's closure is checked (0.0.54)

mcpp.toml

@@ -1,7 +1,7 @@
 [package]
 namespace   = "mcpplibs"
 name        = "xpkg"
-version     = "0.0.53"
+version     = "0.0.54"
 description = "C++23 reference implementation of the xpkg V2 spec (multi-arch)"
 license     = "Apache-2.0"
 repo        = "https://github.com/openxlings/libxpkg"

src/lua-stdlib/xim/libxpkg/elfpatch.lua

@@ -1531,6 +1531,45 @@ function M.host_link_interposer(opt)
               .. "that the package declares its runtime deps.")
     end
 
+    -- The SUBOS view first, the version-pinned payload directories after it.
+    --
+    -- closure_lib_paths returns the payloads the resolver chose, each an exact
+    -- version directory (`.../libX11/1.8.10/lib`), with the subos lib directory
+    -- LAST. Written into an ELF that ordering is a snapshot: upgrade libX11 and
+    -- every entry above still names 1.8.10, and once that payload is collected
+    -- the entry is a dead directory the loader walks past.
+    --
+    -- The subos lib directory is the one name that does not move. It is a
+    -- symlink farm the packages themselves declare into, so it tracks `xlings
+    -- use` and re-points on upgrade -- the same role /run/opengl-driver plays on
+    -- NixOS, /overrides in pressure-vessel and $SNAP/gpu-2404 in a snap. Every
+    -- ecosystem that has to hand a foreign driver a search path converged on a
+    -- stable indirection directory, and we already had one at the wrong end of
+    -- the list.
+    --
+    -- The payload directories STAY, as the fallback. The recipe's own concern is
+    -- real -- installed into a subos that is short of libX11, the farm would be
+    -- quietly missing it -- and with this order the farm answers first while the
+    -- payloads still answer at all. The closure assertion below is what turns
+    -- "quietly missing" into a named failure either way.
+    -- `subos_sysrootdir` is the subos root; its `lib` is the farm that
+    -- `sysroot.declare_libs` (and xvm's `lib` node kind) fill. Read from
+    -- _RUNTIME rather than re-derived from a path in `libdirs`: the last entry
+    -- being the subos directory is a property of closure_lib_paths, not a
+    -- contract, and inferring it would break silently if that changed.
+    local subosdir = nil
+    if _RUNTIME and _RUNTIME.subos_sysrootdir
+       and _RUNTIME.subos_sysrootdir ~= "" then
+        subosdir = path.join(_RUNTIME.subos_sysrootdir, "lib")
+    end
+    if subosdir and os.isdir(subosdir) then
+        local reordered, seen = { subosdir }, { [subosdir] = true }
+        for _, d in ipairs(libdirs) do
+            if not seen[d] then seen[d] = true; table.insert(reordered, d) end
+        end
+        libdirs = reordered
+    end
+
     local tool = _find_tool("patchelf")
     if not tool then
         error("elfpatch.host_link_interposer: patchelf not found")
@@ -1582,9 +1621,57 @@ function M.host_link_interposer(opt)
               .. " -- the vendor's dependencies would resolve from the host")
     end
 
+    -- And the assertion those three were missing: does the RPATH actually
+    -- SERVE the vendor?
+    --
+    -- The three above check the shape of the object we made. All three can hold
+    -- while the thing the object exists for does not work: an RPATH naming
+    -- directories that do not contain the vendor's DT_NEEDED resolves them from
+    -- the host instead, which still renders -- on llvmpipe -- and prints
+    -- nothing. That is this repository's recurring failure shape, and it was
+    -- sitting inside the function written to prevent it.
+    --
+    -- Resolved the way the loader would, not by trusting a list: for each of the
+    -- vendor's DT_NEEDED entries, look for that exact name in the RPATH
+    -- directories in order. Bare SONAMEs only -- an absolute DT_NEEDED needs no
+    -- search, and libc/libm come from the same closure as everything else.
+    local vneeded = _iorun(_shell_quote(tool.program) .. " --print-needed "
+                           .. _shell_quote(vendor)) or ""
+    local missing, vtotal = {}, 0
+    for line in vneeded:gmatch("[^\n]+") do
+        local name = _trim(line)
+        if name ~= "" and not name:find("^/") then
+            vtotal = vtotal + 1
+            local found = false
+            for _, d in ipairs(libdirs) do
+                if os.isfile(path.join(d, name)) then found = true; break end
+            end
+            if not found then table.insert(missing, name) end
+        end
+    end
+    if #missing > 0 then
+        -- A warning, not an error, and the line between them is what this
+        -- ecosystem's convention says: the interposer IS correctly built, and on
+        -- a normal host the loader will find these through its own ld.so cache
+        -- and the result works. Refusing would break a working install. What
+        -- must not happen is that it is INVISIBLE -- inside a sandbox or an
+        -- empty-host container there is no cache to fall back to, and the
+        -- failure then appears as `no device` from a GL call three layers away.
+        _warn(string.format(
+            "interposer %s: %d of the vendor's dependencies are not in the "
+            .. "payload closure and will resolve from the HOST: %s. GL will "
+            .. "work here and fail in a sandbox with no /etc/ld.so.cache. "
+            .. "Declare the missing providers as runtime deps of this package.",
+            soname, #missing, table.concat(missing, ", ")))
+    end
+
+    -- The fraction, not a bare "built". `interposer X -> Y` was true in every
+    -- case above including the one where nothing the vendor needs is reachable.
     _info(string.format(
-        "interposer %s -> %s (%d closure dir(s))", soname, vendor, #libdirs))
-    return { out = out, soname = soname, vendor = vendor, libdirs = libdirs }
+        "interposer %s -> %s (%d closure dir(s), vendor deps resolved %d/%d)",
+        soname, vendor, #libdirs, vtotal - #missing, vtotal))
+    return { out = out, soname = soname, vendor = vendor, libdirs = libdirs,
+             unresolved = missing }
 end
 
 return M

tests/test_executor.cpp

@@ -582,6 +582,91 @@ 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.
+// The vendor's OWN dependency closure has to be checked, not just the shape of
+// the object we made.
+//
+// The three assertions that were here (soname / needed / rpath present) can all
+// hold while the interposer does nothing useful: an RPATH naming directories
+// that do not contain the vendor's DT_NEEDED resolves them from the host, which
+// still renders -- on llvmpipe -- and says nothing. Warn rather than fail: the
+// object IS correctly built and the host's ld.so.cache makes it work outside a
+// sandbox, so refusing would break a working install. Being invisible is the
+// defect.
+TEST(ExecutorTest, HostLinkInterposer_ReportsAnUnservedVendorClosure) {
+#ifdef _WIN32
+    GTEST_SKIP() << "ELF-specific";
+#endif
+    const fs::path temp_dir = make_temp_dir("libxpkg-interposer-closure-");
+    const fs::path install_dir = temp_dir / "install";
+    const fs::path libdir = install_dir / "lib";
+    const fs::path pkg_path = temp_dir / "interposer.lua";
+    const fs::path stub = temp_dir / "stub.so";
+    const fs::path vendor = temp_dir / "libFAKE_vendor.so.550";
+    const fs::path tools = temp_dir / "tools";
+    const fs::path served = temp_dir / "served";
+    fs::create_directories(libdir);
+    fs::create_directories(tools);
+    fs::create_directories(served);
+
+    // One of the two SONAMEs the vendor needs is present in the closure; the
+    // other is not. A fraction, so a pass cannot be produced by an empty list.
+    write_text(served / "libserved.so.1", "x\n");
+
+    // A fake patchelf that answers --print-needed differently for the VENDOR
+    // than for the object under construction. The previous fake could not tell
+    // them apart, which is exactly why this check could not have been tested
+    // with it.
+    write_executable_script(tools / "patchelf",
+        "#!/bin/sh\n"
+        "printf 'patchelf %s\\n' \"$*\" >> \"$ELFPATCH_LOG\"\n"
+        "case \"$1\" in\n"
+        "  --set-soname) echo \"$2\" > \"$ELFPATCH_STATE.soname\" ;;\n"
+        "  --add-needed) echo \"$2\" >> \"$ELFPATCH_STATE.needed\" ;;\n"
+        "  --set-rpath)  echo \"$2\" > \"$ELFPATCH_STATE.rpath\" ;;\n"
+        "  --print-soname) cat \"$ELFPATCH_STATE.soname\" 2>/dev/null ;;\n"
+        "  --print-needed)\n"
+        "     case \"$2\" in\n"
+        "       *libFAKE_vendor.so.550) printf 'libserved.so.1\\nlibmissing.so.7\\n' ;;\n"
+        "       *) cat \"$ELFPATCH_STATE.needed\" 2>/dev/null ;;\n"
+        "     esac ;;\n"
+        "  --print-rpath)  cat \"$ELFPATCH_STATE.rpath\"  2>/dev/null ;;\n"
+        "esac\n"
+        "exit 0\n");
+
+    write_text(stub, "\x7f" "ELF-stub-placeholder\n");
+    write_text(vendor, "\x7f" "ELF-vendor-placeholder\n");
+
+    write_text(pkg_path,
+        "package = { spec = \"1\", name = \"interposer\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/d.tar.gz\", sha256 = \"0\" } } } }\n"
+        "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n"
+        "function install()\n"
+        "    local r = elfpatch.host_link_interposer{\n"
+        "        vendor  = \"" + vendor.string() + "\",\n"
+        "        out     = \"" + (libdir / "libFAKE_vendor.so.0").string() + "\",\n"
+        "        stub    = \"" + stub.string() + "\",\n"
+        "        libdirs = { \"" + served.string() + "\" },\n"
+        "    }\n"
+        "    assert(#r.unresolved == 1, \"expected one unresolved dep\")\n"
+        "    assert(r.unresolved[1] == \"libmissing.so.7\", r.unresolved[1])\n"
+        "    return true\n"
+        "end\n");
+
+    const std::string original_path = std::getenv("PATH") ? std::getenv("PATH") : "";
+    ScopedEnvVar path_env("PATH", tools.string() + ":" + original_path);
+    ScopedEnvVar log_env("ELFPATCH_LOG", (temp_dir / "log.txt").string());
+    ScopedEnvVar st_env("ELFPATCH_STATE", (temp_dir / "state").string());
+
+    auto exec = create_executor(pkg_path);
+    ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error());
+    auto ctx = make_context(install_dir, "linux", tools);
+    auto hook_result = exec->run_hook(HookType::Install, ctx);
+    // The install SUCCEEDS -- the unresolved entry is reported, not fatal.
+    ASSERT_TRUE(hook_result.success) << hook_result.error;
+    EXPECT_TRUE(fs::exists(libdir / "libFAKE_vendor.so.0"));
+
+    fs::remove_all(temp_dir);
+}
+
 // `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