guix shell --check: confusing error message

OpenSubmitted by Chris Marusich.
Details
8 participants
  • Thiago Jung Bauermann
  • bokr
  • Chris Marusich
  • Josselin Poiret
  • Ludovic Courtès
  • Maxime Devos
  • Tobias Geerinckx-Rice
  • (
Owner
unassigned
Severity
normal
Merged with
C
C
Chris Marusich wrote on 19 Jan 04:29 +0100
(address . bug-guix@gnu.org)
87h7a08kub.fsf@gmail.com
Hi,

I've grown so used to using "guix environment," I thought I'd try out
"guix shell." It looks pretty neat! It's good to try to improve the
CLI.

However, when I tried "guix shell," I quickly observed this confusing
behavior:

Toggle snippet (57 lines)
[130] marusich@suzaku:~/guix-master
$ guix shell --container --check -D guix
guix shell: checking the environment variables visible from shell '/bin/bash'...
guix shell: warning: variable 'PKG_CONFIG_PATH' is missing from shell environment
hint: One or more environment variables have a different value in the shell than
the one we set. This means that you may find yourself running code in an
environment different from the one you asked Guix to prepare.

This usually indicates that your shell startup files are unexpectedly
modifying those environment variables. For example, if you are using Bash,
make sure that environment variables are set or modified in
`~/.bash_profile' and _not_ in `~/.bashrc'. For more information on Bash
startup files, run:

info "(bash) Bash Startup Files"

Alternatively, you can avoid the problem by passing the `--container' or
`-C' option. That will give you a fully isolated environment running in a
"container", immune to the issue described above.

[1] marusich@suzaku:~/guix-master
$ env | grep PKG_CONF
[1] marusich@suzaku:~/guix-master
$ guix shell --check -D guix
guix shell: checking the environment variables visible from shell '/bin/bash'...
guix shell: warning: variable 'PKG_CONFIG_PATH' is missing from shell environment
hint: One or more environment variables have a different value in the shell than
the one we set. This means that you may find yourself running code in an
environment different from the one you asked Guix to prepare.

This usually indicates that your shell startup files are unexpectedly
modifying those environment variables. For example, if you are using Bash,
make sure that environment variables are set or modified in
`~/.bash_profile' and _not_ in `~/.bashrc'. For more information on Bash
startup files, run:

info "(bash) Bash Startup Files"

Alternatively, you can avoid the problem by passing the `--container' or
`-C' option. That will give you a fully isolated environment running in a
"container", immune to the issue described above.

[1] marusich@suzaku:~/guix-master
$ guix shell -D guix
[0] [env] marusich@suzaku:~/guix-master
$ env | grep PKG
PKG_CONFIG_PATH=/gnu/store/qr79b2m6cfdj8ar7g0psqg4hglm6djfm-profile/lib/pkgconfig
[0] [env] marusich@suzaku:~/guix-master
$
exit
[0] marusich@suzaku:~/guix-master
$ guix shell --container -D guix
marusich@suzaku ~/guix-master [env]$ env | grep PKG
PKG_CONFIG_PATH=/gnu/store/qr79b2m6cfdj8ar7g0psqg4hglm6djfm-profile/lib/pkgconfig
marusich@suzaku ~/guix-master [env]$

I found the following things to be confusing:

(1) The error message claims that PKG_CONFIG_PATH is "missing from shell
environment." However, it seems to be present when I run "env".

(2) It says I can avoid the problem by passing the `--container' option,
but even when I do that, the problem seems to persist. If that is
expected behavior, then perhaps the wording should be changed to
something less certain, such as "you might be able to avoid the
problem". It does not seem to be the case that I can avoid the problem
by passing the `--container' option in this case.

What's really going on here? It's good to be able to look at this
feature with the eyes of a newbie, since I'm very used to using "guix
environment", but "guix shell" is totally new to me. I thought it would
be a good opportunity to provide feedback.

--
Chris

-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAmHnhXwVHGNtbWFydXNp
Y2hAZ21haWwuY29tAAoJEN1AmhXYIkadWR8QANlUC+4jvnGuBlBna2hZl3Qzups+
YBT9EryqYYJxPMY/9IY/cq62nwgtr15WGy03jZjox2cRfimgMBN4u24eY9Mx51hn
Ew7IQR/Cq951q6QOP077Av1hZ7Ey3kJPtln6aUYbLfzo8AT/8/iUpjJZ19vxPiye
gsP4/6AtoCqMFKfw6KQubHzaJAhOJ7I08Ztyr5DYx5IEjhnT/T2D/hk4gMggNs2y
gaWzw/9FXISYl88rsbB3e0WsWBmtvbXkY5wZLJqytcyfmVhINHcAAWof+OhsA0CK
Ngb8uNXtwn4eB1/O6wSCCKk8pDOlZwZ/PzmmlIKHMwRlTu1EVe+r0eF2+dvyjWRM
L7d5hlFYkCBl/y45IYhEE+W3GuDvZzSYenYVoa5KiRPe/dbFvBlwkA7p/5jCZBNJ
yok5EF3t6/EQhicuTYEGfqJUdQHPwXeQpuIYP59dOsuxwa6KkuzTP6kmMM8rJtjL
FjgB9tq4dAX+x49Oo37mwvG0jjMoJdIVRZBBmhloenPPJLqbdK5T1YEnN3PCwfXW
gqT181jznOFXcFCz1/6KzPlDdyb7dXOi9ZyaOrGJSGt17PKKc04YHBLb+d/2sPKC
pvohNZM+VJlwLkn0JSuX2Pv+YbuSI+U+T31/F0wdSgYlj0R8Z1vYqj1S1wSrep0y
2esEmSKqCxXR2V/s
=E1CA
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 24 Jan 15:35 +0100
(name . Chris Marusich)(address . cmmarusich@gmail.com)(address . 53355@debbugs.gnu.org)
87lez5td4n.fsf@gnu.org
Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (5 lines)
> [130] marusich@suzaku:~/guix-master
> $ guix shell --container --check -D guix
> guix shell: checking the environment variables visible from shell '/bin/bash'...
> guix shell: warning: variable 'PKG_CONFIG_PATH' is missing from shell environment

[...]

Toggle quote (12 lines)
> I found the following things to be confusing:
>
> (1) The error message claims that PKG_CONFIG_PATH is "missing from shell
> environment." However, it seems to be present when I run "env".
>
> (2) It says I can avoid the problem by passing the `--container' option,
> but even when I do that, the problem seems to persist. If that is
> expected behavior, then perhaps the wording should be changed to
> something less certain, such as "you might be able to avoid the
> problem". It does not seem to be the case that I can avoid the problem
> by passing the `--container' option in this case.

What’s confusing is that ‘--check’ does the same job whether or not
‘--container’ is passed: it checks the behavior of your shell *outside*
a container.

I think ‘--check’ should just do nothing when ‘--container’ is used,
possibly emitting a warning saying it’s not doing anything (patch
below).

Now, the diagnostic is hopefully correct if you use, say, ‘--pure’
instead of ‘--container’. Could you check whether this is the case?

Thanks,
Ludo’.
Toggle diff (24 lines)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 510cee727f..ec071402f4 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -1,6 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014, 2015, 2018 David Thompson <davet@gnu.org>
-;;; Copyright © 2015, 2016, 2017, 2018, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015-2022 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2018 Mike Gerwitz <mtg@gnu.org>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -975,7 +975,10 @@ (define manifest
 
                   (mwhen (assoc-ref opts 'check?)
                     (return
-                     (validate-child-shell-environment profile manifest)))
+                     (if container?
+                         (warning (G_ "'--check' is unnecessary \
+when using '--container'; doing nothing~%"))
+                         (validate-child-shell-environment profile manifest))))
 
                   (cond
                    ((assoc-ref opts 'search-paths)
C
C
Chris Marusich wrote on 25 Jan 01:55 +0100
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 53355@debbugs.gnu.org)
87sftc4osu.fsf@gmail.com
Hi Ludo,

Thank you for the response!

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

Toggle quote (11 lines)
> What’s confusing is that ‘--check’ does the same job whether or not
> ‘--container’ is passed: it checks the behavior of your shell *outside*
> a container.
>
> I think ‘--check’ should just do nothing when ‘--container’ is used,
> possibly emitting a warning saying it’s not doing anything (patch
> below).
>
> Now, the diagnostic is hopefully correct if you use, say, ‘--pure’
> instead of ‘--container’. Could you check whether this is the case?

That sounds reasonable. I tested your patch. It now correctly emits a
warning when both --container and --check are provided. However, I now
see that the issue occurs even when --container is omitted. So it seems
like something else might be going on.

Below, I'll provide details of what I did to test your patch.

In one test, I committed your change locally and did "guix pull" to
install the patched Guix into ~/tmpguixprofile. I then tried using it:

Toggle snippet (9 lines)
[0] marusich@suzaku:~/guix-master
$ env -i bash
[0] marusich@suzaku:/home/marusich/guix-master
$ activate-profile ~/tmpguixprofile
[0] marusich@suzaku:/home/marusich/guix-master
$ which guix
/home/marusich/tmpguixprofile/bin/guix

First, I tried without --pure or --container. Below, you can see that
it claims LIBRARY_PATH is missing, but it does not seem to be missing:

Toggle snippet (28 lines)
[0] marusich@suzaku:/home/marusich/guix-master
$ guix shell --check -D guix
guix shell: checking the environment variables visible from shell '/bin/sh'...
guix shell: warning: variable 'LIBRARY_PATH' is missing from shell environment
hint: One or more environment variables have a different value in the shell than
the one we set. This means that you may find yourself running code in an
environment different from the one you asked Guix to prepare.

This usually indicates that your shell startup files are unexpectedly
modifying those environment variables. For example, if you are using Bash,
make sure that environment variables are set or modified in
`~/.bash_profile' and _not_ in `~/.bashrc'. For more information on Bash
startup files, run:

info "(bash) Bash Startup Files"

Alternatively, you can avoid the problem by passing the `--container' or
`-C' option. That will give you a fully isolated environment running in a
"container", immune to the issue described above.

[1] marusich@suzaku:/home/marusich/guix-master
$ env | grep LIBRARY_PATH
[1] marusich@suzaku:/home/marusich/guix-master
$ guix shell -D guix
[0] \u@\H:\w\n$ env | grep LIBRARY_PATH
LIBRARY_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib

Next, I tried with --container - you can see it works as intended:

Toggle snippet (5 lines)
[0] marusich@suzaku:/home/marusich/guix-master
$ guix shell --container --check -D guix
guix shell: warning: '--check' is unnecessary when using '--container'; doing nothing

Next, I tried with --pure and --check - once again, it claims
LIBRARY_PATH is missing, even though it does not seem to be missing:

Toggle snippet (28 lines)
[0] marusich@suzaku:/home/marusich/guix-master
$ guix shell --pure --check -D guix
guix shell: checking the environment variables visible from shell '/bin/sh'...
guix shell: warning: variable 'LIBRARY_PATH' is missing from shell environment
hint: One or more environment variables have a different value in the shell than
the one we set. This means that you may find yourself running code in an
environment different from the one you asked Guix to prepare.

This usually indicates that your shell startup files are unexpectedly
modifying those environment variables. For example, if you are using Bash,
make sure that environment variables are set or modified in
`~/.bash_profile' and _not_ in `~/.bashrc'. For more information on Bash
startup files, run:

info "(bash) Bash Startup Files"

Alternatively, you can avoid the problem by passing the `--container' or
`-C' option. That will give you a fully isolated environment running in a
"container", immune to the issue described above.

[1] marusich@suzaku:/home/marusich/guix-master
$ env | grep LIBRARY_PATH
[1] marusich@suzaku:/home/marusich/guix-master
$ guix shell --pure -D guix
$ env | grep LIBRARY_PATH
LIBRARY_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib

A similar error message occurs if I try the same steps from a checkout
using pre-inst-env. However, in that case the offending environment
variable is different (PKG_CONFIG_PATH in this case):

Toggle snippet (62 lines)
[0] marusich@suzaku:~/guix-master
$ guix environment guix
[0] [env] marusich@suzaku:~/guix-master
$ ./pre-inst-env guix shell --check -D guix -- bash -c 'echo in env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"'
guix shell: checking the environment variables visible from shell '/bin/bash'...
guix shell: warning: variable 'PKG_CONFIG_PATH' is missing from shell environment
hint: One or more environment variables have a different value in the shell than
the one we set. This means that you may find yourself running code in an
environment different from the one you asked Guix to prepare.

This usually indicates that your shell startup files are unexpectedly
modifying those environment variables. For example, if you are using Bash,
make sure that environment variables are set or modified in
`~/.bash_profile' and _not_ in `~/.bashrc'. For more information on Bash
startup files, run:

info "(bash) Bash Startup Files"

Alternatively, you can avoid the problem by passing the `--container' or
`-C' option. That will give you a fully isolated environment running in a
"container", immune to the issue described above.

[1] [env] marusich@suzaku:~/guix-master
$ ./pre-inst-env guix shell --check --pure -D guix -- bash -c 'echo in env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"'
guix shell: checking the environment variables visible from shell '/bin/bash'...
guix shell: warning: variable 'PKG_CONFIG_PATH' is missing from shell environment
hint: One or more environment variables have a different value in the shell than
the one we set. This means that you may find yourself running code in an
environment different from the one you asked Guix to prepare.

This usually indicates that your shell startup files are unexpectedly
modifying those environment variables. For example, if you are using Bash,
make sure that environment variables are set or modified in
`~/.bash_profile' and _not_ in `~/.bashrc'. For more information on Bash
startup files, run:

info "(bash) Bash Startup Files"

Alternatively, you can avoid the problem by passing the `--container' or
`-C' option. That will give you a fully isolated environment running in a
"container", immune to the issue described above.

[1] [env] marusich@suzaku:~/guix-master
$ ./pre-inst-env guix shell --check --container -D guix -- bash -c 'echo in env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"'
guix shell: warning: '--check' is unnecessary when using '--container'; doing nothing
in env, PKG_CONFIG_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib/pkgconfig
[0] [env] marusich@suzaku:~/guix-master
$ ./pre-inst-env guix shell -D guix -- bash -c 'echo in env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"'
in env, PKG_CONFIG_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib/pkgconfig:/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib/pkgconfig
[0] [env] marusich@suzaku:~/guix-master
$ ./pre-inst-env guix shell --pure -D guix -- bash -c 'echo in env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"'
in env, PKG_CONFIG_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib/pkgconfig
[0] [env] marusich@suzaku:~/guix-master
$ ./pre-inst-env guix shell --container -D guix -- bash -c 'echo in env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"'
in env, PKG_CONFIG_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib/pkgconfig
[0] [env] marusich@suzaku:~/guix-master
$ echo out of env, PKG_CONFIG_PATH="$PKG_CONFIG_PATH"
out of env, PKG_CONFIG_PATH=/gnu/store/hvcq6yjfjjc7060pq09zm1rj02mivg4h-profile/lib/pkgconfig
[0] [env] marusich@suzaku:~/guix-master
$

It seems this issue happens regardless of whether I use pre-inst-env or
run Guix from a "guix pull" installation.

--
Chris

-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAmHvSnEVHGNtbWFydXNp
Y2hAZ21haWwuY29tAAoJEN1AmhXYIkad4AQP/1qZY+j1OLqPK4vjC1ij47bnArpM
JlXENik81qBkJZmpPiH3O24xylMoUJ/zQOwdLwnBL8pdpkt2Y+4NzwuLwKUqKoOf
djbeWuD0UwdWHC9ICi489mOb51YlZT5lTp98tqrbjuiqAEo72OtcFr9LpV4wQk6n
WJ35q0wbhyom73uwCGKNj08+bwaM+74Jt5Y0V/k+RlppEF7QP9wzK6JWWFYKsNQw
nlKuwAOZGYfPSXOJGMy8Rtgiqc//UEQaho3qdW1ZdLM+XmL9iydHfwVTGUf2QVVT
RSf9Os1lTp+XxjPtsa3Q4uwTTRKSkpwNLsMDqVOoDnBtjNSqayIkVtfkMKD9uMKu
6k+1+y0dPUK04yIxVuPryZ1H4qStaU+Fd0V174s5gDEt5ujGaTUgMLc/Om/HBSdw
nTD8WKQX0cFZaOUCUZpfRYFUlJdXS4QloaHErg2T5Nl0InVk3SpkPsHf7e2hO/tK
Hmc5nr0XrHz7G9Wz44Y1GGjeVcsyWFejh1kR+n8WBvF4TW4+ohdrFsWxe0NCyNCl
BlG2juHRV+INYy8MlItge4FInY/an4XcdaXEwWjMOb8+Md+K2gxCdDAewhMQP8pb
XES4PyifT/ncCcfQ3BHox3PpuT8MVlL/0/zUaBgjR2xTCcQQIetJn9D+EHa8rhQv
rrGPp0GLc3mueS6R
=wlPY
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 25 Jan 14:39 +0100
(name . Chris Marusich)(address . cmmarusich@gmail.com)(address . 53355@debbugs.gnu.org)
87h79slysd.fsf@gnu.org
Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (16 lines)
> Ludovic Courtès <ludo@gnu.org> writes:
>
>> What’s confusing is that ‘--check’ does the same job whether or not
>> ‘--container’ is passed: it checks the behavior of your shell *outside*
>> a container.
>>
>> I think ‘--check’ should just do nothing when ‘--container’ is used,
>> possibly emitting a warning saying it’s not doing anything (patch
>> below).
>>
>> Now, the diagnostic is hopefully correct if you use, say, ‘--pure’
>> instead of ‘--container’. Could you check whether this is the case?
>
> That sounds reasonable. I tested your patch. It now correctly emits a
> warning when both --container and --check are provided.

Great, I’ll commit it.

Toggle quote (3 lines)
> First, I tried without --pure or --container. Below, you can see that
> it claims LIBRARY_PATH is missing, but it does not seem to be missing:

[...]

Toggle quote (3 lines)
> Next, I tried with --pure and --check - once again, it claims
> LIBRARY_PATH is missing, even though it does not seem to be missing:

It looks like the shell-check machinery is misdiagnosing things, as
Vagrant reported in https://issues.guix.gnu.org/51466 (is this on
Debian too?).

Could you try the debugging tricks I proposed there?

TIA,
Ludo’.
C
C
Chris Marusich wrote on 2 Feb 08:11 +0100
control message for bug #53355
(address . control@debbugs.gnu.org)
87wnid3fqy.fsf@gmail.com
merge 53355 51466
quit
L
L
Ludovic Courtès wrote on 8 Mar 20:07 +0100
Re: bug#53355: guix shell --check: confusing error message
(name . Chris Marusich)(address . cmmarusich@gmail.com)
87ilso461i.fsf_-_@gnu.org
Hi Chris,

Did you have a chance to look into it?


TIA. :-)

Ludo’.

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

Toggle quote (52 lines)
> Hi Chris,
>
> Thanks for debugging this!
>
> Chris Marusich <cmmarusich@gmail.com> skribis:
>
>> From c3eea81846ae71a246e6b592be74062f4bf26474 Mon Sep 17 00:00:00 2001
>> From: Chris Marusich <cmmarusich@gmail.com>
>> Date: Sun, 13 Feb 2022 14:15:14 -0800
>> Subject: [PATCH] environment: Prevent PS1 from clobbering output in 'check'.
>>
>> Fixes: <https://issues.guix.gnu.org/51466>.
>>
>> * guix/scripts/environment.scm (child-shell-environment): In the script
>> executed the child shell, set PS1 to an empty value and then echo three
>> sentinel lines to try to "flush" the original PS1 value before printing the
>> environment variables. In the parent process, read and discard all lines up
>> to and including the last sentinel line. After that, read the remaining lines
>> as usual.
>
> [...]
>
>> + ;; Why print "GUIX_FLUSH" a few times? We are trying to "flush" the
>> + ;; original PS1 value to the port so we can read it (and discard it)
>> + ;; before we start reading the environment variables from the port. If we
>> + ;; don't do this, the original PS1 value can sometimes get interleaved
>> + ;; into the output, which interferes with our parsing logic. It's a hack,
>> + ;; but in practice it seems to do the job. If you know of a more graceful
>> + ;; solution, please implement it! See: https://issues.guix.gnu.org/51466
>> + "PS1=; for i in 1 2 3; do echo GUIX_FLUSH_$i; done; \
>> +env || /usr/bin/env || set; echo GUIX-CHECK-DONE; read x; exit\n")
>
> So you confirm that a single “echo” is not enough, right?
>
> Perhaps we should unroll the ‘for’ loop for portability, to be on the
> safe side. Initially I tested with Bash, Zsh, and Fish:
>
> https://issues.guix.gnu.org/51285#0-lineno49
>
> I think Fish has a very non-POSIX syntax, hence the suggestion to avoid
> ‘for’.
>
> I realized that setting PS1 could interfere with the logic below that
> checks for PS1. And since it doesn’t seem to help, perhaps we can
> remove “PS1=;”?
>
> Thoughts?
>
> Sorry to answer with yet more questions!
>
> Thanks,
> Ludo’.
L
L
Ludovic Courtès wrote on 20 May 23:37 +0200
(name . Chris Marusich)(address . cmmarusich@gmail.com)
8735h3x4w2.fsf@gnu.org
Hi Chris,

A friendly ping. :-)

Ludo’.

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

Toggle quote (64 lines)
> Hi Chris,
>
> Did you have a chance to look into it?
>
> https://issues.guix.gnu.org/53355
>
> TIA. :-)
>
> Ludo’.
>
> Ludovic Courtès <ludo@gnu.org> skribis:
>
>> Hi Chris,
>>
>> Thanks for debugging this!
>>
>> Chris Marusich <cmmarusich@gmail.com> skribis:
>>
>>> From c3eea81846ae71a246e6b592be74062f4bf26474 Mon Sep 17 00:00:00 2001
>>> From: Chris Marusich <cmmarusich@gmail.com>
>>> Date: Sun, 13 Feb 2022 14:15:14 -0800
>>> Subject: [PATCH] environment: Prevent PS1 from clobbering output in 'check'.
>>>
>>> Fixes: <https://issues.guix.gnu.org/51466>.
>>>
>>> * guix/scripts/environment.scm (child-shell-environment): In the script
>>> executed the child shell, set PS1 to an empty value and then echo three
>>> sentinel lines to try to "flush" the original PS1 value before printing the
>>> environment variables. In the parent process, read and discard all lines up
>>> to and including the last sentinel line. After that, read the remaining lines
>>> as usual.
>>
>> [...]
>>
>>> + ;; Why print "GUIX_FLUSH" a few times? We are trying to "flush" the
>>> + ;; original PS1 value to the port so we can read it (and discard it)
>>> + ;; before we start reading the environment variables from the port. If we
>>> + ;; don't do this, the original PS1 value can sometimes get interleaved
>>> + ;; into the output, which interferes with our parsing logic. It's a hack,
>>> + ;; but in practice it seems to do the job. If you know of a more graceful
>>> + ;; solution, please implement it! See: https://issues.guix.gnu.org/51466
>>> + "PS1=; for i in 1 2 3; do echo GUIX_FLUSH_$i; done; \
>>> +env || /usr/bin/env || set; echo GUIX-CHECK-DONE; read x; exit\n")
>>
>> So you confirm that a single “echo” is not enough, right?
>>
>> Perhaps we should unroll the ‘for’ loop for portability, to be on the
>> safe side. Initially I tested with Bash, Zsh, and Fish:
>>
>> https://issues.guix.gnu.org/51285#0-lineno49
>>
>> I think Fish has a very non-POSIX syntax, hence the suggestion to avoid
>> ‘for’.
>>
>> I realized that setting PS1 could interfere with the logic below that
>> checks for PS1. And since it doesn’t seem to help, perhaps we can
>> remove “PS1=;”?
>>
>> Thoughts?
>>
>> Sorry to answer with yet more questions!
>>
>> Thanks,
>> Ludo’.
C
C
Chris Marusich wrote on 24 May 06:42 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
87sfozzglf.fsf_-_@gmail.com
Hi Ludo,

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

Toggle quote (2 lines)
> So you confirm that a single “echo” is not enough, right?

I didn't test one specifically. It might work with just one, but it did
work with three. If we want to proceed with the "echo" approach, let me
know and I'll test just one echo to see if that is reliable enough.

Toggle quote (8 lines)
> Perhaps we should unroll the ‘for’ loop for portability, to be on the
> safe side. Initially I tested with Bash, Zsh, and Fish:
>
> https://issues.guix.gnu.org/51285#0-lineno49
>
> I think Fish has a very non-POSIX syntax, hence the suggestion to avoid
> ‘for’.

I see. Yes, I'll do that if we decide to go with the echo-based
approach.

Toggle quote (4 lines)
> I realized that setting PS1 could interfere with the logic below that
> checks for PS1. And since it doesn’t seem to help, perhaps we can
> remove “PS1=;”?

I recall that I tried removing PS1, and I still had trouble. I believe
it was because even if we unset PS1 as the very first command we do, the
original prompt is still printed. Foreign distros usually set PS1 to
something, and whatever that is will be printed before we have a chance
to input any commands. It's hard to avoid that in general.

Toggle quote (2 lines)
> Thoughts?

One alternative method I tried successfully in a variety of shells was
to use shell redirection (see attached). I like this approach.
However, this will only work in shells that support redirection. I
recall testing with bash, ash (busybox's shell), dash, zsh, fish, ksh,
and csh. I recall that only csh failed, since it doesn't support
redirection.

I personally like the attached patch better than what I proposed
earlier. The earlier patch just echoes a few times. Presumably, it
only works because the PS1 prompt is likely (but not guaranteed) to be
emitted before the last of the echo commands finishes printing. I'd
rather just control the desired output and ignore PS1 entirely, and that
is what the attached patch accomplishes using FDs. However, if support
for shells without redirection is a requirement, then maybe the original
hack (echo a few times) is OK, or perhaps we need something else.

How would you like to proceed? Is it OK to rely on shell redirection?

--
Chris

From 9a1cef589abf01b61e22656f44c76441f4c50ffd Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Fri, 11 Mar 2022 00:20:12 -0800
Subject: [PATCH] environment: Prevent PS1 from clobbering output in 'check'.


* guix/scripts/environment.scm (child-shell-environment) [shell-pipe]
[shell-pipe-in, shell-pipe-out]: New local variables.
[script]: Redirect the stdout of each command to the file descriptor of the
shell-pipe-out port.
[lines]: In the child, close shell-pipe-in before starting the shell. In the
parent, close shell-pipe-out before sending the script to the shell. Read
lines from shell-pipe-in, not port, so that the shell's PS1 prompt cannot
clobber the lines. Close shell-pipe-in just before waiting on the child.
---
guix/scripts/environment.scm | 29 ++++++++++++++++++++++++-----
1 file changed, 24 insertions(+), 5 deletions(-)

Toggle diff (76 lines)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 3216235937..f0cb341aab 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -48,6 +48,7 @@ (define-module (guix scripts environment)
   #:autoload   (gnu packages bash) (bash)
   #:autoload   (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 format)
   #:autoload   (ice-9 rdelim) (read-line)
   #:use-module (ice-9 vlist)
   #:use-module (srfi srfi-1)
@@ -418,11 +419,23 @@ (define (child-shell-environment shell profile manifest)
   (define-values (controller inferior)
     (openpty))
 
+  (define shell-pipe (pipe))
+  (define shell-pipe-in (car shell-pipe))
+  (define shell-pipe-out (cdr shell-pipe))
+
   (define script
-    ;; Script to obtain the list of environment variable values.  On a POSIX
-    ;; shell we can rely on 'set', but on fish we have to use 'env' (fish's
-    ;; 'set' truncates values and prints them in a different format.)
-    "env || /usr/bin/env || set; echo GUIX-CHECK-DONE; read x; exit\n")
+    ;; Script to obtain the list of environment variable values.
+    ;;
+    ;; On a POSIX shell we can rely on 'set', but on fish we have to use 'env'
+    ;; (fish's 'set' truncates values and prints them in a different format).
+    ;;
+    ;; Unless we redirect output to a dedicated file descriptor, there is a
+    ;; risk that the shell's PS1 prompt might clobber the output.  See:
+    ;; https://issues.guix.gnu.org/53355
+    (let ((out-fd (port->fdes shell-pipe-out)))
+      (format
+       #f "env 1>&~d || /usr/bin/env 1>&~d || set 1>&~d; \
+echo GUIX-CHECK-DONE 1>&~d; read x; exit\n" out-fd out-fd out-fd out-fd)))
 
   (define lines
     (match (primitive-fork)
@@ -432,17 +445,22 @@ (define lines
            (load-profile profile manifest #:pure? #t)
            (setenv "GUIX_ENVIRONMENT" profile)
            (close-fdes controller)
+           (close-port shell-pipe-in)
            (login-tty inferior)
            (execl shell shell))
          (lambda _
            (primitive-exit 127))))
       (pid
        (close-fdes inferior)
+       (close-port shell-pipe-out)
        (let* ((port   (fdopen controller "r+l"))
               (result (begin
                         (display script port)
                         (let loop ((lines '()))
-                          (match (read-line port)
+                          ;; Read from our special port, not from the
+                          ;; controller port, to prevent the shell's PS1
+                          ;; prompt from getting mixed into what we read.
+                          (match (read-line shell-pipe-in)
                             ((? eof-object?) (reverse lines))
                             ("GUIX-CHECK-DONE\r"
                              (display "done\n" port)
@@ -452,6 +470,7 @@ (define lines
                              (loop (cons (string-drop-right line 1)
                                          lines))))))))
          (close-port port)
+         (close-port shell-pipe-in)
          (waitpid pid)
          result))))
 

base-commit: adf5ae5a412ed13302186dd4ce8e2df783d4515d
-- 
2.34.0
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAmKMYjwVHGNtbWFydXNp
Y2hAZ21haWwuY29tAAoJEN1AmhXYIkadGUoQAJXWVsqVTXddDRXcOeqM0f0YX3/Y
3JXnXJF+H+uGDU/X7ol+5f/Alq1gH9zbSP4SExwPYHPt5v/kg3RPQB2sWlg7xmrX
pHlh00XhLbZASgAXFtbIzNRAxDtcTKkE5U2dBHj9fFmY30PMfd8nItkkFW2JxYsf
61bHUO9J3kx4z5Ud6MEntqBqa9l1tw4UCmOAW8s7MNQvcyL2N5DIFLnSvG0wiSTZ
jvoPoUX5MGX/ByRGvRtsWObIp9790gmS9ERd/aBYsw7n82bgSTTkj+S9ryJUmFGA
lOodZ1l6ydYisKvDutVhuCNsIzsu88NC9pf3vEnweLtg/Se7V9oRat+e1lOfYImU
kVRKVKkk1XGeePCR1U2FMH3YypiGx3gCoULjBI6Sxk+Eo+MbAHTZKZnHBC1Bxx0j
fGoxM5QYoDZJaIll2uwoWUyJjCVifETFSXETxZeXGrrXPQmqQnd5CRaK7cB//9TC
MWH933SZDanOkUe/bfIjt/I+FtwBJQatlIEJJuskh5B1UqFQ1EN6olG69PAb84IA
BcWKZ14sd/5KChhfdRhCOhW6qFAQe2pfijpRV3kD9dMIWRV2InSbiHymlKfX+W3V
KnH1bKQdT9FPEz0KQCX68HXYa06F0a4Fm5oomZD6h4UJH/csbn7EIDlDxBbzLwia
59Y6quGxi8sRtL7v
=1Fcq
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 13 Jun 12:03 +0200
(name . Chris Marusich)(address . cmmarusich@gmail.com)
871qvsubgv.fsf@gnu.org
Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (7 lines)
> One alternative method I tried successfully in a variety of shells was
> to use shell redirection (see attached). I like this approach.
> However, this will only work in shells that support redirection. I
> recall testing with bash, ash (busybox's shell), dash, zsh, fish, ksh,
> and csh. I recall that only csh failed, since it doesn't support
> redirection.

That’s a good success list in my view; not being a POSIX shell, (t)csh
was already excluded from the list in the original submission I think:

Toggle quote (11 lines)
> I personally like the attached patch better than what I proposed
> earlier. The earlier patch just echoes a few times. Presumably, it
> only works because the PS1 prompt is likely (but not guaranteed) to be
> emitted before the last of the echo commands finishes printing. I'd
> rather just control the desired output and ignore PS1 entirely, and that
> is what the attached patch accomplishes using FDs. However, if support
> for shells without redirection is a requirement, then maybe the original
> hack (echo a few times) is OK, or perhaps we need something else.
>
> How would you like to proceed? Is it OK to rely on shell redirection?

Yeah, I think so. This new approach looks more robust.

Toggle quote (16 lines)
> From 9a1cef589abf01b61e22656f44c76441f4c50ffd Mon Sep 17 00:00:00 2001
> From: Chris Marusich <cmmarusich@gmail.com>
> Date: Fri, 11 Mar 2022 00:20:12 -0800
> Subject: [PATCH] environment: Prevent PS1 from clobbering output in 'check'.
>
> Fixes: <https://issues.guix.gnu.org/51466>.
>
> * guix/scripts/environment.scm (child-shell-environment) [shell-pipe]
> [shell-pipe-in, shell-pipe-out]: New local variables.
> [script]: Redirect the stdout of each command to the file descriptor of the
> shell-pipe-out port.
> [lines]: In the child, close shell-pipe-in before starting the shell. In the
> parent, close shell-pipe-out before sending the script to the shell. Read
> lines from shell-pipe-in, not port, so that the shell's PS1 prompt cannot
> clobber the lines. Close shell-pipe-in just before waiting on the child.

LGTM, please push!

Ludo’.
C
C
Chris Marusich wrote on 19 Jun 22:40 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
87k09cpest.fsf@gmail.com
Hi Ludo,

Thank you for the review!

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

Toggle quote (2 lines)
> LGTM, please push!

Before pushing, I did some more tests to make sure it was still working.
When I did this, I noticed that read-line was no longer returning
strings that end in "\r". This prevents child-shell-environment from
behaving correctly, since it incorrectly assumes that all the lines end
in "\r", stripping it off unconditionally. In the past, I'm sure
read-line was returning strings that end in "\r". I don't know what
changed, but I've attached a second patch that fixes this issue, also.

Unless you have more feedback, I'll go ahead and push both patches to
master in a few days.

--
Chris

From c4fee9e63f8cb694de86ae46bd1e2e4c692eb6f6 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Sun, 19 Jun 2022 13:16:04 -0700
Subject: [PATCH] environment: Don't assume that lines have a trailing "\r".

I've noticed that the child-shell-environment procedure is misbehaving on my
computer because the lines returned by read-line do not have a trailing "\r".
In the past, I recall that such lines did in fact have a trailing "\r". I'm
not sure why it changed, but it seems prudent to just rewrite this code to
tolerate both cases, since it seems that both cases can happen.

* guix/scripts/environment.scm (child-shell-environment) [lines]: Instead of
checking if the line exactly matches "GUIX_CHECK_DONE\r"; check if the line
begins with "GUIX_CHECK_DONE". Instead of always stripping the trailing
character from the line, only do it if the line has a trailing "\r".
---
guix/scripts/environment.scm | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)

Toggle diff (29 lines)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index f0cb341aab..1fb4f5b7c6 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -462,13 +462,18 @@ (define lines
                           ;; prompt from getting mixed into what we read.
                           (match (read-line shell-pipe-in)
                             ((? eof-object?) (reverse lines))
-                            ("GUIX-CHECK-DONE\r"
+                            ((? (lambda (line)
+                                  ;; The line might or might not have a trailing \r.
+                                  (string-prefix? "GUIX-CHECK-DONE" line)))
                              (display "done\n" port)
                              (reverse lines))
                             (line
-                             ;; Drop the '\r' from LINE.
-                             (loop (cons (string-drop-right line 1)
-                                         lines))))))))
+                             ;; Strip the trailing '\r' from LINE if present.
+                             (let ((stripped-line
+                                    (if (string-suffix? "\r" line)
+                                        (string-drop-right line 1)
+                                        line)))
+                               (loop (cons stripped-line lines)))))))))
          (close-port port)
          (close-port shell-pipe-in)
          (waitpid pid)
-- 
2.34.0
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAmKvidMVHGNtbWFydXNp
Y2hAZ21haWwuY29tAAoJEN1AmhXYIkadCbEP/ixl02qd6fKeuKSLzJq+FDSMRNqs
sn4EYxszto77JYFsnHwdIAfMK9kDrZxHqfY9IBLlKS6eUymNFGJs2o64FqezqLOj
bWcneKhUr72BwAzzhstiToRv3CuILB0gyLtTuFWFk/B5+n1zCuNicZ4XKnULChA1
13IyWv/xXZNi2kpa9JVk+Q4y8DHs24O7SsN0C5lKKC7On7aOXaoMd+jE89q84Ick
kzqqXogu04WsxmNhXrgWBq3ByvJjAaL9k8ReoB0rIO3kGj+yNn9shItCti9cGqDb
0pqaedQaidBztol6LIgf3HdZ8sqwhU7bg0ibkpWqXJZte11veFutgXZi/x0kHPo5
/5NmfyPW1ErWBloaUCz6VNZfpPokDbjKtinLntIe+DvNmxqwa3niWZLbWYTbVqO0
dIM4CU5B17cpjhXZR5ApSJv+TpvET7HqmT7EV0dOwsq5YgNliB2ZxM3i2EQC6ZxQ
ASMj1A+gL0vg3ZSgzkad+EwZXHMw5ksPYdWzqyvpNlfXd4PaOVHxxHnlpXnimhh3
BPZdbcf41P6UV2q7sTQBsLKsj7sNkJN1yCnk0kSIZUuWw5bn4WFRNs5dJmIjKeo9
bQe3EVLISSl1rOB+Fj6VF1dcv8VtNqmEGOyjNvz5BcufhV+JPsw3PPaQYgXxZx1d
p7QRW97iTH6UmKBm
=Ni0q
-----END PGP SIGNATURE-----

L
L
Ludovic Courtès wrote on 20 Jun 09:34 +0200
(name . Chris Marusich)(address . cmmarusich@gmail.com)
874k0fbxfa.fsf@gnu.org
Hi,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (8 lines)
> Before pushing, I did some more tests to make sure it was still working.
> When I did this, I noticed that read-line was no longer returning
> strings that end in "\r". This prevents child-shell-environment from
> behaving correctly, since it incorrectly assumes that all the lines end
> in "\r", stripping it off unconditionally. In the past, I'm sure
> read-line was returning strings that end in "\r". I don't know what
> changed, but I've attached a second patch that fixes this issue, also.

Weird, not sure what could have changed.

Toggle quote (3 lines)
> Unless you have more feedback, I'll go ahead and push both patches to
> master in a few days.

Great, thank you!

Ludo’.
B
Re: bug#51466: bug#53355: guix shell --check: confusing error message
(name . Chris Marusich)(address . cmmarusich@gmail.com)
20220620101210.GA19777@LionPure
Hi Chris,

Did you observe this behaviour inside a git repo directory?
I wonder if this git security thing could be relevant:
It makes also me wonder about readline completion stuff
possibly interacting. Isn't that implemented with readline?

I have had some mystery bash parsing errors, and I noticed
set|less
shows a heck of a lot of functions defined that I don't
remember seeing in the past.
Anyway, shouldn't stuff like that have better hygiene than just prefixed
_underscore ? Or maybe set|less doesn't show all that on your system?

Disclaimer: I played a lot of games trying to make stuff conditional
at login, where I renamed .bash_profile and .bashrc (e.g. .my_bashrc)
which brought .profile into play, and I messed with the downstream
of that to source some .my_'s conditionally, so I've go a fragile mess right now ;/

Anyway, did you determine why things changed in the first place?
Or will this be a whack-a-mole game with future weirdnesses? :)

Semms like IWBN to have interfaces governed by contracts :)

Best,
Bengt Richter

On +2022-06-19 13:40:50 -0700, Chris Marusich wrote:
Toggle quote (73 lines)
> Hi Ludo,
>
> Thank you for the review!
>
> Ludovic Courtès <ludo@gnu.org> writes:
>
> > LGTM, please push!
>
> Before pushing, I did some more tests to make sure it was still working.
> When I did this, I noticed that read-line was no longer returning
> strings that end in "\r". This prevents child-shell-environment from
> behaving correctly, since it incorrectly assumes that all the lines end
> in "\r", stripping it off unconditionally. In the past, I'm sure
> read-line was returning strings that end in "\r". I don't know what
> changed, but I've attached a second patch that fixes this issue, also.
>
> Unless you have more feedback, I'll go ahead and push both patches to
> master in a few days.
>
> --
> Chris
>
> PGP: https://savannah.gnu.org/people/viewgpg.php?user_id=106836

> From c4fee9e63f8cb694de86ae46bd1e2e4c692eb6f6 Mon Sep 17 00:00:00 2001
> From: Chris Marusich <cmmarusich@gmail.com>
> Date: Sun, 19 Jun 2022 13:16:04 -0700
> Subject: [PATCH] environment: Don't assume that lines have a trailing "\r".
>
> I've noticed that the child-shell-environment procedure is misbehaving on my
> computer because the lines returned by read-line do not have a trailing "\r".
> In the past, I recall that such lines did in fact have a trailing "\r". I'm
> not sure why it changed, but it seems prudent to just rewrite this code to
> tolerate both cases, since it seems that both cases can happen.
>
> * guix/scripts/environment.scm (child-shell-environment) [lines]: Instead of
> checking if the line exactly matches "GUIX_CHECK_DONE\r"; check if the line
> begins with "GUIX_CHECK_DONE". Instead of always stripping the trailing
> character from the line, only do it if the line has a trailing "\r".
> ---
> guix/scripts/environment.scm | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
> index f0cb341aab..1fb4f5b7c6 100644
> --- a/guix/scripts/environment.scm
> +++ b/guix/scripts/environment.scm
> @@ -462,13 +462,18 @@ (define lines
> ;; prompt from getting mixed into what we read.
> (match (read-line shell-pipe-in)
> ((? eof-object?) (reverse lines))
> - ("GUIX-CHECK-DONE\r"
> + ((? (lambda (line)
> + ;; The line might or might not have a trailing \r.
> + (string-prefix? "GUIX-CHECK-DONE" line)))
> (display "done\n" port)
> (reverse lines))
> (line
> - ;; Drop the '\r' from LINE.
> - (loop (cons (string-drop-right line 1)
> - lines))))))))
> + ;; Strip the trailing '\r' from LINE if present.
> + (let ((stripped-line
> + (if (string-suffix? "\r" line)
> + (string-drop-right line 1)
> + line)))
> + (loop (cons stripped-line lines)))))))))
> (close-port port)
> (close-port shell-pipe-in)
> (waitpid pid)
> --
> 2.34.0
>
B
B
Bengt Richter wrote on 20 Jun 19:56 +0200
Re: bug#53355: bug#51466: bug#53355: guix shell --check: confusing error message
(name . Chris Marusich)(address . cmmarusich@gmail.com)
20220620175656.GA24590@LionPure
Sorry to reply to myself, but forgot to illustrate.

On +2022-06-20 12:12:10 +0200, bokr@bokr.com wrote:
Toggle quote (1 lines)
> Hi Chris,
[...]
Toggle quote (9 lines)
>
> I have had some mystery bash parsing errors, and I noticed
> set|less
> shows a heck of a lot of functions defined that I don't
> remember seeing in the past.
> Anyway, shouldn't stuff like that have better hygiene than just prefixed
> _underscore ? Or maybe set|less doesn't show all that on your system?
>

There are a couple functions without prefixed underscore too,
which invoke some underscore-prefixed ones that look too trusting
of their arguments if you ask me: can someone declare these safe?

I think I can grok quote () ...
(escape single quotes and enclose result in single quotes, trusting bash state)
But what if I want to define my own function quote?? How would I know I was
overriding this? I really don't like my programming space occupied by unknowns :-(

Toggle snippet (7 lines)
quote ()
{
local quoted=${1//\'/\'\\\'\'};
printf "'%s'" "$quoted"
}

but this one below will take more time than I want to spend on code
I'm not intentionally going to use, and which invites name clashes
in my command name space :-(

Toggle snippet (8 lines)
quote_readline ()
{
local quoted;
_quote_readline_by_ref "$1" ret;
printf %s "$ret"
}

where the above calls this:

Toggle snippet (19 lines)
_quote_readline_by_ref ()
{
if [ -z "$1" ]; then
printf -v $2 %s "$1";
else
if [[ $1 == \'* ]]; then
printf -v $2 %s "${1:1}";
else
if [[ $1 == ~* ]]; then
printf -v $2 ~%q "${1:1}";
else
printf -v $2 %q "$1";
fi;
fi;
fi;
[[ ${!2} == \$* ]] && eval $2=${!2}
}

HTH somehow.
--
Regards,
Bengt Richter
B
B
Bengt Richter wrote on 21 Jun 01:27 +0200
(name . Chris Marusich)(address . cmmarusich@gmail.com)
20220620232749.GA25854@LionPure
Sorry again, but I found the source:

tl;dr: These functions are defined in
/usr/share/bash-completion/bash_completion
which looks awful kludgey to me, (however clever :)

There is a reference to
in the header comments for
_quote_readline_by_ref ()

I also found
My bet is there is at least one bug active now.

Completion is really nice when it works, but IMO they certainly
shouldn't use a name like unadorned ``quote'' in their implementation.

And I think it would be prettier in scheme :)
Lots could be prettier if bash could be extended with scheme.

I'm about out of time to chase this, but I expect to bump into it again ;/
HTH.
--
Regards,
Bengt Richter

On +2022-06-20 19:56:56 +0200, Bengt Richter wrote:
Toggle quote (73 lines)
> Sorry to reply to myself, but forgot to illustrate.
>
> On +2022-06-20 12:12:10 +0200, bokr@bokr.com wrote:
> > Hi Chris,
> [...]
> >
> > I have had some mystery bash parsing errors, and I noticed
> > set|less
> > shows a heck of a lot of functions defined that I don't
> > remember seeing in the past.
> > Anyway, shouldn't stuff like that have better hygiene than just prefixed
> > _underscore ? Or maybe set|less doesn't show all that on your system?
> >
>
> There are a couple functions without prefixed underscore too,
> which invoke some underscore-prefixed ones that look too trusting
> of their arguments if you ask me: can someone declare these safe?
>
> I think I can grok quote () ...
> (escape single quotes and enclose result in single quotes, trusting bash state)
> But what if I want to define my own function quote?? How would I know I was
> overriding this? I really don't like my programming space occupied by unknowns :-(
>
> --8<---------------cut here---------------start------------->8---
> quote ()
> {
> local quoted=${1//\'/\'\\\'\'};
> printf "'%s'" "$quoted"
> }
> --8<---------------cut here---------------end--------------->8---
>
> but this one below will take more time than I want to spend on code
> I'm not intentionally going to use, and which invites name clashes
> in my command name space :-(
>
> --8<---------------cut here---------------start------------->8---
> quote_readline ()
> {
> local quoted;
> _quote_readline_by_ref "$1" ret;
> printf %s "$ret"
> }
> --8<---------------cut here---------------end--------------->8---
>
> where the above calls this:
>
> --8<---------------cut here---------------start------------->8---
> _quote_readline_by_ref ()
> {
> if [ -z "$1" ]; then
> printf -v $2 %s "$1";
> else
> if [[ $1 == \'* ]]; then
> printf -v $2 %s "${1:1}";
> else
> if [[ $1 == ~* ]]; then
> printf -v $2 ~%q "${1:1}";
> else
> printf -v $2 %q "$1";
> fi;
> fi;
> fi;
> [[ ${!2} == \$* ]] && eval $2=${!2}
> }
> --8<---------------cut here---------------end--------------->8---
>
> HTH somehow.
> --
> Regards,
> Bengt Richter
>
>
>
T
T
Thiago Jung Bauermann wrote on 21 Jun 06:00 +0200
Re: bug#51466: bug#53355: bug#51466: bug#53355: guix shell --check: confusing error message
(name . Bengt Richter)(address . bokr@bokr.com)
87y1xq1x2y.fsf@kolabnow.com
Hello,

Bengt Richter <bokr@bokr.com> writes:

Toggle quote (2 lines)
> Lots could be prettier if bash could be extended with scheme.

Today is your lucky day. :-)

$ guix show guile-bash | recsel -p name,synopsis
name: guile-bash
synopsis: Extend Bash using Guile

--
Thanks
Thiago
C
C
Chris Marusich wrote on 25 Jun 11:07 +0200
Re: bug#53355: guix shell --check: confusing error message
(name . Ludovic Courtès)(address . ludo@gnu.org)
875ykpdsbd.fsf_-_@gmail.com
Hi Ludo & Everyone,

Chris Marusich <cmmarusich@gmail.com> writes:

Toggle quote (2 lines)
> Is it OK to rely on shell redirection?

It turns out that it is probably not OK to rely on shell redirection in
this case, after all. For example, "dash does not support multi-digit
file descriptors":


Indeed, the patch I proposed earlier to rely on shell redirection caused
a command like

./pre-inst-env env SHELL=/gnu/store/nm0hccsphymxi8c24xmg6ixm9vcf25xb-dash-0.5.11.5/bin/dash guix shell --check --container -D guix

to hang. It hangs because the FD Guile chooses to create and embed in
the script is 19 (on my machine, at least). A redirection like
"env >&19" causes dash to error out, so no environment information gets
sent back to the parent process. The same issue seemed to occur for the
ksh from our oksh package.

To resolve this, I changed the code so that it just writes to a
temporary file. This is simple and more robust. With the attached
patch, I was able to use a command like the one above to verify that
"guix environment --check" works correctly for Guix-built bash, dash,
ksh, fish, zsh, and ash. I also verified that it works for Fedora's
/bin/sh and /bin/bash.

What do you think of this file-based approach? Supporting many
different shells is pretty tricky, but I think this patch does a good
enough job.

--
Chris

From ef8d12a1d44903eafca7153c9344263b1d5d7d56 Mon Sep 17 00:00:00 2001
From: Chris Marusich <cmmarusich@gmail.com>
Date: Fri, 11 Mar 2022 00:20:12 -0800
Subject: [PATCH] environment: Prevent PS1 from clobbering output in 'check'.


* guix/scripts/environment.scm (child-shell-environment) [temporary-file-port]
[temporary-file]: New local variables.
[script]: Redirect stdout to the temporary file.
[lines]: In the parent, send the script to the shell, wait for the shell to
exit, and then read lines from the temporary file. Use a dynamic-wind
expression to ensure we always close port, close temporary-file-port, and
delete temporary-file.
---
guix/scripts/environment.scm | 63 ++++++++++++++++++++++++------------
1 file changed, 43 insertions(+), 20 deletions(-)

Toggle diff (98 lines)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index 3216235937..b02b0771e3 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -48,6 +48,7 @@ (define-module (guix scripts environment)
   #:autoload   (gnu packages bash) (bash)
   #:autoload   (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile)
   #:use-module (ice-9 match)
+  #:use-module (ice-9 format)
   #:autoload   (ice-9 rdelim) (read-line)
   #:use-module (ice-9 vlist)
   #:use-module (srfi srfi-1)
@@ -418,14 +419,27 @@ (define (child-shell-environment shell profile manifest)
   (define-values (controller inferior)
     (openpty))
 
+  (define temporary-file-port (mkstemp "/tmp/guix-env.XXXXXX"))
+
+  (define temporary-file (port-filename temporary-file-port))
+
   (define script
-    ;; Script to obtain the list of environment variable values.  On a POSIX
-    ;; shell we can rely on 'set', but on fish we have to use 'env' (fish's
-    ;; 'set' truncates values and prints them in a different format.)
-    "env || /usr/bin/env || set; echo GUIX-CHECK-DONE; read x; exit\n")
+    ;; Script to obtain the list of environment variable values.
+    ;;
+    ;; On a POSIX shell we can rely on 'set', but on fish we have to use 'env'
+    ;; (fish's 'set' truncates values and prints them in a different format).
+    ;;
+    ;; Unless we redirect output to a file, there is a risk that the shell's
+    ;; PS1 prompt might clobber the output.  See:
+    ;; https://issues.guix.gnu.org/53355
+    (format
+     #f "env >~a || /usr/bin/env >~a || set >~a; \
+echo GUIX-CHECK-DONE >>~a; exit\n"
+     temporary-file temporary-file temporary-file temporary-file))
 
   (define lines
     (match (primitive-fork)
+      ;; Child
       (0
        (catch #t
          (lambda ()
@@ -436,24 +450,33 @@ (define lines
            (execl shell shell))
          (lambda _
            (primitive-exit 127))))
+      ;; Parent
       (pid
        (close-fdes inferior)
-       (let* ((port   (fdopen controller "r+l"))
-              (result (begin
-                        (display script port)
-                        (let loop ((lines '()))
-                          (match (read-line port)
-                            ((? eof-object?) (reverse lines))
-                            ("GUIX-CHECK-DONE\r"
-                             (display "done\n" port)
-                             (reverse lines))
-                            (line
-                             ;; Drop the '\r' from LINE.
-                             (loop (cons (string-drop-right line 1)
-                                         lines))))))))
-         (close-port port)
-         (waitpid pid)
-         result))))
+       (let* ((port   (fdopen controller "r+l")))
+         (dynamic-wind
+           (const #t)
+           (lambda ()
+             (display script port)
+             ;; Wait until the shell is done writing to the temporary file.
+             (waitpid pid)
+             (let loop ((lines '()))
+               ;; Read from the temporary file, not from the controller port, to
+               ;; prevent the shell's PS1 prompt from getting mixed into what we
+               ;; read.  See: https://issues.guix.gnu.org/51466
+               (match (read-line temporary-file-port)
+                 ((? eof-object?) (reverse lines))
+                 ("GUIX-CHECK-DONE"
+                  (reverse lines))
+                 (line
+                  (loop (cons line lines))))))
+           (lambda ()
+             (close-port temporary-file-port)
+             ;; The temporary file might not exist if something weird happens
+             ;; in the child shell that prevents it from even writing to the
+             ;; file (e.g. the shell fails to start for some reason).
+             (false-if-exception (delete-file temporary-file))
+             (close-port port)))))))
 
   (fold (lambda (line table)
           ;; Note: 'set' in fish outputs "NAME VALUE" instead of "NAME=VALUE"

base-commit: 319b8331b2357e12ec9edb9665513c32bef56622
-- 
2.34.0
-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAmK20GYVHGNtbWFydXNp
Y2hAZ21haWwuY29tAAoJEN1AmhXYIkadIeYQAL0LO5v81twAns5KehxoYWL9R+WS
vgWzIixh7PaJygwBd1j8ijY26+ktvRRxUUsU3eYW/YOKdHAFYzVD3RtO4Tmmq5tf
+/9rFAJTFqJGvvyjDm1d44/uO3olR/fwVGPHHjjWmpX7XH5vVlNYzToU2VOEwthe
94/EGBpCzzjXWG7vnQOVJaaRcUL0KA/4eWw4RzXPUgulmGSjtFlpgPgRdJE7Q7g2
VxxER6KeNWSoOGw7AtsCYiTi/qa3dYdXxMORgVMjNBSqnM4OB4QKYilxqxCwT+Ds
TE3wFkOhTrKtVqoPTmUq6FVPpI7RBZdqI23HrfMloeeOzwRZ4VOvKJRjGzl7VWaB
khuibJiwQGQtQtNZKjwwB27NANTCs7F2MnJNd3Lubgr1wlotlxxCfopse3znoIcg
KUUmIeclHszqVAS0Cecp69VWO0Pwj1UobRqtB7BZhuNLGjM9nC7uQWL0JD+k1PdL
TjGGX34fEZsiofgerul+5m7AWJBg4hQMOqG0m33ha19WFqjhM6nggiFSbyFIJb/Y
6K9IjksS8FBhYK9mAXlRjNbM8HmKREK7KY/71k5YCY+qpQTcP13985XNXVBKLfRG
C/lVSRX3pUcZ10QXc8tygVlQ1Ns2F0KM+zvF/xSzWOx0H5oieKszQTSHJdPdt6R7
bMJbCBQ0tsY6PgLE
=Cy8Z
-----END PGP SIGNATURE-----

M
M
Maxime Devos wrote on 25 Jun 11:37 +0200
Re: bug#51466: bug#53355: guix shell --check: confusing error message
0b1765dfff5401fa06ee25779b7f173230bf4ea4.camel@telenet.be
Chris Marusich schreef op za 25-06-2022 om 02:07 [-0700]:
Toggle quote (8 lines)
> It turns out that it is probably not OK to rely on shell redirection
> in
> this case, after all.  For example, "dash does not support multi-
> digit
> file descriptors":
>
> https://bugs.launchpad.net/ubuntu/+source/dash/+bug/249620

I consider temporary files to be more fragile -- you have to take care
of file permissions, removing the file afterwards even after an
interrupt with C-c, deleting the temporary file can fail, there might
be an out-of-space error, in case of file system corruption things
might be remounted read-only, some other program could read, write or
delete the file ..., so I think it would be best to just fix the bug in
dash instead.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrbXahccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7uN1AP4u9LnHq6nIV9yEQ7ccgSkMGy7W
V8Gf1H38sdbvtlwlRgD9EhTjo5zuUvKGHo1lTkI6hseR0SCDTYXCVa7GNcD11Ag=
=frxf
-----END PGP SIGNATURE-----


C
C
Chris Marusich wrote on 25 Jun 18:52 +0200
Re: bug#53355: guix shell --check: confusing error message
(name . Maxime Devos)(address . maximedevos@telenet.be)
87y1xkwur9.fsf_-_@gmail.com
Hi Maxime,

Maxime Devos <maximedevos@telenet.be> writes:

Toggle quote (17 lines)
> Chris Marusich schreef op za 25-06-2022 om 02:07 [-0700]:
>> It turns out that it is probably not OK to rely on shell redirection
>> in
>> this case, after all.  For example, "dash does not support multi-
>> digit
>> file descriptors":
>>
>> https://bugs.launchpad.net/ubuntu/+source/dash/+bug/249620
>
> I consider temporary files to be more fragile -- you have to take care
> of file permissions, removing the file afterwards even after an
> interrupt with C-c, deleting the temporary file can fail, there might
> be an out-of-space error, in case of file system corruption things
> might be remounted read-only, some other program could read, write or
> delete the file ..., so I think it would be best to just fix the bug in
> dash instead.

Yes, I agree those are good reasons to avoid a temporary file if we can.
To that end, do you know if we can somehow force Guile to use a specific
file descriptor for the pipe? In the patch I wrote earlier, which uses
redirection, the problem was that I could not control Guile's choice of
file descriptors. Guile chose file descriptor 19 for one end of the
pipe, and I don't know how to make it use anything else. If we can
arrange for Guile to consistently use file descriptor 7, for example,
then probably it would work in all the shell I've tested.

I wonder if maybe I can just duplicate the file descriptor? I don't
know; if for example Guile reserves all the file descriptors below 10
for other uses, it might be hard.

What do you think? Is there a way to do it?

--
Chris

-----BEGIN PGP SIGNATURE-----

iQJJBAEBCAAzFiEEy/WXVcvn5+/vGD+x3UCaFdgiRp0FAmK3PUoVHGNtbWFydXNp
Y2hAZ21haWwuY29tAAoJEN1AmhXYIkadFZcQAL0FO531KUuCaeazN5nba4ZP/aPS
1qjXKKrWxU9QR2r1HZERoCglSBxOp2Kpkl81mD/3am6hvZWS9Z0h1uIHEh8NzPzB
eU6gfZKG26PLD3h2o8ffrxT8UiTuzzFmyRopPnyKIqIxi6yOu9CZENS0Yxrp9D/I
GllMJBKRaGYJxidLRhq3uhn6AiEKKSNt6FP3BkSbqmZqrPKGXv1JWVCHDkVG0zXS
3A67Dm9Fk6xP974NjyfDLW3gOkRmRFkRekg8dDNDlh607iHErWmODF5i7XMn8qjp
pcXmxGiy9PEGcJd9Y8ZIH9s++96PsZjtWPNvl31gzOaG7RFNuW2itz5xiHhvl8e1
qlXoTNOBKa2qqSXNKn98j+u+RPkxYtnjabLCVh1HPr+RvsMankZgKi3fDX3SQQ+U
kH4bPjCyvxHzQuOtj0dVCAoYkBwWxOLu+qL1+7KRwhWowBKyDXDnGmHweeCGSsEJ
3H0Z5s5RqpHeJDZJkoTsJeS7bjZyeivw1Ktg4Rp/GCk/H/oioEoKPk8YWMrAH/T7
65Bq4tcgFODnPenB29a7XiUa6/s3Ov/GGXtD4Sex97acMtPImiMlGn4Vrr+aazVZ
Ef+QsquE5SqmifhOZeVkW/duLrQucFhNJX+QipNmXzpwcrinr7HSRY6vF9KrJy9F
33Oj910Xl4imyP8K
=0+4i
-----END PGP SIGNATURE-----

M
M
Maxime Devos wrote on 25 Jun 19:40 +0200
(name . Chris Marusich)(address . cmmarusich@gmail.com)
bf5b3b83c03de0c5056255ea60a3d0b4f8036478.camel@telenet.be
Chris Marusich schreef op za 25-06-2022 om 09:52 [-0700]:
Toggle quote (18 lines)
> Yes, I agree those are good reasons to avoid a temporary file if we
> can.
> To that end, do you know if we can somehow force Guile to use a
> specific
> file descriptor for the pipe?  In the patch I wrote earlier, which
> uses
> redirection, the problem was that I could not control Guile's choice
> of
> file descriptors.  Guile chose file descriptor 19 for one end of the
> pipe, and I don't know how to make it use anything else.  If we can
> arrange for Guile to consistently use file descriptor 7, for example,
> then probably it would work in all the shell I've tested.
>
> I wonder if maybe I can just duplicate the file descriptor?  I don't
> know; if for example Guile reserves all the file descriptors below 10
> for other uses, it might be hard.
>

Have a look at ‘(guile)Ports and File Descriptors’. It has lots of
procedures for duplicating and renumbering. That's fragile though, you
might accidentally overwrite an fd that's being used for something
else.

(Normally move->fdes would prevent overwriting things by moving pre-
existing fds out of the way, adjusting ports automatically, but move-
Toggle quote (1 lines)
>fdes doesn't know (yet) about the pipe that Guile uses for
finalisation, see maybe:

I think it would be best to patch the dash appropriately (though fixing
move->fdes would be nice too).

Greetings,
Maximee.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrdIoBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7vQAAPsF2xA7luLZF2c5ndGS+TZax0PZ
bD4L7+9G3f+/kTmeVgD/csEhB34Sh/+JlOiN2WpLjGUFu8O/J/+ZNVhW6+ZSvAk=
=z7BE
-----END PGP SIGNATURE-----


B
(name . Maxime Devos)(address . maximedevos@telenet.be)
20220625200646.GA8075@LionPure
On +2022-06-25 19:40:48 +0200, Maxime Devos wrote:
Toggle quote (36 lines)
> Chris Marusich schreef op za 25-06-2022 om 09:52 [-0700]:
> > Yes, I agree those are good reasons to avoid a temporary file if we
> > can.
> > To that end, do you know if we can somehow force Guile to use a
> > specific
> > file descriptor for the pipe?  In the patch I wrote earlier, which
> > uses
> > redirection, the problem was that I could not control Guile's choice
> > of
> > file descriptors.  Guile chose file descriptor 19 for one end of the
> > pipe, and I don't know how to make it use anything else.  If we can
> > arrange for Guile to consistently use file descriptor 7, for example,
> > then probably it would work in all the shell I've tested.
> >
> > I wonder if maybe I can just duplicate the file descriptor?  I don't
> > know; if for example Guile reserves all the file descriptors below 10
> > for other uses, it might be hard.
> >
>
> Have a look at ‘(guile)Ports and File Descriptors’. It has lots of
> procedures for duplicating and renumbering. That's fragile though, you
> might accidentally overwrite an fd that's being used for something
> else.
>
> (Normally move->fdes would prevent overwriting things by moving pre-
> existing fds out of the way, adjusting ports automatically, but move-
> >fdes doesn't know (yet) about the pipe that Guile uses for
> finalisation, see maybe:
> <https://debbugs.gnu.org/cgi/bugreport.cgi?bug=48563>)
>
> I think it would be best to patch the dash appropriately (though fixing
> move->fdes would be nice too).
>
> Greetings,
> Maximee.

Could this help?:

(from man 2 openat (scroll down a fair bit):
-8<---------------cut here---------------start------------->8---
There are two main use cases for O_TMPFILE:

* Improved tmpfile(3) functionality: race-free creation of temporary files that (1) are automatically
deleted when closed; (2) can never be reached via any pathname; (3) are not subject to symlink at‐
tacks; and (4) do not require the caller to devise unique names.

* Creating a file that is initially invisible, which is then populated with data and adjusted to have
appropriate filesystem attributes (fchown(2), fchmod(2), fsetxattr(2), etc.) before being atomi‐
cally linked into the filesystem in a fully formed state (using linkat(2) as described above).

O_TMPFILE requires support by the underlying filesystem; only a subset of Linux filesystems provide
that support. In the initial implementation, support was provided in the ext2, ext3, ext4, UDF,
Minix, and shmem filesystems. Support for other filesystems has subsequently been added as follows:
XFS (Linux 3.15); Btrfs (Linux 3.16); F2FS (Linux 3.16); and ubifs (Linux 4.9)
Toggle snippet (18 lines)
BTW, IIRC, this can be used to create an invisible file that
can be mmap-ed, and the mmap will persist when you delete
the file. Which then can be used as an anonymous buffer
passed to wayland, along with metadate saying what the buffer
contains, e.g. different kinds of rgb or rgba permutations
and encodings, (or anything, which you can tell wayland just
to keep track of for you.

You need a directory for openat, so probably
XDG_RUNTIME_DIR=/run/user/1000
is suitable if it exists. Worked in my case.

HTH
--
Regards,
Bengt Richter
M
M
Maxime Devos wrote on 25 Jun 23:04 +0200
(address . bokr@bokr.com)
57148fd1c1991ee6bddc88dbccf4118afeaba10a.camel@telenet.be
bokr@bokr.com schreef op za 25-06-2022 om 22:06 [+0200]:
Toggle quote (2 lines)
> BTW, IIRC, this can be used to create an invisible file that

Invisible files don't have file names, so they cannot be put in the
tiny shell script:

Toggle quote (5 lines)
> + (format
> + #f "env >~a || /usr/bin/env >~a || set >~a; \
> +echo GUIX-CHECK-DONE >>~a; exit\n"
> + temporary-file temporary-file temporary-file temporary-file))

Also, I just noticed that this tiny shell script isn't quoting
anything, so you'll get in trouble if $TMPDIR (or was it $TEMPDIR,
whatever) contains " or ' or \ or whatever.

So to avoid messy and fragile escaping, I'd recommend to work with file
descriptors.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrd4ShccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7tm/AQD/oGnY+tMcGXiGGhnWuhszC8rr
Uc43dKvEAmJNnMdfXAEA2r86MfXxbcfBQ/TwOIkKGxijsfu8P6EfYL7xG1RZFw4=
=jqO8
-----END PGP SIGNATURE-----


J
J
Josselin Poiret wrote on 26 Jun 12:33 +0200
87zghzsohq.fsf@jpoiret.xyz
Hello everyone,

Maxime Devos <maximedevos@telenet.be> writes:

Toggle quote (13 lines)
> Chris Marusich schreef op za 25-06-2022 om 09:52 [-0700]:
>> [...]
>> the problem was that I could not control Guile's choice
>> of
>> file descriptors.  Guile chose file descriptor 19 for one end of the
>> pipe, and I don't know how to make it use anything else.  If we can
>> arrange for Guile to consistently use file descriptor 7, for example,
>> then probably it would work in all the shell I've tested.
> Have a look at ‘(guile)Ports and File Descriptors’. It has lots of
> procedures for duplicating and renumbering. That's fragile though, you
> might accidentally overwrite an fd that's being used for something
> else.

Just my 2¢, from my experience, Guile uses a lot of fds (`guile -q` uses
15). I'm not sure it would be safe or advisable to move fds, since we
cannot be sure that they're backing ports or not, and if they're not it
would break things. In general, the Guile ports/fds interface works ok,
but there is a lot of code that is fragile and doesn't handle
edge-cases.

Best,
--
Josselin Poiret
M
M
Maxime Devos wrote on 26 Jun 15:07 +0200
dbfb99620519cc205697520858ece50c603ac9f9.camel@telenet.be
Josselin Poiret schreef op zo 26-06-2022 om 12:33 [+0200]:
Toggle quote (5 lines)
> Just my 2¢, from my experience, Guile uses a lot of fds (`guile -q` uses
> 15).  I'm not sure it would be safe or advisable to move fds, since we
> cannot be sure that they're backing ports or not, and if they're not it
> would break things

As mentioned previously, move->fdes looks in the port table to see if
the file descriptor things are being moved too is still in use, and if
so, moves that (fd, port) out of the way first, so should be safe.

That only works if Guile knows about the fd though, and Guile currently
does not know about it's own finalisation pipe, which I think is the
currently still fragile ...

(To be clear, I still recommend just fixing dash ...)
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrhZ/xccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7kIVAQDT4E/eTeEQjy+tnX8Ld5l+BYlq
RynJC9UhzUm4jfLKuAD/RshvRqPwZuVQT+htxNv08h3+T+wCQSf028EMR600JQY=
=9BuJ
-----END PGP SIGNATURE-----


T
T
Tobias Geerinckx-Rice wrote on 26 Jun 21:45 +0200
A906227E-EF19-4A15-A6FE-6C6F032202E0@tobias.gr
On 26 June 2022 13:07:11 UTC, Maxime Devos <maximedevos@telenet.be> wrote:
Toggle quote (2 lines)
>(To be clear, I still recommend just fixing dash ...)

That's the long-term solution, but Guix needs a work-around regardless.


Kind regards,

T G-R

Sent on the go. Excuse or enjoy my brevity.
L
L
Ludovic Courtès wrote on 27 Jun 12:17 +0200
(name . Chris Marusich)(address . cmmarusich@gmail.com)
87mtdyif6j.fsf@gnu.org
Hi Chris,

Chris Marusich <cmmarusich@gmail.com> skribis:

Toggle quote (6 lines)
> It turns out that it is probably not OK to rely on shell redirection in
> this case, after all. For example, "dash does not support multi-digit
> file descriptors":
>
> https://bugs.launchpad.net/ubuntu/+source/dash/+bug/249620

Bah. :-/

[...]

Toggle quote (11 lines)
> To resolve this, I changed the code so that it just writes to a
> temporary file. This is simple and more robust. With the attached
> patch, I was able to use a command like the one above to verify that
> "guix environment --check" works correctly for Guix-built bash, dash,
> ksh, fish, zsh, and ash. I also verified that it works for Fedora's
> /bin/sh and /bin/bash.
>
> What do you think of this file-based approach? Supporting many
> different shells is pretty tricky, but I think this patch does a good
> enough job.

Like Maxime, I’d rather not create a temporary file.

I agree that Dash should be fixed, but in the meantime, we still want
our stuff to work with the broken Dash (it’s the default on
Debian/Ubuntu, isn’t it?).

I don’t have a better idea to offer though…

Ludo’.
M
M
Maxime Devos wrote on 27 Jun 12:34 +0200
Re: bug#51466: bug#53355: guix shell --check: confusing error message
6e1506e91c5aa75ca366a61d65f7e461eebf9923.camel@telenet.be
Ludovic Courtès schreef op ma 27-06-2022 om 12:17 [+0200]:
Toggle quote (4 lines)
> I agree that Dash should be fixed, but in the meantime, we still want
> our stuff to work with the broken Dash (it’s the default on
> Debian/Ubuntu, isn’t it?).

If Dash is fixed, then it's non-broken, and we don't have to work with
the broken Dash. I don't expect fixing it to be more complicated than
the work-arounds in Guile.

Toggle quote (7 lines)
> On 26 June 2022 13:07:11 UTC, Maxime Devos <maximedevos@telenet.be>
> wrote:
> >(To be clear, I still recommend just fixing dash ...)
>
> That's the long-term solution, but Guix needs a work-around
> regardless.

Fixing dash seems to me something that could be done in the short term?

Greetings,
Maxime
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrmHrRccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7srJAQDBxc6gxrtwdV3oqHrvYuNXcyUU
ybPn+SiL783JBBACYwEA/vgxXuWAr3LXik1J9CWTgIV3EHl9We0V/FXp+okeZA0=
=aiwo
-----END PGP SIGNATURE-----


B
(name . Ludovic Courtès)(address . ludo@gnu.org)
20220627112315.GA7679@LionPure
On +2022-06-27 12:17:08 +0200, Ludovic Courtès wrote:
Toggle quote (37 lines)
> Hi Chris,
>
> Chris Marusich <cmmarusich@gmail.com> skribis:
>
> > It turns out that it is probably not OK to rely on shell redirection in
> > this case, after all. For example, "dash does not support multi-digit
> > file descriptors":
> >
> > https://bugs.launchpad.net/ubuntu/+source/dash/+bug/249620
>
> Bah. :-/
>
> [...]
>
> > To resolve this, I changed the code so that it just writes to a
> > temporary file. This is simple and more robust. With the attached
> > patch, I was able to use a command like the one above to verify that
> > "guix environment --check" works correctly for Guix-built bash, dash,
> > ksh, fish, zsh, and ash. I also verified that it works for Fedora's
> > /bin/sh and /bin/bash.
> >
> > What do you think of this file-based approach? Supporting many
> > different shells is pretty tricky, but I think this patch does a good
> > enough job.
>
> Like Maxime, I’d rather not create a temporary file.
>
> I agree that Dash should be fixed, but in the meantime, we still want
> our stuff to work with the broken Dash (it’s the default on
> Debian/Ubuntu, isn’t it?).
>
> I don’t have a better idea to offer though…
>
> Ludo’.
>
>
>
If this is all about capturing an environment as text,
how about

Toggle snippet (3 lines)
xargs -0 < /proc/$$/environ

(not limited to $$ I would think)
--
Regards,
Bengt Richter
B
B
Bengt Richter wrote on 27 Jun 16:22 +0200
Re: bug#53355: bug#51466: bug#53355: guix shell --check: confusing error message
(name . Ludovic Courtès)(address . ludo@gnu.org)
20220627142232.GA17131@LionPure
On +2022-06-27 13:23:15 +0200, bokr@bokr.com wrote:
Toggle quote (10 lines)
> >
> If this is all about capturing an environment as text,
> how about
>
> --8<---------------cut here---------------start------------->8---
> xargs -0 < /proc/$$/environ
> --8<---------------cut here---------------end--------------->8---
>
> [...]
>
Actually, why fight shell stuff when guile can read it?
Or are these modules not available in your context?
Toggle snippet (4 lines)
scheme@(guile-user)> (use-modules (ice-9 textual-ports))
scheme@(guile-user)> (define ep (open-input-file "/proc/12430/environ"))
scheme@(guile-user)> (define es (get-string-all ep))
es from above got it all, when I tried manually as above.

FWIW I also did the same with (use-modules (rnrs bytevectors))
and the nulls show up as the expected zeroes.
--
Regards,
Bengt Richter
L
L
Ludovic Courtès wrote on 28 Jun 09:45 +0200
Re: bug#51466: bug#53355: guix shell --check: confusing error message
(name . Maxime Devos)(address . maximedevos@telenet.be)
877d51fcz9.fsf@gnu.org
Maxime Devos <maximedevos@telenet.be> skribis:

Toggle quote (7 lines)
> Ludovic Courtès schreef op ma 27-06-2022 om 12:17 [+0200]:
>> I agree that Dash should be fixed, but in the meantime, we still want
>> our stuff to work with the broken Dash (it’s the default on
>> Debian/Ubuntu, isn’t it?).
>
> If Dash is fixed, then it's non-broken

Even if a fix goes upstream today, people will still be using a distro
with the broken Dash for years, literally. That’s why I think we have
to cope with it.

Ludo’.
M
M
Maxime Devos wrote on 28 Jun 12:38 +0200
(name . Ludovic Courtès)(address . ludo@gnu.org)
c51705e5eeb98d0287296b71a80f92c0f8c33d1e.camel@telenet.be
Ludovic Courtès schreef op di 28-06-2022 om 09:45 [+0200]:
Toggle quote (13 lines)
> Maxime Devos <maximedevos@telenet.be> skribis:
>
> > Ludovic Courtès schreef op ma 27-06-2022 om 12:17 [+0200]:
> > > I agree that Dash should be fixed, but in the meantime, we still want
> > > our stuff to work with the broken Dash (it’s the default on
> > > Debian/Ubuntu, isn’t it?).
> >
> > If Dash is fixed, then it's non-broken
>
> Even if a fix goes upstream today, people will still be using a distro
> with the broken Dash for years, literally.  That’s why I think we have
> to cope with it.

Then it could be fixed in that distro? And if the distro intentionally
keeps it broken for years, then that seems more a problem in the distro
than Guix to me.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrraGBccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7k4aAP9TLOzjPUNch0uIQnYszrUpUETF
qu719+XYCJh50eAkygEAkcL+44Gn5YwZIjwxXwHmXmsFi1icNlOvGVIQfrrjdg0=
=2gjF
-----END PGP SIGNATURE-----


(
Re: bug#53355: bug#51466: bug#53355: guix shell --check: confusing error message
CL1XD5FVW8CY.3UBQYT248G4A8@guix-aspire
On Tue Jun 28, 2022 at 11:38 AM BST, Maxime Devos wrote:
Toggle quote (4 lines)
> Then it could be fixed in that distro? And if the distro intentionally
> keeps it broken for years, then that seems more a problem in the distro
> than Guix to me.

I believe Ludo' is referring to LTS distros and other situations where
somebody might not update for a long time.

-- (
M
M
Maxime Devos wrote on 28 Jun 19:31 +0200
34951e5d5c9411ab311f3fd797618cd6e899311b.camel@telenet.be
( schreef op di 28-06-2022 om 17:57 [+0100]:
Toggle quote (10 lines)
> On Tue Jun 28, 2022 at 11:38 AM BST, Maxime Devos wrote:
> > Then it could be fixed in that distro? And if the distro intentionally
> > keeps it broken for years, then that seems more a problem in the distro
> > than Guix to me.
>
> I believe Ludo' is referring to LTS distros and other situations where
> somebody might not update for a long time.
>
>     -- (

I'm thinking so too, though I want to mention LTS is not just ‘don't
update’ -- the third letter means ‘support’, which covers backporting
bug fixes, though I don't know if the distro would include dash > 9 fds
in this.

Greetings,
Maxime.
-----BEGIN PGP SIGNATURE-----

iI0EABYKADUWIQTB8z7iDFKP233XAR9J4+4iGRcl7gUCYrs65BccbWF4aW1lZGV2
b3NAdGVsZW5ldC5iZQAKCRBJ4+4iGRcl7kkhAP9K1Al6BW9M8pJsDjGQTT3rNiDA
rACpyQzbJGCO0bl6cAD/VHTELPiAzm+1zWPW+I56BVA09SPlCujGrb9VXo9CAQE=
=4v1A
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 4 Jul 10:11 +0200
(name . ()(address . paren@disroot.org)
8735fhi9gl.fsf@gnu.org
Hi,

"(" <paren@disroot.org> skribis:

Toggle quote (8 lines)
> On Tue Jun 28, 2022 at 11:38 AM BST, Maxime Devos wrote:
>> Then it could be fixed in that distro? And if the distro intentionally
>> keeps it broken for years, then that seems more a problem in the distro
>> than Guix to me.
>
> I believe Ludo' is referring to LTS distros and other situations where
> somebody might not update for a long time.

Yeah. Whatever the reason, it’s a fact that our users might run Guix on
a system with a broken Dash, which is why I think we have to cope with it.

Ludo’.
?