From: Kevin Boulain <kevinboulain@gmail.com>
To: 51466@debbugs.gnu.org
Cc: Kevin Boulain <kevinboulain@gmail.com>
Subject: bug#51466: [PATCH 1/1] environment: properly parse environment variables during --check
Date: Sun, 27 Mar 2022 19:01:24 +0200 [thread overview]
Message-ID: <20220327170124.2846-2-kevinboulain@gmail.com> (raw)
In-Reply-To: <20220327170124.2846-1-kevinboulain@gmail.com>
Should solve https://issues.guix.gnu.org/51466.
This redirects the env command's output to a temporary file so that
there is no way to get it mixed with the shell's output (like PS1).
Additionnally:
- Remove GUIX-CHECK-DONE and read: I don't think there is a need for
them as discarding the output until the shell exits is enough to
guarantee the environment has been dumped. Sadly, Linux doesn't
report EOF but EIO when the shell exits and the pty gets closed hence
the catch/throw.
- Don't try to parse environment variables by splitting them on \n,
instead use env's -0 option to separate environment variables with
\0. This prevent incorrect parsing of some multi-line variables
(e.g.: f() { echo "hello"; }; declare -fx f) but isn't standard.
* guix/scripts/environment.scm (child-shell-environment): make
environment variables parsing more robust.
* tests/guix-environment.sh: add a simple test that was reliably
failing on my machine.
---
guix/scripts/environment.scm | 65 +++++++++++++++++++++++-------------
tests/guix-environment.sh | 16 +++++++++
2 files changed, 57 insertions(+), 24 deletions(-)
diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index ec071402f4..4ff13c6bde 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -48,7 +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)
- #:autoload (ice-9 rdelim) (read-line)
+ #:autoload (ice-9 rdelim) (read-delimited read-line)
#:use-module (ice-9 vlist)
#:use-module (srfi srfi-1)
#:use-module (srfi srfi-11)
@@ -413,16 +413,22 @@ (define (child-shell-environment shell profile manifest)
"Create a child process, load PROFILE and MANIFEST, and then run SHELL in
interactive mode in it. Return a name/value vhash for all the variables shown
by running 'set' in the shell."
- (define-values (controller inferior)
- (openpty))
-
- (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")
-
(define lines
+ (let* ((environment-port (mkstemp "/tmp/guix-enviroment-XXXXXX"))
+ (environment-file (port-filename environment-port))
+ ;; 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.)
+ ;; We rely on env --null to mark the end of environment variables.
+ ;; This is expected to be safe because POSIX defines environment
+ ;; variables end with '\0' (but does't document the --null option).
+ ;; Some shells, like Bash, allow to export functions, which will span
+ ;; multiple lines and break any trivial parsing relying on '\n'.
+ (script (format #f "env --null > ~a || /usr/bin/env --null > ~a; exit\n"
+ environment-file environment-file)))
+
+ (let-values (((controller inferior) (openpty)))
(match (primitive-fork)
(0
(catch #t
@@ -436,26 +442,37 @@ (define lines
(primitive-exit 127))))
(pid
(close-fdes inferior)
- (let* ((port (fdopen controller "r+l"))
- (result (begin
+ (let ((port (fdopen controller "r+l")))
(display script port)
+ (catch 'system-error
+ (lambda ()
+ ;; We aren't interested in the output of the shell itself,
+ ;; drop it.
+ (while (not (eof-object? (read-line port)))))
+ (lambda args
+ (let ((errno (system-error-errno args)))
+ (cond
+ ((= errno EIO)
+ ;; On Linux, a read won't return EOF but will fail with EIO
+ ;; when the device is closed:
+ ;; https://bugs.python.org/issue5380#msg252544
+ #t)
+ (#t
+ (apply throw args))))))
+ (close-port port)
+ (waitpid pid)))))
+
+ (let ((result (begin
(let loop ((lines '()))
- (match (read-line port)
+ (match (read-delimited "\0" environment-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))))
+ (loop (cons line lines))))))))
+ (close-port environment-port)
+ (delete-file environment-file)
+ result)))
(fold (lambda (line table)
- ;; Note: 'set' in fish outputs "NAME VALUE" instead of "NAME=VALUE"
- ;; but it also truncates values anyway, so don't try to support it.
(let ((index (string-index line #\=)))
(if index
(vhash-cons (string-take line index)
diff --git a/tests/guix-environment.sh b/tests/guix-environment.sh
index 95fe95b437..114bf9fbf5 100644
--- a/tests/guix-environment.sh
+++ b/tests/guix-environment.sh
@@ -258,3 +258,19 @@ then
guix gc --references "$profile" | grep "$dep"
done
fi
+
+# https://issues.guix.gnu.org/51466
+# guix environment --check was sometimes unable to find PKG_CONFIG_PATH
+# because the env command is sent before the prompt gets printed and there
+# is no proper way to deinterleave streams:
+# ;;; (read-line "env || /usr/bin/env || set; echo GUIX-CHECK-DONE; read x; exit\r")
+# ;;; (read-line "bash-5.1$ PKG_CONFIG_PATH=/gnu/store/0ysl5arpf0yf3pn15afr450k676lgdq3-profile/lib/pkgconfig\r")
+# ;;; (read-line "PWD=/home/ether/source/guix\r")
+bash=$(mktemp)
+cat > "$bash" << 'BASH'
+#!/usr/bin/env bash
+exec bash --noprofile --rcfile /dev/null "$@"
+BASH
+chmod +x "$bash"
+env SHELL=$bash guix environment --check --pure guix -- env
+rm -f "$bash"
prev parent reply other threads:[~2022-03-28 17:02 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-28 19:08 bug#51466: guix shell --check reports missing PKG_CONFIG_PATH on Debian bookworm Vagrant Cascadian
2021-10-29 19:06 ` Ludovic Courtès
2021-10-30 6:35 ` Vagrant Cascadian
2021-10-30 14:21 ` Ludovic Courtès
2022-03-27 17:01 ` bug#51466: [PATCH 0/1] environment: properly parse environment variables during --check Kevin Boulain
2022-03-27 17:01 ` Kevin Boulain [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220327170124.2846-2-kevinboulain@gmail.com \
--to=kevinboulain@gmail.com \
--cc=51466@debbugs.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/guix.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.