Activation snippets in reverse order, prevent boot

  • Done
  • quality assurance status badge
Details
6 participants
  • Brian Cully
  • Felix Lechner
  • Jelle Licht
  • Ludovic Courtès
  • Maxim Cournoyer
  • pelzflorian (Florian Pelz)
Owner
unassigned
Submitted by
Ludovic Courtès
Severity
serious
L
L
Ludovic Courtès wrote on 6 Jun 2023 10:15
(address . bug-guix@gnu.org)
877csh9fb9.fsf@inria.fr
Booting a system reconfigured from
eed55a6544d5bda2245ec853e5fa4b28e1865bea fails with shepherd saying:

error: while opening socket '/var/run/shepherd/socket': Address already in use

The root cause appears to be that the ‘boot’ program has expressions
reversed:

Toggle snippet (6 lines)
$ guix gc --references $(guix gc --derivers $(readlink -f /var/guix/profiles/system-236-link)) | grep boot
/gnu/store/21ldhyrpji6lkkdxi4lgr5k9l5fr2b7l-boot.drv
$ cat $(guix build /gnu/store/21ldhyrpji6lkkdxi4lgr5k9l5fr2b7l-boot.drv)
(eval-when (expand load eval) (let ((extensions (quote ())) (prepend (lambda (items lst) (let loop ((items items) (lst lst)) (if (null? items) lst (loop (cdr items) (cons (car items) (delete (car items) lst)))))))) (set! %load-path (prepend (cons "/gnu/store/pj751v3199vmv6i6sf0szp185ryzcfdg-module-import" (map (lambda (extension) (string-append extension "/share/guile/site/" (effective-version))) extensions)) %load-path)) (set! %load-compiled-path (prepend (cons "/gnu/store/pql80c2hy38bb60c68sng74s4xa35vwk-module-import-compiled" (map (lambda (extension) (string-append extension "/lib/guile/" (effective-version) "/site-ccache")) extensions)) %load-compiled-path))))(begin (begin (false-if-exception (delete-file "/run/booted-system")) (symlink (canonicalize-path "/run/current-system") "/run/booted-system") (let loop ((fd 3)) (when (< fd 1024) (false-if-exception (let ((flags (fcntl fd F_GETFD))) (when (zero? (logand flags FD_CLOEXEC)) (fcntl fd F_SETFD (logior FD_CLOEXEC flags))))) (loop (+ fd 1)))) (execl "/gnu/store/wj5i6x3xgai7p8whwqybxwqdjdbmbzha-shepherd-0.10.99-git/bin/shepherd" "shepherd" "--config" "/gnu/store/gnjghlc3n5qbala5jfdslgfi0n3vy8v7-shepherd.conf")) (primitive-load "/gnu/store/riabgidmf6fh76klc1yam7k9j1478vvw-activate.scm") (begin (use-modules (guix build utils)) (letrec-syntax ((fail-safe (syntax-rules () ((_ exp rest ...) (begin (catch (quote system-error) (lambda () exp) (const #f)) (fail-safe rest ...))) ((_) #t)))) (fail-safe (delete-file "/etc/group.lock") (delete-file "/etc/passwd.lock") (delete-file "/etc/.pwd.lock") (setenv "GUIX_LOCPATH" "/gnu/store/5fmqijrs5f7vx8mc2q2pmq26yvhb74sm-glibc-utf8-locales-2.35/lib/locale") (setlocale LC_CTYPE "en_US.utf8") (delete-file-recursively "/tmp") (delete-file-recursively "/var/run") (mkdir "/tmp") (chmod "/tmp" 1023) (mkdir "/var/run") (chmod "/var/run" 493) (delete-file-recursively "/run/udev/watch.old")))))

Namely, (execl "…/bin/shepherd") comes before the cleanup expressions,
which is why /var/run/shepherd/socket is still around when we boot.

Ludo’.
L
L
Ludovic Courtès wrote on 6 Jun 2023 11:44
control message for bug #63921
(address . control@debbugs.gnu.org)
875y80apri.fsf@gnu.org
severity 63921 serious
quit
L
L
Ludovic Courtès wrote on 6 Jun 2023 11:58
Re: bug#63921: Activation snippets in reverse order, prevent boot
(address . 63921@debbugs.gnu.org)(name . Brian Cully)(address . bjc@spork.org)
87y1kw9ajz.fsf@gnu.org
Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (16 lines)
> Booting a system reconfigured from
> eed55a6544d5bda2245ec853e5fa4b28e1865bea fails with shepherd saying:
>
> error: while opening socket '/var/run/shepherd/socket': Address already in use
>
> The root cause appears to be that the ‘boot’ program has expressions
> reversed:
>
> $ guix gc --references $(guix gc --derivers $(readlink -f /var/guix/profiles/system-236-link)) | grep boot
> /gnu/store/21ldhyrpji6lkkdxi4lgr5k9l5fr2b7l-boot.drv
> $ cat $(guix build /gnu/store/21ldhyrpji6lkkdxi4lgr5k9l5fr2b7l-boot.drv)
> (eval-when (expand load eval) (let ((extensions (quote ())) (prepend (lambda (items lst) (let loop ((items items) (lst lst)) (if (null? items) lst (loop (cdr items) (cons (car items) (delete (car items) lst)))))))) (set! %load-path (prepend (cons "/gnu/store/pj751v3199vmv6i6sf0szp185ryzcfdg-module-import" (map (lambda (extension) (string-append extension "/share/guile/site/" (effective-version))) extensions)) %load-path)) (set! %load-compiled-path (prepend (cons "/gnu/store/pql80c2hy38bb60c68sng74s4xa35vwk-module-import-compiled" (map (lambda (extension) (string-append extension "/lib/guile/" (effective-version) "/site-ccache")) extensions)) %load-compiled-path))))(begin (begin (false-if-exception (delete-file "/run/booted-system")) (symlink (canonicalize-path "/run/current-system") "/run/booted-system") (let loop ((fd 3)) (when (< fd 1024) (false-if-exception (let ((flags (fcntl fd F_GETFD))) (when (zero? (logand flags FD_CLOEXEC)) (fcntl fd F_SETFD (logior FD_CLOEXEC flags))))) (loop (+ fd 1)))) (execl "/gnu/store/wj5i6x3xgai7p8whwqybxwqdjdbmbzha-shepherd-0.10.99-git/bin/shepherd" "shepherd" "--config" "/gnu/store/gnjghlc3n5qbala5jfdslgfi0n3vy8v7-shepherd.conf")) (primitive-load "/gnu/store/riabgidmf6fh76klc1yam7k9j1478vvw-activate.scm") (begin (use-modules (guix build utils)) (letrec-syntax ((fail-safe (syntax-rules () ((_ exp rest ...) (begin (catch (quote system-error) (lambda () exp) (const #f)) (fail-safe rest ...))) ((_) #t)))) (fail-safe (delete-file "/etc/group.lock") (delete-file "/etc/passwd.lock") (delete-file "/etc/.pwd.lock") (setenv "GUIX_LOCPATH" "/gnu/store/5fmqijrs5f7vx8mc2q2pmq26yvhb74sm-glibc-utf8-locales-2.35/lib/locale") (setlocale LC_CTYPE "en_US.utf8") (delete-file-recursively "/tmp") (delete-file-recursively "/var/run") (mkdir "/tmp") (chmod "/tmp" 1023) (mkdir "/var/run") (chmod "/var/run" 493) (delete-file-recursively "/run/udev/watch.old")))))
>
> Namely, (execl "…/bin/shepherd") comes before the cleanup expressions,
> which is why /var/run/shepherd/socket is still around when we boot.

Fixed in 181951207339508789b28ba7cb914f983319920f.

The regression came from dbbc7e946131ba257728f1d05b96c4339b7ee88b, which
led ‘modify-services’ to change the order of services (something I had
completely overlooked while reviewing, apologies!). I ended up
rewriting ‘modify-services’. Good news is we now have tests for this.

Ludo’.
L
L
Ludovic Courtès wrote on 6 Jun 2023 11:58
control message for bug #63921
(address . control@debbugs.gnu.org)
87wn0g9ajo.fsf@gnu.org
close 63921
quit
P
P
pelzflorian (Florian Pelz) wrote on 7 Jun 2023 10:57
Re: bug#63921: Activation snippets in reverse order, prevent boot
(name . Ludovic Courtès)(address . ludo@gnu.org)
87a5xb645t.fsf@pelzflorian.de
Hi Ludo, hi all.

Ludovic Courtès <ludo@gnu.org> writes:
Toggle quote (3 lines)
> I ended up
> rewriting ‘modify-services’. Good news is we now have tests for this.

In the rewrite, you wrote:
Toggle quote (5 lines)
> +(define (apply-clauses clauses services)
> + "Apply CLAUSES, an alist as returned by 'clause-alist', to SERVICES, a list
> +of services. Use each clause at most once; raise an error if a clause was not
> +used."

Using clauses at most once broke the greetd example in the manual:
Toggle quote (9 lines)
> (append
> (modify-services %base-services
> ;; greetd-service-type provides "greetd" PAM service
> (delete login-service-type)
> ;; and can be used in place of mingetty-service-type
> (delete mingetty-service-type))
> (list
> (service greetd-service-type

But there are multiple instances of mingetty-service-type in
%base-services. Now an error is raised on reconfigure because there are
two term-tty2 services.

I’m not sure, isn’t it more explicit if we keep your change of deleting
at most once, but change the greetd example?

The following works for me now …

(modify-services %base-services
(delete login-service-type)
(delete mingetty-service-type)
(delete mingetty-service-type)
(delete mingetty-service-type)
(delete mingetty-service-type)
(delete mingetty-service-type)
(delete mingetty-service-type))

Regards,
Florian
P
P
pelzflorian (Florian Pelz) wrote on 7 Jun 2023 11:19
(name . Ludovic Courtès)(address . ludo@gnu.org)
875y7z635j.fsf@pelzflorian.de
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
Toggle quote (2 lines)
> Using clauses at most once broke the greetd example in the manual:

Jelle Licht already reported this on IRC, see the end of

Regards,
Florian
P
P
pelzflorian (Florian Pelz) wrote on 7 Jun 2023 11:30
(name . Jelle Licht)(address . jlicht@fsfe.org)
87ilbzzkjb.fsf@pelzflorian.de
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
Toggle quote (5 lines)
> "pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:
>> Using clauses at most once broke the greetd example in the manual:
> Jelle Licht already reported this on IRC, see the end of
> <http://logs.guix.gnu.org/guix/2023-06-06.log>

Cc Jelle Licht
J
J
Jelle Licht wrote on 7 Jun 2023 19:44
87r0qni2vo.fsf@fsfe.org
"pelzflorian (Florian Pelz)" <pelzflorian@pelzflorian.de> writes:

Toggle quote (2 lines)
> Hi Ludo, hi all.
>
[snip]

Toggle quote (11 lines)
> The following works for me now …
>
> (modify-services %base-services
> (delete login-service-type)
> (delete mingetty-service-type)
> (delete mingetty-service-type)
> (delete mingetty-service-type)
> (delete mingetty-service-type)
> (delete mingetty-service-type)
> (delete mingetty-service-type))

Thanks for the workaround! Is this "thou shall delete N times, and
_exactly_ N times" effect of the recently pushed change functioning as
intended? It imho seems pretty brittle and verbose compared to how
things were before.

- Jelle
B
B
Brian Cully wrote on 8 Jun 2023 00:02
(name . Jelle Licht)(address . jlicht@fsfe.org)
87zg5b7wqj.fsf@psyduck.jhoto.kublai.com
Jelle Licht <jlicht@fsfe.org> writes:

Toggle quote (8 lines)
> Thanks for the workaround! Is this "thou shall delete N times,
> and
> _exactly_ N times" effect of the recently pushed change
> functioning as
> intended? It imho seems pretty brittle and verbose compared to
> how
> things were before.

We could add a ‘delete-all’ in addition to the existing ‘delete’
behavior. Alternately, we could change ‘delete’ back to deleting
everything and adding ‘delete-one’. Or have both ‘delete-all’ and
‘delete-one’ where ‘delete’ is a deprecated alias for ‘delete-all’
to add a path forward for older configs.

Of the three I'm most partial to the last, though I love none of
them. I keep thinking the right solution is to have a delete that
can match with a predicate, but then why not just use ‘filter’ or
‘remove’?

-bjc
P
P
pelzflorian (Florian Pelz) wrote on 8 Jun 2023 01:07
(address . 63921@debbugs.gnu.org)
87bkhqub12.fsf@pelzflorian.de
Jelle is right, multiple (delete mingetty-service-type) is not useful.
With more thought, what I imagined as more explicit is probably more
like (delete "term-tty2") than (delete mingetty-service-type). Also I
wouldn’t actually need it.

And as Brian says, just use filter. modify-services need not do more
than what is already documented.

I would be happy if someone else would undo the change to “delete at
most once”.

Regards,
Florian
P
F
F
Felix Lechner wrote on 26 Jun 2023 21:26
Patch proposed
(address . 63921@debbugs.gnu.org)
CAFHYt54bSQo4PE7Q88UamnNpYRZxnW0diXA0kkdLA=wajtcHGg@mail.gmail.com
Hi,

A patch to resolve this issue was proposed at the other bug


Kind regards
Felix
M
M
Maxim Cournoyer wrote on 1 Sep 2023 13:43
Re: bug#63921: Activation snippets in reverse order, prevent boot
(name . Felix Lechner)(address . felix.lechner@lease-up.com)(address . 63921-done@debbugs.gnu.org)
878r9q5dah.fsf_-_@gmail.com
Hello,

Felix Lechner <felix.lechner@lease-up.com> writes:

Toggle quote (6 lines)
> Hi,
>
> A patch to resolve this issue was proposed at the other bug
>
> https://issues.guix.gnu.org/64106#5

Brian's patch in https://issues.guix.gnu.org/64106#7was installed; it
came with tests and also cause an error to be raised when attempting to
modify a phase already deleted.

Closing, thanks for the reminder.

--
Thanks,
Maxim
Closed
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 63921
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