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

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

[Saku Laesvuori via Guix-patches via]

* 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

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

[宋文武 via Guix-patches via]

Saku Laesvuori <saku@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').

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

[Saku Laesvuori via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 955 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

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

[Ludovic Courtès]

Hi,

Saku Laesvuori <saku@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’.

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

[Saku Laesvuori via Guix-patches via]

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#64765] [PATCH v2] gnu: home: zsh: Load environment when running via ssh

[Saku Laesvuori via Guix-patches via]

* 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

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

[Ludovic Courtès]

Hi,

Saku Laesvuori <saku@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’.