bug#54545: [Guix Home] ‘shepherd’ started twice?
Hi, Andrew Tropin skribis: [...] > for example I can't find info about > Fixes line even with search That part is not written (yet!), but you’ve had the opportunity to look at the Git log I guess. :-) >>> From 56d16b4cd511f6837329b888dade0c6d6da4d89d Mon Sep 17 00:00:00 2001 >>> From: Andrew Tropin >>> Date: Tue, 12 Apr 2022 12:19:50 +0300 >>> Subject: [PATCH 2/3] home: shepherd: Use run-on-change to reload shepherd >>> config. >>> >>> * gnu/home/services/shepherd.scm: Add shepherd configuration to >>> XDG_CONFIG_HOME and use it instead of full path to the store. It's necessary >>> to use run-on-change service. [...] >> How does this relate to the bug at hand? >> >> https://issues.guix.gnu.org/54545 >> > > Almost directly. > > During activation if there is no shepherd process we tried to launch a > new one, which was useful back in the days, when I was testing changes > frequently, but can be kinda unexpected for user, if they stopped > Shepherd for some reason, but during activation it started again > automatically. As we discussed earlier: > Probably we need to do config reload using on-change service and also not trigger on-change stuff if user isn't logged in. > >>> Makes sense. > > it would be nice to reload configuration only if it's changed. To make > run-on-change work we need to store a config somewhere in > home-environment, to make it possible to compare with previous > generation. files/.config/shepherd/init.scm looks like a good match > here, in addition to on-change functionality it increases explorability. Hmm let’s not invent some new fancy way to update Shepherd services and instead use what we already have for Guix System. I’m closing this issue; please open a new one if there’s more to discuss! Thanks, Ludo’.
bug#54545: [Guix Home] ‘shepherd’ started twice?
On 2022-04-12 20:28, Ludovic Courtès wrote:
> Hi,
>
> Andrew Tropin skribis:
>
>> Prepared a patch series, which fixes the issues and sligthly adjusts the
>> way home shepherd reload configuration logic works, now it happens only
>> if configuration is changed and also it doesn't try to be smart and
>> start a shepherd if it's not started yet.
>>
>> From d2578f8924217451ca20f0b61fd6f9b9d31c930d Mon Sep 17 00:00:00 2001
>> From: Andrew Tropin
>> Date: Tue, 12 Apr 2022 11:30:58 +0300
>> Subject: [PATCH 1/3] home: shepherd: Prevent launching the second instance.
>>
>> * gnu/home/services/shepherd.scm: Prevent launching the second instance.
>
> I applied this one with a convention commit log, including a “Fixes”
> line, which is important for traceability. (Please check the ChangeLog
> convention for future patches.)
>
Read it from time to time, but the information about contribution
guidelines is too much spreaded, for example I can't find info about
Fixes line even with search, but maybe I just not very attentive.
https://guix.gnu.org/en/manual/devel/en/guix.html#Sending-a-Patch-Series-1
https://www.gnu.org/prep/standards/standards.html#Change-Logs
>> From 56d16b4cd511f6837329b888dade0c6d6da4d89d Mon Sep 17 00:00:00 2001
>> From: Andrew Tropin
>> Date: Tue, 12 Apr 2022 12:19:50 +0300
>> Subject: [PATCH 2/3] home: shepherd: Use run-on-change to reload shepherd
>> config.
>>
>> * gnu/home/services/shepherd.scm: Add shepherd configuration to
>> XDG_CONFIG_HOME and use it instead of full path to the store. It's necessary
>> to use run-on-change service.
>
> [...]
>
>> +++ b/gnu/home/services/shepherd.scm
>> @@ -105,27 +105,30 @@ (define (launch-shepherd-gexp config)
>> (system*
>> #$(file-append shepherd "/bin/shepherd")
>> "--logfile"
>> - (string-append log-dir "/shepherd.log")
>> - "--config"
>> - #$(home-shepherd-configuration-file services shepherd)
>> + (string-append log-dir "/shepherd.log")
>> #~"")))
>>
>> (define (reload-configuration-gexp config)
>>(let* ((shepherd (home-shepherd-configuration-shepherd config))
>> (services (home-shepherd-configuration-services config)))
>> -#~(system*
>> - #$(file-append shepherd "/bin/herd")
>> - "load" "root"
>> - #$(home-shepherd-configuration-file services shepherd
>> +#~(when (file-exists?
>> + (string-append
>> + (or (getenv "XDG_RUNTIME_DIR")
>> + (format #f "/run/user/~a" (getuid)))
>> + "/shepherd/socket"))
>> +(system*
>> + #$(file-append shepherd "/bin/herd")
>> + "load" "root"
>> + #$(home-shepherd-configuration-file services shepherd)
>>
>> -(define (ensure-shepherd-gexp config)
>> - #~(if (file-exists?
>> - (string-append
>> - (or (getenv "XDG_RUNTIME_DIR")
>> - (format #f "/run/user/~a" (getuid)))
>> - "/shepherd/socket"))
>> -#$(reload-configuration-gexp config)
>> -#$(launch-shepherd-gexp config)))
>> +(define (add-shepherd-configuration config)
>> + (let* ((shepherd (home-shepherd-configuration-shepherd config))
>> + (services (home-shepherd-configuration-services config)))
>> +`(("shepherd/init.scm"
>> + ,(home-shepherd-configuration-file services shepherd)
>> +
>> +(define (home-shepherd-run-on-change config)
>> + `(("files/.config/shepherd/init.scm" ,(reload-configuration-gexp
>> config
>
> How does this relate to the bug at hand?
>
> https://issues.guix.gnu.org/54545
>
Almost directly.
During activation if there is no shepherd process we tried to launch a
new one, which was useful back in the days, when I was testing changes
frequently, but can be kinda unexpected for user, if they stopped
Shepherd for some reason, but during activation it started again
automatically. As we discussed earlier:
>>> Probably we need to do config reload using on-change service and also
>>> not trigger on-change stuff if user isn't logged in.
>> Makes sense.
it would be nice to reload configuration only if it's changed. To make
run-on-change work we need to store a config somewhere in
home-environment, to make it possible to compare with previous
generation. files/.config/shepherd/init.scm looks like a good match
here, in addition to on-change functionality it increases explorability.
> As discussed elsewhere, I find it less ambiguous to pass store file
> names for configuration files.
>
I don't know how much value in it, but we can keep those lines:
>> - "--config"
>> - #$(home-shepherd-configuration-file services shepherd)
>
> That can be a drawback in some cases, for daemons that won’t be able
> to load a new config from a different location, but shepherd is not in
> that category: it can load a config file via ‘herd load root’ from any
> place.
>
bug#54545: [Guix Home] ‘shepherd’ started twice?
Hi,
Andrew Tropin skribis:
> Prepared a patch series, which fixes the issues and sligthly adjusts the
> way home shepherd reload configuration logic works, now it happens only
> if configuration is changed and also it doesn't try to be smart and
> start a shepherd if it's not started yet.
>
> From d2578f8924217451ca20f0b61fd6f9b9d31c930d Mon Sep 17 00:00:00 2001
> From: Andrew Tropin
> Date: Tue, 12 Apr 2022 11:30:58 +0300
> Subject: [PATCH 1/3] home: shepherd: Prevent launching the second instance.
>
> * gnu/home/services/shepherd.scm: Prevent launching the second instance.
I applied this one with a convention commit log, including a “Fixes”
line, which is important for traceability. (Please check the ChangeLog
convention for future patches.)
> From 56d16b4cd511f6837329b888dade0c6d6da4d89d Mon Sep 17 00:00:00 2001
> From: Andrew Tropin
> Date: Tue, 12 Apr 2022 12:19:50 +0300
> Subject: [PATCH 2/3] home: shepherd: Use run-on-change to reload shepherd
> config.
>
> * gnu/home/services/shepherd.scm: Add shepherd configuration to
> XDG_CONFIG_HOME and use it instead of full path to the store. It's necessary
> to use run-on-change service.
[...]
> +++ b/gnu/home/services/shepherd.scm
> @@ -105,27 +105,30 @@ (define (launch-shepherd-gexp config)
> (system*
> #$(file-append shepherd "/bin/shepherd")
> "--logfile"
> - (string-append log-dir "/shepherd.log")
> - "--config"
> - #$(home-shepherd-configuration-file services shepherd)
> + (string-append log-dir "/shepherd.log")
> #~"")))
>
> (define (reload-configuration-gexp config)
>(let* ((shepherd (home-shepherd-configuration-shepherd config))
> (services (home-shepherd-configuration-services config)))
> -#~(system*
> - #$(file-append shepherd "/bin/herd")
> - "load" "root"
> - #$(home-shepherd-configuration-file services shepherd
> +#~(when (file-exists?
> + (string-append
> + (or (getenv "XDG_RUNTIME_DIR")
> + (format #f "/run/user/~a" (getuid)))
> + "/shepherd/socket"))
> +(system*
> + #$(file-append shepherd "/bin/herd")
> + "load" "root"
> + #$(home-shepherd-configuration-file services shepherd)
>
> -(define (ensure-shepherd-gexp config)
> - #~(if (file-exists?
> - (string-append
> - (or (getenv "XDG_RUNTIME_DIR")
> - (format #f "/run/user/~a" (getuid)))
> - "/shepherd/socket"))
> -#$(reload-configuration-gexp config)
> -#$(launch-shepherd-gexp config)))
> +(define (add-shepherd-configuration config)
> + (let* ((shepherd (home-shepherd-configuration-shepherd config))
> + (services (home-shepherd-configuration-services config)))
> +`(("shepherd/init.scm"
> + ,(home-shepherd-configuration-file services shepherd)
> +
> +(define (home-shepherd-run-on-change config)
> + `(("files/.config/shepherd/init.scm" ,(reload-configuration-gexp config
How does this relate to the bug at hand?
https://issues.guix.gnu.org/54545
As discussed elsewhere, I find it less ambiguous to pass store file
names for configuration files.
That can be a drawback in some cases, for daemons that won’t be able to
load a new config from a different location, but shepherd is not in that
category: it can load a config file via ‘herd load root’ from any place.
Last, it would be nice if we could use the (guix scripts system
reconfigure) machinery like ‘guix system reconfigure’ and ‘guix deploy’
to upgrade services. An idea for future work. :-)
> From e80e9fae6f6bcd478fa904aad8eb426da3f42f10 Mon Sep 17 00:00:00 2001
> From: Andrew Tropin
> Date: Tue, 12 Apr 2022 12:23:26 +0300
> Subject: [PATCH 3/3] home: run-on-first-login: Add a startup message to the
> script.
>
> gnu/home/services.scm: Add a startup message to the script to make it clear
> when it begins.
> ---
> gnu/home/services.scm | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/gnu/home/services.scm b/gnu/home/services.scm
> index 49bd6e3555..e2c51910a8 100644
> --- a/gnu/home/services.scm
> +++ b/gnu/home/services.scm
> @@ -344,6 +344,7 @@ (define (compute-on-first-login-script _ gexps)
> #~(begin
> (use-modules (guix i18n))
> #$%initialize-gettext
> + (display (G_ "Starting run-on-first-login script.\n\n"))
I’m not fond of unconditional low-level logging. In Guix there’s
relatively little logging and whatever logging there is is controlled by
‘-v’; I think we should follow that approach as much as possible.
Thank you!
Ludo’.
bug#54545: [Guix Home] ‘shepherd’ started twice?
On 2022-04-04 22:16, Ludovic Courtès wrote: > Hi, > > Andrew Tropin skribis: > >> Activation script tries to load latest shepherd configuration with `herd >> load root ./path/to/config.scm` and it starts a shepherd process. Login >> shell starts the shepherd process as well. Probably we need to do >> config reload using on-change service and also not trigger on-change >> stuff if user isn't logged in. > > Makes sense. > >> I can think on the proper solution and make a patch with a fix later >> this week or beginning of the next week. > > Awesome, thanks for taking a look! > > Ludo’. Prepared a patch series, which fixes the issues and sligthly adjusts the way home shepherd reload configuration logic works, now it happens only if configuration is changed and also it doesn't try to be smart and start a shepherd if it's not started yet. From d2578f8924217451ca20f0b61fd6f9b9d31c930d Mon Sep 17 00:00:00 2001 From: Andrew Tropin Date: Tue, 12 Apr 2022 11:30:58 +0300 Subject: [PATCH 1/3] home: shepherd: Prevent launching the second instance. * gnu/home/services/shepherd.scm: Prevent launching the second instance. --- gnu/home/services/shepherd.scm | 26 +++--- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/gnu/home/services/shepherd.scm b/gnu/home/services/shepherd.scm index 012585fea4..df6bbb30e6 100644 --- a/gnu/home/services/shepherd.scm +++ b/gnu/home/services/shepherd.scm @@ -93,17 +93,21 @@ (define (launch-shepherd-gexp config) (services (home-shepherd-configuration-services config))) (if (home-shepherd-configuration-auto-start? config) (with-imported-modules '((guix build utils)) - #~(let ((log-dir (or (getenv "XDG_LOG_HOME") - (format #f "~a/.local/var/log" (getenv "HOME") - ((@ (guix build utils) mkdir-p) log-dir) - (system* - #$(file-append shepherd "/bin/shepherd") - "--logfile" - (string-append -log-dir -"/shepherd.log") - "--config" - #$(home-shepherd-configuration-file services shepherd + #~(unless (file-exists? + (string-append + (or (getenv "XDG_RUNTIME_DIR") + (format #f "/run/user/~a" (getuid))) + "/shepherd/socket")) + (let ((log-dir (or (getenv "XDG_LOG_HOME") + (format #f "~a/.local/var/log" + (getenv "HOME") +((@ (guix build utils) mkdir-p) log-dir) +(system* + #$(file-append shepherd "/bin/shepherd") + "--logfile" + (string-append log-dir "/shepherd.log") + "--config" + #$(home-shepherd-configuration-file services shepherd) #~""))) (define (reload-configuration-gexp config) -- 2.35.1 From 56d16b4cd511f6837329b888dade0c6d6da4d89d Mon Sep 17 00:00:00 2001 From: Andrew Tropin Date: Tue, 12 Apr 2022 12:19:50 +0300 Subject: [PATCH 2/3] home: shepherd: Use run-on-change to reload shepherd config. * gnu/home/services/shepherd.scm: Add shepherd configuration to XDG_CONFIG_HOME and use it instead of full path to the store. It's necessary to use run-on-change service. --- gnu/home/services/shepherd.scm | 40 +++--- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/gnu/home/services/shepherd.scm b/gnu/home/services/shepherd.scm index df6bbb30e6..9a99fed2b5 100644 --- a/gnu/home/services/shepherd.scm +++ b/gnu/home/services/shepherd.scm @@ -105,27 +105,30 @@ (define (launch-shepherd-gexp config) (system* #$(file-append shepherd "/bin/shepherd") "--logfile" - (string-append log-dir "/shepherd.log") - "--config" - #$(home-shepherd-configuration-file services shepherd) + (string-append log-dir "/shepherd.log") #~""))) (define (reload-configuration-gexp config) (let* ((shepherd (home-shepherd-configuration-shepherd config)) (services (home-shepherd-configuration-services config))) -#~(system* - #$(file-append shepherd "/bin/herd") - "load" "root" - #$(home-shepherd-configuration-file services shepherd +#~(when (file-exists? + (string-append + (or (getenv "XDG_RUNTIME_DIR") + (format #f "/run/user/~a" (getuid))) + "/shepherd/socket")) +(system* + #$(file-append shepherd "/bin/herd") + "load" "root" + #$(home-shepherd-configuration-file services shepherd) -(define (ensure-shepherd-gexp config) - #~(if (file-exists? - (string-append - (or (getenv "XDG_RUNTIME_DIR") - (format #f "/run/user/~a" (getuid))) -
bug#54545: [Guix Home] ‘shepherd’ started twice?
Hi, Andrew Tropin skribis: > Activation script tries to load latest shepherd configuration with `herd > load root ./path/to/config.scm` and it starts a shepherd process. Login > shell starts the shepherd process as well. Probably we need to do > config reload using on-change service and also not trigger on-change > stuff if user isn't logged in. Makes sense. > I can think on the proper solution and make a patch with a fix later > this week or beginning of the next week. Awesome, thanks for taking a look! Ludo’.
bug#54545: [Guix Home] ‘shepherd’ started twice?
On 2022-03-24 15:21, Ludovic Courtès wrote:
> Hi,
>
> From what can be seen in ‘guix home container’, it would seem that
> ‘shepherd’ is started twice, leading to this error while attempting to
> bind(2) the second time (thus it’s actually harmless, but suboptimal):
>
> --8<---cut here---start->8---
> $ ./pre-inst-env guix home container /tmp/t.scm
> WARNING: (guile-user): imported module (guix build utils) overrides core
> binding `delete'
> Symlinking /home/ludo/.bash_profile ->
> /gnu/store/flqaxzvgfv2g3415mhmq6c0zbzdzv2k4-bash_profile... done
> Symlinking /home/ludo/.profile ->
> /gnu/store/dann7r1095xll0kji5yl0ql07096rc8j-shell-profile... done
> Symlinking /home/ludo/.bashrc ->
> /gnu/store/g78w0adqg25z3jl8jq71n0n0z32f7dbx-bashrc... done
> Symlinking /home/ludo/.config/fontconfig/fonts.conf ->
> /gnu/store/4261pxafny0g2myhh9yj1771ry7k05lc-fonts.conf... done
> done
> Finished updating symlinks.
>
> Comparing /gnu/store/non-existing-generation/profile/share/fonts and
>
> /gnu/store/vvfrdbvmb0g41k00xxmd9qpgzavkvd32-home/profile/share/fonts... done
> (same)
> Evaluating on-change gexps.
>
> On-change gexps evaluation finished.
>
> Service root has been started.
> WARNING: Use of `load' in declarative module (#{ g56}#). Add #:declarative?
> #f to your define-module invocation.
> Starting services...
> Service mcron has been started.
>
> Service root has been started.
> WARNING: Use of `load' in declarative module (#{ g56}#). Add #:declarative?
> #f to your define-module invocation.
> Starting services...
> Service mcron has been started.
>
> Backtrace:
>4 (primitive-load "/gnu/store/vza48khbaq0fdmcsrn27xj5y5yy?")
> In shepherd.scm:
> ~$316:10 3 (main "--logfile" "/home/ludo/.local/var/log/shepherd.?" ?)
> 56:14 2 (call-with-server-socket "/run/user/1000/shepherd/sock?" ?)
> 49:6 1 (open-server-socket "/run/user/1000/shepherd/socket")
> In unknown file:
>0 (bind # #(1 "/run/user/1000?") #)
>
> ERROR: In procedure bind:
> In procedure bind: Address already in use
> --8<---cut here---end--->8---
>
> I suspect the problem is in activation snippets, but I’m open to other
> hypotheses. :-)
>
> Thoughts?
>
> Ludo’.
>
>
Can confirm.
Activation script tries to load latest shepherd configuration with `herd
load root ./path/to/config.scm` and it starts a shepherd process. Login
shell starts the shepherd process as well. Probably we need to do
config reload using on-change service and also not trigger on-change
stuff if user isn't logged in. I can think on the proper solution and
make a patch with a fix later this week or beginning of the next week.
--
Best regards,
Andrew Tropin
signature.asc
Description: PGP signature
bug#54545: [Guix Home] ‘shepherd’ started twice?
Hi,
>From what can be seen in ‘guix home container’, it would seem that
‘shepherd’ is started twice, leading to this error while attempting to
bind(2) the second time (thus it’s actually harmless, but suboptimal):
--8<---cut here---start->8---
$ ./pre-inst-env guix home container /tmp/t.scm
WARNING: (guile-user): imported module (guix build utils) overrides core
binding `delete'
Symlinking /home/ludo/.bash_profile ->
/gnu/store/flqaxzvgfv2g3415mhmq6c0zbzdzv2k4-bash_profile... done
Symlinking /home/ludo/.profile ->
/gnu/store/dann7r1095xll0kji5yl0ql07096rc8j-shell-profile... done
Symlinking /home/ludo/.bashrc ->
/gnu/store/g78w0adqg25z3jl8jq71n0n0z32f7dbx-bashrc... done
Symlinking /home/ludo/.config/fontconfig/fonts.conf ->
/gnu/store/4261pxafny0g2myhh9yj1771ry7k05lc-fonts.conf... done
done
Finished updating symlinks.
Comparing /gnu/store/non-existing-generation/profile/share/fonts and
/gnu/store/vvfrdbvmb0g41k00xxmd9qpgzavkvd32-home/profile/share/fonts... done
(same)
Evaluating on-change gexps.
On-change gexps evaluation finished.
Service root has been started.
WARNING: Use of `load' in declarative module (#{ g56}#). Add #:declarative? #f
to your define-module invocation.
Starting services...
Service mcron has been started.
Service root has been started.
WARNING: Use of `load' in declarative module (#{ g56}#). Add #:declarative? #f
to your define-module invocation.
Starting services...
Service mcron has been started.
Backtrace:
4 (primitive-load "/gnu/store/vza48khbaq0fdmcsrn27xj5y5yy?")
In shepherd.scm:
~$316:10 3 (main "--logfile" "/home/ludo/.local/var/log/shepherd.?" ?)
56:14 2 (call-with-server-socket "/run/user/1000/shepherd/sock?" ?)
49:6 1 (open-server-socket "/run/user/1000/shepherd/socket")
In unknown file:
0 (bind # #(1 "/run/user/1000?") #)
ERROR: In procedure bind:
In procedure bind: Address already in use
--8<---cut here---end--->8---
I suspect the problem is in activation snippets, but I’m open to other
hypotheses. :-)
Thoughts?
Ludo’.
