bug#46209: 46043 breaks booting with custom shepherd package

bug#46209: 46043 breaks booting with custom shepherd package

[Oleg Pykhalov]

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

Before applied patch [1] I used a hack to override shepherd package:

    (module-set! (resolve-module '(gnu packages admin)) 'shepherd shepherd-patched)

Now on Guix commit eb6b061 I cannot use the previous override, and also
the mentioned in [1] way overriding shepherd package doesn't work:

    (shepherd-root-service-type config => (shepherd-configuration
                                           (inherit config)
                                           (shepherd shepherd-patched)))

I got error about undefined ‘#:supplementary-groups’ which added in
‘shepherd-patched’, so I assume my ‘shepherd-patched’ package not used.

[1] https://issues.guix.gnu.org/46043

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

bug#46209: 46043 breaks booting with custom shepherd package

[Maxime Devos]

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

On Sun, 2021-01-31 at 15:46 +0300, Oleg Pykhalov wrote:
> Before applied patch [1] I used a hack to override shepherd package:
> 
>     (module-set! (resolve-module '(gnu packages admin)) 'shepherd shepherd-patched)
> 
> Now on Guix commit eb6b061 I cannot use the previous override, and also
> the mentioned in [1] way overriding shepherd package doesn't work:
> 
>     (shepherd-root-service-type config => (shepherd-configuration
>                                            (inherit config)
>                                            (shepherd shepherd-patched)))
> 
> I got error about undefined ‘#:supplementary-groups’ which added in
> ‘shepherd-patched’, so I assume my ‘shepherd-patched’ package not used.
> 
> [1] https://issues.guix.gnu.org/46043

Can you post your operating system configuration definition?
Also, were does the error about undefined "#:supplementary-groups"
appear? When booting, when reconfiguring a ‘live’ system, when
guix is compiling the shepherd services to .go?

A pitfall to take in mind: it's the ‘essential-services’ field
you need to modify, not the ‘services’ field of ‘operating-system’.
My original example at [1] was incorrect.  A corrected example:

(operating-system
 [...]
 (services [don't modify this])
 (essential-services
  (modify-services (operating-system-default-essential-services
                    this-operating-system)
    (shepherd-root-service-type c =>
                                (shepherd-configuration
                                 (inherit c)
                                 (shepherd-patched))))))


Maxime
-- 
Maxime Devos <maximedevos@telenet.be>
PGP Key: C1F3 3EE2 0C52 8FDB 7DD7  011F 49E3 EE22 1917 25EE
Freenode handle: mdevos

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#46209: 46043 breaks booting with custom shepherd package

[Oleg Pykhalov]

[-- Attachment #1.1: Type: text/plain, Size: 1852 bytes --]

Hello,

Thank you for the fast responce.

First of all I want to ask did you try to override Shepherd yourself?

It would be helpful to have a paragraph in the documentation how to
override the Shepherd, and if you know a proper way, please add a small
example of operating-system definition with overriden Shepherd and a
basic service like OpenSSH.

Maxime Devos <maximedevos@telenet.be> writes:

> On Sun, 2021-01-31 at 15:46 +0300, Oleg Pykhalov wrote:
>> Before applied patch [1] I used a hack to override shepherd package:
>> 
>>     (module-set! (resolve-module '(gnu packages admin)) 'shepherd shepherd-patched)
>> 
>> Now on Guix commit eb6b061 I cannot use the previous override, and also
>> the mentioned in [1] way overriding shepherd package doesn't work:
>> 
>>     (shepherd-root-service-type config => (shepherd-configuration
>>                                            (inherit config)
>>                                            (shepherd shepherd-patched)))
>> 
>> I got error about undefined ‘#:supplementary-groups’ which added in
>> ‘shepherd-patched’, so I assume my ‘shepherd-patched’ package not used.
>> 
>> [1] https://issues.guix.gnu.org/46043
>
> Can you post your operating system configuration definition?
> Also, were does the error about undefined "#:supplementary-groups"
> appear? When booting, when reconfiguring a ‘live’ system, when
> guix is compiling the shepherd services to .go?

While booting (as I understand in initrd).  If I run (system "/bin/sh"),
then "ps auxwww" I got only shepherd and /bin/sh in the process list.

I can publish, but it's too big [1], so if you don't mind I prepared a
virtual-machine to reproduce the issue.  Basically I don't even need to
use a patched shepherd to break the VM (result attached as a screenshot
below).


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: virtual-machine-definition --]
[-- Type: text/x-scheme, Size: 3206 bytes --]

;; Generation 120	Jan 31 2021 01:15:12	(current)
;;   guix eb6b061
;;     repository URL: https://git.savannah.gnu.org/git/guix.git
;;     branch: master
;;     commit: eb6b061320418fdbd49ecb72c366e43ff7a38dbc
;;
;; env GUIX_PACKAGE_PATH=$PWD guix system build vm-image.tmpl
;; env GUIX_PACKAGE_PATH=$PWD guix system vm vm-image.tmpl

(use-modules (gnu) (guix) (srfi srfi-1))
(use-service-modules shepherd networking ssh)
(use-package-modules admin bootloaders certs nano)

(let ((base-system
       (operating-system
         (host-name "gnu")
         (timezone "Etc/UTC")
         (locale "en_US.utf8")

         (firmware '())

         ;; Below we assume /dev/vda is the VM's hard disk.
         ;; Adjust as needed.
         (bootloader (bootloader-configuration
                      (bootloader grub-bootloader)
                      (target "/dev/vda")
                      (terminal-outputs '(console))))
         (file-systems (cons (file-system
                               (mount-point "/")
                               (device "/dev/vda1")
                               (type "ext4"))
                             %base-file-systems))

         (users (cons (user-account
                       (name "guest")
                       (comment "GNU Guix Live")
                       (password "")                     ;no password
                       (group "users")
                       (supplementary-groups '("wheel" "netdev"
                                               "audio" "video")))
                      %base-user-accounts))

         ;; Our /etc/sudoers file.  Since 'guest' initially has an empty password,
         ;; allow for password-less sudo.
         (sudoers-file (plain-file "sudoers" "\
root ALL=(ALL) ALL
%wheel ALL=NOPASSWD: ALL\n"))

         (packages (append (list nano nss-certs)
                           %base-packages))

         (services
          (append (list ;; Uncomment the line below to add an SSH server.
                   ;;(service openssh-service-type)

                   ;; Use the DHCP client service rather than NetworkManager.
                   (service dhcp-client-service-type))

                  ;; Remove GDM, ModemManager, NetworkManager, and wpa-supplicant,
                  ;; which don't make sense in a VM.
                  (remove (lambda (service)
                            (let ((type (service-kind service)))
                              (or (memq type
                                        (list wpa-supplicant-service-type
                                              network-manager-service-type
                                              modem-manager-service-type))
                                  (eq? 'network-manager-applet
                                       (service-type-name type)))))
                          %base-services))))))
  (operating-system
    (inherit base-system)
    (essential-services
     (modify-services (operating-system-default-essential-services base-system)
       (shepherd-root-service-type config => (shepherd-configuration
                                              (inherit config)
                                              (shepherd shepherd)))))))

[-- Attachment #1.3: Type: text/plain, Size: 633 bytes --]


> A pitfall to take in mind: it's the ‘essential-services’ field
> you need to modify, not the ‘services’ field of ‘operating-system’.
> My original example at [1] was incorrect.  A corrected example:
>
> (operating-system
>  [...]
>  (services [don't modify this])
>  (essential-services
>   (modify-services (operating-system-default-essential-services
>                     this-operating-system)
>     (shepherd-root-service-type c =>
>                                 (shepherd-configuration
>                                  (inherit c)
>                                  (shepherd-patched))))))


[-- Attachment #1.4: QEMU Screenshot --]
[-- Type: image/png, Size: 11036 bytes --]

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

bug#46209: 46043 breaks booting with custom shepherd package

[Maxime Devos]

[-- Attachment #1.1.1: Type: text/plain, Size: 1051 bytes --]

On Sun, 2021-01-31 at 22:49 +0300, Oleg Pykhalov wrote:
>   (operating-system
> 
>     (inherit base-system)
> 
>     (essential-services
> 
>      (modify-services (operating-system-default-essential-services base-system)
> 
>        (shepherd-root-service-type config => (shepherd-configuration
> 
>                                               (inherit config)
> 
>                                               (shepherd shepherd)))))))

I'm currently running "guix time-machine etcetera", which hasn't completed yet,
but here's some quick speculation on what could have went wrong:

You shouldn't write (operating-system-default-essential-services base-system),
but rather (operating-system-default-essential-services this-operating-system).

The essential-services field is thunked, basically that means that the expression
under essential-services can refer to the operating-system definition itself,
using the macro this-operating-system.

A  potentially fixed configuration is attached, I'll try to run it in a VM.

[-- Attachment #1.1.2: Type: text/html, Size: 1908 bytes --]

[-- Attachment #1.2: reproduce-shepherd-bug.scm --]
[-- Type: text/x-scheme, Size: 2741 bytes --]

;; Generation 120	Jan 31 2021 01:15:12	(current)
;;   guix eb6b061
;;     repository URL: https://git.savannah.gnu.org/git/guix.git
;;     branch: master
;;     commit: eb6b061320418fdbd49ecb72c366e43ff7a38dbc
;;
;; env GUIX_PACKAGE_PATH=$PWD guix system build vm-image.tmpl
;; env GUIX_PACKAGE_PATH=$PWD guix system vm vm-image.tmpl

(use-modules (gnu) (guix) (srfi srfi-1))
(use-service-modules shepherd networking ssh)
(use-package-modules admin bootloaders certs nano)

(operating-system
 (host-name "gnu")
 (timezone "Etc/UTC")
 (locale "en_US.utf8")

 (firmware '())

 ;; Below we assume /dev/vda is the VM's hard disk.
 ;; Adjust as needed.
 (bootloader (bootloader-configuration
              (bootloader grub-bootloader)
              (target "/dev/vda")
              (terminal-outputs '(console))))
 (file-systems (cons (file-system
                      (mount-point "/")
                      (device "/dev/vda1")
                      (type "ext4"))
                     %base-file-systems))

 (users (cons (user-account
               (name "guest")
               (comment "GNU Guix Live")
               (password "")	;no password
               (group "users")
               (supplementary-groups '("wheel" "netdev"
                                       "audio" "video")))
              %base-user-accounts))

 ;; Our /etc/sudoers file.  Since 'guest' initially has an empty password,
 ;; allow for password-less sudo.
 (sudoers-file (plain-file "sudoers" "
root ALL=(ALL) ALL
%wheel ALL=NOPASSWD: ALL\n"))

 (packages (append (list nano nss-certs)
                   %base-packages))

 (essential-services
  (modify-services
   (operating-system-default-essential-services this-operating-system)
   (shepherd-root-service-type config => (shepherd-configuration
					  (inherit config)
					  (shepherd shepherd)))))
 (services
  (append (list ;; Uncomment the line below to add an SSH server.
           ;;(service openssh-service-type)

           ;; Use the DHCP client service rather than NetworkManager.
           (service dhcp-client-service-type))

          ;; Remove GDM, ModemManager, NetworkManager, and wpa-supplicant,
          ;; which don't make sense in a VM.
          (remove (lambda (service)
		    (let ((type (service-kind service)))
                      (or (memq type
                                (list wpa-supplicant-service-type
                                      network-manager-service-type
                                      modem-manager-service-type))
                          (eq? 'network-manager-applet
                               (service-type-name type)))))
                  %base-services))))


[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#46209: 46043 breaks booting with custom shepherd package

[Maxime Devos]

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

When using the operating system configuration
adjusted to use this-operating-system, the VM
boots just fine for me (adjusted configuration
should be sent with previous e-mail).  Could
you confirm?

I haven't tried using the original system
configuration yet.

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#46209: 46043 breaks booting with custom shepherd package

[Oleg Pykhalov]

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

Maxime Devos <maximedevos@telenet.be> writes:

> When using the operating system configuration
> adjusted to use this-operating-system, the VM
> boots just fine for me (adjusted configuration
> should be sent with previous e-mail).  Could
> you confirm?

Thank you!  VM boots, and my desktop machine boots with shepherd-patched
and #:supplementary-groups field

    (essential-services
     (modify-services (operating-system-default-essential-services this-operating-system)
       (shepherd-root-service-type config => (shepherd-configuration
                                              (inherit config)
                                              (shepherd shepherd-patched)))))

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

bug#46209: 46043 breaks booting with custom shepherd package

[Maxime Devos]

[-- Attachment #1.1: Type: text/plain, Size: 1710 bytes --]

On Sun, 2021-01-31 at 22:49 +0300, Oleg Pykhalov wrote:
> [...]
> First of all I want to ask did you try to override Shepherd yourself?

Yes, and I can confirm I could use #:supplementary-groups in
service definitions.  I override Shepherd with the mechanism in [1],
not with module-set!, however.

> It would be helpful to have a paragraph in the documentation how to
> override the Shepherd, and if you know a proper way, please add a small
> example of operating-system definition with overriden Shepherd and a
> basic service like OpenSSH.

Yes, would be very helpful, especially as it's easy to get wrong.
I'm currently occupied with other things, though.  As for an example:
your system configuration might do (as adjusted per previous e-mails),
with the following block ...

 (essential-services
  (modify-services
   (operating-system-default-essential-services this-operating-system)
   (shepherd-root-service-type 
config => (shepherd-configuration
                                          (inherit config)
                                          (shep
herd shepherd)))))
replaced by ...

 (essential-services
  (modify-services
   (operating-system-default-essential-services this-operating-system)
   (shepherd-root-service-type 
config => (shepherd-configuration
                                          (inherit config)
                                          (shep
herd shepherd-patched)))))

(otherwise no change in Shepherd is made), where shepherd-patched should
be defined somewhere in the beginning of the configuration file.
You could use your own shepherd-patched, or my shepherd-latest (attached):

[1] https://issues.guix.gnu.org/46043


[-- Attachment #1.2: extra-packages.scm --]
[-- Type: text/x-scheme, Size: 2690 bytes --]

;;; GNU Guix --- Functional package management for GNU
;;; Copyright © 2019, 2020 Pierre Langlois <pierre.langlois@gmx.com>
;;; Copyright © 2020 Tobias Geerinckx-Rice <me@tobias.gr>
;;; Copyright © 2021 Maxime Devos <maximedevos@telenet.be>
;;;
;;; This file is part of GNU Guix.
;;;
;;; GNU Guix is free software; you can redistribute it and/or modify it
;;; under the terms of the GNU General Public License as published by
;;; the Free Software Foundation; either version 3 of the License, or (at
;;; your option) any later version.
;;;
;;; GNU Guix is distributed in the hope that it will be useful, but
;;; WITHOUT ANY WARRANTY; without even the implied warranty of
;;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
;;; GNU General Public License for more details.
;;;
;;; You should have received a copy of the GNU General Public License
;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.

(define-module (extra-packages)
  #:use-module (guix packages)
  #:use-module (guix git-download)
  #:use-module (gnu packages autotools)
  #:use-module (gnu packages texinfo)
  #:use-module (gnu packages gettext)
  #:use-module (gnu packages admin)
  #:use-module (gnu packages man))

(define-public shepherd-latest
  (let ((commit "4c5176f5a7a5a1e7d7f258f585e8ed127a21b99a")
        (revision "1"))
    (package
      (inherit shepherd)
      (version (git-version "0.8.1" revision commit))
      (source
       (origin
         (method git-fetch)
         (uri (git-reference
               (url "https://git.savannah.gnu.org/git/shepherd.git")
               (commit commit)))
         (file-name (git-file-name "shepherd" version))
         (sha256
          (base32 "0x7njnvhxmay4xz4pyh9b982bhxys089nysz95paz3vhf1253fr6"))))
      (arguments
       '(#:configure-flags '("--localstatedir=/var")
         #:phases
         (modify-phases %standard-phases
           ;; XXX less duplication (gpodder)
           ;; 'msgmerge' introduces non-determinism by resetting the
           ;; POT-Creation-Date in .po files.
           (add-after 'bootstrap 'do-not-run-msgmerge
             (lambda _
               (substitute* "Makefile.in"
                 (("msgmerge") "true"))
               #t))
           (add-after 'unpack 'make-po-files-writable
             (lambda _
               (for-each
                (lambda (f)
                  (chmod f #o664))
                (find-files "po"))
               #t)))))
      (native-inputs
       `(("autoconf" ,autoconf)
         ("automake" ,automake)
         ("gettext" ,gettext-minimal)
         ("texinfo" ,texinfo)
         ("help2man" ,help2man)
         ,@(package-native-inputs shepherd))))))

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#46209: 46043 breaks booting with custom shepherd package

[Oleg Pykhalov]

[-- Attachment #1.1: Type: text/plain, Size: 501 bytes --]

Maxime Devos <maximedevos@telenet.be> writes:

[…]

>> It would be helpful to have a paragraph in the documentation how to
>> override the Shepherd, and if you know a proper way, please add a small
>> example of operating-system definition with overriden Shepherd and a
>> basic service like OpenSSH.
>
> Yes, would be very helpful, especially as it's easy to get wrong.
> I'm currently occupied with other things, though.  

If you don't mind I could push the following instruction:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1.2: Explain how to set custom Shepherd package. --]
[-- Type: text/x-patch, Size: 1271 bytes --]

From f94a63bb35f6f05bd429703ec3102d15cfb1e382 Mon Sep 17 00:00:00 2001
From: Oleg Pykhalov <go.wigust@gmail.com>
Date: Mon, 1 Feb 2021 01:49:00 +0300
Subject: [PATCH] doc: Explain how to set custom Shepherd package.

* doc/guix.texi (Shepherd Services): Give example on how to set custom
Shepherd package.
---
 doc/guix.texi | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index aca4657d6a..76a4e55e50 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -32826,6 +32826,24 @@ mechanism instead (@pxref{Shepherd Services}).
 @end table
 @end deftp
 
+The following example specifies the Shepherd package for operating
+system:
+
+@lisp
+(operating-system
+  ;; ...
+  (services (append (list openssh-service-type))
+            ;; ...
+            %desktop-services)
+  ;; ...
+  ;; Use own Shepherd package.
+  (essential-services
+   (modify-services (operating-system-default-essential-services this-operating-system)
+     (shepherd-root-service-type config => (shepherd-configuration
+					    (inherit config)
+					    (shepherd my-shepherd))))))
+@end lisp
+
 @defvr {Scheme Variable} %shepherd-root-service
 This service represents PID@tie{}1.
 @end defvr
-- 
2.30.0


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

bug#46209: 46043 breaks booting with custom shepherd package

[Maxime Devos]

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

> If you don't mind I could push the following instruction: [...]

Linguistic nitpick: In ‘The following example specifies the Shepherd package
for operating system’ doesn't seem to be grammatical.  Perhaps write
‘[...] for the operating system’ or ‘[...] of the operating system.’?

Otherwise LGTM!

Thanks,
Maxime

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 260 bytes --]

bug#46209: 46043 breaks booting with custom shepherd package

[Ludovic Courtès]

Hi Oleg,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

> From f94a63bb35f6f05bd429703ec3102d15cfb1e382 Mon Sep 17 00:00:00 2001
> From: Oleg Pykhalov <go.wigust@gmail.com>
> Date: Mon, 1 Feb 2021 01:49:00 +0300
> Subject: [PATCH] doc: Explain how to set custom Shepherd package.
>
> * doc/guix.texi (Shepherd Services): Give example on how to set custom
> Shepherd package.

[...]

> +The following example specifies the Shepherd package for operating
                                                           ^
Missing “the”, as Maxime wrote.

> +   (modify-services (operating-system-default-essential-services this-operating-system)
> +     (shepherd-root-service-type config => (shepherd-configuration
> +					    (inherit config)
> +					    (shepherd my-shepherd))))))

Please make sure there are no tabs and shorten lines in the manual to
~72 chars so that the PDF output looks good.

Otherwise LGTM too, thanks!

Ludo’.

bug#46209: 46043 breaks booting with custom shepherd package

[Oleg Pykhalov]

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

Pushed to master as 847c816ddd3a38c865da460cb7b22cf665db162f

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