[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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 64765@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 64765
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch