From b25b1ebf5ec07abcc9e2742b4a72377b7be6a40d Mon Sep 17 00:00:00 2001
From: Maximilian Bosch <mb@flyingcircus.io>
Date: Tue, 12 Nov 2024 16:46:38 +0100
Subject: [PATCH] FixELFRunPath: shrink & add-rpath in a single operation

FC-41616

The core issue is that each `--add-rpath` makes the ELF header a little
larger whereas `--shrink-rpath` doesn't remove the extra space allocated
by `--add-rpath`. If this isn't done in a `nix-build` where the
increased space is probably neglible, but in an imperatively managed
virtualenv (which is the case here), then the dynamic libraries will
blow up and at some point fail with a segfault when Python tries to open
those[1].

This change basically applies a `patchelf(1)` patch that provides a flag
`--add-rpath-and-shrink` which performs both operations at once so that
there's no additional space allocated in the first place.

[1] Interestingly, I failed to trigger the exact error with a `dlopen(2)`
    call in a C program.
---
 ..._092716_mb_FC_41616_patchelf_size_issue.md |  7 ++++
 src/batou_ext/python.py                       | 33 ++++++++++++++++---
 2 files changed, 36 insertions(+), 4 deletions(-)
 create mode 100644 CHANGES.d/20241115_092716_mb_FC_41616_patchelf_size_issue.md

diff --git a/CHANGES.d/20241115_092716_mb_FC_41616_patchelf_size_issue.md b/CHANGES.d/20241115_092716_mb_FC_41616_patchelf_size_issue.md
new file mode 100644
index 0000000..a80b094
--- /dev/null
+++ b/CHANGES.d/20241115_092716_mb_FC_41616_patchelf_size_issue.md
@@ -0,0 +1,7 @@
+- The component `batou_ext.python.FixELFRunPath` now uses a patched version of patchelf to make sure that the
+  dynamic libraries don't get larger per deploy.
+
+  When a certain threshold is exceeded, Python will fail to import these.
+
+  If the component got regularly executed in deployments, you may want to consider recreating
+  the virtualenv once.
diff --git a/src/batou_ext/python.py b/src/batou_ext/python.py
index 7e86c5a..7678eeb 100644
--- a/src/batou_ext/python.py
+++ b/src/batou_ext/python.py
@@ -1,6 +1,7 @@
 import os.path
 import shlex
 from glob import glob
+from textwrap import dedent
 
 import batou.component
 import batou.lib.python
@@ -217,13 +218,13 @@ def update(self):
                 return
 
             # add user env to DT_RPATH
-            self.__patchelf(["--add-rpath", directories], files_to_fix)
-            # drop everything from DT_RPATH except self.env_directory
+            # & drop everything from DT_RPATH except self.env_directory
             # and directories in the venv (to allow shared libraries from numpy
             # to load other shared libraries from numpy).
             self.__patchelf(
                 [
-                    "--shrink-rpath",
+                    "--add-rpath-and-shrink",
+                    directories,
                     "--allowed-rpath-prefixes",
                     f"{directories}:$ORIGIN",
                 ],
@@ -231,12 +232,36 @@ def update(self):
             )
 
     def __patchelf(self, args, paths):
+        # The idea behind the `pkgs.patchelf-venv or` is to move the expression
+        # into fc-nixos eventually to not rebuild patchelf on-demand (not too urgent though
+        # since the patchelf build is relatively small).
+        # If the patch gets accepted upstream, we should remove the entire dance here. If not,
+        # the platform approach will be taken.
+        patchelf_expr = shlex.quote(
+            dedent(
+                """
+        with import <nixpkgs> {}; {
+          patchelf = pkgs.patchelf-venv or patchelf.overrideAttrs ({ patches ? [], ... }: {
+            patches = patches ++ [
+              (fetchpatch {
+                url = "https://github.com/flyingcircusio/patchelf/commit/6ffde887d77275323c81c2e091891251b021abb3.patch";
+                hash = "sha256-4Qct2Ez3v6DyUG26JTWt6/tkaqB9h1gYaoaObqhvFS8=";
+              })
+            ];
+          });
+        }
+        """
+            )
+        )
+
         args_ = " ".join(shlex.quote(arg) for arg in args)
         # `--force-rpath` because we need `rpath` for $ORIGIN since rpath
         # works for all ELFs below in the dependency tree in contrast to DT_RUNPATH.
         # It's impossible to use both at the same time because DT_RPATH will always
         # be ignored then. For more context, see `ld.so(8)`.
-        patchelf = "nix run -f '<nixpkgs>' -- patchelf --force-rpath"
+        patchelf = (
+            f"nix run --impure --expr {patchelf_expr} -- patchelf --force-rpath"
+        )
         cmd = f"xargs -P {self.patchelf_jobs} {patchelf} {args_}"
         proc = self.cmd(cmd, communicate=False)