guix shell emulate-fhs option can have wrong glibc package

  • Done
  • quality assurance status badge
Details
3 participants
  • John Kehayias
  • Ludovic Courtès
  • zimoun
Owner
unassigned
Submitted by
John Kehayias
Severity
normal
J
J
John Kehayias wrote on 29 Oct 2022 07:31
(name . Guix Bugs)(address . bug-guix@gnu.org)(name . Ludovic Courtès)(address . ludo@gnu.org)
87tu3n9oxd.fsf@protonmail.com
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):

Toggle snippet (6 lines)
? 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

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:

Toggle snippet (15 lines)
? 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

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

Toggle snippet (15 lines)
? ./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

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
Z
Z
zimoun wrote on 2 Nov 2022 13:47
(name . Ludovic Courtès)(address . ludo@gnu.org)
87cza5frq6.fsf@gmail.com
Hi,

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

Toggle quote (6 lines)
> --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.



Cheers,
simon
J
J
John Kehayias wrote on 2 Nov 2022 16:50
(name . zimoun)(address . zimon.toutoune@gmail.com)
87y1st73vk.fsf@protonmail.com
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


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

Toggle quote (19 lines)
> 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.
L
L
Ludovic Courtès wrote on 2 Nov 2022 16:50
(name . John Kehayias)(address . john.kehayias@protonmail.com)(address . 58861@debbugs.gnu.org)
87sfj1tkwy.fsf@gnu.org
Hi John,

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

Toggle quote (11 lines)
> 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’.
J
J
John Kehayias wrote on 3 Nov 2022 19:50
[PATCH] shell: Fix '--emulate-fhs' sometimes not including 'glibc-for-fhs'.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 58861@debbugs.gnu.org)
87pme3dg9s.fsf@protonmail.com
Hi Ludo’,

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

Toggle quote (31 lines)
> 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.

Toggle quote (3 lines)
> 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.kehayias@protonmail.com>
Date: Thu, 3 Nov 2022 14:25:09 -0400
Subject: [PATCH] shell: Fix '--emulate-fhs' sometimes not including
'glibc-for-fhs'.
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(-)
Toggle diff (70 lines)
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
L
L
Ludovic Courtès wrote on 6 Nov 2022 12:18
(name . John Kehayias)(address . john.kehayias@protonmail.com)(address . 58861-done@debbugs.gnu.org)
87mt94gwl7.fsf@gnu.org
Hi,

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

Toggle quote (24 lines)
> 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’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 58861@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 58861
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch