diff --git a/.github/workflows/nix-build.yml b/.github/workflows/nix-build.yml index 2332e3d6b..a8531c9ac 100644 --- a/.github/workflows/nix-build.yml +++ b/.github/workflows/nix-build.yml @@ -198,41 +198,56 @@ jobs: # green with the hand-added RPATH missing entirely -- validating # everything except the one thing this packaging adds. # - # Read the RPATH out of the binary instead, and require that one of its - # directories actually holds the soname the shim hands to dlopen. - HELPER=$(wrapper_value OPENSCREEN_LINUX_CURSOR_HELPER_EXE) - if [ -n "$HELPER" ]; then - echo "--- helper dlopen contract ---" - # DT_RPATH or DT_RUNPATH: the derivation asks for the former, but read + # Read the RPATH out of the object instead, and require that one of its + # directories actually holds the soname handed to dlopen. + # + # Both components have one. The helper's libpipewire entry is the one + # that caught a real regression; the addon's libvulkan entry is the same + # construction and was silently being stripped too, unnoticed because + # ubuntu-latest has a system libvulkan that satisfies the dlopen anyway. + # That is exactly why it needs asserting rather than exercising: the + # runner cannot reproduce the host where it matters. + check_dlopen_rpath() { + local what="$1" obj="$2" soname="$3" + [ -n "$obj" ] || return 0 + echo "--- $what dlopen contract ---" + # DT_RPATH or DT_RUNPATH: the derivations ask for the former, but read # whichever is there rather than asserting which, so a silent # conversion is reported instead of looking like an absent RPATH. - RPATH=$(readelf -d "$HELPER" | sed -nE 's/.*\((RPATH|RUNPATH)\).*\[(.*)\]/\2/p' | tail -1) - echo "rpath: ${RPATH:-}" - found="" - origin=$(dirname "$HELPER") - IFS=: read -ra dirs <<<"$RPATH" + local rpath + rpath=$(readelf -d "$obj" | sed -nE 's/.*\((RPATH|RUNPATH)\).*\[(.*)\]/\2/p' | tail -1) + echo "rpath: ${rpath:-}" + local origin found="" dirs dir + origin=$(dirname "$obj") + IFS=: read -ra dirs <<<"$rpath" for dir in ${dirs[@]+"${dirs[@]}"}; do - # $ORIGIN is relative to the binary; the ffmpeg entries use it. + # $ORIGIN is relative to the object; the ffmpeg entries use it. dir=${dir//\$ORIGIN/$origin} - if [ -e "$dir/libpipewire-0.3.so.0" ]; then + if [ -e "$dir/$soname" ]; then found="$dir" break fi done if [ -z "$found" ]; then - echo "::error::no RPATH entry of the helper holds libpipewire-0.3.so.0; its dlopen will fail on any host without an ld.so.cache" + echo "::error::no RPATH entry of the $what holds $soname; its dlopen will fail on any host without an ld.so.cache" missing=$((missing + 1)) else - echo "libpipewire-0.3.so.0 resolves from $found" + echo "$soname resolves from $found" fi # Unresolved DT_NEEDED entries are a different fault, and this is # where they would show. - if ldd "$HELPER" | grep -q "not found"; then - echo "::error::the helper has unresolved DT_NEEDED libraries" - ldd "$HELPER" | grep "not found" + if ldd "$obj" | grep -q "not found"; then + echo "::error::the $what has unresolved DT_NEEDED libraries" + ldd "$obj" | grep "not found" missing=$((missing + 1)) fi - fi + } + + check_dlopen_rpath "PipeWire helper" \ + "$(wrapper_value OPENSCREEN_LINUX_CURSOR_HELPER_EXE)" libpipewire-0.3.so.0 + check_dlopen_rpath "compositor addon" \ + "$(wrapper_value OPENSCREEN_COMPOSITOR_VIEW_NODE)" libvulkan.so.1 + [ "$missing" -eq 0 ] # Electron initialises Chromium even for the headless CLI, so it needs a diff --git a/nix/compositor-view.nix b/nix/compositor-view.nix index e939750d1..ecf8b0770 100644 --- a/nix/compositor-view.nix +++ b/nix/compositor-view.nix @@ -201,13 +201,7 @@ rustPlatform.buildRustPackage { # time rather than rewriting afterwards). patchelf --add-rpath '$ORIGIN' "$out/lib/compositor_view.node" - # The Vulkan loader, which nothing else pulls in: wgpu reaches Vulkan through - # ash's dlopen("libvulkan.so.1"), never a DT_NEEDED, which is why this - # derivation builds without it and then fails at export time. The ICD stays - # the host's job -- forcing a rasteriser would put every user with a real GPU - # into software rendering -- but the loader cannot be, because NixOS has no - # ld.so.cache and /run/opengl-driver/lib carries ICDs, not libvulkan.so.1. - patchelf --add-rpath "${lib.makeLibraryPath [ vulkan-loader ]}" "$out/lib/compositor_view.node" + # The Vulkan loader is added in postFixup, not here -- see the comment there. # What the reference build ends with (assertNoUnprefixedFfmpegImports in # scripts/build-linux-compositor-addon.mjs). preBuild's `count -eq 0` proves @@ -226,6 +220,24 @@ rustPlatform.buildRustPackage { runHook postInstall ''; + # The Vulkan loader, which nothing else pulls in: wgpu reaches Vulkan through + # ash's dlopen("libvulkan.so.1"), never a DT_NEEDED, which is why this + # derivation builds without it and then fails at export time. The ICD stays the + # host's job -- forcing a rasteriser would put every user with a real GPU into + # software rendering -- but the loader cannot be, because NixOS has no + # ld.so.cache and /run/opengl-driver/lib carries ICDs, not libvulkan.so.1. + # + # postFixup, and this was in installPhase until the PipeWire helper proved why + # that does not work: fixupPhase runs `patchelf --shrink-rpath`, which drops + # every RPATH entry no DT_NEEDED library needs -- which is the definition of an + # entry added for a dlopen. It was being added and then stripped, and the + # runner never noticed because ubuntu-latest has a system libvulkan that + # satisfies the dlopen regardless. On NixOS it would not have. + postFixup = '' + patchelf --force-rpath --add-rpath "${lib.makeLibraryPath [ vulkan-loader ]}" \ + "$out/lib/compositor_view.node" + ''; + meta = { description = "Native compositor addon for OpenScreen"; homepage = "https://github.com/getopenscreen/openscreen"; diff --git a/nix/pipewire-helper.nix b/nix/pipewire-helper.nix index 761e408f0..953171c94 100644 --- a/nix/pipewire-helper.nix +++ b/nix/pipewire-helper.nix @@ -96,7 +96,21 @@ rustPlatform.buildRustPackage { # wants DT_RPATH rather than DT_RUNPATH, so that the entries apply to the # transitive ffmpeg libraries too. patchelf defaults to writing DT_RUNPATH, # which would silently undo that choice. - postInstall = '' + # + # postFixup AND NOT postInstall, which is where this was and where it did not + # survive. stdenv's fixupPhase runs `patchelf --shrink-rpath`, which drops every + # RPATH entry no DT_NEEDED library needs -- and an entry that exists solely for a + # dlopen is, by construction, exactly what that describes. Added in postInstall + # it was stripped minutes later and the binary shipped with: + # + # $ORIGIN/helper-ffmpeg:/nix/store/...-ffmpeg-tree.../lib: + # /nix/store/...-glibc.../lib:/nix/store/...-gcc-lib/lib + # + # i.e. precisely the entries the linker could justify, and not the one that + # matters. postFixup runs at the end of fixupPhase, after the shrink, so the + # entry survives. dontPatchELF would also work and is worse: it disables the + # shrink for every output, to fix one entry. + postFixup = '' patchelf --force-rpath --add-rpath "${lib.makeLibraryPath [ pipewire ]}" \ "$out/bin/openscreen-pipewire-helper" '';