[bug#70350] [PATCH] pack: ‘-R’ (once) does not include fakechroot fallback.

[bug#70350] [PATCH] pack: ‘-R’ (once) does not include fakechroot fallback.

[Ludovic Courtès]

From: Ludovic Courtès <ludovic.courtes@inria.fr>

Previously, ‘guix pack -R’ would build a wrapper containing both the
“userns” and “fakechroot” engines, instead of providing nothing but the
“userns” engine as the manual says.  This patch fixes it.

* guix/scripts/pack.scm (wrapped-package): Add #:fakechroot?
[build]: When FAKECHROOT? is false, ‘elf-loader-compile-flags’ always
returns '().

Change-Id: Ic75cc8c36bf0a3881f299b274d78bd9fc2d4e2bb
---
 guix/scripts/pack.scm | 78 ++++++++++++++++++++++---------------------
 1 file changed, 40 insertions(+), 38 deletions(-)

Hello!

I stumbled upon the bug whereby ‘guix pack -RR’, just like (guix build
gremlins), loads entire ELF files in memory just to parse them, which
can OOM if said files are large enough:

  https://issues.guix.gnu.org/59365#4

I thought passing a single ‘-R’ would allow me to work around the
problem, since the fakechroot engine was not supposed to be compiled
in this case, but it turns out it was.

This patch makes ‘guix pack’ conform with the doc: with a single ‘-R’,
only the “userns” engine gets compiled.

Thoughts?

Ludo’.

diff --git a/guix/scripts/pack.scm b/guix/scripts/pack.scm
index 3e45c34895..fe4df042d7 100644
--- a/guix/scripts/pack.scm
+++ b/guix/scripts/pack.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2015, 2017-2023 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015, 2017-2024 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017, 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Konrad Hinsen <konrad.hinsen@fastmail.net>
 ;;; Copyright © 2018 Chris Marusich <cmmarusich@gmail.com>
@@ -1066,10 +1066,11 @@ (define* (wrapped-package package
                           #:optional
                           (output* "out")
                           (compiler (c-compiler))
-                          #:key proot?)
+                          #:key proot? (fakechroot? proot?))
   "Return the OUTPUT of PACKAGE with its binaries wrapped such that they are
 relocatable.  When PROOT? is true, include PRoot in the result and use it as a
-last resort for relocation."
+last resort for relocation.  When FAKECHROOT? is true, include
+libfakechroot.so and related ld.so machinery as a fallback."
   (define runner
     (local-file (search-auxiliary-file "run-in-namespace.c")))
 
@@ -1161,43 +1162,44 @@ (define* (wrapped-package package
           (define (elf-loader-compile-flags program)
             ;; Return the cpp flags defining macros for the ld.so/fakechroot
             ;; wrapper of PROGRAM.
+            #$(if fakechroot?
+                  ;; TODO: Handle scripts by wrapping their interpreter.
+                  #~(if (elf-file? program)
+                        (let* ((bv      (call-with-input-file program
+                                          get-bytevector-all))
+                               (elf     (parse-elf bv))
+                               (interp  (elf-interpreter elf))
+                               (gconv   (and interp
+                                             (string-append (dirname interp)
+                                                            "/gconv"))))
+                          (if interp
+                              (list (string-append "-DPROGRAM_INTERPRETER=\""
+                                                   interp "\"")
+                                    (string-append "-DFAKECHROOT_LIBRARY=\""
+                                                   #$(fakechroot-library) "\"")
 
-            ;; TODO: Handle scripts by wrapping their interpreter.
-            (if (elf-file? program)
-                (let* ((bv      (call-with-input-file program
-                                  get-bytevector-all))
-                       (elf     (parse-elf bv))
-                       (interp  (elf-interpreter elf))
-                       (gconv   (and interp
-                                     (string-append (dirname interp)
-                                                    "/gconv"))))
-                  (if interp
-                      (list (string-append "-DPROGRAM_INTERPRETER=\""
-                                           interp "\"")
-                            (string-append "-DFAKECHROOT_LIBRARY=\""
-                                           #$(fakechroot-library) "\"")
+                                    (string-append "-DLOADER_AUDIT_MODULE=\""
+                                                   #$(audit-module) "\"")
 
-                            (string-append "-DLOADER_AUDIT_MODULE=\""
-                                           #$(audit-module) "\"")
-
-                            ;; XXX: Normally (runpath #$(audit-module)) is
-                            ;; enough.  However, to work around
-                            ;; <https://sourceware.org/bugzilla/show_bug.cgi?id=26634>
-                            ;; (glibc <= 2.32), pass the whole search path of
-                            ;; PROGRAM, which presumably is a superset of that
-                            ;; of the audit module.
-                            (string-append "-DLOADER_AUDIT_RUNPATH={ "
-                                           (string-join
-                                            (map object->string
-                                                 (runpath program))
-                                            ", " 'suffix)
-                                           "NULL }")
-                            (if gconv
-                                (string-append "-DGCONV_DIRECTORY=\""
-                                               gconv "\"")
-                                "-UGCONV_DIRECTORY"))
-                      '()))
-                '()))
+                                    ;; XXX: Normally (runpath #$(audit-module)) is
+                                    ;; enough.  However, to work around
+                                    ;; <https://sourceware.org/bugzilla/show_bug.cgi?id=26634>
+                                    ;; (glibc <= 2.32), pass the whole search path of
+                                    ;; PROGRAM, which presumably is a superset of that
+                                    ;; of the audit module.
+                                    (string-append "-DLOADER_AUDIT_RUNPATH={ "
+                                                   (string-join
+                                                    (map object->string
+                                                         (runpath program))
+                                                    ", " 'suffix)
+                                                   "NULL }")
+                                    (if gconv
+                                        (string-append "-DGCONV_DIRECTORY=\""
+                                                       gconv "\"")
+                                        "-UGCONV_DIRECTORY"))
+                              '()))
+                        '())
+                  #~'()))
 
           (define (build-wrapper program)
             ;; Build a user-namespace wrapper for PROGRAM.

base-commit: 4e7337536ba41e888a601c92fada8a4adca9d2c6
-- 
2.41.0

bug#70350: [PATCH] pack: ‘-R’ (once) does not include fakechroot fallback.

[Ludovic Courtès]

Ludovic Courtès <ludo@gnu.org> skribis:

> From: Ludovic Courtès <ludovic.courtes@inria.fr>
>
> Previously, ‘guix pack -R’ would build a wrapper containing both the
> “userns” and “fakechroot” engines, instead of providing nothing but the
> “userns” engine as the manual says.  This patch fixes it.
>
> * guix/scripts/pack.scm (wrapped-package): Add #:fakechroot?
> [build]: When FAKECHROOT? is false, ‘elf-loader-compile-flags’ always
> returns '().
>
> Change-Id: Ic75cc8c36bf0a3881f299b274d78bd9fc2d4e2bb

Pushed as a15db2ee5090441c08d9a642d9284ef3ccdd95d0.

Ludo'.