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

  • Done
  • quality assurance status badge
Details
3 participants
  • ???
  • Ludovic Courtès
  • Saku Laesvuori
Owner
unassigned
Submitted by
Saku Laesvuori
Severity
normal
S
S
Saku Laesvuori wrote on 21 Jul 2023 12:51
(address . guix-patches@gnu.org)(name . Saku Laesvuori)(address . saku@laesvuori.fi)
20230721105801.10840-1-saku@laesvuori.fi
* 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(-)

Toggle diff (50 lines)
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
?
?
??? wrote on 21 Jul 2023 14:42
(name . Saku Laesvuori)(address . saku@laesvuori.fi)(address . 64765@debbugs.gnu.org)
87bkg5h1vp.fsf@envs.net
Saku Laesvuori <saku@laesvuori.fi> writes:

Toggle quote (10 lines)
> * 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').
S
S
Saku Laesvuori wrote on 21 Jul 2023 16:44
(name . ???)(address . iyzsong@envs.net)(address . 64765@debbugs.gnu.org)
20230721144458.w3we4ve6oqw4m7pn@X-kone
Toggle quote (14 lines)
> > * 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.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEoMkZR3NPB29fCOn/JX0oSiodOjIFAmS6meoACgkQJX0oSiod
OjJ+XBAAsWpZpl3koXu4/EY0eHILF/UaQNC1dT4vFukPinhQkNQPp6P+Gx5DNBSN
L3EE/FteS99MLTywh+JM3H6YqkYDOz08yoqk6DbJV/62N4WWixzme2j4XbVI8hJm
ZSDIwQzV0HWtjVh07YD8NbUhiOYXKX2ekYc2NKzy1/7FCiWDbpmGLtki8IMMSp9q
ReGFEpjZy1v8qWWG4LZ2gOUTBOVJf42q/qtcULN293p5GRv9SB+K/IYdwHWqsiH9
7b0IsWlkR3TEkOO2vSIZxeEZhR5PcdnmWRCFvgRobvJANfn044kCsTaCVJYNjQqH
xn8Nje2u4/fsH6EhdUYeDWgxwWnUYCydnqQYPONEFtvE8ir06JWs0UVwmUFDm5ey
50/gejNAU0EteqqyVOvb5cgQn/yGSDg4ycT1kVV8EvP+HJD9IxRxuN2aLxGegBnt
ltSQf8YcIaokKgqisgXpVNyuQQDvfiCX/5Wz7P/0sWUQtaGIpa7boQ83x44wskbC
b2zK7w6sU5yEY5v6tqjB4J0gtiUR52BG/wjrGuQ4tbW7ThaFVxTXOs0BvodP23i4
qtKS5D0PeFyzr0Y5pyflTpiUSlVZziROOvsNw5uNEtdeTJomzRbAGpVqCvvKVfzv
YkHdO3lDFDxSthfDW2yjLKfjRUcf7SxtWBPfJVvDj80Q5iLTgzQ=
=A30l
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 16 Aug 2023 22:48
(name . Saku Laesvuori)(address . saku@laesvuori.fi)
87sf8i3e8j.fsf_-_@gnu.org
Hi,

Saku Laesvuori <saku@laesvuori.fi> skribis:

Toggle quote (19 lines)
>> > * 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’.
S
S
Saku Laesvuori wrote on 17 Aug 2023 09:36
(name . Ludovic Courtès)(address . ludo@gnu.org)
20230817073640.kowdgkn7fz3tatho@X-kone
Toggle quote (15 lines)
> >> 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.
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEoMkZR3NPB29fCOn/JX0oSiodOjIFAmTdzggACgkQJX0oSiod
OjJsYxAAwhWQBEi+GwLPJNpEOnHhiO6HD6ZVNtfFNAYWTR09pzAGd9ZFTynlhJ33
weC2TxldDtQ4b0DjBfi6j1/zLUtzJOUSMBv3vlSvq4EsbOiRBJcnktkfLJPTvKEV
1snMVAftfaNuAD4CsX+M61AiOQIEiNKUiVzudxUgh+keJ1sLkfHX2DAdl8J2WkzM
sLYzKvVOT8mtdOT2es4fNjdDHrbMSTM5QrDNae/CLAaA9Le8uoRhUImeN1NvDP8O
2RixZHHqLag1cGI20QnUSvaGxQx8k6rpqvsi1QCy570Ykxj1fHlFZdL/nPnEN0FB
8EriLLJn+lOb3wcRLM1UvPYXzyuornCiTRoTVv8xrVnKjFp2kXFN4e7rCZpZWMPN
/o3+XuCKHLlpTDcKkwbV+pZWTUhZ5ws4/vK2UxUt6BaXe5gUPswIsTln1kPw9WqI
TLs8thKBIUcMF3XJYgmyIHFmhsB+oXCsjma8+o+C4TTBGlVME2MBqKSTSYx7CKLZ
VDHiVILOUEKidX2lhPvi0Ul2smcOAe34jMKE2jQq34D7slCD16oI+9jZjcEzDBt9
bXtKGB5nHiZ93cY9yqq05oqIm9+trKLVU8WDhFaASUzJynDDdwhIRYDqNCKqgEMG
dtdXa8pZ+tpXgs+H+7G0BffXlFXbkblEr2SkRAtZunpC5IzdBag=
=Akzn
-----END PGP SIGNATURE-----


S
S
Saku Laesvuori wrote on 17 Aug 2023 09:38
[PATCH v2] gnu: home: zsh: Load environment when running via ssh
(address . 64765@debbugs.gnu.org)(name . Saku Laesvuori)(address . saku@laesvuori.fi)
d54948f15db64b0cc3a9e36d649f0c71f71c49e1.1692257928.git.saku@laesvuori.fi
* 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(-)

Toggle diff (29 lines)
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
L
L
Ludovic Courtès wrote on 17 Sep 2023 15:10
Re: bug#64765: [PATCH] gnu: home: zsh: Also load enviroment in non-login shells
(name . Saku Laesvuori)(address . saku@laesvuori.fi)
87msxlx7w9.fsf_-_@gnu.org
Hi,

Saku Laesvuori <saku@laesvuori.fi> skribis:

Toggle quote (4 lines)
> * 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’.
Closed
?