Hi Ludo’, On Wed, Nov 02, 2022 at 04:50 PM, Ludovic Courtès wrote:
> Hi John, > > John Kehayias <john.kehay...@protonmail.com> skribis: > >> After commit >> <https://git.savannah.gnu.org/cgit/guix.git/commit/?id=c07b55eb94f8cfa9d0f56cfd97a16f2f7d842652> >> I noticed a changed in behavior of guix shell with the emulate-fhs option >> for a >> container. I tracked it down to the wrong glibc package appearing in the >> container, i.e. >> the standard Guix version rather than glibc-for-fhs (which reads a global ld >> cache). >> >> The cause I believe is related to <https://issues.guix.gnu.org/58859>, >> namely that >> package input order for a profile can matter. But it is slightly different >> here since >> the glibc-for-fhs package is added internally. >> >> We can see this demonstrated by comparing the FHS container with a -D input >> so that a >> glibc package is implicitly included (here from the gnu-build-system): >> >> ❯ guix shell -CFD hello coreutils >> john@narya ~/Files/UPenn/canvasgrading [env]$ ls /lib/ld* -la >> lrwxrwxrwx 1 65534 overflow 69 Jan 1 1970 /lib/ld-2.33.so -> >> /gnu/store/5h2w4qi9hk1qzzgi1w83220ydslinr4s-glibc-2.33/lib/ld-2.33.so >> lrwxrwxrwx 1 65534 overflow 79 Jan 1 1970 /lib/ld-linux-x86-64.so.2 -> >> /gnu/store/5h2w4qi9hk1qzzgi1w83220ydslinr4s-glibc-2.33/lib/ld-linux-x86-64.so.2 > > How about fixing it by moving the (alist-cons 'expression …) thing right > before the ‘options-with-caching’ call in ‘parse-args’? > > That way it would no longer be sensitive to the position of ‘-F’ on the > command line. > Good idea, that worked! I didn't think right away of an easier way of doing this, so I added another let binding to easily check for '--emulate-fhs' in the parsed arguments. > Could you give it a try and add a test? > I added a test that explicitly includes 'glibc' in the 'guix shell' invocation and checked the link to '/lib/libc.so' was from 'glibc-for-fhs'. Again, not sure if there is a better way here, but the test does pass now and fails without the change you proposed. I also checked against the examples I gave originally and looked good there too. Patch attached. I included an explanation (and link) of this bug and the fix in the commit message. Thanks and let me know if there is anything to improve here! John
From 72be4a15a10916ae8d51dfb2998d6179bc57be59 Mon Sep 17 00:00:00 2001 From: John Kehayias <john.kehay...@protonmail.com> Date: Thu, 3 Nov 2022 14:25:09 -0400 Subject: [PATCH] shell: Fix '--emulate-fhs' sometimes not including 'glibc-for-fhs'. Fixes <https://issues.guix.gnu.org/58861>. Previously the order of the options giving to 'guix shell' could mean that the 'glibc-for-fhs' package included with the '--emulate-fhs' option would not appear in the container. For example, using the development option with a package using the 'gnu-build-system', e.g. 'guix shell -CFD hello', would include the regular 'glibc' package. The option ordered mattered: 'guix shell -CD hello -F' would include the expected 'glibc-for-fhs'. We fix this by having 'glibc-for-fhs' added to the package list just before calling 'options-with-caching' so the option order given by the user does not matter. * guix/scripts/shell.scm (%options): Move the '--emulate-fhs' (expression . ...) component from here... (parse-args): ... to here. * tests/guix-environment-container.sh: Add a test to check that 'glibc-for-fhs' is in the container even when 'glibc' is included in the 'guix shell' package list. --- guix/scripts/shell.scm | 26 ++++++++++++++------------ tests/guix-environment-container.sh | 10 ++++++++++ 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/guix/scripts/shell.scm b/guix/scripts/shell.scm index a2836629ad..f334bd57ae 100644 --- a/guix/scripts/shell.scm +++ b/guix/scripts/shell.scm @@ -143,16 +143,7 @@ (define %options (option '(#\F "emulate-fhs") #f #f (lambda (opt name arg result) - (let ((result - ;; For an FHS-container, add the (hidden) - ;; package glibc-for-fhs which uses the global - ;; cache at /etc/ld.so.cache. - (alist-cons - 'expression - '(ad-hoc-package - "(@@ (gnu packages base) glibc-for-fhs)") - result))) - (alist-cons 'emulate-fhs? #t result))))) + (alist-cons 'emulate-fhs? #t result)))) (filter-map (lambda (opt) (and (not (any (lambda (name) (member name to-remove)) @@ -173,8 +164,19 @@ (define (parse-args args) ;; The '--' token is used to separate the command to run from the rest of ;; the operands. (let ((args command (break (cut string=? "--" <>) args))) - (let ((opts (parse-command-line args %options (list %default-options) - #:argument-handler handle-argument))) + (let* ((args-parsed (parse-command-line args %options (list %default-options) + #:argument-handler handle-argument)) + ;; For an FHS-container, add the (hidden) package glibc-for-fhs + ;; which uses the global cache at /etc/ld.so.cache. We handle + ;; adding this package here to ensure it will always appear in the + ;; container as it is the first package in OPTS. + (opts (if (assoc-ref args-parsed 'emulate-fhs?) + (alist-cons + 'expression + '(ad-hoc-package + "(@@ (gnu packages base) glibc-for-fhs)") + args-parsed) + args-parsed))) (options-with-caching (auto-detect-manifest (match command diff --git a/tests/guix-environment-container.sh b/tests/guix-environment-container.sh index f233c3fcc0..fb2c19b193 100644 --- a/tests/guix-environment-container.sh +++ b/tests/guix-environment-container.sh @@ -1,5 +1,6 @@ # GNU Guix --- Functional package management for GNU # Copyright © 2015 David Thompson <da...@gnu.org> +# Copyright © 2022 John Kehayias <john.kehay...@protonmail.com> # # This file is part of GNU Guix. # @@ -231,3 +232,12 @@ guix shell -C --emulate-fhs --bootstrap guile-bootstrap \ # Test that the ld cache was generated and can be successfully read. guix shell -CF --bootstrap guile-bootstrap \ -- guile -c '(execlp "ldconfig" "ldconfig" "-p")' + +# Test that the package glibc-for-fhs is in the container even if there is the +# regular glibc package from another source. See +# <https://issues.guix.gnu.org/58861>. +guix shell -CF --bootstrap guile-bootstrap glibc \ + -- guile -c '(exit (if (string-contains (readlink "/lib/libc.so") + "glibc-for-fhs") + 0 + 1))' -- 2.38.0
