GNU bug report logs - #60365
[PATCH 0/4] Rationalize 'PS1' handling in home, system, and shell

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 60365 in the body.
You can then email your comments to 60365 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: guix-patches <at> gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 0/4] Rationalize 'PS1' handling in home, system, and shell
Date: Tue, 27 Dec 2022 22:23:42 +0100

Hello!

I found some things could be improved in how we deal with ‘PS1’ in
Guix Home, Guix System, and ‘guix shell’ (my initial motivation was
making it easy to change ‘PS1’ from a Home configuration).

These patches fiddle with ‘PS1’.  The most visible effect is that
Guix Home’s ~/.bashrc will no longer provide a default ‘PS1’.

Thoughts?

Ludo’.

Ludovic Courtès (4):
  system, home: Factorize default '.bashrc'.
  system: Define default 'PS1' in /etc/bashrc rather than ~/.bashrc.
  environment: Simplify 'PS1' suggestion on '--check'.
  environment: Avoid false positive on 'PS1' check.

 gnu/home/services/shells.scm | 41 +++---------------------------------
 gnu/system.scm               |  3 +++
 gnu/system/shadow.scm        | 33 +++++++++++++----------------
 guix/scripts/environment.scm | 12 ++++++-----
 4 files changed, 28 insertions(+), 61 deletions(-)


base-commit: 9369c1ccf47d9bf6f2e28a9454c1c329a2044f19
prerequisite-patch-id: aae3af39aa7753dd7ed6d0584587a06d51402386
-- 
2.38.1

Message #8 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 60365 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 1/4] system, home: Factorize default '.bashrc'.
Date: Tue, 27 Dec 2022 22:26:15 +0100

* gnu/system/shadow.scm (%default-bashrc): New variable.  Source
/etc/bashrc only if it exists.
(default-skeletons): Use it.
* gnu/home/services/shells.scm (guix-bashrc): Remove.
(add-bash-configuration): Refer to '%default-bashrc' instead.
---
 gnu/home/services/shells.scm | 41 +++---------------------------------
 gnu/system/shadow.scm        | 26 +++++++++++++----------
 2 files changed, 18 insertions(+), 49 deletions(-)

diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index b529c8e798..d7dd579293 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -19,6 +19,7 @@
 
 (define-module (gnu home services shells)
   #:use-module (gnu services configuration)
+  #:autoload   (gnu system shadow) (%default-bashrc)
   #:use-module (gnu home services utils)
   #:use-module (gnu home services)
   #:use-module (gnu packages shells)
@@ -370,43 +371,6 @@ (define-configuration home-bash-configuration
 won't be read in some cases (if the shell terminates by exec'ing
 another process for example)."))
 
-;; TODO: Use value from (gnu system shadow)
-(define guix-bashrc
-  "\
-# Bash initialization for interactive non-login shells and
-# for remote shells (info \"(bash) Bash Startup Files\").
-
-# Export 'SHELL' to child processes.  Programs such as 'screen'
-# honor it and otherwise use /bin/sh.
-export SHELL
-
-if [[ $- != *i* ]]
-then
-    # We are being invoked from a non-interactive shell.  If this
-    # is an SSH session (as in \"ssh host command\"), source
-    # /etc/profile so we get PATH and other essential variables.
-    [[ -n \"$SSH_CLIENT\" ]] && source /etc/profile
-
-    # Don't do anything else.
-    return
-fi
-
-# Source the system-wide file.
-if [[ -e /etc/bashrc ]]; then
-    source /etc/bashrc
-fi
-
-# Adjust the prompt depending on whether we're in 'guix environment'.
-if [ -n \"$GUIX_ENVIRONMENT\" ]
-then
-    PS1='\\u@\\h \\w [env]\\$ '
-else
-    PS1='\\u@\\h \\w\\$ '
-fi
-alias ls='ls -p --color=auto'
-alias ll='ls -l'
-alias grep='grep --color=auto'\n")
-
 (define (add-bash-configuration config)
   (define (filter-fields field)
     (filter-configuration-fields home-bash-configuration-fields
@@ -449,7 +413,8 @@ (define* (file-if-not-empty field #:optional (extra-content #f))
      ,@(list (file-if-not-empty
               'bashrc
               (if (home-bash-configuration-guix-defaults? config)
-                  (list (serialize-field 'aliases) guix-bashrc)
+                  (list (serialize-field 'aliases)
+                        (plain-file-content %default-bashrc))
                   (list (serialize-field 'aliases))))
              (file-if-not-empty 'bash-logout)))))
 
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index 59f0a02c8b..4360a5ebd1 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2013, 2014, 2015, 2016, 2017, 2018, 2019, 2020 Ludovic Courtès <ludo <at> gnu.org>
+;;; Copyright © 2013-2020, 2022 Ludovic Courtès <ludo <at> gnu.org>
 ;;; Copyright © 2016 Alex Griffin <a <at> ajgrf.com>
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke <at> gnu.org>
 ;;; Copyright © 2020 Efraim Flashner <efraim <at> flashner.co.il>
@@ -63,7 +63,8 @@ (define-module (gnu system shadow)
                user-group-id
                user-group-system?)
 
-  #:export (default-skeletons
+  #:export (%default-bashrc
+            default-skeletons
             skeleton-directory
             %base-groups
             %base-user-accounts
@@ -118,14 +119,8 @@ (define %base-user-accounts
          (create-home-directory? #f)
          (system? #t))))
 
-(define (default-skeletons)
-  "Return the default skeleton files for /etc/skel.  These files are copied by
-'useradd' in the home directory of newly created user accounts."
-
-  (let ((profile (plain-file "bash_profile" "\
-# Honor per-interactive-shell startup file
-if [ -f ~/.bashrc ]; then . ~/.bashrc; fi\n"))
-        (bashrc  (plain-file "bashrc" "\
+(define %default-bashrc
+  (plain-file "bashrc" "\
 # Bash initialization for interactive non-login shells and
 # for remote shells (info \"(bash) Bash Startup Files\").
 
@@ -145,7 +140,7 @@ (define (default-skeletons)
 fi
 
 # Source the system-wide file.
-source /etc/bashrc
+[ -f /etc/bashrc ] && source /etc/bashrc
 
 # Adjust the prompt depending on whether we're in 'guix environment'.
 if [ -n \"$GUIX_ENVIRONMENT\" ]
@@ -157,6 +152,15 @@ (define (default-skeletons)
 alias ls='ls -p --color=auto'
 alias ll='ls -l'
 alias grep='grep --color=auto'\n"))
+
+(define (default-skeletons)
+  "Return the default skeleton files for /etc/skel.  These files are copied by
+'useradd' in the home directory of newly created user accounts."
+
+  (let ((profile (plain-file "bash_profile" "\
+# Honor per-interactive-shell startup file
+if [ -f ~/.bashrc ]; then . ~/.bashrc; fi\n"))
+        (bashrc  %default-bashrc)
         (zprofile    (plain-file "zprofile" "\
 # Honor system-wide environment variables
 source /etc/profile\n"))
-- 
2.38.1

Message #11 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 60365 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 2/4] system: Define default 'PS1' in /etc/bashrc rather than
 ~/.bashrc.
Date: Tue, 27 Dec 2022 22:26:16 +0100

Users can override 'PS1' in ~/.bashrc if they wish.

Previously, on Guix Home, the "default" 'PS1' would be set in ~/.bashrc
when 'home-bash-configuration-guix-defaults?' is true, preventing users
from overriding it via the 'environment-variables' field of
'home-bash-extension'.

* gnu/system/shadow.scm (%default-bashrc): Remove 'PS1' setting.
* gnu/system.scm (operating-system-etc-service): Define PS1 in
/etc/bashrc.
---
 gnu/system.scm        | 3 +++
 gnu/system/shadow.scm | 7 -------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/gnu/system.scm b/gnu/system.scm
index 62c8e0c2b6..d67f9a615b 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -1047,6 +1047,9 @@ (define* (operating-system-etc-service os)
         (bashrc    (plain-file "bashrc" "\
 # Bash-specific initialization.
 
+# Provide a default prompt.  The user's ~/.bashrc can override it.
+PS1='\\u@\\h \\w${GUIX_ENVIRONMENT:+ [env]}\\$ '
+
 # The 'bash-completion' package.
 if [ -f /run/current-system/profile/etc/profile.d/bash_completion.sh ]
 then
diff --git a/gnu/system/shadow.scm b/gnu/system/shadow.scm
index 4360a5ebd1..2e87928368 100644
--- a/gnu/system/shadow.scm
+++ b/gnu/system/shadow.scm
@@ -142,13 +142,6 @@ (define %default-bashrc
 # Source the system-wide file.
 [ -f /etc/bashrc ] && source /etc/bashrc
 
-# Adjust the prompt depending on whether we're in 'guix environment'.
-if [ -n \"$GUIX_ENVIRONMENT\" ]
-then
-    PS1='\\u@\\h \\w [env]\\$ '
-else
-    PS1='\\u@\\h \\w\\$ '
-fi
 alias ls='ls -p --color=auto'
 alias ll='ls -l'
 alias grep='grep --color=auto'\n"))
-- 
2.38.1

Message #14 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 60365 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 3/4] environment: Simplify 'PS1' suggestion on '--check'.
Date: Tue, 27 Dec 2022 22:26:17 +0100

* guix/scripts/environment.scm (validate-child-shell-environment): In
'PS1' suggestion, remove 'export' and avoid 'if'.
---
 guix/scripts/environment.scm | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index ab11b35335..bdbfa03fcf 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -610,10 +610,7 @@ (define-syntax-rule (warn exp ...)
 @file{~/.bashrc}:
 
 @example
-if [ -n \"$GUIX_ENVIRONMENT\" ]
-then
-  export PS1=\"\\u@@\\h \\w [env]\\$ \"
-fi
+PS1='\\u@@\\h \\w${GUIX_ENVIRONMENT:+ [env]}\\$ '
 @end example
 "))))))
 
-- 
2.38.1

Message #17 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: 60365 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: [PATCH 4/4] environment: Avoid false positive on 'PS1' check.
Date: Tue, 27 Dec 2022 22:26:18 +0100

* guix/scripts/environment.scm (validate-child-shell-environment): Do
not warn when 'PS1' refers to 'GUIX_ENVIRONMENT'.
---
 guix/scripts/environment.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/guix/scripts/environment.scm b/guix/scripts/environment.scm
index bdbfa03fcf..c7fd8fd340 100644
--- a/guix/scripts/environment.scm
+++ b/guix/scripts/environment.scm
@@ -601,7 +601,12 @@ (define-syntax-rule (warn exp ...)
       (match (vhash-assoc "PS1" actual)
         (#f #f)
         ((_ . str)
-         (when (and (getenv "PS1") (string=? str (getenv "PS1")))
+         (when (and (getenv "PS1") (string=? str (getenv "PS1"))
+
+                    ;; 'PS1' might be conditional on 'GUIX_ENVIRONMENT', as
+                    ;; shown in the hint below.
+                    (not (or (string-contains str "$GUIX_ENVIRONMENT")
+                             (string-contains str "${GUIX_ENVIRONMENT"))))
            (warning (G_ "'PS1' is the same in sub-shell~%"))
            (display-hint (G_ "Consider setting a different prompt for
 environment shells to make them distinguishable.
-- 
2.38.1

Message #20 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Ludovic Courtès <ludo <at> gnu.org>, 60365 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: [bug#60365] [PATCH 0/4] Rationalize 'PS1' handling in home,
 system, and shell
Date: Wed, 28 Dec 2022 11:23:03 +0400

[Message part 1 (text/plain, inline)]

On 2022-12-27 22:23, Ludovic Courtès wrote:

> Hello!
>

Hi Ludovic!

> I found some things could be improved in how we deal with ‘PS1’ in
> Guix Home, Guix System, and ‘guix shell’ (my initial motivation was
> making it easy to change ‘PS1’ from a Home configuration).
>
> These patches fiddle with ‘PS1’.  The most visible effect is that
> Guix Home’s ~/.bashrc will no longer provide a default ‘PS1’.
>
> Thoughts?

Factorization of bashrc is great!

Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
distros and guix environment won't be noticable in the prompt.  Warning
is useful, but I would expect prompt work OOB, when guix-defualts? is
#t.

The possible alternative solution is to factorize bash_profile and put
PS1 here, so both skeleton and home provided bashrc have it, after that
setting PS1 with environment-variables field will start working.

>
> Ludo’.
>
> Ludovic Courtès (4):
>   system, home: Factorize default '.bashrc'.
>   system: Define default 'PS1' in /etc/bashrc rather than ~/.bashrc.
>   environment: Simplify 'PS1' suggestion on '--check'.
>   environment: Avoid false positive on 'PS1' check.
>
>  gnu/home/services/shells.scm | 41 +++---------------------------------
>  gnu/system.scm               |  3 +++
>  gnu/system/shadow.scm        | 33 +++++++++++++----------------
>  guix/scripts/environment.scm | 12 ++++++-----
>  4 files changed, 28 insertions(+), 61 deletions(-)
>
>
> base-commit: 9369c1ccf47d9bf6f2e28a9454c1c329a2044f19
> prerequisite-patch-id: aae3af39aa7753dd7ed6d0584587a06d51402386

-- 
Best regards,
Andrew Tropin

[signature.asc (application/pgp-signature, inline)]

Message #23 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 60365 <at> debbugs.gnu.org
Subject: Re: bug#60365: [PATCH 0/4] Rationalize 'PS1' handling in home,
 system, and shell
Date: Tue, 03 Jan 2023 23:46:53 +0100

[Message part 1 (text/plain, inline)]

Hi Andrew!

Andrew Tropin <andrew <at> trop.in> skribis:

>> I found some things could be improved in how we deal with ‘PS1’ in
>> Guix Home, Guix System, and ‘guix shell’ (my initial motivation was
>> making it easy to change ‘PS1’ from a Home configuration).
>>
>> These patches fiddle with ‘PS1’.  The most visible effect is that
>> Guix Home’s ~/.bashrc will no longer provide a default ‘PS1’.
>>
>> Thoughts?
>
> Factorization of bashrc is great!
>
> Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
> distros and guix environment won't be noticable in the prompt.  Warning
> is useful, but I would expect prompt work OOB, when guix-defualts? is
> #t.

Yes, you’re right.

> The possible alternative solution is to factorize bash_profile and put
> PS1 here, so both skeleton and home provided bashrc have it, after that
> setting PS1 with environment-variables field will start working.

How about this:

[Message part 2 (text/x-patch, inline)]

diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index d7dd579293..8d7db6b774 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -407,6 +407,9 @@ (define* (file-if-not-empty field #:optional (extra-content #f))
 # Honor per-interactive-shell startup file
 if [ -f ~/.bashrc ]; then source ~/.bashrc; fi
 "
+        (if (home-bash-configuration-guix-defaults? config)
+            "export PS1=...\n"
+            "")
         (serialize-field 'bash-profile)
         (serialize-field 'environment-variables)))
 

[Message part 3 (text/plain, inline)]

That way, if one adds ‘PS1’ to ‘environment-variables’, that will
override the default.

WDYT?

Thanks for your feedback!

Ludo’.

Message #26 received at 60365 <at> debbugs.gnu.org (full text, mbox):

From: Andrew Tropin <andrew <at> trop.in>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 60365 <at> debbugs.gnu.org
Subject: Re: bug#60365: [PATCH 0/4] Rationalize 'PS1' handling in home,
 system, and shell
Date: Wed, 04 Jan 2023 10:00:18 +0400

[Message part 1 (text/plain, inline)]

On 2023-01-03 23:46, Ludovic Courtès wrote:

> Hi Andrew!
>
> Andrew Tropin <andrew <at> trop.in> skribis:
>
>>> I found some things could be improved in how we deal with ‘PS1’ in
>>> Guix Home, Guix System, and ‘guix shell’ (my initial motivation was
>>> making it easy to change ‘PS1’ from a Home configuration).
>>>
>>> These patches fiddle with ‘PS1’.  The most visible effect is that
>>> Guix Home’s ~/.bashrc will no longer provide a default ‘PS1’.
>>>
>>> Thoughts?
>>
>> Factorization of bashrc is great!
>>
>> Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
>> distros and guix environment won't be noticable in the prompt.  Warning
>> is useful, but I would expect prompt work OOB, when guix-defualts? is
>> #t.
>
> Yes, you’re right.
>
>> The possible alternative solution is to factorize bash_profile and put
>> PS1 here, so both skeleton and home provided bashrc have it, after that
>> setting PS1 with environment-variables field will start working.
>
> How about this:
>
> diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
> index d7dd579293..8d7db6b774 100644
> --- a/gnu/home/services/shells.scm
> +++ b/gnu/home/services/shells.scm
> @@ -407,6 +407,9 @@ (define* (file-if-not-empty field #:optional (extra-content #f))
>  # Honor per-interactive-shell startup file
>  if [ -f ~/.bashrc ]; then source ~/.bashrc; fi
>  "
> +        (if (home-bash-configuration-guix-defaults? config)
> +            "export PS1=...\n"

Yep, something like that should work.  Ideally, is to make
%default-bash-profile or even more generic %default-profile and reuse it
across system and home, but it may be not that trivial, so we can
postpone it for now.

> +            "")
>          (serialize-field 'bash-profile)
>          (serialize-field 'environment-variables)))
>  
>
> That way, if one adds ‘PS1’ to ‘environment-variables’, that will
> override the default.
>
> WDYT?
>
> Thanks for your feedback!
>
> Ludo’.

-- 
Best regards,
Andrew Tropin

[signature.asc (application/pgp-signature, inline)]

Message #31 received at 60365-done <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Andrew Tropin <andrew <at> trop.in>
Cc: 60365-done <at> debbugs.gnu.org
Subject: Re: bug#60365: [PATCH 0/4] Rationalize 'PS1' handling in home,
 system, and shell
Date: Thu, 05 Jan 2023 15:20:13 +0100

Andrew Tropin <andrew <at> trop.in> skribis:

> On 2023-01-03 23:46, Ludovic Courtès wrote:

[...]

>>> Not sure about moving PS1 to /etc/bashrc as it won't work on foreign
>>> distros and guix environment won't be noticable in the prompt.  Warning
>>> is useful, but I would expect prompt work OOB, when guix-defualts? is
>>> #t.
>>
>> Yes, you’re right.
>>
>>> The possible alternative solution is to factorize bash_profile and put
>>> PS1 here, so both skeleton and home provided bashrc have it, after that
>>> setting PS1 with environment-variables field will start working.
>>
>> How about this:
>>
>> diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
>> index d7dd579293..8d7db6b774 100644
>> --- a/gnu/home/services/shells.scm
>> +++ b/gnu/home/services/shells.scm
>> @@ -407,6 +407,9 @@ (define* (file-if-not-empty field #:optional (extra-content #f))
>>  # Honor per-interactive-shell startup file
>>  if [ -f ~/.bashrc ]; then source ~/.bashrc; fi
>>  "
>> +        (if (home-bash-configuration-guix-defaults? config)
>> +            "export PS1=...\n"
>
> Yep, something like that should work.  Ideally, is to make
> %default-bash-profile or even more generic %default-profile and reuse it
> across system and home, but it may be not that trivial, so we can
> postpone it for now.

I adjusted things as proposed and pushed:

  b5e3701f1a environment: Avoid false positive on 'PS1' check.
  632e2f7ce1 environment: Simplify 'PS1' suggestion on '--check'.
  086df6ef21 system: Define default 'PS1' in /etc/bashrc rather than ~/.bashrc.
  2127e56bd8 system, home: Factorize default '.bashrc'.

Thanks!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.