Hi Ludo, Ludovic Courtès <l...@gnu.org> writes:
> 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. > 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. > 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. > 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 PGP: https://savannah.gnu.org/people/viewgpg.php?user_id=106836
From 9a1cef589abf01b61e22656f44c76441f4c50ffd Mon Sep 17 00:00:00 2001 From: Chris Marusich <cmmarus...@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. --- guix/scripts/environment.scm | 29 ++++++++++++++++++++++++----- 1 file changed, 24 insertions(+), 5 deletions(-) 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
signature.asc
Description: PGP signature
