From patchwork Tue Oct 19 10:13:10 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ludovic_Court=C3=A8s?= X-Patchwork-Id: 33946 Return-Path: X-Original-To: patchwork@mira.cbaines.net Delivered-To: patchwork@mira.cbaines.net Received: by mira.cbaines.net (Postfix, from userid 113) id 9898427BBE3; Tue, 19 Oct 2021 11:16:24 +0100 (BST) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on mira.cbaines.net X-Spam-Level: X-Spam-Status: No, score=-2.9 required=5.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_MSPIKE_H2,SPF_HELO_PASS autolearn=unavailable autolearn_force=no version=3.4.2 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) by mira.cbaines.net (Postfix) with ESMTPS id 0BF1427BBE1 for ; Tue, 19 Oct 2021 11:16:24 +0100 (BST) Received: from localhost ([::1]:53422 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mcmAQ-0006qX-Uw for patchwork@mira.cbaines.net; Tue, 19 Oct 2021 06:16:22 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:35512) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1mcm8B-0003yQ-AJ for guix-patches@gnu.org; Tue, 19 Oct 2021 06:14:03 -0400 Received: from debbugs.gnu.org ([209.51.188.43]:37918) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mcm8B-0007ny-0K for guix-patches@gnu.org; Tue, 19 Oct 2021 06:14:03 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1mcm8A-0007Ij-SD for guix-patches@gnu.org; Tue, 19 Oct 2021 06:14:02 -0400 X-Loop: help-debbugs@gnu.org Subject: [bug#51285] [PATCH 2/3] environment: Add '--check'. Resent-From: Ludovic =?utf-8?q?Court=C3=A8s?= Original-Sender: "Debbugs-submit" Resent-CC: guix-patches@gnu.org Resent-Date: Tue, 19 Oct 2021 10:14:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 51285 X-GNU-PR-Package: guix-patches X-GNU-PR-Keywords: patch To: 51285@debbugs.gnu.org Cc: Ludovic =?utf-8?q?Court=C3=A8s?= Received: via spool by 51285-submit@debbugs.gnu.org id=B51285.163463842728022 (code B ref 51285); Tue, 19 Oct 2021 10:14:02 +0000 Received: (at 51285) by debbugs.gnu.org; 19 Oct 2021 10:13:47 +0000 Received: from localhost ([127.0.0.1]:49462 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mcm7u-0007Ho-LH for submit@debbugs.gnu.org; Tue, 19 Oct 2021 06:13:47 -0400 Received: from eggs.gnu.org ([209.51.188.92]:33478) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1mcm7r-0007HF-FN for 51285@debbugs.gnu.org; Tue, 19 Oct 2021 06:13:44 -0400 Received: from fencepost.gnu.org ([2001:470:142:3::e]:47972) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1mcm7l-0007U8-7w; Tue, 19 Oct 2021 06:13:37 -0400 Received: from 91-160-117-201.subs.proxad.net ([91.160.117.201]:59236 helo=gnu.org) by fencepost.gnu.org with esmtpsa (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1mcm7k-0005AA-Tw; Tue, 19 Oct 2021 06:13:37 -0400 From: Ludovic =?utf-8?q?Court=C3=A8s?= Date: Tue, 19 Oct 2021 12:13:10 +0200 Message-Id: <20211019101311.10174-2-ludo@gnu.org> X-Mailer: git-send-email 2.33.0 In-Reply-To: <20211019101311.10174-1-ludo@gnu.org> References: <20211019101311.10174-1-ludo@gnu.org> MIME-Version: 1.0 X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: guix-patches@gnu.org List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-patches-bounces+patchwork=mira.cbaines.net@gnu.org Sender: "Guix-patches" X-getmail-retrieved-from-mailbox: Patches From: Ludovic Courtès * guix/scripts/environment.scm (show-environment-options-help) (%options): Add '--check'. * guix/scripts/environment.scm (child-shell-environment) (validate-child-shell-environment): New procedures. (guix-environment*): Call 'validate-child-shell-environment' when 'check?' key is in OPTS. * doc/guix.texi (Invoking guix shell): Shorten footnote about Bash startup files. Document '--check' and mention startup files. (Invoking guix environment): Document '--check'. --- doc/guix.texi | 39 ++++++--- guix/scripts/environment.scm | 162 ++++++++++++++++++++++++++++++++++- 2 files changed, 189 insertions(+), 12 deletions(-) diff --git a/doc/guix.texi b/doc/guix.texi index dabd7fea1e..e860ccc9b2 100644 --- a/doc/guix.texi +++ b/doc/guix.texi @@ -5640,17 +5640,11 @@ environment, where the new packages are added to search path environment variables such as @code{PATH}. You can, instead, choose to create an @emph{isolated} environment containing nothing but the packages you asked for. Passing the @option{--pure} option clears environment -variable definitions found in the parent environment@footnote{Users -sometimes wrongfully augment environment variables such as @env{PATH} in -their @file{~/.bashrc} file. As a consequence, when @command{guix -environment} launches it, Bash may read @file{~/.bashrc}, thereby -introducing ``impurities'' in these environment variables. It is an -error to define such environment variables in @file{.bashrc}; instead, -they should be defined in @file{.bash_profile}, which is sourced only by -log-in shells. @xref{Bash Startup Files,,, bash, The GNU Bash Reference -Manual}, for details on Bash start-up files.}; passing -@option{--container} goes one step further by spawning a @dfn{container} -isolated from the rest of the system: +variable definitions found in the parent environment@footnote{Be sure to +use the @option{--check} option the first time you use @command{guix +shell} interactively to make sure the shell does not undo the effect of +@option{--pure}.}; passing @option{--container} goes one step further by +spawning a @dfn{container} isolated from the rest of the system: @example guix shell --container emacs gcc-toolchain @@ -5699,6 +5693,24 @@ $ ls "$GUIX_ENVIRONMENT/bin" The available options are summarized below. @table @code +@item --check +Set up the environment and check whether the shell would clobber +environment variables. It's a good idea to use this option the first +time you run @command{guix shell} for an interactive session to make +sure your setup is correct. + +For example, if the shell modifies the @env{PATH} environment variable, +report it since you would get a different environment than what you +asked for. + +Such problems usually indicate that the 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 @file{~/.bash_profile} and @emph{not} in @file{~/.bashrc}---the +former is sourced only by log-in shells. @xref{Bash Startup Files,,, +bash, The GNU Bash Reference Manual}, for details on Bash start-up +files. + @item --development @itemx -D Cause @command{guix shell} to include in the environment the @@ -6065,6 +6077,11 @@ guix environment --preserve='^DISPLAY$' --container --network \ The available options are summarized below. @table @code +@item --check +Set up the environment and check whether the shell would clobber +environment variables. @xref{Invoking guix shell, @option{--check}}, +for more info. + @item --root=@var{file} @itemx -r @var{file} @cindex persistent environment diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm index 05a43659da..7b97a8e39a 100644 --- a/guix/scripts/environment.scm +++ b/guix/scripts/environment.scm @@ -41,12 +41,14 @@ (define-module (guix scripts environment) #:autoload (gnu build accounts) (password-entry group-entry password-entry-name password-entry-directory write-passwd write-group) - #:autoload (guix build syscalls) (set-network-interface-up) + #:autoload (guix build syscalls) (set-network-interface-up openpty login-tty) #:use-module (gnu system file-systems) #:autoload (gnu packages) (specification->package+output) #:autoload (gnu packages bash) (bash) #:autoload (gnu packages bootstrap) (bootstrap-executable %bootstrap-guile) #:use-module (ice-9 match) + #:autoload (ice-9 rdelim) (read-line) + #:use-module (ice-9 vlist) #:use-module (srfi srfi-1) #:use-module (srfi srfi-11) #:use-module (srfi srfi-26) @@ -83,6 +85,8 @@ (define (show-environment-options-help) -m, --manifest=FILE create environment with the manifest from FILE")) (display (G_ " -p, --profile=PATH create environment from profile at PATH")) + (display (G_ " + --check check if the shell clobbers environment variables")) (display (G_ " --pure unset existing environment variables")) (display (G_ " @@ -178,6 +182,9 @@ (define %options (option '(#\V "version") #f #f (lambda args (show-version-and-exit "guix environment"))) + (option '("check") #f #f + (lambda (opt name arg result) + (alist-cons 'check? #t result))) (option '("pure") #f #f (lambda (opt name arg result) (alist-cons 'pure #t result))) @@ -396,6 +403,155 @@ (define* (launch-environment command profile manifest ((program . args) (apply execlp program program args)))) +(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 + (match (primitive-fork) + (0 + (catch #t + (lambda () + (load-profile profile manifest #:pure? #t) + (setenv "GUIX_ENVIRONMENT" profile) + (close-fdes controller) + (login-tty inferior) + (execl shell shell)) + (lambda _ + (primitive-exit 127)))) + (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)))) + + (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) + (string-drop line (+ 1 index)) + table) + table))) + vlist-null + lines)) + +(define* (validate-child-shell-environment profile manifest + #:optional (shell %default-shell)) + "Run SHELL in interactive mode in an environment for PROFILE and MANIFEST +and report clobbered environment variables." + (define warned? #f) + (define-syntax-rule (warn exp ...) + (begin + (set! warned? #t) + (warning exp ...))) + + (info (G_ "checking the environment variables visible from shell '~a'...~%") + shell) + (let ((actual (child-shell-environment shell profile manifest))) + (when (vlist-null? actual) + (leave (G_ "failed to determine environment of shell '~a'~%") + shell)) + (for-each (match-lambda + ((spec . expected) + (let ((name (search-path-specification-variable spec))) + (match (vhash-assoc name actual) + (#f + (warn (G_ "variable '~a' is missing from shell \ +environment~%") + name)) + ((_ . actual) + (cond ((string=? expected actual) + #t) + ((string-prefix? expected actual) + (warn (G_ "variable '~a' has unexpected \ +suffix '~a'~%") + name + (string-drop actual + (string-length expected)))) + (else + (warn (G_ "variable '~a' is clobbered: '~a'~%") + name actual)))))))) + (profile-search-paths profile manifest)) + + ;; Special case. + (match (vhash-assoc "GUIX_ENVIRONMENT" actual) + (#f + (warn (G_ "'GUIX_ENVIRONMENT' is missing from the shell \ +environment~%"))) + ((_ . value) + (unless (string=? value profile) + (warn (G_ "'GUIX_ENVIRONMENT' is set to '~a' instead of '~a'~%") + value profile)))) + + ;; Check the prompt unless we have more important warnings. + (unless warned? + (match (vhash-assoc "PS1" actual) + (#f #f) + (str + (when (and (getenv "PS1") (string=? str (getenv "PS1"))) + (warning (G_ "'PS1' is the same in sub-shell~%")) + (display-hint (G_ "Consider setting a different prompt for +environment shells to make them distinguishable. + +If you are using Bash, you can do that by adding these lines to +@file{~/.bashrc}: + +@example +if [ -n \"$GUIX_ENVIRONMENT\" ] +then + export PS1=\"\\u@@\\h \\w [env]\\$ \" +fi +@end example +")))))) + + (if warned? + (begin + (display-hint (G_ "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 +@file{~/.bash_profile} and @emph{not} in @file{~/.bashrc}. For more +information on Bash startup files, run: + +@example +info \"(bash) Bash Startup Files\" +@end example + +Alternatively, you can avoid the problem by passing the @option{--container} +or @option{-C} option. That will give you a fully isolated environment +running in a \"container\", immune to the issue described above.")) + (exit 1)) + (info (G_ "All is good! The shell gets correct environment \ +variables.~%"))))) + (define* (launch-environment/fork command profile manifest #:key pure? (white-list '())) "Run COMMAND in a new process with an environment containing PROFILE, with @@ -775,6 +931,10 @@ (define manifest (mwhen gc-root (register-gc-root profile gc-root)) + (mwhen (assoc-ref opts 'check?) + (return + (validate-child-shell-environment profile manifest))) + (cond ((assoc-ref opts 'search-paths) (show-search-paths profile manifest #:pure? pure?)