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'.

Fixes: <https://issues.guix.gnu.org/51466>.

* 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(-)

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