GNU bug report logs - #54986
[PATCH] gnu: mpd: Add support for socket activation.

Previous Next

To reply to this bug, email your comments to 54986 AT debbugs.gnu.org.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: guix-patches <at> gnu.org
Subject: [PATCH] gnu: mpd: Add support for socket activation.
Date: Sun, 17 Apr 2022 12:01:47 +0200

* gnu/packages/mpd.scm (mpd)[#:configure-flags]: Convert to G-Expression.
Add “-Dsystemd=enabled”.
[#:phases]: New argument.
[inputs]: Add elogind.
---
 gnu/packages/mpd.scm | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/mpd.scm b/gnu/packages/mpd.scm
index 1ee6806735..40e6a99ad7 100644
--- a/gnu/packages/mpd.scm
+++ b/gnu/packages/mpd.scm
@@ -47,6 +47,7 @@ (define-module (gnu packages mpd)
   #:use-module (gnu packages boost)
   #:use-module (gnu packages cdrom)
   #:use-module (gnu packages cmake) ;for MPD
+  #:use-module (gnu packages freedesktop) ;elogind
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages gnome)
   #:use-module (gnu packages gnupg)
@@ -119,12 +120,28 @@ (define-public mpd
                 "1v969w7h3660ph3h2bdlkrzc05pfz95bmxjqdbzzf7pfwf795ifb"))))
     (build-system meson-build-system)
     (arguments
-     `(#:configure-flags '("-Ddocumentation=enabled")))
+     (list
+      #:configure-flags #~(list "-Ddocumentation=enabled"
+                                "-Dsystemd=enabled")
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'unpack 'enable-elogind
+            (lambda _
+              (substitute* "src/lib/systemd/meson.build"
+                (("libsystemd") "libelogind"))
+              ;; XXX: systemd dependency overwritten internally, leads to bad
+              ;;      errors
+              (substitute* "src/lib/systemd/meson.build"
+                (("systemd_dep = declare_dependency" all)
+                 (string-append "_" all)))
+              (substitute* "meson.build"
+                (("systemd_dep,") "systemd_dep, _systemd_dep,")))))))
     (inputs (list ao
                   alsa-lib
                   avahi
                   boost
                   curl
+                  elogind
                   ffmpeg
                   flac
                   fmt
-- 
2.35.1

Message #8 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Sun, 17 Apr 2022 23:06:16 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> * gnu/packages/mpd.scm (mpd)[#:configure-flags]: Convert to G-Expression.
> Add “-Dsystemd=enabled”.
> [#:phases]: New argument.
> [inputs]: Add elogind.

LGTM!

Are you planning to update the service as well?

Thanks,
Ludo’.

Message #11 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Sun, 17 Apr 2022 23:57:23 +0200

Am Sonntag, dem 17.04.2022 um 23:06 +0200 schrieb Ludovic Courtès:
> Hi,
> 
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
> 
> > * gnu/packages/mpd.scm (mpd)[#:configure-flags]: Convert to G-
> > Expression.
> > Add “-Dsystemd=enabled”.
> > [#:phases]: New argument.
> > [inputs]: Add elogind.
> 
> LGTM!
> 
> Are you planning to update the service as well?
I'm not sure if updating the service is a good idea.  On the upside,
people who report mpd starting too early will be alleviated of one
issue.  On the downside, it appears as though mpd too easily escapes
shepherd's management.  My current observations are more or less
consistent with what I saw with Emacs: killing the mpd service won't
stop playing music, and shepherd won't restart a killed MPD without
asked to.  IMHO shepherd's socket activation still needs some work
before we can make it the default for every service.

Cheers

Message #14 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Mon, 18 Apr 2022 23:05:47 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> I'm not sure if updating the service is a good idea.  On the upside,
> people who report mpd starting too early will be alleviated of one
> issue.  On the downside, it appears as though mpd too easily escapes
> shepherd's management.  My current observations are more or less
> consistent with what I saw with Emacs: killing the mpd service won't
> stop playing music,

Do you mean that ‘herd stop mpd’ doesn’t stop the mpd process?

What does /var/log/messages say?

> and shepherd won't restart a killed MPD without asked to.

Weird.  What you describe sounds as if shepherd is not looking at the
right process or something.

If you have a service definition to reproduce this, I’d be happy to take
a look!

Thanks,
Ludo’.

Message #17 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Mon, 18 Apr 2022 23:19:12 +0200

Am Montag, dem 18.04.2022 um 23:05 +0200 schrieb Ludovic Courtès:
> Hi,
> 
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
> 
> > I'm not sure if updating the service is a good idea.  On the
> > upside, people who report mpd starting too early will be alleviated
> > of one issue.  On the downside, it appears as though mpd too easily
> > escapes shepherd's management.  My current observations are more or
> > less consistent with what I saw with Emacs: killing the mpd service
> > won't stop playing music,
> 
> Do you mean that ‘herd stop mpd’ doesn’t stop the mpd process?
Yep.
> What does /var/log/messages say?
I don't think there's anything meaningful there to inspect, I'm running
this as a user service.  Shepherd's own logs are rather empty.

Interestingly, the running value for the mpd service remains
(("unknown" . "#<input-output: socket 17>")) even after MPD started. 
Should that be the case?

> > and shepherd won't restart a killed MPD without asked to.
> 
> Weird.  What you describe sounds as if shepherd is not looking at the
> right process or something.
> 
> If you have a service definition to reproduce this, I’d be happy to
> take a look!
The second is probably just me forgetting to set #:respawn? #t.  One
"bug" down, one more to go.

Message #20 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: [PATCH v2 1/3] gnu: mpd: Add support for socket activation.
Date: Sun, 17 Apr 2022 12:01:47 +0200

* gnu/packages/mpd.scm (mpd)[#:configure-flags]: Convert to G-Expression.
Add “-Dsystemd=enabled”.
[#:phases]: New argument.
[inputs]: Add elogind.
---
 gnu/packages/mpd.scm | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/mpd.scm b/gnu/packages/mpd.scm
index 1ee6806735..40e6a99ad7 100644
--- a/gnu/packages/mpd.scm
+++ b/gnu/packages/mpd.scm
@@ -47,6 +47,7 @@ (define-module (gnu packages mpd)
   #:use-module (gnu packages boost)
   #:use-module (gnu packages cdrom)
   #:use-module (gnu packages cmake) ;for MPD
+  #:use-module (gnu packages freedesktop) ;elogind
   #:use-module (gnu packages gettext)
   #:use-module (gnu packages gnome)
   #:use-module (gnu packages gnupg)
@@ -119,12 +120,28 @@ (define-public mpd
                 "1v969w7h3660ph3h2bdlkrzc05pfz95bmxjqdbzzf7pfwf795ifb"))))
     (build-system meson-build-system)
     (arguments
-     `(#:configure-flags '("-Ddocumentation=enabled")))
+     (list
+      #:configure-flags #~(list "-Ddocumentation=enabled"
+                                "-Dsystemd=enabled")
+      #:phases
+      #~(modify-phases %standard-phases
+          (add-after 'unpack 'enable-elogind
+            (lambda _
+              (substitute* "src/lib/systemd/meson.build"
+                (("libsystemd") "libelogind"))
+              ;; XXX: systemd dependency overwritten internally, leads to bad
+              ;;      errors
+              (substitute* "src/lib/systemd/meson.build"
+                (("systemd_dep = declare_dependency" all)
+                 (string-append "_" all)))
+              (substitute* "meson.build"
+                (("systemd_dep,") "systemd_dep, _systemd_dep,")))))))
     (inputs (list ao
                   alsa-lib
                   avahi
                   boost
                   curl
+                  elogind
                   ffmpeg
                   flac
                   fmt
-- 
2.35.1

Message #23 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: [PATCH v2 2/3 WIP] services: shepherd: Add support for socket
 activation endpoints.
Date: Sat, 23 Apr 2022 16:25:55 +0200

* gnu/services/shepherd.scm (<shepherd-endpoint>): New record type.
(shepherd-endpoint->sexp): New variable.
* doc/guix.texi (Shepherd Services): Document it.
---
 doc/guix.texi             | 29 +++++++++++++++++++
 gnu/services/shepherd.scm | 60 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 89 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index 5399584cb0..38aeda1d2d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -37675,6 +37675,35 @@ This, as you can see, is a fairly sophisticated way to say hello.
 info on actions.
 @end deftp
 
+@deftp {Data Type} shepherd-endpoint
+This is an endpoint that a systemd-style shepherd service listens on.
+
+@table @code
+@item address
+This is the socket address that shepherd binds and forwards to the service.
+
+@item name
+‾\_(ツ)_/‾
+
+@item style
+‾\_(ツ)_/‾
+
+@item backlog
+‾\_(ツ)_/‾
+
+@item socket-owner
+This is the user ID owning the socket.
+
+@item socket-group
+This is the group ID owning the socket.
+
+@item socket-directory-permissions
+These are the UNIX permissions to set for the directory the socket
+resides in.
+
+@end table
+@end deftp
+
 @defvr {Scheme Variable} shepherd-root-service-type
 The service type for the Shepherd ``root service''---i.e., PID <at> tie{}1.
 
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index 4fd4b2a497..3ef0023d7f 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -66,6 +66,16 @@ (define-module (gnu services shepherd)
             shepherd-action-documentation
             shepherd-action-procedure
 
+            shepherd-endpoint
+            shepherd-endpoint-address
+            shepherd-endpoint-name
+            shepherd-endpoint-style
+            shepherd-endpoint-backlog
+            shepherd-endpoint-socket-owner
+            shepherd-endpoint-socket-group
+            shepherd-endpoint-socket-directory-permissions
+            shepherd-endpoint->sexp
+
             %default-modules
 
             shepherd-service-file
@@ -183,6 +193,56 @@ (define %default-modules
     ((guix build utils) #:hide (delete))
     (guix build syscalls)))
 
+(define-record-type* <shepherd-endpoint>
+  shepherd-endpoint make-shepherd-endpoint
+  shepherd-endpoint?
+  (address                      shepherd-endpoint-address)      ; sockaddr
+  (name                         shepherd-endpoint-name          ; string | #f
+                                (default #f))
+  (style                        shepherd-endpoint-style         ; int | #f
+                                (default #f))
+  (backlog                      shepherd-endpoint-backlog       ; int | #f
+                                (default #f))
+  (socket-owner                 shepherd-endpoint-socket-owner  ; uid | #f
+                                (default #f))
+  (socket-group                 shepherd-endpoint-socket-group  ; gid | #f
+                                (default #f))
+  (socket-directory-permissions
+   shepherd-endpoint-socket-directory-permissions ; chmod pattern (int) | #f
+   (default #f)))
+
+(define (shepherd-endpoint->sexp endpoint)
+  (match endpoint
+    (($ <shepherd-endpoint> address
+                            name style backlog socket-owner socket-group
+                            socket-directory-permissions)
+     `(endpoint
+       ,(match (sockaddr:fam address)
+          ((? (cute = <> AF_INET) _)
+           `(make-socket-addr AF_INET
+                              ,(sockaddr:addr address)
+                              ,(sockaddr:port address)))
+          ((? (cute = <> AF_INET6) _)
+           `(make-socket-addr AF_INET6
+                              ,(sockaddr:addr address)
+                              ,(sockaddr:port address)
+                              ,(sockaddr:flowinfo address)
+                              ,(sockaddr:scopeid address)))
+          ((? (cute = <> AF_UNIX) _)
+           `(make-socket-addr AF_UNIX
+                              ,(sockaddr:path address))))
+       ,@(fold
+          (match-lambda*
+            (((key value) seed)
+             (if value (cons* key value seed) seed)))
+          '()
+          `((#:name ,name)
+            (#:style ,style)
+            (#:backlog ,backlog)
+            (#:socket-owner ,socket-owner)
+            (#:socket-group ,socket-group)
+            (#:socket-directory-permissions ,socket-directory-permissions)))))))
+
 (define-record-type* <shepherd-service>
   shepherd-service make-shepherd-service
   shepherd-service?
-- 
2.35.1

Message #26 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: [PATCH v2 3/3 WIP] services: mpd: Support socket activation.
Date: Sat, 23 Apr 2022 16:39:59 +0200

* gnu/services/audio.scm (<mpd-configuration>)[shepherd-endpoints]: New field.
(mpd-shepherd-service): Use it.
* doc/guix.texi (Music Player Daemon): Document it.
---
 doc/guix.texi          |  7 +++++++
 gnu/services/audio.scm | 45 ++++++++++++++++++++++++++++++------------
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 38aeda1d2d..6bfa854b1d 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -30883,6 +30883,13 @@ The port to run mpd on.
 The address that mpd will bind to.  To use a Unix domain socket,
 an absolute path can be specified here.
 
+@item @code{shepherd-endpoints} (default: @code{'()})
+The endpoints shepherd shall bind and spawn MPD from.  If this field is
+not empty (checked via @code{null?}), a systemd-style service is used
+rather than a forkexec-service.  This delays the start of MPD until the
+first client connects.  As a side effect @code{port} and @code{address}
+may be ignored.
+
 @item @code{outputs} (default: @code{"(list (mpd-output))"})
 The audio outputs that MPD can use.  By default this is a single output using pulseaudio.
 
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index c60053f33c..b184ac596a 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -78,6 +78,8 @@ (define-record-type* <mpd-configuration>
                 (default "6600"))
   (address      mpd-configuration-address
                 (default "any"))
+  (shepherd-endpoints mpd-configuration-shepherd-endpoints
+                      (default '())) ; list of <shepherd-endpoint>
   (outputs      mpd-configuration-outputs
                 (default (list (mpd-output)))))
 
@@ -140,19 +142,36 @@ (define (mpd-shepherd-service config)
    (documentation "Run the MPD (Music Player Daemon)")
    (requirement '(user-processes))
    (provision '(mpd))
-   (start #~(make-forkexec-constructor
-             (list #$(file-append mpd "/bin/mpd")
-                   "--no-daemon"
-                   #$(mpd-config->file config))
-             #:environment-variables
-             ;; Required to detect PulseAudio when run under a user account.
-             (list (string-append
-                    "XDG_RUNTIME_DIR=/run/user/"
-                    (number->string
-                     (passwd:uid
-                      (getpwnam #$(mpd-configuration-user config))))))
-             #:log-file #$(mpd-file-name config "log")))
-   (stop  #~(make-kill-destructor))))
+   (start (if (null? (mpd-configuration-shepherd-endpoints config))
+              #~(make-forkexec-constructor
+                 (list #$(file-append mpd "/bin/mpd")
+                       "--no-daemon"
+                       #$(mpd-config->file config))
+                 #:environment-variables
+                 ;; Required to detect PulseAudio when run under a user account.
+                 (list (string-append
+                        "XDG_RUNTIME_DIR=/run/user/"
+                        (number->string
+                         (passwd:uid
+                          (getpwnam #$(mpd-configuration-user config))))))
+                 #:log-file #$(mpd-file-name config "log"))
+              #~(make-systemd-constructor
+                 (list #$(file-append mpd "/bin/mpd")
+                       "--systemd"
+                       #$(mpd-config->file config))
+                 (list #$@(map shepherd-endpoint->sexp
+                               (mpd-configuration-shepherd-endpoints config)))
+                 #:environment-variables
+                 ;; Required to detect PulseAudio when run under a user account.
+                 (list (string-append
+                        "XDG_RUNTIME_DIR=/run/user/"
+                        (number->string
+                         (passwd:uid
+                          (getpwnam #$(mpd-configuration-user config))))))
+                 #:log-file #$(mpd-file-name config "log"))))
+   (stop  (if (null? (mpd-configuration-shepherd-endpoints config))
+              #~(make-kill-destructor)
+              #~(make-systemd-destructor)))))
 
 (define (mpd-service-activation config)
   (with-imported-modules '((guix build utils))
-- 
2.35.1

Message #29 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: Re: [bug#54986] [PATCH v2 3/3 WIP] services: mpd: Support socket
 activation.
Date: Sat, 23 Apr 2022 19:31:34 +0200

[Message part 1 (text/plain, inline)]

> +(define (shepherd-endpoint->sexp endpoint)
> +  (match endpoint
> +    (($ <shepherd-endpoint> address
> +                            name style backlog socket-owner socket-
group
> +                            socket-directory-permissions)
> +     `(endpoint
> +       ,(match (sockaddr:fam address)
> +          ((? (cute = <> AF_INET) _)
> +           `(make-socket-addr AF_INET
> +                              ,(sockaddr:addr address)
> +                              ,(sockaddr:port address)))

Liliana Marie Prikler schreef op za 23-04-2022 om 16:39 [+0200]:
> +                 (list #$@(map shepherd-endpoint->sexp
> +                               (mpd-configuration-shepherd-endpoints config))) 

For hygiene reasons, should 'shepherd-endpoint->sexp' use @?

  `((@ (the module) endpoint)
    ,(match [...]
        ((@ [...] make-socket-addr (@ [...] AF_UNIX) ...)
        ...))

That way, no assumptions are made on what modules are imported and it
avoids hygiene problems like in

  ;; There are two ‘endpoints’ here: the ‘API endpoint’,
  ;; and Shepherd endpoints.
  #~(let ((endpoint "http://localhost:1234/api"))
      (make-systemd-constructor
         (list #$(file-append soft "/bin/ware")
               "--endpoint" #$endpoint)
         (list (shepherd-endpoint->sexp ...))))

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #32 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Maxime Devos <maximedevos <at> telenet.be>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: Re: [bug#54986] [PATCH v2 3/3 WIP] services: mpd: Support socket
 activation.
Date: Sat, 23 Apr 2022 19:35:46 +0200

[Message part 1 (text/plain, inline)]

Liliana Marie Prikler schreef op za 23-04-2022 om 16:39 [+0200]:
> +@item @code{shepherd-endpoints} (default: @code{'()})
> +The endpoints shepherd shall bind and spawn MPD from.  If this field is
> +not empty (checked via @code{null?}), a systemd-style service is used
> +rather than a forkexec-service.

systemd-style / forkexec-service looks like an implementation detail to
me, not something useful to the user.

>  This delays the start of MPD until the
> +first client connects.
> 

Likewise.

>   As a side effect @code{port} and @code{address}
> +may be ignored.

Could these three fields be unified, keeping 'port' and 'address' only
for some backwards compatibility?  E.g.:

  (mpd-configuration
    (endpoints (list (shepherd-endpoint [a AF_INET6 address])
                     (shepherd-endpoint [a AF_UNIX address]))))

Greetings,
Maxime.

[signature.asc (application/pgp-signature, inline)]

Message #35 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Maxime Devos <maximedevos <at> telenet.be>, 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: Re: [bug#54986] [PATCH v2 3/3 WIP] services: mpd: Support socket
 activation.
Date: Sat, 23 Apr 2022 20:28:30 +0200

Am Samstag, dem 23.04.2022 um 19:35 +0200 schrieb Maxime Devos:
> Liliana Marie Prikler schreef op za 23-04-2022 om 16:39 [+0200]:
> > +@item @code{shepherd-endpoints} (default: @code{'()})
> > +The endpoints shepherd shall bind and spawn MPD from.  If this
> > field is
> > +not empty (checked via @code{null?}), a systemd-style service is
> > used
> > +rather than a forkexec-service.
> 
> systemd-style / forkexec-service looks like an implementation detail
> to me, not something useful to the user.
> 
> >  This delays the start of MPD until the
> > +first client connects.
> 
> Likewise.
Even if this was just an implementation detail, I shall point to
Hyrum's law.

> >   As a side effect @code{port} and @code{address}
> > +may be ignored.
> 
> Could these three fields be unified, keeping 'port' and 'address'
> only for some backwards compatibility?  E.g.:
> 
>   (mpd-configuration
>     (endpoints (list (shepherd-endpoint [a AF_INET6 address])
>                      (shepherd-endpoint [a AF_UNIX address]))))
Not AFAIK.  MPD is less strict than Guile when it comes to addresses,
so converting from address+port to socket addresses is nontrivial. 
That being said, I'm not sure if they are even ignored.  MPD only warns
about pid_file, so they might also be used.

Cheers

Message #38 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Maxime Devos <maximedevos <at> telenet.be>, 54986 <at> debbugs.gnu.org
Cc: ludo <at> gnu.org
Subject: Re: [bug#54986] [PATCH v2 3/3 WIP] services: mpd: Support socket
 activation.
Date: Sat, 23 Apr 2022 20:36:19 +0200

Am Samstag, dem 23.04.2022 um 19:31 +0200 schrieb Maxime Devos:
> > +(define (shepherd-endpoint->sexp endpoint)
> > +  (match endpoint
> > +    (($ <shepherd-endpoint> address
> > +                            name style backlog socket-owner
> > socket-
> group
> > +                            socket-directory-permissions)
> > +     `(endpoint
> > +       ,(match (sockaddr:fam address)
> > +          ((? (cute = <> AF_INET) _)
> > +           `(make-socket-addr AF_INET
> > +                              ,(sockaddr:addr address)
> > +                              ,(sockaddr:port address)))
> 
> Liliana Marie Prikler schreef op za 23-04-2022 om 16:39 [+0200]:
> > +                 (list #$@(map shepherd-endpoint->sexp
> > +                               (mpd-configuration-shepherd-
> > endpoints config))) 
> 
> For hygiene reasons, should 'shepherd-endpoint->sexp' use @?
> 
>   `((@ (the module) endpoint)
>     ,(match [...]
>         ((@ [...] make-socket-addr (@ [...] AF_UNIX) ...)
>         ...))
> 
> That way, no assumptions are made on what modules are imported and it
> avoids hygiene problems like in
> 
>   ;; There are two ‘endpoints’ here: the ‘API endpoint’,
>   ;; and Shepherd endpoints.
>   #~(let ((endpoint "http://localhost:1234/api"))
>       (make-systemd-constructor
>          (list #$(file-append soft "/bin/ware")
>                "--endpoint" #$endpoint)
>          (list (shepherd-endpoint->sexp ...))))
Good point, that's should probably done for defensive programming. 
Note also that I'm missing half of the documentation still, so it won't
be pushed too soon, though.

Message #41 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Wed, 27 Apr 2022 22:56:50 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

[...]

>> Do you mean that ‘herd stop mpd’ doesn’t stop the mpd process?
> Yep.
>> What does /var/log/messages say?
> I don't think there's anything meaningful there to inspect, I'm running
> this as a user service.  Shepherd's own logs are rather empty.

Then it’s in ~/.local/state (by default) or
~/.local/var/log/shepherd.log if you’re using Guix Home.

> Interestingly, the running value for the mpd service remains
> (("unknown" . "#<input-output: socket 17>")) even after MPD started. 
> Should that be the case?

No.

>> > and shepherd won't restart a killed MPD without asked to.
>> 
>> Weird.  What you describe sounds as if shepherd is not looking at the
>> right process or something.
>> 
>> If you have a service definition to reproduce this, I’d be happy to
>> take a look!
> The second is probably just me forgetting to set #:respawn? #t.  One
> "bug" down, one more to go.

:-)

Ludo’.

Message #44 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: [PATCH v2 2/3 WIP] services: shepherd: Add support for socket
 activation endpoints.
Date: Thu, 28 Apr 2022 00:05:19 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> * gnu/services/shepherd.scm (<shepherd-endpoint>): New record type.
> (shepherd-endpoint->sexp): New variable.
> * doc/guix.texi (Shepherd Services): Document it.

[...]

> +++ b/gnu/services/shepherd.scm
> @@ -66,6 +66,16 @@ (define-module (gnu services shepherd)
>              shepherd-action-documentation
>              shepherd-action-procedure
>  
> +            shepherd-endpoint
> +            shepherd-endpoint-address
> +            shepherd-endpoint-name
> +            shepherd-endpoint-style
> +            shepherd-endpoint-backlog
> +            shepherd-endpoint-socket-owner
> +            shepherd-endpoint-socket-group
> +            shepherd-endpoint-socket-directory-permissions
> +            shepherd-endpoint->sexp
> +
>              %default-modules
>  
>              shepherd-service-file
> @@ -183,6 +193,56 @@ (define %default-modules
>      ((guix build utils) #:hide (delete))
>      (guix build syscalls)))
>  
> +(define-record-type* <shepherd-endpoint>

I don’t think this is necessary: Shepherd endpoints are created in a
#~(make-systemd-constructor …) gexp, it’s OK to use the Shepherd
endpoint API there.

Ludo’.

Message #47 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: [PATCH v2 3/3 WIP] services: mpd: Support socket activation.
Date: Thu, 28 Apr 2022 00:08:55 +0200

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> * gnu/services/audio.scm (<mpd-configuration>)[shepherd-endpoints]: New field.
> (mpd-shepherd-service): Use it.
> * doc/guix.texi (Music Player Daemon): Document it.

[...]

> +++ b/gnu/services/audio.scm
> @@ -78,6 +78,8 @@ (define-record-type* <mpd-configuration>
>                  (default "6600"))
>    (address      mpd-configuration-address
>                  (default "any"))
> +  (shepherd-endpoints mpd-configuration-shepherd-endpoints
> +                      (default '())) ; list of <shepherd-endpoint>

The way I see it, service configuration should be oblivious to whether
it’s started as “forkexec”, systemd, or inetd.

There’s already an ‘address’ field above, so my suggestion would be to
reuse it.  This is what I did for example for the openssh service, and
also for bitlbee and dicod here:

  https://issues.guix.gnu.org/54997#5

WDYT?

Thanks,
Ludo’.

Message #50 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Thu, 28 Apr 2022 18:45:13 +0200

Am Mittwoch, dem 27.04.2022 um 22:56 +0200 schrieb Ludovic Courtès:
> 
> > Interestingly, the running value for the mpd service remains
> > (("unknown" . "#<input-output: socket 17>")) even after MPD
> > started. Should that be the case?
> 
> No.
It turns out the issue here was that I rewrote the service to put its
log into XDG_STATE_HOME as well, but under a directory that did not yet
exist.  #55080 addresses this.

Cheers

Message #53 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Fri, 13 May 2022 17:55:36 +0200

Hi Liliana,

What’s the status of this patch series?  Would be nice to have it in!

Ludo’.

Ludovic Courtès <ludo <at> gnu.org> skribis:

> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
>
>> * gnu/services/audio.scm (<mpd-configuration>)[shepherd-endpoints]: New field.
>> (mpd-shepherd-service): Use it.
>> * doc/guix.texi (Music Player Daemon): Document it.
>
> [...]
>
>> +++ b/gnu/services/audio.scm
>> @@ -78,6 +78,8 @@ (define-record-type* <mpd-configuration>
>>                  (default "6600"))
>>    (address      mpd-configuration-address
>>                  (default "any"))
>> +  (shepherd-endpoints mpd-configuration-shepherd-endpoints
>> +                      (default '())) ; list of <shepherd-endpoint>
>
> The way I see it, service configuration should be oblivious to whether
> it’s started as “forkexec”, systemd, or inetd.
>
> There’s already an ‘address’ field above, so my suggestion would be to
> reuse it.  This is what I did for example for the openssh service, and
> also for bitlbee and dicod here:
>
>   https://issues.guix.gnu.org/54997#5
>
> WDYT?
>
> Thanks,
> Ludo’.

Message #56 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Fri, 13 May 2022 19:00:41 +0200

Am Freitag, dem 13.05.2022 um 17:55 +0200 schrieb Ludovic Courtès:
> Hi Liliana,
> 
> What’s the status of this patch series?  Would be nice to have it in!
The patch for MPD itself is good to go as far as I'm aware, the patch
for the service type is work in progress.

This question
> Ludovic Courtès <ludo <at> gnu.org> skribis:
> > The way I see it, service configuration should be oblivious to
> > whether it’s started as “forkexec”, systemd, or inetd.
has me in a bit of a bind in multiple ways.  For one, I don't see a
direct translation from MPD's configuration scheme to shepherd's.  For
another, the distinct semantics between forkexec and lazy loading cause
observable differences as in “What the fuck, I only ran mpc status, why
is the music now playing?” – this can be avoided if the user knows that
shepherd will be lazy-loading mpd and that it might as a result to
starting start playing on the first received command.

Moreover, I think the patch I added to make endpoint records
configurable from Guix could also serve to solve other bugs, e.g. SSH
only listening on IPv4 addresses, which would require us to be able to
specify whether to listen on IPv4, IPv6 or both through Guix.  Long
term, I think we should only keep the distinction between forkexec and
inetd/systemd for those services where it makes an observable
difference, like mpd.  For SSH, apart from bugs and perhaps people
using old shepherd, I don't think there'd be any reason to keep
forkexec beyond a certain point (in a future as distant as you would
want it to be).

Maxime also raised hygiene concerns that would be comparatively easy to
solve.

TL;DR: v2 [1/3] is good, [2/3 WIP] and [3/3 WIP] should be kept back
for now.

Cheers

Message #59 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v3 2/2] services: mpd: Support socket activation.
Date: Sat, 23 Apr 2022 16:39:59 +0200

* gnu/services/audio.scm (<mpd-endpoint>): New record.
(<mpd-configuration>)[port, address]: Removed fields.
[endpoints]: New field.
(sanitize-mpd-endpoints, mpd-configuration-shepherd-endpoints): New variables.
(mpd-shepherd-service): Use mpd-configuration-shepherd-endpoints.
* doc/guix.texi (Music Player Daemon): Document it.
---
 doc/guix.texi          | 23 ++++++++--
 gnu/services/audio.scm | 98 +++++++++++++++++++++++++++++++++---------
 2 files changed, 97 insertions(+), 24 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 30cc3b6d45..d5b9de0de2 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -32192,15 +32192,30 @@ The location of the file that stores current MPD's state.
 @item @code{sticker-file} (default: @code{"~/.mpd/sticker.sql"})
 The location of the sticker database.
 
-@item @code{port} (default: @code{"6600"})
-The port to run mpd on.
+@item @code{endpoints} (default: @code{(list (mpd-endpoint))})
+The endpoints to bind MPD to.  This can be an mpd-endpoint (see below)
+or a list of shepherd endpoints.  If using shepherd endpoints, MPD
+will be spawned as a systemd-style service rather than as a
+forkexec-style one.  This delays the start of MPD until the first client
+connects.
+
+@item @code{outputs} (default: @code{"(list (mpd-output))"})
+The audio outputs that MPD can use.  By default this is a single output using pulseaudio.
+
+@end table
+@end deftp
+
+@deftp {Data Type} mpd-endpoint
+Data type representing the configuration of @command{mpd}.
+
+@table @asis
 
 @item @code{address} (default: @code{"any"})
 The address that mpd will bind to.  To use a Unix domain socket,
 an absolute path can be specified here.
 
-@item @code{outputs} (default: @code{"(list (mpd-output))"})
-The audio outputs that MPD can use.  By default this is a single output using pulseaudio.
+@item @code{port} (default: @code{"6600"})
+The port to run mpd on.
 
 @end table
 @end deftp
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index c60053f33c..6e8fd460a1 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -20,6 +20,8 @@
 
 (define-module (gnu services audio)
   #:use-module (guix gexp)
+  #:use-module (guix diagnostics)
+  #:use-module (guix i18n)
   #:use-module (gnu services)
   #:use-module (gnu services shepherd)
   #:use-module (gnu system shadow)
@@ -28,8 +30,14 @@ (define-module (gnu services audio)
   #:use-module (guix records)
   #:use-module (ice-9 match)
   #:use-module (ice-9 format)
+  #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-26)
+  #:use-module (srfi srfi-34)
+  #:use-module (srfi srfi-35)
   #:export (mpd-output
             mpd-output?
+            mpd-endpoint
+            mpd-endpoint?
             mpd-configuration
             mpd-configuration?
             mpd-service-type))
@@ -59,6 +67,30 @@ (define-record-type* <mpd-output>
   (extra-options mpd-output-extra-options
                  (default '())))
 
+(define-record-type* <mpd-endpoint>
+  mpd-endpoint make-mpd-endpoint
+  mpd-endpoint?
+  (address mpd-endpoint-address (default "any"))
+  (port    mpd-endpoint-port (default "6600")))
+
+(define-with-syntax-properties (sanitize-mpd-endpoints (endpoints properties))
+  (match endpoints
+    ((? mpd-endpoint? endpoint) (list endpoint))
+    (((? mpd-endpoint?)) endpoints)
+    (((? mpd-endpoint?) . _)
+     (raise (make-compound-condition
+             (condition (&message
+                         (message
+                          (G_ "\
+<mpd-endpoint> is mutually exclusive with other endpoints."))))
+             (condition (&error-location
+                         (location (source-properties->location properties))))
+             (condition (&fix-hint
+                         (hint "\
+Make sure you only need one endpoint or use shepherd endpoints instead."))))))
+    (((? shepherd-endpoint?) . (? (cute every shepherd-endpoint? <>)))
+     endpoints)))
+
 (define-record-type* <mpd-configuration>
   mpd-configuration make-mpd-configuration
   mpd-configuration?
@@ -74,10 +106,9 @@ (define-record-type* <mpd-configuration>
                 (default "~/.mpd/state"))
   (sticker-file mpd-configuration-sticker-file
                 (default "~/.mpd/sticker.sql"))
-  (port         mpd-configuration-port
-                (default "6600"))
-  (address      mpd-configuration-address
-                (default "any"))
+  (endpoints    mpd-configuration-endpoints     ; list of <shepherd-endpoint>
+                (default (list (mpd-endpoint))) ;       | <mpd-endpoint>
+                (sanitize sanitize-mpd-endpoints))
   (outputs      mpd-configuration-outputs
                 (default (list (mpd-output)))))
 
@@ -124,9 +155,12 @@ (define (mpd-config->file config)
                   ("playlist_directory" ,mpd-configuration-playlist-dir)
                   ("db_file" ,mpd-configuration-db-file)
                   ("state_file" ,mpd-configuration-state-file)
-                  ("sticker_file" ,mpd-configuration-sticker-file)
-                  ("port" ,mpd-configuration-port)
-                  ("bind_to_address" ,mpd-configuration-address))))))
+                  ("sticker_file" ,mpd-configuration-sticker-file)))
+           (match (mpd-configuration-endpoints config)
+             (($ <mpd-endpoint> address port)
+              `(("bind_to_address" ,address)
+                ("port" ,port)))
+             (_ '())))))
 
 (define (mpd-file-name config file)
   "Return a path in /var/run/mpd/ that is writable
@@ -135,24 +169,48 @@ (define (mpd-file-name config file)
                  (mpd-configuration-user config)
                  "/" file))
 
+(define (mpd-configuration-shepherd-endpoints config)
+  "Return @code{(mpd-configuration-endpoints config)}"
+  (let ((endpoints (mpd-configuration-endpoints config)))
+    (match endpoints
+      (((? shepherd-endpoint?) . _) endpoints)
+      (_ #f))))
+
 (define (mpd-shepherd-service config)
   (shepherd-service
    (documentation "Run the MPD (Music Player Daemon)")
    (requirement '(user-processes))
    (provision '(mpd))
-   (start #~(make-forkexec-constructor
-             (list #$(file-append mpd "/bin/mpd")
-                   "--no-daemon"
-                   #$(mpd-config->file config))
-             #:environment-variables
-             ;; Required to detect PulseAudio when run under a user account.
-             (list (string-append
-                    "XDG_RUNTIME_DIR=/run/user/"
-                    (number->string
-                     (passwd:uid
-                      (getpwnam #$(mpd-configuration-user config))))))
-             #:log-file #$(mpd-file-name config "log")))
-   (stop  #~(make-kill-destructor))))
+   (start (if (mpd-configuration-shepherd-endpoints config)
+              #~(make-systemd-constructor
+                 (list #$(file-append mpd "/bin/mpd")
+                       "--systemd"
+                       #$(mpd-config->file config))
+                 (list #$@(map shepherd-endpoint->sexp
+                               (mpd-configuration-shepherd-endpoints config)))
+                 #:environment-variables
+                 ;; Required to detect PulseAudio when run under a user account.
+                 (list (string-append
+                        "XDG_RUNTIME_DIR=/run/user/"
+                        (number->string
+                         (passwd:uid
+                          (getpwnam #$(mpd-configuration-user config))))))
+                 #:log-file #$(mpd-file-name config "log"))
+              #~(make-forkexec-constructor
+                 (list #$(file-append mpd "/bin/mpd")
+                       "--no-daemon"
+                       #$(mpd-config->file config))
+                 #:environment-variables
+                 ;; Required to detect PulseAudio when run under a user account.
+                 (list (string-append
+                        "XDG_RUNTIME_DIR=/run/user/"
+                        (number->string
+                         (passwd:uid
+                          (getpwnam #$(mpd-configuration-user config))))))
+                 #:log-file #$(mpd-file-name config "log"))))
+   (stop  (if (mpd-configuration-shepherd-endpoints config)
+              #~(make-systemd-destructor)
+              #~(make-kill-destructor)))))
 
 (define (mpd-service-activation config)
   (with-imported-modules '((guix build utils))
-- 
2.37.3

Message #62 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: [PATCH v3 1/2] services: shepherd: Add support for socket
 activation endpoints.
Date: Sat, 23 Apr 2022 16:25:55 +0200

* gnu/services/shepherd.scm (<shepherd-endpoint>): New record type.
(shepherd-endpoint->sexp): New variable.
* doc/guix.texi (Shepherd Services): Document it.
---
 doc/guix.texi             | 32 ++++++++++++++++++++
 gnu/services/shepherd.scm | 64 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index c21235f28d..30cc3b6d45 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -39332,6 +39332,38 @@ This, as you can see, is a fairly sophisticated way to say hello.
 info on actions.
 @end deftp
 
+@deftp {Data Type} shepherd-endpoint
+This data type represents an endpoint for inetd-style and systemd-style
+services.  Its fields reflect the arguments to shepherd's
+@code{endpoint} procedure.
+
+@table @code
+@item address
+The socket address to bind via shepherd and forward to the service.
+
+@item name
+A ``descriptive'' name to forward to the service along with the socket.
+
+@item style
+The socket style to use.
+@xref{Network Sockets and Communication,,, guile, GNU Guile Reference
+Manual}.
+
+@item backlog
+The maximum length of the queue for pending connections.
+
+@item socket-owner
+The user ID owning the socket.
+
+@item socket-group
+The group ID owning the socket.
+
+@item socket-directory-permissions
+The UNIX permissions to set for the directory the socket resides in.
+
+@end table
+@end deftp
+
 @defvr {Scheme Variable} shepherd-root-service-type
 The service type for the Shepherd ``root service''---i.e., PID <at> tie{}1.
 
diff --git a/gnu/services/shepherd.scm b/gnu/services/shepherd.scm
index 4fd4b2a497..f0d204ff3a 100644
--- a/gnu/services/shepherd.scm
+++ b/gnu/services/shepherd.scm
@@ -66,6 +66,17 @@ (define-module (gnu services shepherd)
             shepherd-action-documentation
             shepherd-action-procedure
 
+            shepherd-endpoint
+            shepherd-endpoint?
+            shepherd-endpoint-address
+            shepherd-endpoint-name
+            shepherd-endpoint-style
+            shepherd-endpoint-backlog
+            shepherd-endpoint-socket-owner
+            shepherd-endpoint-socket-group
+            shepherd-endpoint-socket-directory-permissions
+            shepherd-endpoint->sexp
+
             %default-modules
 
             shepherd-service-file
@@ -183,6 +194,59 @@ (define %default-modules
     ((guix build utils) #:hide (delete))
     (guix build syscalls)))
 
+(define-record-type* <shepherd-endpoint>
+  shepherd-endpoint make-shepherd-endpoint
+  shepherd-endpoint?
+  (address                      shepherd-endpoint-address)      ; sockaddr
+  (name                         shepherd-endpoint-name          ; string | #f
+                                (default #f))
+  (style                        shepherd-endpoint-style         ; int | #f
+                                (default #f))
+  (backlog                      shepherd-endpoint-backlog       ; int | #f
+                                (default #f))
+  (socket-owner                 shepherd-endpoint-socket-owner  ; uid | #f
+                                (default #f))
+  (socket-group                 shepherd-endpoint-socket-group  ; gid | #f
+                                (default #f))
+  (socket-directory-permissions
+   shepherd-endpoint-socket-directory-permissions ; chmod pattern (int) | #f
+   (default #f)))
+
+(define (shepherd-endpoint->sexp endpoint)
+  (match endpoint
+    (($ <shepherd-endpoint> address
+                            name style backlog socket-owner socket-group
+                            socket-directory-permissions)
+     `(endpoint
+       ,(match (sockaddr:fam address)
+          ((? (cute = <> AF_INET) _)
+           `((@ (guile) make-socket-addr)
+             AF_INET
+             ,(sockaddr:addr address)
+             ,(sockaddr:port address)))
+          ((? (cute = <> AF_INET6) _)
+           `((@ (guile) make-socket-addr)
+             AF_INET6
+             ,(sockaddr:addr address)
+             ,(sockaddr:port address)
+             ,(sockaddr:flowinfo address)
+             ,(sockaddr:scopeid address)))
+          ((? (cute = <> AF_UNIX) _)
+           `((@ (guile) make-socket-addr)
+             AF_UNIX
+             ,(sockaddr:path address))))
+       ,@(fold
+          (match-lambda*
+            (((key value) seed)
+             (if value (cons* key value seed) seed)))
+          '()
+          `((#:name ,name)
+            (#:style ,style)
+            (#:backlog ,backlog)
+            (#:socket-owner ,socket-owner)
+            (#:socket-group ,socket-group)
+            (#:socket-directory-permissions ,socket-directory-permissions)))))))
+
 (define-record-type* <shepherd-service>
   shepherd-service make-shepherd-service
   shepherd-service?
-- 
2.37.3

Message #65 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: [PATCH v3 2/2] services: mpd: Support socket activation.
Date: Sun, 23 Oct 2022 13:38:56 +0200

Am Samstag, dem 23.04.2022 um 16:39 +0200 schrieb Liliana Marie
Prikler:
> * gnu/services/audio.scm (<mpd-endpoint>): New record.
> (<mpd-configuration>)[port, address]: Removed fields.
> [endpoints]: New field.
> (sanitize-mpd-endpoints, mpd-configuration-shepherd-endpoints): New
> variables.
> (mpd-shepherd-service): Use mpd-configuration-shepherd-endpoints.
> * doc/guix.texi (Music Player Daemon): Document it.
> ---
Bump

Message #68 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: 54986 <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: [PATCH v3 2/2] services: mpd: Support socket activation.
Date: Sat, 26 Nov 2022 13:59:30 +0100

Am Sonntag, dem 23.10.2022 um 13:38 +0200 schrieb Liliana Marie
Prikler:
> Am Samstag, dem 23.04.2022 um 16:39 +0200 schrieb Liliana Marie
> Prikler:
> > * gnu/services/audio.scm (<mpd-endpoint>): New record.
> > (<mpd-configuration>)[port, address]: Removed fields.
> > [endpoints]: New field.
> > (sanitize-mpd-endpoints, mpd-configuration-shepherd-endpoints): New
> > variables.
> > (mpd-shepherd-service): Use mpd-configuration-shepherd-endpoints.
> > * doc/guix.texi (Music Player Daemon): Document it.
> > ---
> Bump
Bumpy bump

Message #71 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: mirai <mirai <at> makinata.eu>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 59866 <at> debbugs.gnu.org
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] services: mpd: Refactor MPD service
Date: Thu, 8 Dec 2022 13:11:29 +0000

On 2022-12-07 18:27, Liliana Marie Prikler wrote:
> Note that there is [1], which attempts to make it so that shepherd
> endpoints can be specified in lieu of MPD endpoints.  You don't need to
> implement this logic – you are free to do so if you want to – but could
> you make it so that there is an explicit endpoint abstraction that
> would allow for extension later on?
> 
> Cheers
> 
> [1] https://issues.guix.gnu.org/54986

Hi,

After reading issue #54986, regarding mpd escaping shepherd management,
my guess is that there could be two issues at play here:

* mpd.conf was serialized with pid_file [1]. The safest is
to actually not serialize any unused directives and let MPD decide
based on what's actually present. pid_file should only be set if
MPD is to be launched as a "daemon process". [2]

* But the service definition for MPD is launched with '--no-daemon' option
as seen in [3], which could be triggering a bug in MPD. It's probably worth
testing the combinations of having pid_file set and invoking --no-daemon.


> but could
> you make it so that there is an explicit endpoint abstraction that
> would allow for extension later on?

If I'm understanding correctly what [1] is about, I think this can be
implemented through the 'extend' interface. See how
certbot adds a nginx-server-configuration to a nginx-service-type
through the 'extend' interface. For MPD, this would amount to
appending the 'adresses' field with "adequately formatted" values.


[1]: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/audio.scm?id=1b6172d5f6ca22f0fa02cd1335b1b90e9501b731#n116
[2]: https://mpd.readthedocs.io/en/latest/user.html#starting-and-stopping-mpd
[3]: https://git.savannah.gnu.org/cgit/guix.git/tree/gnu/services/audio.scm?id=1b6172d5f6ca22f0fa02cd1335b1b90e9501b731#n145

Message #74 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: mirai <mirai <at> makinata.eu>, 59866 <at> debbugs.gnu.org
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] services: mpd: Refactor MPD service
Date: Thu, 08 Dec 2022 14:35:55 +0100

Am Donnerstag, dem 08.12.2022 um 13:11 +0000 schrieb mirai:
> On 2022-12-07 18:27, Liliana Marie Prikler wrote:
> > Note that there is [1], which attempts to make it so that shepherd
> > endpoints can be specified in lieu of MPD endpoints.  You don't
> > need to implement this logic – you are free to do so if you want to
> > – but could you make it so that there is an explicit endpoint
> > abstraction that would allow for extension later on?
> > 
> > Cheers
> > 
> > [1] https://issues.guix.gnu.org/54986
> 
> Hi,
> 
> After reading issue #54986, regarding mpd escaping shepherd
> management, my guess is that there could be two issues at play here:
> 
> * mpd.conf was serialized with pid_file [1]. The safest is
> to actually not serialize any unused directives and let MPD decide
> based on what's actually present. pid_file should only be set if
> MPD is to be launched as a "daemon process". [2]
> 
> * But the service definition for MPD is launched with '--no-daemon'
> option as seen in [3], which could be triggering a bug in MPD. It's
> probably worth testing the combinations of having pid_file set and
> invoking --no-daemon.
Nice catch, but completely unrelated to the issue.  In neither case do
we ever spawn MPD as a regular daemon, yet only in the systemd case
does shepherd have a problem with it.  That statement was concerning
shepherd's (mis)management of sockets, not MPD.

> > but could you make it so that there is an explicit endpoint
> > abstraction that would allow for extension later on?
> 
> If I'm understanding correctly what [1] is about, I think this can be
> implemented through the 'extend' interface. See how certbot adds a
> nginx-server-configuration to a nginx-service-type through the
> 'extend' interface. For MPD, this would amount to appending the
> 'adresses' field with "adequately formatted" values.
This doesn't work for #54986, which makes it so that in-file addresses
are ignored in favour of handing over the sockets directly through
shepherd.  Looking at [4], it appears the meaning of "port" is closer
to that of a default port, as addresses can have ports in them.  But I
would still prefer addresses to be "endpoints", which if they happen to
be a list of strings are taken as MPD addresses and if they happen to
be shepherd endpoints are passed on to the shepherd service.

WDYT?

[4] https://mpd.readthedocs.io/en/stable/user.html#client-connections

Message #77 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: mirai <mirai <at> makinata.eu>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 59866 <at> debbugs.gnu.org
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] services: mpd: Refactor MPD service
Date: Fri, 9 Dec 2022 13:44:58 +0000

On 2022-12-08 13:35, Liliana Marie Prikler wrote:
> This doesn't work for #54986, which makes it so that in-file addresses
> are ignored in favour of handing over the sockets directly through
> shepherd.  Looking at [4], it appears the meaning of "port" is closer
> to that of a default port, as addresses can have ports in them.  But I
> would still prefer addresses to be "endpoints", which if they happen to
> be a list of strings are taken as MPD addresses and if they happen to
> be shepherd endpoints are passed on to the shepherd service.

Are you proposing for the 'addresses' field to be a
"maybe-list-of-string-or-shepherd-endpoint"? (more of a xor as they can't
be used simultaneously)
Example:

--8<---------------cut here---------------start------------->8---
;; should fire a error message during guix system reconfigure
(mpd-configuration
  (addresses `("[::]:6645"  
               ,(shepherd-endpoint
                  (address "/var/run/mpd-shepherd-socket")))))
--8<---------------cut here---------------end--------------->8---

I don't think it breaks backward compatibility to introduce this
after #59866 is merged.
The type of field 'addresses' could be changed transparently to something like:

--8<---------------cut here---------------start------------->8---
(define list-of-addresses (list-of (lambda (x) (or (string? x) (shepherd-endpoint? x)))))
--8<---------------cut here---------------end--------------->8---

Message #80 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: mirai <mirai <at> makinata.eu>, 59866 <at> debbugs.gnu.org
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] services: mpd: Refactor MPD service
Date: Fri, 09 Dec 2022 20:22:06 +0100

Am Freitag, dem 09.12.2022 um 13:44 +0000 schrieb mirai:
> On 2022-12-08 13:35, Liliana Marie Prikler wrote:
> > This doesn't work for #54986, which makes it so that in-file
> > addresses are ignored in favour of handing over the sockets
> > directly through shepherd.  Looking at [4], it appears the meaning
> > of "port" is closer to that of a default port, as addresses can
> > have ports in them. 
> > But I would still prefer addresses to be "endpoints", which if they
> > happen to be a list of strings are taken as MPD addresses and if
> > they happen to be shepherd endpoints are passed on to the shepherd
> > service.
> 
> Are you proposing for the 'addresses' field to be a
> "maybe-list-of-string-or-shepherd-endpoint"? (more of a xor as they
> can't be used simultaneously)
> Example:
> 
> --8<---------------cut here---------------start------------->8---
> ;; should fire a error message during guix system reconfigure
> (mpd-configuration
>   (addresses `("[::]:6645"  
>                ,(shepherd-endpoint
>                   (address "/var/run/mpd-shepherd-socket")))))
> --8<---------------cut here---------------end--------------->8---
> 
> I don't think it breaks backward compatibility to introduce this
> after #59866 is merged.
> The type of field 'addresses' could be changed transparently to
> something like:
> 
> --8<---------------cut here---------------start------------->8---
> (define list-of-addresses (list-of (lambda (x) (or (string? x)
> (shepherd-endpoint? x)))))
> --8<---------------cut here---------------end--------------->8---
Something like that, but I don't think the vocabulary matches 1:1.  In
my opinion, an address is an endpoint – not a shepherd endpoint, but an
endpoint still – while a shepherd endpoint is not an address.  Thus, I
propose changing the vocabulary now to not break backwards
compatibility later.  IIUC, the change from the previous records to
define-configuration is already an API change, so it'd be good to have
both in the same series.

Cheers

Message #83 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: mirai <mirai <at> makinata.eu>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>, 59866 <at> debbugs.gnu.org
Cc: 54986 <at> debbugs.gnu.org
Subject: Re: [PATCH 0/2] services: mpd: Refactor MPD service
Date: Sun, 11 Dec 2022 12:05:38 +0000

On 2022-12-09 19:22, Liliana Marie Prikler wrote:
> Something like that, but I don't think the vocabulary matches 1:1.  In
> my opinion, an address is an endpoint – not a shepherd endpoint, but an
> endpoint still – while a shepherd endpoint is not an address.  Thus, I
> propose changing the vocabulary now to not break backwards
> compatibility later.
You prefer the 'addresses' field to be renamed to 'endpoints', is that correct?

IIUC, the change from the previous records to
> define-configuration is already an API change, so it'd be good to have
> both in the same series.
The first patch in the series changes from define-record-type* to define-configuration
but it does not add anything, I don't think there are actually any (exported)
API changes visible to the user.

Only the second patch starts to introduce visible API changes.

Cheers

Message #86 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Tue, 25 Apr 2023 20:33:15 -0400

Hi!

Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:

> * gnu/services/shepherd.scm (<shepherd-endpoint>): New record type.
> (shepherd-endpoint->sexp): New variable.
> * doc/guix.texi (Shepherd Services): Document it.

Like Ludovic, I'm wondering what duplicating the Shepherd endpoints API
in Guix buys us?  It sometimes feel a bit contrived to have to work
inside the service's gexp expression, but other than that, I think it's
good to:

1. Avoid duplication.
2. Keep it as internal/hidden as possible from users.

-- 
Thanks,
Maxim

Message #89 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Wed, 26 Apr 2023 06:28:16 +0200

Am Dienstag, dem 25.04.2023 um 20:33 -0400 schrieb Maxim Cournoyer:
> Hi!
> 
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
> 
> > * gnu/services/shepherd.scm (<shepherd-endpoint>): New record type.
> > (shepherd-endpoint->sexp): New variable.
> > * doc/guix.texi (Shepherd Services): Document it.
> 
> Like Ludovic, I'm wondering what duplicating the Shepherd endpoints
> API in Guix buys us?  It sometimes feel a bit contrived to have to
> work inside the service's gexp expression, but other than that, I
> think it's good to:
> 
> 1. Avoid duplication.
> 2. Keep it as internal/hidden as possible from users.
I agree with the point about avoiding duplication, but I want users to
be able to specify endpoints for socket activation.  This has several
benefits: It firstly allows users to specify that they want a specific
service to be started on demand rather than on boot, and it also allows
them to bind to multiple endpoints, e.g. any IPv4 address, any IPv6
address or both.  Duplicating the API here is merely a means of
allowing users to express the above in a Guixy fashion.  It also gives
us type-checking which a simple quote or quasiquote doesn't.  If the
same can be achieved by inspecting Shepherd's records and we can allow
Guix to depend on Shepherd, that'd be fine by me. 

Cheers

Message #92 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Mon, 01 May 2023 11:53:52 -0400

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:

> Am Dienstag, dem 25.04.2023 um 20:33 -0400 schrieb Maxim Cournoyer:
>> Hi!
>> 
>> Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:
>> 
>> > * gnu/services/shepherd.scm (<shepherd-endpoint>): New record type.
>> > (shepherd-endpoint->sexp): New variable.
>> > * doc/guix.texi (Shepherd Services): Document it.
>> 
>> Like Ludovic, I'm wondering what duplicating the Shepherd endpoints
>> API in Guix buys us?  It sometimes feel a bit contrived to have to
>> work inside the service's gexp expression, but other than that, I
>> think it's good to:
>> 
>> 1. Avoid duplication.
>> 2. Keep it as internal/hidden as possible from users.
> I agree with the point about avoiding duplication, but I want users to
> be able to specify endpoints for socket activation.  This has several
> benefits: It firstly allows users to specify that they want a specific
> service to be started on demand rather than on boot, and it also allows
> them to bind to multiple endpoints, e.g. any IPv4 address, any IPv6
> address or both.  Duplicating the API here is merely a means of
> allowing users to express the above in a Guixy fashion.  It also gives
> us type-checking which a simple quote or quasiquote doesn't.  If the
> same can be achieved by inspecting Shepherd's records and we can allow
> Guix to depend on Shepherd, that'd be fine by me. 

Instead of replicating the Shepherd API in Guix, could we use the
Shepherd API directly?  It's Scheme, and already depended on by Guix, so
the question arises.

It may not be a good idea, but I need to be refreshed on the reasons
:-).

-- 
Thanks,
Maxim

Message #95 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Mon, 01 May 2023 18:09:58 +0200

Am Montag, dem 01.05.2023 um 11:53 -0400 schrieb Maxim Cournoyer:
> > > Like Ludovic, I'm wondering what duplicating the Shepherd
> > > endpoints API in Guix buys us?  [...]
> 
> Instead of replicating the Shepherd API in Guix, could we use the
> Shepherd API directly?  It's Scheme, and already depended on by Guix,
> so the question arises.
In theory, it'd be possible, albeit with some caveats:
1. Shepherd doesn't (didn't) have a full guix-style records API, which
might cause discrepancies in otherwise normal-looking Scheme code.
2. It'd probably make shepherd a compile-time dependency, which is
avoided in other places in the code, i.e. (gnu build shepherd)
3. Shepherd records are (to my knowledge) not print-readable, so we'd
have to move them through G-Expressions through some of our own code
anyway; how strongly that would replicate the API is up to
debate/speculation.

Cheers 

Message #98 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>,
 Maxime Devos <maximedevos <at> telenet.be>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Tue, 02 May 2023 23:17:46 -0400

Hi Liliana,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> writes:

> Am Montag, dem 01.05.2023 um 11:53 -0400 schrieb Maxim Cournoyer:
>> > > Like Ludovic, I'm wondering what duplicating the Shepherd
>> > > endpoints API in Guix buys us?  [...]
>>
>> Instead of replicating the Shepherd API in Guix, could we use the
>> Shepherd API directly?  It's Scheme, and already depended on by Guix,
>> so the question arises.
> In theory, it'd be possible, albeit with some caveats:
> 1. Shepherd doesn't (didn't) have a full guix-style records API, which
> might cause discrepancies in otherwise normal-looking Scheme code.

Shepherd's code is pretty normal-looking Scheme code to me, last I
checked :-) I think Ludovic has also been working toward bringing it
closer to their standards, e.g. deprecating the use of GOOPS.

> 2. It'd probably make shepherd a compile-time dependency, which is
> avoided in other places in the code, i.e. (gnu build shepherd)

I wonder if that could be an acceptable price to pay?  It seems
reasonable to me that the close integration of Shepherd within Guix
introduces a dependency on it.

> 3. Shepherd records are (to my knowledge) not print-readable, so we'd
> have to move them through G-Expressions through some of our own code
> anyway; how strongly that would replicate the API is up to
> debate/speculation.

As far as I can tell, both are based on SRFI-9 records and use the same
representation, or I miss the fine details from the source.

-- 
Thanks,
Maxim

Message #101 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Wed, 03 May 2023 15:27:31 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Montag, dem 01.05.2023 um 11:53 -0400 schrieb Maxim Cournoyer:
>> > > Like Ludovic, I'm wondering what duplicating the Shepherd
>> > > endpoints API in Guix buys us?  [...]
>> 
>> Instead of replicating the Shepherd API in Guix, could we use the
>> Shepherd API directly?  It's Scheme, and already depended on by Guix,
>> so the question arises.
> In theory, it'd be possible, albeit with some caveats:

It’s not just possible: several services in (gnu services …) and (gnu
home services …) use endpoints for systemd or inetd-style startup.

> 1. Shepherd doesn't (didn't) have a full guix-style records API, which
> might cause discrepancies in otherwise normal-looking Scheme code.

I’m not convinced.  :-)

> 2. It'd probably make shepherd a compile-time dependency, which is
> avoided in other places in the code, i.e. (gnu build shepherd)

(gnu build shepherd) is 75% deprecated; the introduction of endpoints in
the Shepherd didn’t have any effect on it.

> 3. Shepherd records are (to my knowledge) not print-readable, so we'd
> have to move them through G-Expressions through some of our own code
> anyway; how strongly that would replicate the API is up to
> debate/speculation.

When would you want to print those <endpoint> records?

Thanks,
Ludo’.

Message #104 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Wed, 03 May 2023 18:54:25 +0200

Am Mittwoch, dem 03.05.2023 um 15:27 +0200 schrieb Ludovic Courtès:
> It’s not just possible: several services in (gnu services …) and (gnu
> home services …) use endpoints for systemd or inetd-style startup.
True, but to my knowledge they don't yet allow the user to specify
those endpoints directly.  At the very least, they didn't when I
started this thread, which was shortly after shepherd itself gained
endpoints.  I'm happy to be proven wrong on this point.

> > 1. Shepherd doesn't (didn't) have a full guix-style records API,
> > which might cause discrepancies in otherwise normal-looking Scheme
> > code.
> 
> I’m not convinced.  :-)
For more information, I'm a little worried that someone would attempt
  (endpoint (inherit some-other-endpoint) (field ...))
though perhaps that's a little overengineered problem and shepherd
itself is moving towards a more declarative API as we speak.

> > 2. It'd probably make shepherd a compile-time dependency, which is
> > avoided in other places in the code, i.e. (gnu build shepherd)
> 
> (gnu build shepherd) is 75% deprecated; the introduction of endpoints
> in the Shepherd didn’t have any effect on it.
Good to know.

> > 3. Shepherd records are (to my knowledge) not print-readable, so
> > we'd have to move them through G-Expressions through some of our
> > own code anyway; how strongly that would replicate the API is up to
> > debate/speculation.
> 
> When would you want to print those <endpoint> records?
When writing them to a shepherd init.scm :)

Cheers

Message #107 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Wed, 10 May 2023 17:14:14 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Mittwoch, dem 03.05.2023 um 15:27 +0200 schrieb Ludovic Courtès:
>> It’s not just possible: several services in (gnu services …) and (gnu
>> home services …) use endpoints for systemd or inetd-style startup.
> True, but to my knowledge they don't yet allow the user to specify
> those endpoints directly.  At the very least, they didn't when I
> started this thread, which was shortly after shepherd itself gained
> endpoints.  I'm happy to be proven wrong on this point.

They don’t let users specify the endpoints as such, but closely enough.
For instance, the ‘interface’ field of <bitlbee-configuration> is used
to build it endpoint, and similarly for ‘openssh’ and ‘dicod’.

Overall, it seems to me we don’t need a first-class <endpoint> type in
Guix System itself.

I hope that makes sense!

Ludo’.

Message #110 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
To: Ludovic Courtès <ludo <at> gnu.org>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Wed, 10 May 2023 20:32:26 +0200

Am Mittwoch, dem 10.05.2023 um 17:14 +0200 schrieb Ludovic Courtès:
> Hi,
> 
> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
> 
> > Am Mittwoch, dem 03.05.2023 um 15:27 +0200 schrieb Ludovic Courtès:
> > > It’s not just possible: several services in (gnu services …) and
> > > (gnu home services …) use endpoints for systemd or inetd-style
> > > startup.
> > True, but to my knowledge they don't yet allow the user to specify
> > those endpoints directly.  At the very least, they didn't when I
> > started this thread, which was shortly after shepherd itself gained
> > endpoints.  I'm happy to be proven wrong on this point.
> 
> They don’t let users specify the endpoints as such, but closely
> enough.  For instance, the ‘interface’ field of <bitlbee-
> configuration> is used to build it endpoint, and similarly for
> ‘openssh’ and ‘dicod’.
> 
> Overall, it seems to me we don’t need a first-class <endpoint> type
> in Guix System itself.
It's funny you would argue that, because imho openssh would actually be
a good candidate for supporting first-class endpoints:  Doing so would
allow the user to specify whether IPv4, IPv6 or both (default) should
be allowed for connections.  For other service that support sockets as
well as TCP/IP ports, the benefit would be even greater.

I understand that copypasting all the fields into Guix records is a bit
of a non-starter, but I don't think it's a good idea to simply give up.
I just need some pointers in which direction to continue.

Cheers

Message #113 received at 54986 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Liliana Marie Prikler <liliana.prikler <at> gmail.com>
Cc: 54986 <at> debbugs.gnu.org, Maxime Devos <maximedevos <at> telenet.be>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#54986: [PATCH] gnu: mpd: Add support for socket activation.
Date: Thu, 11 May 2023 12:52:17 +0200

Hi,

Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:

> Am Mittwoch, dem 10.05.2023 um 17:14 +0200 schrieb Ludovic Courtès:
>> Hi,
>> 
>> Liliana Marie Prikler <liliana.prikler <at> gmail.com> skribis:
>> 
>> > Am Mittwoch, dem 03.05.2023 um 15:27 +0200 schrieb Ludovic Courtès:
>> > > It’s not just possible: several services in (gnu services …) and
>> > > (gnu home services …) use endpoints for systemd or inetd-style
>> > > startup.
>> > True, but to my knowledge they don't yet allow the user to specify
>> > those endpoints directly.  At the very least, they didn't when I
>> > started this thread, which was shortly after shepherd itself gained
>> > endpoints.  I'm happy to be proven wrong on this point.
>> 
>> They don’t let users specify the endpoints as such, but closely
>> enough.  For instance, the ‘interface’ field of <bitlbee-
>> configuration> is used to build it endpoint, and similarly for
>> ‘openssh’ and ‘dicod’.
>> 
>> Overall, it seems to me we don’t need a first-class <endpoint> type
>> in Guix System itself.
> It's funny you would argue that, because imho openssh would actually be
> a good candidate for supporting first-class endpoints:  Doing so would
> allow the user to specify whether IPv4, IPv6 or both (default) should
> be allowed for connections.  For other service that support sockets as
> well as TCP/IP ports, the benefit would be even greater.

For the services I mentioned, I don’t feel that lack of first-class
endpoints is a hindrance in terms of flexibility.  We’re trading
expressivity for ease of use.

> I understand that copypasting all the fields into Guix records is a bit
> of a non-starter, but I don't think it's a good idea to simply give up.
> I just need some pointers in which direction to continue.

Like I wrote, I’m kinda skeptical about the idea.  :-)

Now, if you find good motivating examples and find a way to express
endpoints that remain concise in common cases, that may be more
appealing to me!

Ludo’.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.