bug#58861: guix shell emulate-fhs option can have wrong glibc package

bug#58861: guix shell emulate-fhs option can have wrong glibc package

[John Kehayias via Bug reports for GNU Guix]

Hi Guixers,

(cc'ing Ludo’ as author of the commit referenced below)

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):

--8<---------------cut here---------------start------------->8---
❯ 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
--8<---------------cut here---------------end--------------->8---

Note that the loader comes from the standard glibc package. This means it won't read from the global cache.

However, if we change the order, placing the FHS option after the (implicit) glibc input, we do get the glibc-for-fhs package. This is similar to #58859 which I just reported:

--8<---------------cut here---------------start------------->8---
❯ guix shell -CD hello -F coreutils
The following derivation will be built:
  /gnu/store/1hvdkgp68nak827qx6vhmrixdixnl6yl-profile.drv

building CA certificate bundle...
listing Emacs sub-directories...
building fonts directory...
building directory of Info manuals...
building profile with 23 packages...

[env]$ ls /lib/ld* -la
lrwxrwxrwx 1 65534 overflow 77 Jan  1  1970 /lib/ld-2.33.so -> /gnu/store/dhd4a04vxs6nzz0kqnhp0f2sm1q1xbkq-glibc-for-fhs-2.33/lib/ld-2.33.so
lrwxrwxrwx 1 65534 overflow 87 Jan  1  1970 /lib/ld-linux-x86-64.so.2 -> /gnu/store/dhd4a04vxs6nzz0kqnhp0f2sm1q1xbkq-glibc-for-fhs-2.33/lib/ld-linux-x86-64.so.2
--8<---------------cut here---------------end--------------->8---

Here the ld loader is as it should be for the FHS container.

This was not the behavior before 8b192c5550213911f930594f4fd7386f36618237, where the option handling was moved to shell rather than environment for emulate-fhs. Reverting this commit and doing the same thing, I get

--8<---------------cut here---------------start------------->8---
❯ ./pre-inst-env guix shell -CFD hello coreutils

[env]$ ls -la /lib/ld*
lrwxrwxrwx 1 65534 overflow 77 Jan  1  1970 /lib/ld-2.33.so -> /gnu/store/dhd4a04vxs6nzz0kqnhp0f2sm1q1xbkq-glibc-for-fhs-2.33/lib/ld-2.33.so
lrwxrwxrwx 1 65534 overflow 87 Jan  1  1970 /lib/ld-linux-x86-64.so.2 -> /gnu/store/dhd4a04vxs6nzz0kqnhp0f2sm1q1xbkq-glibc-for-fhs-2.33/lib/ld-linux-x86-64.so.2

[env]$ exit

❯ ./pre-inst-env guix shell -CD hello -F coreutils

[env]$ ls -la /lib/ld*
lrwxrwxrwx 1 65534 overflow 77 Jan  1  1970 /lib/ld-2.33.so -> /gnu/store/dhd4a04vxs6nzz0kqnhp0f2sm1q1xbkq-glibc-for-fhs-2.33/lib/ld-2.33.so
lrwxrwxrwx 1 65534 overflow 87 Jan  1  1970 /lib/ld-linux-x86-64.so.2 -> /gnu/store/dhd4a04vxs6nzz0kqnhp0f2sm1q1xbkq-glibc-for-fhs-2.33/lib/ld-linux-x86-64.so.2
--8<---------------cut here---------------end--------------->8---

Both cases have the expected behavior. The glibc-for-fhs package being added to the profile is done last, when creating the manifest, so I think it is the last package in the list and thus "wins out" over the glibc from the --development input (in keeping with the behavior noted in #58859).

Further, I don't reproduce the bug that the commit above was supposed to fix: running the same FHS container shell multiple times (so the profile is cached) does not give me any errors. Although I didn't test for this specifically before the final FHS patches, I did (and do) use the same cached profiles repeatedly.

Was the bug in using the --profile option in combination with --emulate-fhs? I haven't tested that, but I could see that being the problem instead.

Assuming there is a problem with profiles and emulate-fhs, what is the best fix here? My guess is to put the glibc-for-fhs package always last to make sure it is the glibc of the profile in this case to always have the same (expected) behavior.

Thanks!
John

bug#58861: guix shell emulate-fhs option can have wrong glibc package

[zimoun]

Hi,

On sam., 29 oct. 2022 at 05:31, John Kehayias via Bug reports for GNU Guix <bug-guix@gnu.org> wrote:

> --8<---------------cut here---------------start------------->8---
> ❯ guix shell -CFD hello coreutils

> ❯ guix shell -CD hello -F coreutils
> --8<---------------cut here---------------end--------------->8---

Unrelated, “guix package” provides some ’%actions’ and as reported in
[1], the command-line order matters – when it should not be.  Maybe, as
proposed in [1], “guix shell” could process a « plan » with always the
same order, whatever the command-line order is.

1: <http://issues.guix.gnu.org/issue/50473>


Cheers,
simon

bug#58861: guix shell emulate-fhs option can have wrong glibc package

[John Kehayias via Bug reports for GNU Guix]

Hi simon and Ludo’,

Before I forget, nckx helpfully pointed out that I linked to the wrong commit, which made this all the more confusing. The correct commit is

<https://git.savannah.gnu.org/cgit/guix.git/commit/?id=8b192c5550213911f930594f4fd7386f36618237>

On Wed, Nov 02, 2022 at 01:47 PM, zimoun wrote:

> Hi,
>
> On sam., 29 oct. 2022 at 05:31, John Kehayias via Bug reports for GNU Guix
> <bug-guix@gnu.org> wrote:
>
>> --8<---------------cut here---------------start------------->8---
>> ❯ guix shell -CFD hello coreutils
>
>> ❯ guix shell -CD hello -F coreutils
>> --8<---------------cut here---------------end--------------->8---
>
> Unrelated, “guix package” provides some ’%actions’ and as reported in
> [1], the command-line order matters – when it should not be.  Maybe, as
> proposed in [1], “guix shell” could process a « plan » with always the
> same order, whatever the command-line order is.
>
> 1: <http://issues.guix.gnu.org/issue/50473>
>

Thanks, I'll take a look. Seems like we may want to have a more systematic method here.

bug#58861: guix shell emulate-fhs option can have wrong glibc package

[Ludovic Courtès]

Hi John,

John Kehayias <john.kehayias@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.

Could you give it a try and add a test?

Thanks,
Ludo’.

bug#58861: [PATCH] shell: Fix '--emulate-fhs' sometimes not including 'glibc-for-fhs'.

[John Kehayias via Bug reports for GNU Guix]

[-- Attachment #1: Type: text/plain, Size: 2339 bytes --]

Hi Ludo’,

On Wed, Nov 02, 2022 at 04:50 PM, Ludovic Courtès wrote:

> Hi John,
>
> John Kehayias <john.kehayias@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

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-shell-Fix-emulate-fhs-sometimes-not-including-glibc-.patch --]
[-- Type: text/x-patch; name=0001-shell-Fix-emulate-fhs-sometimes-not-including-glibc-.patch, Size: 4870 bytes --]

From 72be4a15a10916ae8d51dfb2998d6179bc57be59 Mon Sep 17 00:00:00 2001
From: John Kehayias <john.kehayias@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 <davet@gnu.org>
+# Copyright © 2022 John Kehayias <john.kehayias@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

bug#58861: [PATCH] shell: Fix '--emulate-fhs' sometimes not including 'glibc-for-fhs'.

[Ludovic Courtès]

Hi,

John Kehayias <john.kehayias@protonmail.com> skribis:

> From 72be4a15a10916ae8d51dfb2998d6179bc57be59 Mon Sep 17 00:00:00 2001
> From: John Kehayias <john.kehayias@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.

Perfect; applied, thanks!

Ludo’.