GNU bug report logs - #64765
[PATCH] gnu: home: zsh: Also load enviroment in non-login shells

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 64765 in the body.
You can then email your comments to 64765 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: Saku Laesvuori <saku <at> laesvuori.fi>
To: guix-patches <at> gnu.org
Cc: Saku Laesvuori <saku <at> laesvuori.fi>
Subject: [PATCH] gnu: home: zsh: Also load enviroment in non-login shells
Date: Fri, 21 Jul 2023 13:51:19 +0300

* gnu/home/services/shells.scm (zsh-file-zshenv): Add snippet to source
profiles.
(zsh-file-zprofile): Remove profile sourcing snippet.
(zsh-get-configuration-files): Always add .zshenv as it is never empty.
Check that .zprofile is not empty before adding it.
---
The service incorrectly assumed that shells are either login shells or
started from another shell. For example, ssh with a command argument
starts shells that aren't login shells nor started from another shell.

 gnu/home/services/shells.scm | 22 +++++++++-------------
 1 file changed, 9 insertions(+), 13 deletions(-)

diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index 7960590e7c..93a3b38267 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -182,21 +182,18 @@ (define* (zsh-field-not-empty? config field)
 (define (zsh-file-zshenv config)
   (mixed-text-file
    "zshenv"
-   (zsh-serialize-field config 'zshenv)
-   (zsh-serialize-field config 'environment-variables)))
-
-(define (zsh-file-zprofile config)
-  (mixed-text-file
-   "zprofile"
    "\
 # Set up the system, user profile, and related variables.
 source /etc/profile
 # Set up the home environment profile.
 source ~/.profile
-
-# It's only necessary if zsh is a login shell, otherwise profiles will
-# be already sourced by bash
 "
+   (zsh-serialize-field config 'zshenv)
+   (zsh-serialize-field config 'environment-variables)))
+
+(define (zsh-file-zprofile config)
+  (mixed-text-file
+   "zprofile"
    (zsh-serialize-field config 'zprofile)))
 
 (define (zsh-file-by-field config field)
@@ -208,10 +205,9 @@ (define (zsh-file-by-field config field)
         (zsh-serialize-field config field)))))
 
 (define (zsh-get-configuration-files config)
-  `((".zprofile" ,(zsh-file-by-field config 'zprofile)) ;; Always non-empty
-    ,@(if (or (zsh-field-not-empty? config 'zshenv)
-              (zsh-field-not-empty? config 'environment-variables))
-          `((".zshenv" ,(zsh-file-by-field config 'zshenv))) '())
+  `((".zshenv" ,(zsh-file-by-field config 'zshenv)) ;; Always non-empty
+    ,@(if (zsh-field-not-empty? config 'zprofile)
+          `((".zprofile" ,(zsh-file-by-field config 'zprofile))) '())
     ,@(if (zsh-field-not-empty? config 'zshrc)
           `((".zshrc" ,(zsh-file-by-field config 'zshrc))) '())
     ,@(if (zsh-field-not-empty? config 'zlogin)

base-commit: e401eff97706dc6cdaf20b01dd12e291d7d13c2b
-- 
2.41.0

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

From: 宋文武 <iyzsong <at> envs.net>
To: Saku Laesvuori <saku <at> laesvuori.fi>
Cc: 64765 <at> debbugs.gnu.org
Subject: Re: bug#64765: [PATCH] gnu: home: zsh: Also load enviroment in
 non-login shells
Date: Fri, 21 Jul 2023 20:42:34 +0800

Saku Laesvuori <saku <at> laesvuori.fi> writes:

> * gnu/home/services/shells.scm (zsh-file-zshenv): Add snippet to source
> profiles.
> (zsh-file-zprofile): Remove profile sourcing snippet.
> (zsh-get-configuration-files): Always add .zshenv as it is never empty.
> Check that .zprofile is not empty before adding it.
> ---
> The service incorrectly assumed that shells are either login shells or
> started from another shell. For example, ssh with a command argument
> starts shells that aren't login shells nor started from another shell.

Hello, this looks reasonable to me, only one question:
Will ~/.guix-home/profile/etc/profile be sourced multiple times with
duplicated search-path entries?  (eg: check 'env' in 'zsh' in 'zsh').

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

From: Saku Laesvuori <saku <at> laesvuori.fi>
To: 宋文武 <iyzsong <at> envs.net>
Cc: 64765 <at> debbugs.gnu.org
Subject: Re: bug#64765: [PATCH] gnu: home: zsh: Also load enviroment in
 non-login shells
Date: Fri, 21 Jul 2023 17:44:58 +0300

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

> > * gnu/home/services/shells.scm (zsh-file-zshenv): Add snippet to source
> > profiles.
> > (zsh-file-zprofile): Remove profile sourcing snippet.
> > (zsh-get-configuration-files): Always add .zshenv as it is never empty.
> > Check that .zprofile is not empty before adding it.
> > ---
> > The service incorrectly assumed that shells are either login shells or
> > started from another shell. For example, ssh with a command argument
> > starts shells that aren't login shells nor started from another shell.
> 
> Hello, this looks reasonable to me, only one question:
> Will ~/.guix-home/profile/etc/profile be sourced multiple times with
> duplicated search-path entries?  (eg: check 'env' in 'zsh' in 'zsh').

Yes, but I don't think it causes any problems aside from adding useless
data to the environment. This could be prevented by exporting
GUIX_PROFILE_SOURCED=1 or something similar and only sourcing profiles
if it isn't set.

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

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Saku Laesvuori <saku <at> laesvuori.fi>
Cc: 64765 <at> debbugs.gnu.org, 宋文武 <iyzsong <at> envs.net>
Subject: Re: bug#64765: [PATCH] gnu: home: zsh: Also load enviroment in
 non-login shells
Date: Wed, 16 Aug 2023 22:48:44 +0200

Hi,

Saku Laesvuori <saku <at> laesvuori.fi> skribis:

>> > * gnu/home/services/shells.scm (zsh-file-zshenv): Add snippet to source
>> > profiles.
>> > (zsh-file-zprofile): Remove profile sourcing snippet.
>> > (zsh-get-configuration-files): Always add .zshenv as it is never empty.
>> > Check that .zprofile is not empty before adding it.
>> > ---
>> > The service incorrectly assumed that shells are either login shells or
>> > started from another shell. For example, ssh with a command argument
>> > starts shells that aren't login shells nor started from another shell.
>> 
>> Hello, this looks reasonable to me, only one question:
>> Will ~/.guix-home/profile/etc/profile be sourced multiple times with
>> duplicated search-path entries?  (eg: check 'env' in 'zsh' in 'zsh').
>
> Yes, but I don't think it causes any problems aside from adding useless
> data to the environment. This could be prevented by exporting
> GUIX_PROFILE_SOURCED=1 or something similar and only sourcing profiles
> if it isn't set.

I doesn’t sound great though, and I’m sure it could break obscure
things, like #include_next in C.

Is there a way this could be avoided?  (I’m not familiar with Zsh so I’m
not offering to help; just looking for pending patches to apply.  :-))

Thanks,
Ludo’.

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

From: Saku Laesvuori <saku <at> laesvuori.fi>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 64765 <at> debbugs.gnu.org, 宋文武 <iyzsong <at> envs.net>
Subject: Re: bug#64765: [PATCH] gnu: home: zsh: Also load enviroment in
 non-login shells
Date: Thu, 17 Aug 2023 10:36:40 +0300

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

> >> Hello, this looks reasonable to me, only one question:
> >> Will ~/.guix-home/profile/etc/profile be sourced multiple times with
> >> duplicated search-path entries?  (eg: check 'env' in 'zsh' in 'zsh').
> >
> > Yes, but I don't think it causes any problems aside from adding useless
> > data to the environment. This could be prevented by exporting
> > GUIX_PROFILE_SOURCED=1 or something similar and only sourcing profiles
> > if it isn't set.
> 
> I doesn’t sound great though, and I’m sure it could break obscure
> things, like #include_next in C.
> 
> Is there a way this could be avoided?  (I’m not familiar with Zsh so I’m
> not offering to help; just looking for pending patches to apply.  :-))

It could be avoided properly by making guix-generated
profiles ensure that they can't be sourced multiple times
(e.g. [ -z "$GUIX_PROFILE_SOURCED" ] || return ; GUIX_PROFILE_SOURCED=1).
The specific problem I was facing was with running commands with ssh,
which the bash service seems to fix by sourcing /etc/profile from bashrc
when used via ssh. I'll send a patch for this ssh-specific solution but
I'm not sure if it's the best way to fix this.

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

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

From: Saku Laesvuori <saku <at> laesvuori.fi>
To: 64765 <at> debbugs.gnu.org
Cc: Saku Laesvuori <saku <at> laesvuori.fi>
Subject: [PATCH v2] gnu: home: zsh: Load environment when running via ssh
Date: Thu, 17 Aug 2023 10:38:48 +0300

* gnu/home/services/shells.scm (zsh-file-zshenv): Add snippet to source
/etc/profile when running via ssh.
(zsh-get-configuration-files): Always add .zshenv as it is never empty.
---
 gnu/home/services/shells.scm | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/gnu/home/services/shells.scm b/gnu/home/services/shells.scm
index 7960590e7c..9dd56f634a 100644
--- a/gnu/home/services/shells.scm
+++ b/gnu/home/services/shells.scm
@@ -183,7 +183,8 @@ (define (zsh-file-zshenv config)
   (mixed-text-file
    "zshenv"
    (zsh-serialize-field config 'zshenv)
-   (zsh-serialize-field config 'environment-variables)))
+   (zsh-serialize-field config 'environment-variables)
+   "[ -n \"$SSH_CLIENT\" ] && source /etc/profile"))
 
 (define (zsh-file-zprofile config)
   (mixed-text-file
@@ -209,9 +210,7 @@ (define (zsh-file-by-field config field)
 
 (define (zsh-get-configuration-files config)
   `((".zprofile" ,(zsh-file-by-field config 'zprofile)) ;; Always non-empty
-    ,@(if (or (zsh-field-not-empty? config 'zshenv)
-              (zsh-field-not-empty? config 'environment-variables))
-          `((".zshenv" ,(zsh-file-by-field config 'zshenv))) '())
+    (".zshenv" ,(zsh-file-by-field config 'zshenv)) ;; Always non-empty
     ,@(if (zsh-field-not-empty? config 'zshrc)
           `((".zshrc" ,(zsh-file-by-field config 'zshrc))) '())
     ,@(if (zsh-field-not-empty? config 'zlogin)

base-commit: ad4520b92662e42d7d0b1e648b2068300dbb95c8
-- 
2.41.0

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

From: Ludovic Courtès <ludo <at> gnu.org>
To: Saku Laesvuori <saku <at> laesvuori.fi>
Cc: paren <at> disroot.org, 64765-done <at> debbugs.gnu.org,
 Andrew Tropin <andrew <at> trop.in>
Subject: Re: bug#64765: [PATCH] gnu: home: zsh: Also load enviroment in
 non-login shells
Date: Sun, 17 Sep 2023 15:10:30 +0200

Hi,

Saku Laesvuori <saku <at> laesvuori.fi> skribis:

> * gnu/home/services/shells.scm (zsh-file-zshenv): Add snippet to source
> /etc/profile when running via ssh.
> (zsh-get-configuration-files): Always add .zshenv as it is never empty.

Applied, thanks!

Ludo’.

Previous Next

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