[Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

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


Hi Guix, Emacs,

As promised to Stefan a few months ago, here's a use case of
Shepherd/Emacs implementation that we developped in RDE.

We're using the --daemon option on the Shepherd side to launch the
server in the background, include code in Emacs configuration to make it
create a pid-file as soon as the server has started, and redefine
kill-emacs to be managed by the Shepherd.

Originally I asked Stefan because I wasn't happy with the current
Emacs/Shepherd interaction possibilities, as Emacs doesn't provide a
native --pid-file option. He advised me to share once we found a
solution to highlight the situation, here it is.

Cheers,
Nicolas


-------------------- Start of forwarded message --------------------
From: Nicolas Graves <ngraves@ngraves.fr>
To: ~abcdw/rde-devel@lists.sr.ht
Cc: ngraves@ngraves.fr
Subject: [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
Date: Thu, 11 Apr 2024 01:48:13 +0200

---
 src/rde/features/emacs.scm      | 42 ++++++++++++++++++++++++++++++++-
 src/rde/home/services/emacs.scm |  9 ++++---
 2 files changed, 47 insertions(+), 4 deletions(-)

diff --git a/src/rde/features/emacs.scm b/src/rde/features/emacs.scm
index 7d055377..74ba306e 100644
--- a/src/rde/features/emacs.scm
+++ b/src/rde/features/emacs.scm
@@ -440,7 +440,47 @@ Prefix argument can be used to kill a few words."
        ;; Configure ediff for window manager.
        (setq ediff-diff-options "-w"
              ediff-split-window-function 'split-window-horizontally
-             ediff-window-setup-function 'ediff-setup-windows-plain))
+             ediff-window-setup-function 'ediff-setup-windows-plain)
+
+       ;; Configure emacs background server.
+       ,@(if (get-value 'emacs-server-mode? config)
+             `((add-hook
+                'emacs-startup-hook
+                (lambda ()
+                  (when server-mode
+                    (advice-add
+                     'kill-emacs :override
+                     (lambda (&optional arg restart)
+                       "\
+Make GNU Shepherd kill the Emacs server.
+
+If RESTART is non-nil, instead of just exiting at the end, herd will restart
+an Emacs server. ARG is ignored.
+
+Note: This is added through RDE. Redefining a primitive is not advised in
+Emacs, but this one is high-level (present in few other functions), and
+tested."
+                       (interactive)
+                       (if restart
+                           (call-process-shell-command
+                            (concat "herd restart emacs-" server-name)
+                            nil 0)
+                           (call-process-shell-command
+                            (concat "herd stop emacs-" server-name)
+                            nil 0))))
+                    ;; Tell shepherd that emacs is started through pid-file.
+                    (with-temp-file (concat (getenv "XDG_RUNTIME_DIR")
+                                            "/emacs/" server-name ".pid")
+                                    (insert (number-to-string (emacs-pid)))))))
+               (add-hook
+                'kill-emacs-hook
+                (lambda ()
+                  (when server-mode
+                    (let ((pidfile (concat (getenv "XDG_RUNTIME_DIR")
+                                           "/emacs/" server-name ".pid")))
+                      (when (file-exists-p pidfile)
+                        (delete-file pidfile)))))))
+             '()))
      #:summary "General settings, better defaults"
      #:commentary "\
 It can contain settings not yet moved to separate features."
diff --git a/src/rde/home/services/emacs.scm b/src/rde/home/services/emacs.scm
index 9b0b37af..95156a4d 100644
--- a/src/rde/home/services/emacs.scm
+++ b/src/rde/home/services/emacs.scm
@@ -188,11 +188,14 @@ Same as @code{init-el}, but result will go to @file{early-init.el}."))
    (start #~(make-forkexec-constructor
              (list #$(file-append
                       (home-emacs-configuration-emacs config)
-                      "/bin/emacs") #$(format #f "--fg-daemon=~a" name))
+                      "/bin/emacs") #$(format #f "--daemon=~a" name))
              #:log-file (string-append
                          (getenv "XDG_STATE_HOME") "/log"
-                         "/emacs-" #$(symbol->string name) ".log")))
-   (stop #~(make-kill-destructor))))
+                         "/emacs-" #$(symbol->string name) ".log")
+             #:pid-file (string-append (getenv "XDG_RUNTIME_DIR") "/emacs/"
+                                       #$(symbol->string name) ".pid")))
+   (stop #~(make-kill-destructor))
+   (respawn? #f)))
 
 (define (add-emacs-shepherd-service config)
   (if (not (null? (home-emacs-configuration-emacs-servers config)))
-- 
2.41.0

-------------------- End of forwarded message --------------------

-------------------- Start of forwarded message --------------------
From: Andrew Tropin <andrew@trop.in>
To: Nicolas Graves <ngraves@ngraves.fr>, ~abcdw/rde-devel@lists.sr.ht
Cc: ngraves@ngraves.fr
Subject: Re: [PATCH v6 00/10] Emacs server background mode.
Date: Thu, 11 Apr 2024 12:15:58 +0300


[-- Attachment #2.1: Type: text/plain, Size: 1261 bytes --]

On 2024-04-11 01:48, Nicolas Graves wrote:

> v6 of the previous patch series, rebased and squased
>
> Nicolas Graves (10):
>   rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
>   rde: emacs: Add smart emacs-client-alternate-editor
>   rde: emacs: Make minibuffer programs fail without emacs-server
>   rde: sway: Add shepherd value
>   rde: emacs: Use absolute path for herd binary
>   rde: wm: Sort package modules
>   rde: swaynotificationcenter: Add value libnotify
>   rde: emacs: Move emacs-minibuffer-program to feature value
>   rde: emacs: Make emacs-minibuffer-program depend on config
>   rde: emacs: Propagate herd and libnotify paths
>
>  src/rde/features/emacs-xyz.scm      |   6 +-
>  src/rde/features/emacs.scm          | 168 +++++++++++++++++++++-------
>  src/rde/features/password-utils.scm |   3 +-
>  src/rde/features/wm.scm             |  38 ++++---
>  src/rde/features/xdisorg.scm        |   4 +-
>  src/rde/home/services/emacs.scm     |   9 +-
>  6 files changed, 165 insertions(+), 63 deletions(-)

Very nice, works flawlessly so far and notifications, when server is
starting are great.  No more half-way loaded emacsclient.

Thank you very much for you work!  Great job, Nicolas!

-- 
Best regards,
Andrew Tropin

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

[-- Attachment #3: Type: text/plain, Size: 101 bytes --]

-------------------- End of forwarded message --------------------

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Ludovic Courtès]

Hi Nicolas,

Nicolas Graves skribis:

> As promised to Stefan a few months ago, here's a use case of
> Shepherd/Emacs implementation that we developped in RDE.

Would be nice to have it in Guix Home!

> We're using the --daemon option on the Shepherd side to launch the
> server in the background, include code in Emacs configuration to make it
> create a pid-file as soon as the server has started, and redefine
> kill-emacs to be managed by the Shepherd.

Emacs supports systemd-style socket activation so, instead of using a
PID file, you could use ‘make-systemd-constructor’.

Now, that code in emacs.c is unfortunately implemented via libsystemd
and thus disabled in Guix.  Using libsystemd in this case is unnecessary
(and increases the attack surface, as we’ve seen with the xz backdoor):
it could read the ‘LISTEN_FDS’ and ‘LISTEN_PID’ environment variables
instead of calling the sd_* functions.

  https://www.freedesktop.org/software/systemd/man/latest/sd_listen_fds.html
  https://www.gnu.org/software/shepherd/manual/html_node/Service-De_002d-and-Constructors.html#index-make_002dsystemd_002dconstructor

Thanks,
Ludo’.

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

On 2024-04-12 22:38, Ludovic Courtès wrote:

> Hi Nicolas,
>
> Nicolas Graves skribis:
>
>> As promised to Stefan a few months ago, here's a use case of
>> Shepherd/Emacs implementation that we developped in RDE.
>
> Would be nice to have it in Guix Home!

I am commited to merge RDE contributions upstream when this is possible,
I will when it's mature enough / adapted for Guix ;)

>
>> We're using the --daemon option on the Shepherd side to launch the
>> server in the background, include code in Emacs configuration to make it
>> create a pid-file as soon as the server has started, and redefine
>> kill-emacs to be managed by the Shepherd.
>
> Emacs supports systemd-style socket activation so, instead of using a
> PID file, you could use ‘make-systemd-constructor’.
>
> Now, that code in emacs.c is unfortunately implemented via libsystemd
> and thus disabled in Guix.  Using libsystemd in this case is unnecessary
> (and increases the attack surface, as we’ve seen with the xz backdoor):
> it could read the ‘LISTEN_FDS’ and ‘LISTEN_PID’ environment variables
> instead of calling the sd_* functions.
>
>   https://www.freedesktop.org/software/systemd/man/latest/sd_listen_fds.html
>   https://www.gnu.org/software/shepherd/manual/html_node/Service-De_002d-and-Constructors.html#index-make_002dsystemd_002dconstructor

Thanks Ludo, that is precisely the feedback I was looking for.

Maybe some feedback on the Emacs side about this? There are indeed very
few places where systemd sd_* functions are called in emacs.c, should we
try and re-implement them instead of using the library as is? Would that
be a contribution Emacs devs would be interested in? That would
definitely be beneficial for Emacs on Guix as highlighted by Ludo'.

>
> Thanks,
> Ludo’.

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

On 2024-04-13 16:20, Nicolas Graves wrote:

> On 2024-04-12 22:38, Ludovic Courtès wrote:
>
>> Hi Nicolas,
>>
>> Nicolas Graves skribis:
>>
>>> As promised to Stefan a few months ago, here's a use case of
>>> Shepherd/Emacs implementation that we developped in RDE.
>>
>> Would be nice to have it in Guix Home!
>
> I am commited to merge RDE contributions upstream when this is possible,
> I will when it's mature enough / adapted for Guix ;)
>
>>
>>> We're using the --daemon option on the Shepherd side to launch the
>>> server in the background, include code in Emacs configuration to make it
>>> create a pid-file as soon as the server has started, and redefine
>>> kill-emacs to be managed by the Shepherd.
>>
>> Emacs supports systemd-style socket activation so, instead of using a
>> PID file, you could use ‘make-systemd-constructor’.
>>
>> Now, that code in emacs.c is unfortunately implemented via libsystemd
>> and thus disabled in Guix.  Using libsystemd in this case is unnecessary
>> (and increases the attack surface, as we’ve seen with the xz backdoor):
>> it could read the ‘LISTEN_FDS’ and ‘LISTEN_PID’ environment variables
>> instead of calling the sd_* functions.
>>
>>   https://www.freedesktop.org/software/systemd/man/latest/sd_listen_fds.html
>>   https://www.gnu.org/software/shepherd/manual/html_node/Service-De_002d-and-Constructors.html#index-make_002dsystemd_002dconstructor
>
> Thanks Ludo, that is precisely the feedback I was looking for.
>
> Maybe some feedback on the Emacs side about this? There are indeed very
> few places where systemd sd_* functions are called in emacs.c, should we
> try and re-implement them instead of using the library as is? Would that
> be a contribution Emacs devs would be interested in? That would
> definitely be beneficial for Emacs on Guix as highlighted by Ludo'.

For the Guix side: it seems to be possible without Emacs source code
change and by using elogind instead of systemd, there seems to be some
code in configure.ac to handle this case. Will try this.

>
>>
>> Thanks,
>> Ludo’.

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Stefan Monnier]

> Maybe some feedback on the Emacs side about this? There are indeed very
> few places where systemd sd_* functions are called in emacs.c, should we
> try and re-implement them instead of using the library as is? Would that
> be a contribution Emacs devs would be interested in? That would
> definitely be beneficial for Emacs on Guix as highlighted by Ludo'.

It's hard to tell without seeing the actual patch.

But if the code is sufficiently simple, it implements a protocol that's
well documented, and it allows us to eliminate the dependency on the
systemd library, we might like it.


        Stefan

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

On 2024-04-12 22:38, Ludovic Courtès wrote:

> Hi Nicolas,
>
> Nicolas Graves skribis:
>
>> As promised to Stefan a few months ago, here's a use case of
>> Shepherd/Emacs implementation that we developped in RDE.
>
> Would be nice to have it in Guix Home!

Something like this seems to work:

(define (emacs-shepherd-service config name)
  (shepherd-service
   (documentation
    (format #f "Emacs server.  Use ~a to connect to it."
            (if (eq? 'server name)
                "@code{emacsclient}"
                (format #f "@code{emacsclient -s ~a}" name))))
   (provision `(,(symbol-append 'emacs- name)))
   (requirement '(emacs))
   (modules '((shepherd support)))  ;for '%user-runtime-dir'
   (start #~(make-systemd-constructor
             (list #$(file-append
                      (home-emacs-configuration-emacs config)
                      "/bin/emacs") #$(format #f "--fg-daemon=~a" name))
             (list (endpoint
                    (make-socket-address
                     AF_UNIX
                     (string-append %user-runtime-dir
                                    "/emacs/" #$(symbol->string name)))
                    #:name '#$(format #f "emacs-~a" name)
                    #:socket-directory-permissions #o700))
             #:log-file (string-append
                         (getenv "XDG_STATE_HOME") "/log"
                         "/emacs-" #$(symbol->string name) ".log")))
   (stop #~(make-systemd-destructor))))

But I'm not sure it's better regarding user experience. On RDE we
implemented a notifier that parses the result of
herd eval root "(and=> (lookup-service 'emacs-server) service-status)"
thus giving a nice "Emacs is currently starting" notification.

This evaluation doesn't seem to work using make-systemd-constructor,
although it has its advantages (indeed launches a frame when available).
It would be nice if service-status could "sync" with
make-systemd-constructor in this case.

I would be happy to send such a patch for Guix (is there already some
patch series on which to graft this?), WDYT @Ludo?

I can also send this in RDE, it simplifies a lot although I'm still not
sure it yields a better user experience. @Andrew?

>> We're using the --daemon option on the Shepherd side to launch the
>> server in the background, include code in Emacs configuration to make it
>> create a pid-file as soon as the server has started, and redefine
>> kill-emacs to be managed by the Shepherd.
>
> Emacs supports systemd-style socket activation so, instead of using a
> PID file, you could use ‘make-systemd-constructor’.
>
> Now, that code in emacs.c is unfortunately implemented via libsystemd
> and thus disabled in Guix.  Using libsystemd in this case is unnecessary
> (and increases the attack surface, as we’ve seen with the xz backdoor):
> it could read the ‘LISTEN_FDS’ and ‘LISTEN_PID’ environment variables
> instead of calling the sd_* functions.
>
>   https://www.freedesktop.org/software/systemd/man/latest/sd_listen_fds.html
>   https://www.gnu.org/software/shepherd/manual/html_node/Service-De_002d-and-Constructors.html#index-make_002dsystemd_002dconstructor
>
> Thanks,
> Ludo’.

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Felix Lechner via Development of GNU Guix and the GNU System distribution.]

Hi Nicolas,

On Fri, Apr 12 2024, Ludovic Courtès wrote:

> Emacs supports systemd-style socket activation so, instead of using a
> PID file, you could use ‘make-systemd-constructor’.

Ludo' may have meant to write "Shepherd" instead of Emacs.  Here are a
few examples for how you might use that beautiful constructor. [1][2][3]

Alas, I don't much like the name, which refers to the true daemon called
systemd.  Instead I would prefer "make-socket-listener-constructor" or
similar.

Kind regards
Felix

[1] https://codeberg.org/lechner/system-config/src/commit/db9edb46caf36fe15bc6f8abc5d1df184b6d5c5f/host/wallace-server/operating-system.scm#L958-L987
[2] https://codeberg.org/lechner/system-config/src/commit/db9edb46caf36fe15bc6f8abc5d1df184b6d5c5f/host/wallace-server/operating-system.scm#L989-L1018
[3] https://codeberg.org/lechner/system-config/src/commit/db9edb46caf36fe15bc6f8abc5d1df184b6d5c5f/host/wallace-server/operating-system.scm#L1020-L1047

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Björn Bidar]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> Maybe some feedback on the Emacs side about this? There are indeed very
>> few places where systemd sd_* functions are called in emacs.c, should we
>> try and re-implement them instead of using the library as is? Would that
>> be a contribution Emacs devs would be interested in? That would
>> definitely be beneficial for Emacs on Guix as highlighted by Ludo'.
>
> It's hard to tell without seeing the actual patch.
>
> But if the code is sufficiently simple, it implements a protocol that's
> well documented, and it allows us to eliminate the dependency on the
> systemd library, we might like it.

Would that make sense on systems where systemd is used? If libsystem is
already installed it would be more convenient for the user to use the
already installed and very likely loaded libsystemd instead of
reimplementing the feature.

Ideally the support for other initrd system could implement a function
that is then called instead of the systemd codepath  be it something
different or to reimplenent sd-notify. Maybe shepherd as something like
sd-notify of it's own?

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Stefan Monnier]

> Would that make sense on systems where systemd is used? If libsystem is
> already installed it would be more convenient for the user to use the
> already installed and very likely loaded libsystemd instead of
> reimplementing the feature.

You might be right, but it's hard to say without seeing the replacement
code and the actual doc of the protocol.


        Stefan

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Ludovic Courtès]

Hi,

Björn Bidar <bjorn.bidar@thaodan.de> skribis:

> Stefan Monnier <monnier@iro.umontreal.ca> writes:
>
>>> Maybe some feedback on the Emacs side about this? There are indeed very
>>> few places where systemd sd_* functions are called in emacs.c, should we
>>> try and re-implement them instead of using the library as is? Would that
>>> be a contribution Emacs devs would be interested in? That would
>>> definitely be beneficial for Emacs on Guix as highlighted by Ludo'.
>>
>> It's hard to tell without seeing the actual patch.
>>
>> But if the code is sufficiently simple, it implements a protocol that's
>> well documented, and it allows us to eliminate the dependency on the
>> systemd library, we might like it.
>
> Would that make sense on systems where systemd is used? If libsystem is
> already installed it would be more convenient for the user to use the
> already installed and very likely loaded libsystemd instead of
> reimplementing the feature.

As I wrote, in the wake of the xz backdoor, many came to the conclusion
that linking against libsystemd “just” to check a couple of environment
variables (for socket activation) is hard to justify (libsystemd
provides much more functionality than this bit.)

> Ideally the support for other initrd system could implement a function
> that is then called instead of the systemd codepath  be it something
> different or to reimplenent sd-notify. Maybe shepherd as something like
> sd-notify of it's own?

Shepherd does not implement the sd-notify protocol, but it implements
socket activation, which is what Emacs currently uses AFAICS.

HTH,
Ludo’.

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Ludovic Courtès]

Stefan Monnier <monnier@iro.umontreal.ca> skribis:

>> Would that make sense on systems where systemd is used? If libsystem is
>> already installed it would be more convenient for the user to use the
>> already installed and very likely loaded libsystemd instead of
>> reimplementing the feature.
>
> You might be right, but it's hard to say without seeing the replacement
> code and the actual doc of the protocol.

As an example, here’s C++ code that checks the LISTEN_* environment
variables I mentioned earlier:

  https://git.savannah.gnu.org/cgit/guix.git/tree/nix/nix-daemon/guix-daemon.cc#n437

Ludo’.

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Ludovic Courtès]

Hi,

Nicolas Graves <ngraves@ngraves.fr> skribis:

> (define (emacs-shepherd-service config name)
>   (shepherd-service
>    (documentation
>     (format #f "Emacs server.  Use ~a to connect to it."
>             (if (eq? 'server name)
>                 "@code{emacsclient}"
>                 (format #f "@code{emacsclient -s ~a}" name))))
>    (provision `(,(symbol-append 'emacs- name)))
>    (requirement '(emacs))
>    (modules '((shepherd support)))  ;for '%user-runtime-dir'
>    (start #~(make-systemd-constructor
>              (list #$(file-append
>                       (home-emacs-configuration-emacs config)
>                       "/bin/emacs") #$(format #f "--fg-daemon=~a" name))
>              (list (endpoint
>                     (make-socket-address
>                      AF_UNIX
>                      (string-append %user-runtime-dir
>                                     "/emacs/" #$(symbol->string name)))
>                     #:name '#$(format #f "emacs-~a" name)
>                     #:socket-directory-permissions #o700))
>              #:log-file (string-append
>                          (getenv "XDG_STATE_HOME") "/log"
>                          "/emacs-" #$(symbol->string name) ".log")))
>    (stop #~(make-systemd-destructor))))

Note that in this case the server is started lazily, the first time
someone connects to the socket.

You can pass #:lazy-start? #false if you instead want to start it
upfront.

> But I'm not sure it's better regarding user experience. On RDE we
> implemented a notifier that parses the result of
> herd eval root "(and=> (lookup-service 'emacs-server) service-status)"
> thus giving a nice "Emacs is currently starting" notification.

With socket activation, the service is immediately in “running” state:
starting it requires nothing more than accepting connections on the
socket.

(Maybe that’s one of the rare situations where sd-notify would help?)

> This evaluation doesn't seem to work using make-systemd-constructor,
> although it has its advantages (indeed launches a frame when available).
> It would be nice if service-status could "sync" with
> make-systemd-constructor in this case.
>
> I would be happy to send such a patch for Guix (is there already some
> patch series on which to graft this?), WDYT @Ludo?

Sure, feel free to send a patch for this; there are unfortunately
several attempts at an Emacs service in the issue tracker, but none of
them made it to the tree so far, in part because some were very
ambitious.

Thanks,
Ludo’.

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Rudolf Schlatte]

Ludovic Courtès <ludo@gnu.org> writes:

> Stefan Monnier <monnier@iro.umontreal.ca> skribis:
>
>>> Would that make sense on systems where systemd is used? If libsystem is
>>> already installed it would be more convenient for the user to use the
>>> already installed and very likely loaded libsystemd instead of
>>> reimplementing the feature.
>>
>> You might be right, but it's hard to say without seeing the replacement
>> code and the actual doc of the protocol.
>
> As an example, here’s C++ code that checks the LISTEN_* environment
> variables I mentioned earlier:
>
>   https://git.savannah.gnu.org/cgit/guix.git/tree/nix/nix-daemon/guix-daemon.cc#n437
>
> Ludo’.

The systemd documentation contains code to implement sd_notify without
using libsystemd at
https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes

Lennart Poettering said on Mastodon that the notify protocol is stable
and is independent of libsystemd.

https://mastodon.social/@pid_eins/112202687764571433
https://mastodon.social/@pid_eins/112202696589971319

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Stefan Monnier]

> The systemd documentation contains code to implement sd_notify without
> using libsystemd at
> https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
>
> Lennart Poettering said on Mastodon that the notify protocol is stable
> and is independent of libsystemd.
>
> https://mastodon.social/@pid_eins/112202687764571433
> https://mastodon.social/@pid_eins/112202696589971319

Looks quite promising.


        Stefan

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

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


On 2024-04-13 11:16, Stefan Monnier wrote:

>> Maybe some feedback on the Emacs side about this? There are indeed very
>> few places where systemd sd_* functions are called in emacs.c, should we
>> try and re-implement them instead of using the library as is? Would that
>> be a contribution Emacs devs would be interested in? That would
>> definitely be beneficial for Emacs on Guix as highlighted by Ludo'.
>
> It's hard to tell without seeing the actual patch.

Seems to work properly on my side with the following patch, on Guix with
the shepherd service I sent a few weeks ago. The patch is actually
pretty simple.

Some choices :

- I removed most of the code in former function sd_listen_fds' upstream
source code, because it didn't seem related to integration functionality
with emacs but rather to systemd inner workings. Some edge cases might
not be as well covered (strtol with defaults instead of systemd's
safe_atoi), but it works on my side. The functionality is directly
integrated in emacs.c rather than the function itself reproduced.

- I simplified the sd_is_socket function to instead use the upstream
untouched sd_is_socket_internal function. This is because the default
family argument was set in emacs.c.

- The sd_notify code is taken from
https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes

Disclaimer : I'm not a seasoned C nor emacs developer, some things are
probably odd, but I would need for feedback to make things more emacsy /
rigorous in C.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Implement-systemd-socket.patch --]
[-- Type: text/x-patch, Size: 19503 bytes --]

From c9f3af7dc4accc4da352ef8ffdb95478bcf2bd16 Mon Sep 17 00:00:00 2001
From: Nicolas Graves <ngraves@ngraves.fr>
Date: Sat, 13 Apr 2024 19:37:34 +0200
Subject: [PATCH] Implement systemd socket.

---
 configure.ac     |  19 +-----
 lib/Makefile.in  |   2 +-
 lib/gnulib.mk.in |   2 -
 lib/sd-socket.c  | 149 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/sd-socket.h  | 127 ++++++++++++++++++++++++++++++++++++++++
 msdos/sed1v2.inp |   3 -
 src/Makefile.in  |   9 +--
 src/deps.mk      |   2 +-
 src/emacs.c      |  50 ++++++++--------
 9 files changed, 308 insertions(+), 55 deletions(-)
 create mode 100644 lib/sd-socket.c
 create mode 100644 lib/sd-socket.h

diff --git a/configure.ac b/configure.ac
index 29b71ea2730..a245a7b5ccc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -457,7 +457,6 @@ AC_DEFUN
 OPTION_DEFAULT_ON([webp],[don't compile with WebP image support])
 OPTION_DEFAULT_ON([sqlite3],[don't compile with sqlite3 support])
 OPTION_DEFAULT_ON([lcms2],[don't compile with Little CMS support])
-OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support])
 OPTION_DEFAULT_ON([cairo],[don't compile with Cairo drawing])
 OPTION_DEFAULT_OFF([cairo-xcb], [use XCB surfaces for Cairo support])
 OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
@@ -3203,21 +3202,6 @@ AC_DEFUN
 AC_SUBST([LIBGNUTLS_LIBS])
 AC_SUBST([LIBGNUTLS_CFLAGS])
 
-HAVE_LIBSYSTEMD=no
-if test "${with_libsystemd}" = "yes" ; then
-  dnl This code has been tested with libsystemd 222 and later.
-  dnl FIXME: Find the earliest version number for which Emacs should work,
-  dnl and change '222' to that number.
-  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
-    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
-  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
-    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
-  fi
-fi
-
-AC_SUBST([LIBSYSTEMD_LIBS])
-AC_SUBST([LIBSYSTEMD_CFLAGS])
-
 HAVE_JSON=no
 JSON_OBJ=
 
@@ -6652,7 +6636,7 @@ AC_DEFUN
 optsep=
 emacs_config_features=
 for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
- HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
+ HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
  M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
  SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
  UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
@@ -6721,7 +6705,6 @@ AC_DEFUN
   Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
   Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
   Does Emacs use -lxft?                                   ${HAVE_XFT}
-  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
   Does Emacs use -ljansson?                               ${HAVE_JSON}
   Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
   Does Emacs use the GMP library?                         ${HAVE_GMP}
diff --git a/lib/Makefile.in b/lib/Makefile.in
index 71199c32277..d934a755e85 100644
--- a/lib/Makefile.in
+++ b/lib/Makefile.in
@@ -74,7 +74,7 @@ Makefile:
 not_emacs_OBJECTS = regex.o malloc/%.o free.o
 
 libgnu_a_OBJECTS = fingerprint.o $(gl_LIBOBJS) \
-  $(patsubst %.c,%.o,$(filter %.c,$(libgnu_a_SOURCES)))
+  $(patsubst %.c,%.o,$(filter %.c,$(libgnu_a_SOURCES))) sd-socket.o
 for_emacs_OBJECTS = $(filter-out $(not_emacs_OBJECTS),$(libgnu_a_OBJECTS))
 libegnu_a_OBJECTS = $(patsubst %.o,e-%.o,$(for_emacs_OBJECTS))
 
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index 9ab4b741595..f9ed4a4cb23 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -912,8 +912,6 @@ LIBSECCOMP_CFLAGS = @LIBSECCOMP_CFLAGS@
 LIBSECCOMP_LIBS = @LIBSECCOMP_LIBS@
 LIBSELINUX_LIBS = @LIBSELINUX_LIBS@
 LIBSOUND = @LIBSOUND@
-LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
-LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
 LIBS_ECLIENT = @LIBS_ECLIENT@
 LIBS_GNUSTEP = @LIBS_GNUSTEP@
 LIBS_MAIL = @LIBS_MAIL@
diff --git a/lib/sd-socket.c b/lib/sd-socket.c
new file mode 100644
index 00000000000..6350882006d
--- /dev/null
+++ b/lib/sd-socket.c
@@ -0,0 +1,149 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* Copied and adapted from systemd source code. */
+
+#define _GNU_SOURCE 1
+
+#include <assert.h>
+#include <config.h>
+#include <errno.h>
+#include <inttypes.h>
+#include <limits.h>
+#include <mqueue.h>
+#include <netinet/in.h>
+#include <poll.h>
+#include <signal.h>
+#include <stdarg.h>
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/un.h>
+#include <time.h>
+#include <unistd.h>
+
+#include "sd-socket.h"
+#define _cleanup_(f) __attribute__((cleanup(f)))
+
+int sd_is_socket(int fd, int type, int listening) {
+        struct stat st_fd;
+
+        assert(fd >= 0);
+        assert(type >= 0);
+
+        if (fstat(fd, &st_fd) < 0)
+                return -errno;
+
+        if (!S_ISSOCK(st_fd.st_mode))
+                return 0;
+
+        if (type != 0) {
+                int other_type = 0;
+                socklen_t l = sizeof(other_type);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &other_type, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(other_type))
+                        return -EINVAL;
+
+                if (other_type != type)
+                        return 0;
+        }
+
+        if (listening >= 0) {
+                int accepting = 0;
+                socklen_t l = sizeof(accepting);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &accepting, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(accepting))
+                        return -EINVAL;
+
+                if (!accepting != !listening)
+                        return 0;
+        }
+
+        return 1;
+}
+
+/* SPDX-License-Identifier: MIT-0 */
+
+/* Implement the systemd notify protocol without external dependencies.
+ * Supports both readiness notification on startup and on reloading,
+ * according to the protocol defined at:
+ * https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
+ * This protocol is guaranteed to be stable as per:
+ * https://systemd.io/PORTABILITY_AND_STABILITY/ */
+
+static void closep(int *fd) {
+  if (!fd || *fd < 0)
+    return;
+
+  close(*fd);
+  *fd = -1;
+}
+
+static int sd_notify(const char *message) {
+  union sockaddr_union {
+    struct sockaddr sa;
+    struct sockaddr_un sun;
+  } socket_addr = {
+    .sun.sun_family = AF_UNIX,
+  };
+  size_t path_length, message_length;
+  _cleanup_(closep) int fd = -1;
+  const char *socket_path;
+
+  /* Verify the argument first */
+  if (!message)
+    return -EINVAL;
+
+  message_length = strlen(message);
+  if (message_length == 0)
+    return -EINVAL;
+
+  /* If the variable is not set, the protocol is a noop */
+  socket_path = getenv("NOTIFY_SOCKET");
+  if (!socket_path)
+    return 0; /* Not set? Nothing to do */
+
+  /* Only AF_UNIX is supported, with path or abstract sockets */
+  if (socket_path[0] != '/' && socket_path[0] != '@')
+    return -EAFNOSUPPORT;
+
+  path_length = strlen(socket_path);
+  /* Ensure there is room for NUL byte */
+  if (path_length >= sizeof(socket_addr.sun.sun_path))
+    return -E2BIG;
+
+  memcpy(socket_addr.sun.sun_path, socket_path, path_length);
+
+  /* Support for abstract socket */
+  if (socket_addr.sun.sun_path[0] == '@')
+    socket_addr.sun.sun_path[0] = 0;
+
+  fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
+  if (fd < 0)
+    return -errno;
+
+  if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0)
+    return -errno;
+
+  ssize_t written = write(fd, message, message_length);
+  if (written != (ssize_t) message_length)
+    return written < 0 ? -errno : -EPROTO;
+
+  return 1; /* Notified! */
+}
+
+int sd_notify_ready(void) {
+  return sd_notify("READY=1");
+}
+
+int sd_notify_stopping(void) {
+  return sd_notify("STOPPING=1");
+}
diff --git a/lib/sd-socket.h b/lib/sd-socket.h
new file mode 100644
index 00000000000..31450c731e9
--- /dev/null
+++ b/lib/sd-socket.h
@@ -0,0 +1,127 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* Copied and adapted from systemd source code. */
+#ifndef _SD_SOCKET_H
+#define _SD_SOCKET_H 1
+
+#include <inttypes.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+/*
+  The following functionality is provided:
+
+  - Support for logging with log levels on stderr
+  - File descriptor passing for socket-based activation
+  - Daemon startup and status notification
+  - Detection of systemd boots
+
+  See sd-daemon(3) for more information.
+*/
+
+/* The first passed file descriptor is fd 3 */
+#define SD_LISTEN_FDS_START 3
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is a socket of type (SOCK_DGRAM, SOCK_STREAM,
+  ...), 0 otherwise. If type is 0 a socket type check will not be done
+  and the call only verifies if the file descriptor refers to a
+  socket. If listening is > 0 it is verified that the socket is in
+  listening mode. (i.e. listen() has been called) If listening is == 0
+  it is verified that the socket is not in listening mode. If
+  listening is < 0 no listening mode check is done. Returns a negative
+  errno style error code on failure.
+
+  See sd_is_socket(3) for more information.
+*/
+int sd_is_socket(int fd, int type, int listening);
+
+/*
+  Informs systemd about changed daemon state. This takes a number of
+  newline separated environment-style variable assignments in a
+  string. The following variables are known:
+
+     MAINPID=...  The main PID of a daemon, in case systemd did not
+                  fork off the process itself. Example: "MAINPID=4711"
+
+     READY=1      Tells systemd that daemon startup or daemon reload
+                  is finished (only relevant for services of Type=notify).
+                  The passed argument is a boolean "1" or "0". Since there
+                  is little value in signaling non-readiness the only
+                  value daemons should send is "READY=1".
+
+     RELOADING=1  Tell systemd that the daemon began reloading its
+                  configuration. When the configuration has been
+                  reloaded completely, READY=1 should be sent to inform
+                  systemd about this.
+
+     STOPPING=1   Tells systemd that the daemon is about to go down.
+
+     STATUS=...   Passes a single-line status string back to systemd
+                  that describes the daemon state. This is free-form
+                  and can be used for various purposes: general state
+                  feedback, fsck-like programs could pass completion
+                  percentages and failing programs could pass a human
+                  readable error message. Example: "STATUS=Completed
+                  66% of file system check..."
+
+     NOTIFYACCESS=...
+                  Reset the access to the service status notification socket.
+                  Example: "NOTIFYACCESS=main"
+
+     ERRNO=...    If a daemon fails, the errno-style error code,
+                  formatted as string. Example: "ERRNO=2" for ENOENT.
+
+     BUSERROR=... If a daemon fails, the D-Bus error-style error
+                  code. Example: "BUSERROR=org.freedesktop.DBus.Error.TimedOut"
+
+     WATCHDOG=1   Tells systemd to update the watchdog timestamp.
+                  Services using this feature should do this in
+                  regular intervals. A watchdog framework can use the
+                  timestamps to detect failed services. Also see
+                  sd_watchdog_enabled() below.
+
+     WATCHDOG_USEC=...
+                  Reset watchdog_usec value during runtime.
+                  To reset watchdog_usec value, start the service again.
+                  Example: "WATCHDOG_USEC=20000000"
+
+     FDSTORE=1    Store the file descriptors passed along with the
+                  message in the per-service file descriptor store,
+                  and pass them to the main process again on next
+                  invocation. This variable is only supported with
+                  sd_pid_notify_with_fds().
+
+     FDSTOREREMOVE=1
+                  Remove one or more file descriptors from the file
+                  descriptor store, identified by the name specified
+                  in FDNAME=, see below.
+
+     FDNAME=      A name to assign to new file descriptors stored in the
+                  file descriptor store, or the name of the file descriptors
+                  to remove in case of FDSTOREREMOVE=1.
+
+  Daemons can choose to send additional variables. However, it is
+  recommended to prefix variable names not listed above with X_.
+
+  Returns a negative errno-style error code on failure. Returns > 0
+  if systemd could be notified, 0 if it couldn't possibly because
+  systemd is not running.
+
+  Example: When a daemon finished starting up, it could issue this
+  call to notify systemd about it:
+
+     sd_notify(0, "READY=1");
+
+  See sd_notifyf() for more complete examples.
+
+  See sd_notify(3) for more information.
+*/
+
+/* This is actually a simplified version that you can find right there :
+   https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
+ */
+int sd_notify_ready(void);
+int sd_notify_stopping(void);
+
+#endif
diff --git a/msdos/sed1v2.inp b/msdos/sed1v2.inp
index 4d4c80a6b1a..d6d80ff809c 100644
--- a/msdos/sed1v2.inp
+++ b/msdos/sed1v2.inp
@@ -138,8 +138,6 @@ s/ *@WEBP_LIBS@//
 /^LIBMODULES *=/s/@LIBMODULES@//
 /^MODULES_OBJ *=/s/@MODULES_OBJ@//
 /^LIBSELINUX_LIBS *=/s/@LIBSELINUX_LIBS@//
-/^LIBSYSTEMD_LIBS *=/s/@LIBSYSTEMD_LIBS@//
-/^LIBSYSTEMD_CFLAGS *=/s/@LIBSYSTEMD_CFLAGS@//
 /^LIB_CLOCK_GETTIME *=/s/@[^@\n]*@//g
 /^LIB_TIMER_TIME *=/s/@[^@\n]*@//g
 /^LIB_EXECINFO *=/s/@[^@\n]*@//g
@@ -269,7 +267,6 @@ s/echo.*buildobj.lst/dj&/
 # Make the GCC command line fit one screen line
 /^[ 	][ 	]*\$(GNUSTEP_CFLAGS)/d
 /^[ 	][ 	]*\$(LIBGNUTLS_CFLAGS)/d
-/^[ 	][ 	]*\$(LIBSYSTEMD_CFLAGS)/d
 /^[ 	][ 	]*\$(XRANDR_CFLAGS)/d
 /^[ 	][ 	]*\$(WEBKIT_CFLAGS)/d
 /^[ 	][ 	]*\$(SETTINGS_CFLAGS)/d
diff --git a/src/Makefile.in b/src/Makefile.in
index 9bc53c072ea..d3a7f432ea8 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -336,9 +336,6 @@ LIBSELINUX_LIBS =
 LIBGNUTLS_LIBS = @LIBGNUTLS_LIBS@
 LIBGNUTLS_CFLAGS = @LIBGNUTLS_CFLAGS@
 
-LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
-LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
-
 JSON_LIBS = @JSON_LIBS@
 JSON_CFLAGS = @JSON_CFLAGS@
 JSON_OBJ = @JSON_OBJ@
@@ -405,11 +402,11 @@ EMACS_CFLAGS=
   $(C_SWITCH_MACHINE) $(C_SWITCH_SYSTEM) $(C_SWITCH_X_SITE) \
   $(GNUSTEP_CFLAGS) $(CFLAGS_SOUND) $(RSVG_CFLAGS) $(IMAGEMAGICK_CFLAGS) \
   $(PNG_CFLAGS) $(LIBXML2_CFLAGS) $(LIBGCCJIT_CFLAGS) $(DBUS_CFLAGS) \
-  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFLAGS) \
+  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFAGS) \
   $(XINPUT_CFLAGS) $(WEBP_CFLAGS) $(WEBKIT_CFLAGS) $(LCMS2_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(HARFBUZZ_CFLAGS) $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
-  $(LIBSYSTEMD_CFLAGS) $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
+  $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
   $(LIBGNUTLS_CFLAGS) $(NOTIFY_CFLAGS) $(CAIRO_CFLAGS) \
   $(WERROR_CFLAGS) $(HAIKU_CFLAGS) $(XCOMPOSITE_CFLAGS) $(XSHAPE_CFLAGS)
 ALL_CFLAGS = $(EMACS_CFLAGS) $(WARN_CFLAGS) $(CFLAGS)
@@ -567,7 +564,7 @@ LIBES =
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(HARFBUZZ_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
    $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LCMS2_LIBS) \
-   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS) \
+   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) \
    $(JSON_LIBS) $(LIBGMP) $(LIBGCCJIT_LIBS) $(XINPUT_LIBS) $(HAIKU_LIBS) \
    $(TREE_SITTER_LIBS) $(SQLITE3_LIBS) $(XCOMPOSITE_LIBS) $(XSHAPE_LIBS)
 
diff --git a/src/deps.mk b/src/deps.mk
index a7c8ae11f72..b932764f267 100644
--- a/src/deps.mk
+++ b/src/deps.mk
@@ -92,7 +92,7 @@ editfns.o:
 emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
    termhooks.h buffer.h atimer.h systime.h $(INTERVALS_H) lisp.h $(config_h) \
    globals.h ../lib/unistd.h window.h dispextern.h keyboard.h keymap.h \
-   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h
+   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h ../lib/sd-socket.h
 fileio.o: fileio.c window.h buffer.h systime.h $(INTERVALS_H) character.h \
    coding.h msdos.h blockinput.h atimer.h lisp.h $(config_h) frame.h \
    commands.h globals.h ../lib/unistd.h
diff --git a/src/emacs.c b/src/emacs.c
index dde305edbc2..c0381c23c37 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -57,10 +57,8 @@ #define MAIN_PROGRAM
 #include "dosfns.h"
 #endif
 
-#ifdef HAVE_LIBSYSTEMD
-# include <systemd/sd-daemon.h>
-# include <sys/socket.h>
-#endif
+#include "sd-socket.h"
+#include <sys/socket.h>
 
 #if defined HAVE_LINUX_SECCOMP_H && defined HAVE_LINUX_FILTER_H \
   && HAVE_DECL_SECCOMP_SET_MODE_FILTER                          \
@@ -1719,20 +1717,19 @@ main (int argc, char **argv)
             }
         } /* daemon_type == 2 */
 
-#ifdef HAVE_LIBSYSTEMD
       /* Read the number of sockets passed through by systemd.  */
-      int systemd_socket = sd_listen_fds (1);
-
-      if (systemd_socket > 1)
-        fputs (("\n"
-		"Warning: systemd passed more than one socket to Emacs.\n"
-		"Try 'Accept=false' in the Emacs socket unit file.\n"),
-	       stderr);
-      else if (systemd_socket == 1
-	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
-				     AF_UNSPEC, SOCK_STREAM, 1)))
-	sockfd = SD_LISTEN_FDS_START;
-#endif /* HAVE_LIBSYSTEMD */
+      const char *fds = getenv("LISTEN_FDS");
+      if (fds) {
+	int systemd_socket = strtol(fds, NULL, 0);
+	if (systemd_socket > 1)
+	  fputs (("\n"
+		  "Warning: systemd passed more than one socket to Emacs.\n"
+		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
+		 stderr);
+	else if (systemd_socket == 1
+		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
+	  sockfd = SD_LISTEN_FDS_START;
+      }
 
       /* On X, the bug happens because we call abort to avoid GLib
 	 crashes upon a longjmp in our X error handler.
@@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
     }
 #endif
 
-#ifdef HAVE_LIBSYSTEMD
   /* Notify systemd we are shutting down, but only if we have notified
      it about startup.  */
-  if (daemon_type == -1)
-    sd_notify(0, "STOPPING=1");
-#endif /* HAVE_LIBSYSTEMD */
+  if (daemon_type == -1){
+    int r = sd_notify_stopping();
+    if (r < 0) {
+      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
+      exit (EXIT_FAILURE);
+    }
+  }
 
   /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
      set.  */
@@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
 
   if (daemon_type == 1)
     {
-#ifdef HAVE_LIBSYSTEMD
-      sd_notify(0, "READY=1");
-#endif /* HAVE_LIBSYSTEMD */
+      int r = sd_notify_ready();
+      if (r < 0) {
+	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
+	exit (EXIT_FAILURE);
+      }
     }
 
   if (daemon_type == 2)
-- 
2.41.0


[-- Attachment #3: Type: text/plain, Size: 242 bytes --]


>
> But if the code is sufficiently simple, it implements a protocol that's
> well documented, and it allows us to eliminate the dependency on the
> systemd library, we might like it.
>
>
>         Stefan
>

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

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


A lightly cleaned-up version attached.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Implement-systemd-socket.patch --]
[-- Type: text/x-patch, Size: 15761 bytes --]

From 8ba972aec7026878bfbbe5cfb387c5c723672380 Mon Sep 17 00:00:00 2001
From: Nicolas Graves <ngraves@ngraves.fr>
Date: Sat, 13 Apr 2024 19:37:34 +0200
Subject: [PATCH] Implement systemd socket.

---
 configure.ac     |  19 +------
 lib/Makefile.in  |   2 +-
 lib/gnulib.mk.in |   2 -
 lib/sd-socket.c  | 137 +++++++++++++++++++++++++++++++++++++++++++++++
 lib/sd-socket.h  |  57 ++++++++++++++++++++
 msdos/sed1v2.inp |   3 --
 src/Makefile.in  |   9 ++--
 src/deps.mk      |   2 +-
 src/emacs.c      |  50 ++++++++---------
 9 files changed, 226 insertions(+), 55 deletions(-)
 create mode 100644 lib/sd-socket.c
 create mode 100644 lib/sd-socket.h

diff --git a/configure.ac b/configure.ac
index 29b71ea2730..a245a7b5ccc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -457,7 +457,6 @@ AC_DEFUN
 OPTION_DEFAULT_ON([webp],[don't compile with WebP image support])
 OPTION_DEFAULT_ON([sqlite3],[don't compile with sqlite3 support])
 OPTION_DEFAULT_ON([lcms2],[don't compile with Little CMS support])
-OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support])
 OPTION_DEFAULT_ON([cairo],[don't compile with Cairo drawing])
 OPTION_DEFAULT_OFF([cairo-xcb], [use XCB surfaces for Cairo support])
 OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
@@ -3203,21 +3202,6 @@ AC_DEFUN
 AC_SUBST([LIBGNUTLS_LIBS])
 AC_SUBST([LIBGNUTLS_CFLAGS])
 
-HAVE_LIBSYSTEMD=no
-if test "${with_libsystemd}" = "yes" ; then
-  dnl This code has been tested with libsystemd 222 and later.
-  dnl FIXME: Find the earliest version number for which Emacs should work,
-  dnl and change '222' to that number.
-  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
-    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
-  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
-    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
-  fi
-fi
-
-AC_SUBST([LIBSYSTEMD_LIBS])
-AC_SUBST([LIBSYSTEMD_CFLAGS])
-
 HAVE_JSON=no
 JSON_OBJ=
 
@@ -6652,7 +6636,7 @@ AC_DEFUN
 optsep=
 emacs_config_features=
 for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
- HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
+ HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
  M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
  SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
  UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
@@ -6721,7 +6705,6 @@ AC_DEFUN
   Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
   Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
   Does Emacs use -lxft?                                   ${HAVE_XFT}
-  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
   Does Emacs use -ljansson?                               ${HAVE_JSON}
   Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
   Does Emacs use the GMP library?                         ${HAVE_GMP}
diff --git a/lib/Makefile.in b/lib/Makefile.in
index 71199c32277..d934a755e85 100644
--- a/lib/Makefile.in
+++ b/lib/Makefile.in
@@ -74,7 +74,7 @@ Makefile:
 not_emacs_OBJECTS = regex.o malloc/%.o free.o
 
 libgnu_a_OBJECTS = fingerprint.o $(gl_LIBOBJS) \
-  $(patsubst %.c,%.o,$(filter %.c,$(libgnu_a_SOURCES)))
+  $(patsubst %.c,%.o,$(filter %.c,$(libgnu_a_SOURCES))) sd-socket.o
 for_emacs_OBJECTS = $(filter-out $(not_emacs_OBJECTS),$(libgnu_a_OBJECTS))
 libegnu_a_OBJECTS = $(patsubst %.o,e-%.o,$(for_emacs_OBJECTS))
 
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index 9ab4b741595..f9ed4a4cb23 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -912,8 +912,6 @@ LIBSECCOMP_CFLAGS = @LIBSECCOMP_CFLAGS@
 LIBSECCOMP_LIBS = @LIBSECCOMP_LIBS@
 LIBSELINUX_LIBS = @LIBSELINUX_LIBS@
 LIBSOUND = @LIBSOUND@
-LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
-LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
 LIBS_ECLIENT = @LIBS_ECLIENT@
 LIBS_GNUSTEP = @LIBS_GNUSTEP@
 LIBS_MAIL = @LIBS_MAIL@
diff --git a/lib/sd-socket.c b/lib/sd-socket.c
new file mode 100644
index 00000000000..205ddcf433d
--- /dev/null
+++ b/lib/sd-socket.c
@@ -0,0 +1,137 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* Copied and adapted from systemd source code. */
+
+#define _GNU_SOURCE 1
+
+#include <assert.h>
+#include <errno.h>
+#include <signal.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/un.h>
+
+#include "sd-socket.h"
+#define _cleanup_(f) __attribute__((cleanup(f)))
+
+int sd_is_socket(int fd, int type, int listening) {
+        struct stat st_fd;
+
+        assert(fd >= 0);
+        assert(type >= 0);
+
+        if (fstat(fd, &st_fd) < 0)
+                return -errno;
+
+        if (!S_ISSOCK(st_fd.st_mode))
+                return 0;
+
+        if (type != 0) {
+                int other_type = 0;
+                socklen_t l = sizeof(other_type);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &other_type, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(other_type))
+                        return -EINVAL;
+
+                if (other_type != type)
+                        return 0;
+        }
+
+        if (listening >= 0) {
+                int accepting = 0;
+                socklen_t l = sizeof(accepting);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &accepting, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(accepting))
+                        return -EINVAL;
+
+                if (!accepting != !listening)
+                        return 0;
+        }
+
+        return 1;
+}
+
+/* SPDX-License-Identifier: MIT-0 */
+
+/* Implement the systemd notify protocol without external dependencies.
+ * Supports both readiness notification on startup and on reloading,
+ * according to the protocol defined at:
+ * https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
+ * This protocol is guaranteed to be stable as per:
+ * https://systemd.io/PORTABILITY_AND_STABILITY/ */
+
+static void closep(int *fd) {
+  if (!fd || *fd < 0)
+    return;
+
+  close(*fd);
+  *fd = -1;
+}
+
+static int sd_notify(const char *message) {
+  union sockaddr_union {
+    struct sockaddr sa;
+    struct sockaddr_un sun;
+  } socket_addr = {
+    .sun.sun_family = AF_UNIX,
+  };
+  size_t path_length, message_length;
+  _cleanup_(closep) int fd = -1;
+  const char *socket_path;
+
+  /* Verify the argument first */
+  if (!message)
+    return -EINVAL;
+
+  message_length = strlen(message);
+  if (message_length == 0)
+    return -EINVAL;
+
+  /* If the variable is not set, the protocol is a noop */
+  socket_path = getenv("NOTIFY_SOCKET");
+  if (!socket_path)
+    return 0; /* Not set? Nothing to do */
+
+  /* Only AF_UNIX is supported, with path or abstract sockets */
+  if (socket_path[0] != '/' && socket_path[0] != '@')
+    return -EAFNOSUPPORT;
+
+  path_length = strlen(socket_path);
+  /* Ensure there is room for NUL byte */
+  if (path_length >= sizeof(socket_addr.sun.sun_path))
+    return -E2BIG;
+
+  memcpy(socket_addr.sun.sun_path, socket_path, path_length);
+
+  /* Support for abstract socket */
+  if (socket_addr.sun.sun_path[0] == '@')
+    socket_addr.sun.sun_path[0] = 0;
+
+  fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
+  if (fd < 0)
+    return -errno;
+
+  if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0)
+    return -errno;
+
+  ssize_t written = write(fd, message, message_length);
+  if (written != (ssize_t) message_length)
+    return written < 0 ? -errno : -EPROTO;
+
+  return 1; /* Notified! */
+}
+
+int sd_notify_ready(void) {
+  return sd_notify("READY=1");
+}
+
+int sd_notify_stopping(void) {
+  return sd_notify("STOPPING=1");
+}
diff --git a/lib/sd-socket.h b/lib/sd-socket.h
new file mode 100644
index 00000000000..217dbfce8a6
--- /dev/null
+++ b/lib/sd-socket.h
@@ -0,0 +1,57 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* Copied and adapted from systemd source code. */
+#ifndef _SD_SOCKET_H
+#define _SD_SOCKET_H 1
+
+#include <inttypes.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+/*
+  The following functionality is provided:
+
+  - Support for logging with log levels on stderr
+  - File descriptor passing for socket-based activation
+  - Daemon startup and status notification
+  - Detection of systemd boots
+
+  See sd-daemon(3) for more information.
+*/
+
+/* The first passed file descriptor is fd 3 */
+#define SD_LISTEN_FDS_START 3
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is a socket of type (SOCK_DGRAM, SOCK_STREAM,
+  ...), 0 otherwise. If type is 0 a socket type check will not be done
+  and the call only verifies if the file descriptor refers to a
+  socket. If listening is > 0 it is verified that the socket is in
+  listening mode. (i.e. listen() has been called) If listening is == 0
+  it is verified that the socket is not in listening mode. If
+  listening is < 0 no listening mode check is done. Returns a negative
+  errno style error code on failure.
+
+  See sd_is_socket(3) for more information.
+*/
+int sd_is_socket(int fd, int type, int listening);
+
+/*
+  Tells systemd that daemon startup or daemon reload is finished (only
+  relevant for services of Type=notify).
+
+  See https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
+  for more information.
+*/
+int sd_notify_ready(void);
+
+/*
+  Tells systemd that the daemon is about to go down.
+
+  See https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
+  for more information.
+
+ */
+int sd_notify_stopping(void);
+
+#endif
diff --git a/msdos/sed1v2.inp b/msdos/sed1v2.inp
index 4d4c80a6b1a..d6d80ff809c 100644
--- a/msdos/sed1v2.inp
+++ b/msdos/sed1v2.inp
@@ -138,8 +138,6 @@ s/ *@WEBP_LIBS@//
 /^LIBMODULES *=/s/@LIBMODULES@//
 /^MODULES_OBJ *=/s/@MODULES_OBJ@//
 /^LIBSELINUX_LIBS *=/s/@LIBSELINUX_LIBS@//
-/^LIBSYSTEMD_LIBS *=/s/@LIBSYSTEMD_LIBS@//
-/^LIBSYSTEMD_CFLAGS *=/s/@LIBSYSTEMD_CFLAGS@//
 /^LIB_CLOCK_GETTIME *=/s/@[^@\n]*@//g
 /^LIB_TIMER_TIME *=/s/@[^@\n]*@//g
 /^LIB_EXECINFO *=/s/@[^@\n]*@//g
@@ -269,7 +267,6 @@ s/echo.*buildobj.lst/dj&/
 # Make the GCC command line fit one screen line
 /^[ 	][ 	]*\$(GNUSTEP_CFLAGS)/d
 /^[ 	][ 	]*\$(LIBGNUTLS_CFLAGS)/d
-/^[ 	][ 	]*\$(LIBSYSTEMD_CFLAGS)/d
 /^[ 	][ 	]*\$(XRANDR_CFLAGS)/d
 /^[ 	][ 	]*\$(WEBKIT_CFLAGS)/d
 /^[ 	][ 	]*\$(SETTINGS_CFLAGS)/d
diff --git a/src/Makefile.in b/src/Makefile.in
index 9bc53c072ea..d3a7f432ea8 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -336,9 +336,6 @@ LIBSELINUX_LIBS =
 LIBGNUTLS_LIBS = @LIBGNUTLS_LIBS@
 LIBGNUTLS_CFLAGS = @LIBGNUTLS_CFLAGS@
 
-LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
-LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
-
 JSON_LIBS = @JSON_LIBS@
 JSON_CFLAGS = @JSON_CFLAGS@
 JSON_OBJ = @JSON_OBJ@
@@ -405,11 +402,11 @@ EMACS_CFLAGS=
   $(C_SWITCH_MACHINE) $(C_SWITCH_SYSTEM) $(C_SWITCH_X_SITE) \
   $(GNUSTEP_CFLAGS) $(CFLAGS_SOUND) $(RSVG_CFLAGS) $(IMAGEMAGICK_CFLAGS) \
   $(PNG_CFLAGS) $(LIBXML2_CFLAGS) $(LIBGCCJIT_CFLAGS) $(DBUS_CFLAGS) \
-  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFLAGS) \
+  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFAGS) \
   $(XINPUT_CFLAGS) $(WEBP_CFLAGS) $(WEBKIT_CFLAGS) $(LCMS2_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(HARFBUZZ_CFLAGS) $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
-  $(LIBSYSTEMD_CFLAGS) $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
+  $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
   $(LIBGNUTLS_CFLAGS) $(NOTIFY_CFLAGS) $(CAIRO_CFLAGS) \
   $(WERROR_CFLAGS) $(HAIKU_CFLAGS) $(XCOMPOSITE_CFLAGS) $(XSHAPE_CFLAGS)
 ALL_CFLAGS = $(EMACS_CFLAGS) $(WARN_CFLAGS) $(CFLAGS)
@@ -567,7 +564,7 @@ LIBES =
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(HARFBUZZ_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
    $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LCMS2_LIBS) \
-   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS) \
+   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) \
    $(JSON_LIBS) $(LIBGMP) $(LIBGCCJIT_LIBS) $(XINPUT_LIBS) $(HAIKU_LIBS) \
    $(TREE_SITTER_LIBS) $(SQLITE3_LIBS) $(XCOMPOSITE_LIBS) $(XSHAPE_LIBS)
 
diff --git a/src/deps.mk b/src/deps.mk
index a7c8ae11f72..b932764f267 100644
--- a/src/deps.mk
+++ b/src/deps.mk
@@ -92,7 +92,7 @@ editfns.o:
 emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
    termhooks.h buffer.h atimer.h systime.h $(INTERVALS_H) lisp.h $(config_h) \
    globals.h ../lib/unistd.h window.h dispextern.h keyboard.h keymap.h \
-   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h
+   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h ../lib/sd-socket.h
 fileio.o: fileio.c window.h buffer.h systime.h $(INTERVALS_H) character.h \
    coding.h msdos.h blockinput.h atimer.h lisp.h $(config_h) frame.h \
    commands.h globals.h ../lib/unistd.h
diff --git a/src/emacs.c b/src/emacs.c
index dde305edbc2..c0381c23c37 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -57,10 +57,8 @@ #define MAIN_PROGRAM
 #include "dosfns.h"
 #endif
 
-#ifdef HAVE_LIBSYSTEMD
-# include <systemd/sd-daemon.h>
-# include <sys/socket.h>
-#endif
+#include "sd-socket.h"
+#include <sys/socket.h>
 
 #if defined HAVE_LINUX_SECCOMP_H && defined HAVE_LINUX_FILTER_H \
   && HAVE_DECL_SECCOMP_SET_MODE_FILTER                          \
@@ -1719,20 +1717,19 @@ main (int argc, char **argv)
             }
         } /* daemon_type == 2 */
 
-#ifdef HAVE_LIBSYSTEMD
       /* Read the number of sockets passed through by systemd.  */
-      int systemd_socket = sd_listen_fds (1);
-
-      if (systemd_socket > 1)
-        fputs (("\n"
-		"Warning: systemd passed more than one socket to Emacs.\n"
-		"Try 'Accept=false' in the Emacs socket unit file.\n"),
-	       stderr);
-      else if (systemd_socket == 1
-	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
-				     AF_UNSPEC, SOCK_STREAM, 1)))
-	sockfd = SD_LISTEN_FDS_START;
-#endif /* HAVE_LIBSYSTEMD */
+      const char *fds = getenv("LISTEN_FDS");
+      if (fds) {
+	int systemd_socket = strtol(fds, NULL, 0);
+	if (systemd_socket > 1)
+	  fputs (("\n"
+		  "Warning: systemd passed more than one socket to Emacs.\n"
+		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
+		 stderr);
+	else if (systemd_socket == 1
+		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
+	  sockfd = SD_LISTEN_FDS_START;
+      }
 
       /* On X, the bug happens because we call abort to avoid GLib
 	 crashes upon a longjmp in our X error handler.
@@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
     }
 #endif
 
-#ifdef HAVE_LIBSYSTEMD
   /* Notify systemd we are shutting down, but only if we have notified
      it about startup.  */
-  if (daemon_type == -1)
-    sd_notify(0, "STOPPING=1");
-#endif /* HAVE_LIBSYSTEMD */
+  if (daemon_type == -1){
+    int r = sd_notify_stopping();
+    if (r < 0) {
+      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
+      exit (EXIT_FAILURE);
+    }
+  }
 
   /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
      set.  */
@@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
 
   if (daemon_type == 1)
     {
-#ifdef HAVE_LIBSYSTEMD
-      sd_notify(0, "READY=1");
-#endif /* HAVE_LIBSYSTEMD */
+      int r = sd_notify_ready();
+      if (r < 0) {
+	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
+	exit (EXIT_FAILURE);
+      }
     }
 
   if (daemon_type == 2)
-- 
2.41.0


[-- Attachment #3: Type: text/plain, Size: 22436 bytes --]


On 2024-05-11 22:15, Nicolas Graves wrote:

> On 2024-04-13 11:16, Stefan Monnier wrote:
>
>>> Maybe some feedback on the Emacs side about this? There are indeed very
>>> few places where systemd sd_* functions are called in emacs.c, should we
>>> try and re-implement them instead of using the library as is? Would that
>>> be a contribution Emacs devs would be interested in? That would
>>> definitely be beneficial for Emacs on Guix as highlighted by Ludo'.
>>
>> It's hard to tell without seeing the actual patch.
>
> Seems to work properly on my side with the following patch, on Guix with
> the shepherd service I sent a few weeks ago. The patch is actually
> pretty simple.
>
> Some choices :
>
> - I removed most of the code in former function sd_listen_fds' upstream
> source code, because it didn't seem related to integration functionality
> with emacs but rather to systemd inner workings. Some edge cases might
> not be as well covered (strtol with defaults instead of systemd's
> safe_atoi), but it works on my side. The functionality is directly
> integrated in emacs.c rather than the function itself reproduced.
>
> - I simplified the sd_is_socket function to instead use the upstream
> untouched sd_is_socket_internal function. This is because the default
> family argument was set in emacs.c.
>
> - The sd_notify code is taken from
> https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
>
> Disclaimer : I'm not a seasoned C nor emacs developer, some things are
> probably odd, but I would need for feedback to make things more emacsy /
> rigorous in C.
>
> From c9f3af7dc4accc4da352ef8ffdb95478bcf2bd16 Mon Sep 17 00:00:00 2001
> From: Nicolas Graves <ngraves@ngraves.fr>
> Date: Sat, 13 Apr 2024 19:37:34 +0200
> Subject: [PATCH] Implement systemd socket.
>
> ---
>  configure.ac     |  19 +-----
>  lib/Makefile.in  |   2 +-
>  lib/gnulib.mk.in |   2 -
>  lib/sd-socket.c  | 149 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/sd-socket.h  | 127 ++++++++++++++++++++++++++++++++++++++++
>  msdos/sed1v2.inp |   3 -
>  src/Makefile.in  |   9 +--
>  src/deps.mk      |   2 +-
>  src/emacs.c      |  50 ++++++++--------
>  9 files changed, 308 insertions(+), 55 deletions(-)
>  create mode 100644 lib/sd-socket.c
>  create mode 100644 lib/sd-socket.h
>
> diff --git a/configure.ac b/configure.ac
> index 29b71ea2730..a245a7b5ccc 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -457,7 +457,6 @@ AC_DEFUN
>  OPTION_DEFAULT_ON([webp],[don't compile with WebP image support])
>  OPTION_DEFAULT_ON([sqlite3],[don't compile with sqlite3 support])
>  OPTION_DEFAULT_ON([lcms2],[don't compile with Little CMS support])
> -OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support])
>  OPTION_DEFAULT_ON([cairo],[don't compile with Cairo drawing])
>  OPTION_DEFAULT_OFF([cairo-xcb], [use XCB surfaces for Cairo support])
>  OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
> @@ -3203,21 +3202,6 @@ AC_DEFUN
>  AC_SUBST([LIBGNUTLS_LIBS])
>  AC_SUBST([LIBGNUTLS_CFLAGS])
>  
> -HAVE_LIBSYSTEMD=no
> -if test "${with_libsystemd}" = "yes" ; then
> -  dnl This code has been tested with libsystemd 222 and later.
> -  dnl FIXME: Find the earliest version number for which Emacs should work,
> -  dnl and change '222' to that number.
> -  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
> -    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
> -  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
> -    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
> -  fi
> -fi
> -
> -AC_SUBST([LIBSYSTEMD_LIBS])
> -AC_SUBST([LIBSYSTEMD_CFLAGS])
> -
>  HAVE_JSON=no
>  JSON_OBJ=
>  
> @@ -6652,7 +6636,7 @@ AC_DEFUN
>  optsep=
>  emacs_config_features=
>  for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
> - HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
> + HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
>   M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
>   SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
>   UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
> @@ -6721,7 +6705,6 @@ AC_DEFUN
>    Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
>    Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
>    Does Emacs use -lxft?                                   ${HAVE_XFT}
> -  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
>    Does Emacs use -ljansson?                               ${HAVE_JSON}
>    Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
>    Does Emacs use the GMP library?                         ${HAVE_GMP}
> diff --git a/lib/Makefile.in b/lib/Makefile.in
> index 71199c32277..d934a755e85 100644
> --- a/lib/Makefile.in
> +++ b/lib/Makefile.in
> @@ -74,7 +74,7 @@ Makefile:
>  not_emacs_OBJECTS = regex.o malloc/%.o free.o
>  
>  libgnu_a_OBJECTS = fingerprint.o $(gl_LIBOBJS) \
> -  $(patsubst %.c,%.o,$(filter %.c,$(libgnu_a_SOURCES)))
> +  $(patsubst %.c,%.o,$(filter %.c,$(libgnu_a_SOURCES))) sd-socket.o
>  for_emacs_OBJECTS = $(filter-out $(not_emacs_OBJECTS),$(libgnu_a_OBJECTS))
>  libegnu_a_OBJECTS = $(patsubst %.o,e-%.o,$(for_emacs_OBJECTS))
>  
> diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
> index 9ab4b741595..f9ed4a4cb23 100644
> --- a/lib/gnulib.mk.in
> +++ b/lib/gnulib.mk.in
> @@ -912,8 +912,6 @@ LIBSECCOMP_CFLAGS = @LIBSECCOMP_CFLAGS@
>  LIBSECCOMP_LIBS = @LIBSECCOMP_LIBS@
>  LIBSELINUX_LIBS = @LIBSELINUX_LIBS@
>  LIBSOUND = @LIBSOUND@
> -LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
> -LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
>  LIBS_ECLIENT = @LIBS_ECLIENT@
>  LIBS_GNUSTEP = @LIBS_GNUSTEP@
>  LIBS_MAIL = @LIBS_MAIL@
> diff --git a/lib/sd-socket.c b/lib/sd-socket.c
> new file mode 100644
> index 00000000000..6350882006d
> --- /dev/null
> +++ b/lib/sd-socket.c
> @@ -0,0 +1,149 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/* Copied and adapted from systemd source code. */
> +
> +#define _GNU_SOURCE 1
> +
> +#include <assert.h>
> +#include <config.h>
> +#include <errno.h>
> +#include <inttypes.h>
> +#include <limits.h>
> +#include <mqueue.h>
> +#include <netinet/in.h>
> +#include <poll.h>
> +#include <signal.h>
> +#include <stdarg.h>
> +#include <stdbool.h>
> +#include <stddef.h>
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <sys/socket.h>
> +#include <sys/stat.h>
> +#include <sys/un.h>
> +#include <time.h>
> +#include <unistd.h>
> +
> +#include "sd-socket.h"
> +#define _cleanup_(f) __attribute__((cleanup(f)))
> +
> +int sd_is_socket(int fd, int type, int listening) {
> +        struct stat st_fd;
> +
> +        assert(fd >= 0);
> +        assert(type >= 0);
> +
> +        if (fstat(fd, &st_fd) < 0)
> +                return -errno;
> +
> +        if (!S_ISSOCK(st_fd.st_mode))
> +                return 0;
> +
> +        if (type != 0) {
> +                int other_type = 0;
> +                socklen_t l = sizeof(other_type);
> +
> +                if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &other_type, &l) < 0)
> +                        return -errno;
> +
> +                if (l != sizeof(other_type))
> +                        return -EINVAL;
> +
> +                if (other_type != type)
> +                        return 0;
> +        }
> +
> +        if (listening >= 0) {
> +                int accepting = 0;
> +                socklen_t l = sizeof(accepting);
> +
> +                if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &accepting, &l) < 0)
> +                        return -errno;
> +
> +                if (l != sizeof(accepting))
> +                        return -EINVAL;
> +
> +                if (!accepting != !listening)
> +                        return 0;
> +        }
> +
> +        return 1;
> +}
> +
> +/* SPDX-License-Identifier: MIT-0 */
> +
> +/* Implement the systemd notify protocol without external dependencies.
> + * Supports both readiness notification on startup and on reloading,
> + * according to the protocol defined at:
> + * https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
> + * This protocol is guaranteed to be stable as per:
> + * https://systemd.io/PORTABILITY_AND_STABILITY/ */
> +
> +static void closep(int *fd) {
> +  if (!fd || *fd < 0)
> +    return;
> +
> +  close(*fd);
> +  *fd = -1;
> +}
> +
> +static int sd_notify(const char *message) {
> +  union sockaddr_union {
> +    struct sockaddr sa;
> +    struct sockaddr_un sun;
> +  } socket_addr = {
> +    .sun.sun_family = AF_UNIX,
> +  };
> +  size_t path_length, message_length;
> +  _cleanup_(closep) int fd = -1;
> +  const char *socket_path;
> +
> +  /* Verify the argument first */
> +  if (!message)
> +    return -EINVAL;
> +
> +  message_length = strlen(message);
> +  if (message_length == 0)
> +    return -EINVAL;
> +
> +  /* If the variable is not set, the protocol is a noop */
> +  socket_path = getenv("NOTIFY_SOCKET");
> +  if (!socket_path)
> +    return 0; /* Not set? Nothing to do */
> +
> +  /* Only AF_UNIX is supported, with path or abstract sockets */
> +  if (socket_path[0] != '/' && socket_path[0] != '@')
> +    return -EAFNOSUPPORT;
> +
> +  path_length = strlen(socket_path);
> +  /* Ensure there is room for NUL byte */
> +  if (path_length >= sizeof(socket_addr.sun.sun_path))
> +    return -E2BIG;
> +
> +  memcpy(socket_addr.sun.sun_path, socket_path, path_length);
> +
> +  /* Support for abstract socket */
> +  if (socket_addr.sun.sun_path[0] == '@')
> +    socket_addr.sun.sun_path[0] = 0;
> +
> +  fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
> +  if (fd < 0)
> +    return -errno;
> +
> +  if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0)
> +    return -errno;
> +
> +  ssize_t written = write(fd, message, message_length);
> +  if (written != (ssize_t) message_length)
> +    return written < 0 ? -errno : -EPROTO;
> +
> +  return 1; /* Notified! */
> +}
> +
> +int sd_notify_ready(void) {
> +  return sd_notify("READY=1");
> +}
> +
> +int sd_notify_stopping(void) {
> +  return sd_notify("STOPPING=1");
> +}
> diff --git a/lib/sd-socket.h b/lib/sd-socket.h
> new file mode 100644
> index 00000000000..31450c731e9
> --- /dev/null
> +++ b/lib/sd-socket.h
> @@ -0,0 +1,127 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/* Copied and adapted from systemd source code. */
> +#ifndef _SD_SOCKET_H
> +#define _SD_SOCKET_H 1
> +
> +#include <inttypes.h>
> +#include <sys/types.h>
> +#include <sys/socket.h>
> +
> +/*
> +  The following functionality is provided:
> +
> +  - Support for logging with log levels on stderr
> +  - File descriptor passing for socket-based activation
> +  - Daemon startup and status notification
> +  - Detection of systemd boots
> +
> +  See sd-daemon(3) for more information.
> +*/
> +
> +/* The first passed file descriptor is fd 3 */
> +#define SD_LISTEN_FDS_START 3
> +
> +/*
> +  Helper call for identifying a passed file descriptor. Returns 1 if
> +  the file descriptor is a socket of type (SOCK_DGRAM, SOCK_STREAM,
> +  ...), 0 otherwise. If type is 0 a socket type check will not be done
> +  and the call only verifies if the file descriptor refers to a
> +  socket. If listening is > 0 it is verified that the socket is in
> +  listening mode. (i.e. listen() has been called) If listening is == 0
> +  it is verified that the socket is not in listening mode. If
> +  listening is < 0 no listening mode check is done. Returns a negative
> +  errno style error code on failure.
> +
> +  See sd_is_socket(3) for more information.
> +*/
> +int sd_is_socket(int fd, int type, int listening);
> +
> +/*
> +  Informs systemd about changed daemon state. This takes a number of
> +  newline separated environment-style variable assignments in a
> +  string. The following variables are known:
> +
> +     MAINPID=...  The main PID of a daemon, in case systemd did not
> +                  fork off the process itself. Example: "MAINPID=4711"
> +
> +     READY=1      Tells systemd that daemon startup or daemon reload
> +                  is finished (only relevant for services of Type=notify).
> +                  The passed argument is a boolean "1" or "0". Since there
> +                  is little value in signaling non-readiness the only
> +                  value daemons should send is "READY=1".
> +
> +     RELOADING=1  Tell systemd that the daemon began reloading its
> +                  configuration. When the configuration has been
> +                  reloaded completely, READY=1 should be sent to inform
> +                  systemd about this.
> +
> +     STOPPING=1   Tells systemd that the daemon is about to go down.
> +
> +     STATUS=...   Passes a single-line status string back to systemd
> +                  that describes the daemon state. This is free-form
> +                  and can be used for various purposes: general state
> +                  feedback, fsck-like programs could pass completion
> +                  percentages and failing programs could pass a human
> +                  readable error message. Example: "STATUS=Completed
> +                  66% of file system check..."
> +
> +     NOTIFYACCESS=...
> +                  Reset the access to the service status notification socket.
> +                  Example: "NOTIFYACCESS=main"
> +
> +     ERRNO=...    If a daemon fails, the errno-style error code,
> +                  formatted as string. Example: "ERRNO=2" for ENOENT.
> +
> +     BUSERROR=... If a daemon fails, the D-Bus error-style error
> +                  code. Example: "BUSERROR=org.freedesktop.DBus.Error.TimedOut"
> +
> +     WATCHDOG=1   Tells systemd to update the watchdog timestamp.
> +                  Services using this feature should do this in
> +                  regular intervals. A watchdog framework can use the
> +                  timestamps to detect failed services. Also see
> +                  sd_watchdog_enabled() below.
> +
> +     WATCHDOG_USEC=...
> +                  Reset watchdog_usec value during runtime.
> +                  To reset watchdog_usec value, start the service again.
> +                  Example: "WATCHDOG_USEC=20000000"
> +
> +     FDSTORE=1    Store the file descriptors passed along with the
> +                  message in the per-service file descriptor store,
> +                  and pass them to the main process again on next
> +                  invocation. This variable is only supported with
> +                  sd_pid_notify_with_fds().
> +
> +     FDSTOREREMOVE=1
> +                  Remove one or more file descriptors from the file
> +                  descriptor store, identified by the name specified
> +                  in FDNAME=, see below.
> +
> +     FDNAME=      A name to assign to new file descriptors stored in the
> +                  file descriptor store, or the name of the file descriptors
> +                  to remove in case of FDSTOREREMOVE=1.
> +
> +  Daemons can choose to send additional variables. However, it is
> +  recommended to prefix variable names not listed above with X_.
> +
> +  Returns a negative errno-style error code on failure. Returns > 0
> +  if systemd could be notified, 0 if it couldn't possibly because
> +  systemd is not running.
> +
> +  Example: When a daemon finished starting up, it could issue this
> +  call to notify systemd about it:
> +
> +     sd_notify(0, "READY=1");
> +
> +  See sd_notifyf() for more complete examples.
> +
> +  See sd_notify(3) for more information.
> +*/
> +
> +/* This is actually a simplified version that you can find right there :
> +   https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
> + */
> +int sd_notify_ready(void);
> +int sd_notify_stopping(void);
> +
> +#endif
> diff --git a/msdos/sed1v2.inp b/msdos/sed1v2.inp
> index 4d4c80a6b1a..d6d80ff809c 100644
> --- a/msdos/sed1v2.inp
> +++ b/msdos/sed1v2.inp
> @@ -138,8 +138,6 @@ s/ *@WEBP_LIBS@//
>  /^LIBMODULES *=/s/@LIBMODULES@//
>  /^MODULES_OBJ *=/s/@MODULES_OBJ@//
>  /^LIBSELINUX_LIBS *=/s/@LIBSELINUX_LIBS@//
> -/^LIBSYSTEMD_LIBS *=/s/@LIBSYSTEMD_LIBS@//
> -/^LIBSYSTEMD_CFLAGS *=/s/@LIBSYSTEMD_CFLAGS@//
>  /^LIB_CLOCK_GETTIME *=/s/@[^@\n]*@//g
>  /^LIB_TIMER_TIME *=/s/@[^@\n]*@//g
>  /^LIB_EXECINFO *=/s/@[^@\n]*@//g
> @@ -269,7 +267,6 @@ s/echo.*buildobj.lst/dj&/
>  # Make the GCC command line fit one screen line
>  /^[ 	][ 	]*\$(GNUSTEP_CFLAGS)/d
>  /^[ 	][ 	]*\$(LIBGNUTLS_CFLAGS)/d
> -/^[ 	][ 	]*\$(LIBSYSTEMD_CFLAGS)/d
>  /^[ 	][ 	]*\$(XRANDR_CFLAGS)/d
>  /^[ 	][ 	]*\$(WEBKIT_CFLAGS)/d
>  /^[ 	][ 	]*\$(SETTINGS_CFLAGS)/d
> diff --git a/src/Makefile.in b/src/Makefile.in
> index 9bc53c072ea..d3a7f432ea8 100644
> --- a/src/Makefile.in
> +++ b/src/Makefile.in
> @@ -336,9 +336,6 @@ LIBSELINUX_LIBS =
>  LIBGNUTLS_LIBS = @LIBGNUTLS_LIBS@
>  LIBGNUTLS_CFLAGS = @LIBGNUTLS_CFLAGS@
>  
> -LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
> -LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
> -
>  JSON_LIBS = @JSON_LIBS@
>  JSON_CFLAGS = @JSON_CFLAGS@
>  JSON_OBJ = @JSON_OBJ@
> @@ -405,11 +402,11 @@ EMACS_CFLAGS=
>    $(C_SWITCH_MACHINE) $(C_SWITCH_SYSTEM) $(C_SWITCH_X_SITE) \
>    $(GNUSTEP_CFLAGS) $(CFLAGS_SOUND) $(RSVG_CFLAGS) $(IMAGEMAGICK_CFLAGS) \
>    $(PNG_CFLAGS) $(LIBXML2_CFLAGS) $(LIBGCCJIT_CFLAGS) $(DBUS_CFLAGS) \
> -  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFLAGS) \
> +  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFAGS) \
>    $(XINPUT_CFLAGS) $(WEBP_CFLAGS) $(WEBKIT_CFLAGS) $(LCMS2_CFLAGS) \
>    $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
>    $(HARFBUZZ_CFLAGS) $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
> -  $(LIBSYSTEMD_CFLAGS) $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
> +  $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
>    $(LIBGNUTLS_CFLAGS) $(NOTIFY_CFLAGS) $(CAIRO_CFLAGS) \
>    $(WERROR_CFLAGS) $(HAIKU_CFLAGS) $(XCOMPOSITE_CFLAGS) $(XSHAPE_CFLAGS)
>  ALL_CFLAGS = $(EMACS_CFLAGS) $(WARN_CFLAGS) $(CFLAGS)
> @@ -567,7 +564,7 @@ LIBES =
>     $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
>     $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(HARFBUZZ_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
>     $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LCMS2_LIBS) \
> -   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS) \
> +   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) \
>     $(JSON_LIBS) $(LIBGMP) $(LIBGCCJIT_LIBS) $(XINPUT_LIBS) $(HAIKU_LIBS) \
>     $(TREE_SITTER_LIBS) $(SQLITE3_LIBS) $(XCOMPOSITE_LIBS) $(XSHAPE_LIBS)
>  
> diff --git a/src/deps.mk b/src/deps.mk
> index a7c8ae11f72..b932764f267 100644
> --- a/src/deps.mk
> +++ b/src/deps.mk
> @@ -92,7 +92,7 @@ editfns.o:
>  emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
>     termhooks.h buffer.h atimer.h systime.h $(INTERVALS_H) lisp.h $(config_h) \
>     globals.h ../lib/unistd.h window.h dispextern.h keyboard.h keymap.h \
> -   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h
> +   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h ../lib/sd-socket.h
>  fileio.o: fileio.c window.h buffer.h systime.h $(INTERVALS_H) character.h \
>     coding.h msdos.h blockinput.h atimer.h lisp.h $(config_h) frame.h \
>     commands.h globals.h ../lib/unistd.h
> diff --git a/src/emacs.c b/src/emacs.c
> index dde305edbc2..c0381c23c37 100644
> --- a/src/emacs.c
> +++ b/src/emacs.c
> @@ -57,10 +57,8 @@ #define MAIN_PROGRAM
>  #include "dosfns.h"
>  #endif
>  
> -#ifdef HAVE_LIBSYSTEMD
> -# include <systemd/sd-daemon.h>
> -# include <sys/socket.h>
> -#endif
> +#include "sd-socket.h"
> +#include <sys/socket.h>
>  
>  #if defined HAVE_LINUX_SECCOMP_H && defined HAVE_LINUX_FILTER_H \
>    && HAVE_DECL_SECCOMP_SET_MODE_FILTER                          \
> @@ -1719,20 +1717,19 @@ main (int argc, char **argv)
>              }
>          } /* daemon_type == 2 */
>  
> -#ifdef HAVE_LIBSYSTEMD
>        /* Read the number of sockets passed through by systemd.  */
> -      int systemd_socket = sd_listen_fds (1);
> -
> -      if (systemd_socket > 1)
> -        fputs (("\n"
> -		"Warning: systemd passed more than one socket to Emacs.\n"
> -		"Try 'Accept=false' in the Emacs socket unit file.\n"),
> -	       stderr);
> -      else if (systemd_socket == 1
> -	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
> -				     AF_UNSPEC, SOCK_STREAM, 1)))
> -	sockfd = SD_LISTEN_FDS_START;
> -#endif /* HAVE_LIBSYSTEMD */
> +      const char *fds = getenv("LISTEN_FDS");
> +      if (fds) {
> +	int systemd_socket = strtol(fds, NULL, 0);
> +	if (systemd_socket > 1)
> +	  fputs (("\n"
> +		  "Warning: systemd passed more than one socket to Emacs.\n"
> +		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
> +		 stderr);
> +	else if (systemd_socket == 1
> +		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
> +	  sockfd = SD_LISTEN_FDS_START;
> +      }
>  
>        /* On X, the bug happens because we call abort to avoid GLib
>  	 crashes upon a longjmp in our X error handler.
> @@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
>      }
>  #endif
>  
> -#ifdef HAVE_LIBSYSTEMD
>    /* Notify systemd we are shutting down, but only if we have notified
>       it about startup.  */
> -  if (daemon_type == -1)
> -    sd_notify(0, "STOPPING=1");
> -#endif /* HAVE_LIBSYSTEMD */
> +  if (daemon_type == -1){
> +    int r = sd_notify_stopping();
> +    if (r < 0) {
> +      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
> +      exit (EXIT_FAILURE);
> +    }
> +  }
>  
>    /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
>       set.  */
> @@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
>  
>    if (daemon_type == 1)
>      {
> -#ifdef HAVE_LIBSYSTEMD
> -      sd_notify(0, "READY=1");
> -#endif /* HAVE_LIBSYSTEMD */
> +      int r = sd_notify_ready();
> +      if (r < 0) {
> +	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
> +	exit (EXIT_FAILURE);
> +      }
>      }
>  
>    if (daemon_type == 2)
> -- 
> 2.41.0
>
>
>>
>> But if the code is sufficiently simple, it implements a protocol that's
>> well documented, and it allows us to eliminate the dependency on the
>> systemd library, we might like it.
>>
>>
>>         Stefan
>>

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Eli Zaretskii]

> Cc: Ludovic Courtès <ludo@gnu.org>, emacs-devel@gnu.org,
>  Andrew Tropin <andrew@trop.in>, guix-devel@gnu.org, bjorn.bidar@thaodan.de,
>  rudi@constantly.at, felix.lechner@lease-up.com
> Date: Sun, 12 May 2024 01:07:51 +0200
> From:  Nicolas Graves via "Emacs development discussions." <emacs-devel@gnu.org>
> 
> A lightly cleaned-up version attached.

Thanks.  What was the motivation for this, again?

>  configure.ac     |  19 +------
>  lib/Makefile.in  |   2 +-
>  lib/gnulib.mk.in |   2 -
>  lib/sd-socket.c  | 137 +++++++++++++++++++++++++++++++++++++++++++++++
>  lib/sd-socket.h  |  57 ++++++++++++++++++++
>  msdos/sed1v2.inp |   3 --
>  src/Makefile.in  |   9 ++--
>  src/deps.mk      |   2 +-
>  src/emacs.c      |  50 ++++++++---------
>  9 files changed, 226 insertions(+), 55 deletions(-)
>  create mode 100644 lib/sd-socket.c
>  create mode 100644 lib/sd-socket.h

Your code is not from Gnulib, so it is wrong to place it in lib/.  It
should be in src/, and probably just an addition to sysdep.c.  Then
many of the changes to the build infrastructure will not be needed.

> -HAVE_LIBSYSTEMD=no
> -if test "${with_libsystemd}" = "yes" ; then
> -  dnl This code has been tested with libsystemd 222 and later.
> -  dnl FIXME: Find the earliest version number for which Emacs should work,
> -  dnl and change '222' to that number.
> -  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
> -    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
> -  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
> -    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
> -  fi
> -fi
> -
> -AC_SUBST([LIBSYSTEMD_LIBS])
> -AC_SUBST([LIBSYSTEMD_CFLAGS])

This test should be replaced by a test to determine whether the
systemd interface should be compiled into Emacs.  Not all of the
supported platform can use it, so we need to determine that at
configure time.

> -
>  HAVE_JSON=no
>  JSON_OBJ=
>  
> @@ -6652,7 +6636,7 @@ AC_DEFUN
>  optsep=
>  emacs_config_features=
>  for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
> - HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
> + HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
>   M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
>   SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
>   UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
> @@ -6721,7 +6705,6 @@ AC_DEFUN
>    Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
>    Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
>    Does Emacs use -lxft?                                   ${HAVE_XFT}
> -  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
>    Does Emacs use -ljansson?                               ${HAVE_JSON}
>    Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
>    Does Emacs use the GMP library?                         ${HAVE_GMP}

The above summary of the configuration should still call out the
result of the configure test for systemd interface, and I think the
list of features should include some string which tells us whether
systemd interface is supported.

> -#ifdef HAVE_LIBSYSTEMD
>        /* Read the number of sockets passed through by systemd.  */
> -      int systemd_socket = sd_listen_fds (1);
> -
> -      if (systemd_socket > 1)
> -        fputs (("\n"
> -		"Warning: systemd passed more than one socket to Emacs.\n"
> -		"Try 'Accept=false' in the Emacs socket unit file.\n"),
> -	       stderr);
> -      else if (systemd_socket == 1
> -	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
> -				     AF_UNSPEC, SOCK_STREAM, 1)))
> -	sockfd = SD_LISTEN_FDS_START;
> -#endif /* HAVE_LIBSYSTEMD */
> +      const char *fds = getenv("LISTEN_FDS");
> +      if (fds) {
> +	int systemd_socket = strtol(fds, NULL, 0);
> +	if (systemd_socket > 1)
> +	  fputs (("\n"
> +		  "Warning: systemd passed more than one socket to Emacs.\n"
> +		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
> +		 stderr);
> +	else if (systemd_socket == 1
> +		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
> +	  sockfd = SD_LISTEN_FDS_START;
> +      }

The new code to interface with systemd cannot be compiled
unconditionally, it should have some #ifdef condition, determined by
the configure script, because not all the platforms we support can use
systemd.

> @@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
>      }
>  #endif
>  
> -#ifdef HAVE_LIBSYSTEMD
>    /* Notify systemd we are shutting down, but only if we have notified
>       it about startup.  */
> -  if (daemon_type == -1)
> -    sd_notify(0, "STOPPING=1");
> -#endif /* HAVE_LIBSYSTEMD */
> +  if (daemon_type == -1){
> +    int r = sd_notify_stopping();
> +    if (r < 0) {
> +      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
> +      exit (EXIT_FAILURE);
> +    }
> +  }

Same here.

>    /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
>       set.  */
> @@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
>  
>    if (daemon_type == 1)
>      {
> -#ifdef HAVE_LIBSYSTEMD
> -      sd_notify(0, "READY=1");
> -#endif /* HAVE_LIBSYSTEMD */
> +      int r = sd_notify_ready();
> +      if (r < 0) {
> +	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
> +	exit (EXIT_FAILURE);
> +      }
>      }

And here.

> > Seems to work properly on my side with the following patch, on Guix with
> > the shepherd service I sent a few weeks ago. The patch is actually
> > pretty simple.

How sure we are that the code you wrote will not need any significant
maintenance due to changes in how systemd works?  The significant
advantage of using libsystemd is that the systemd developers take care
of updating it as needed.  With these changes, that burden is now on
us.  I don't think we'd be happy maintaining system-level code that
have very little relevance to Emacs.

> > - The sd_notify code is taken from
> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes

I'm not sure what would be the legal aspects of such code borrowing.

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

On 2024-05-12 09:29, Eli Zaretskii wrote:

>> Cc: Ludovic Courtès <ludo@gnu.org>, emacs-devel@gnu.org,
>>  Andrew Tropin <andrew@trop.in>, guix-devel@gnu.org, bjorn.bidar@thaodan.de,
>>  rudi@constantly.at, felix.lechner@lease-up.com
>> Date: Sun, 12 May 2024 01:07:51 +0200
>> From:  Nicolas Graves via "Emacs development discussions." <emacs-devel@gnu.org>
>> 
>> A lightly cleaned-up version attached.
>
> Thanks.  What was the motivation for this, again?

A light summary (all is in the preceding exchanges in the mailing list):

- Ludovic Courtès suggested this change because linking with systemd is
unnecessary (very light usage), and increases the attack surface.

- Rudolf Schlatte highlights that Lennart Poettering says that the
notify protocol is stable and independent of libsystemd.
          https://mastodon.social/@pid_eins/112202687764571433
          https://mastodon.social/@pid_eins/112202696589971319
  This mastondon thread itself contains a lot of info/answers about
  this, including examples of similar work on other projects:
  - https://github.com/FRRouting/frr/pull/8508
  - https://github.com/OISF/suricata/pull/10757
  Lennart Poettering also says that the protocol has been stable for a
  decade and will surely be for at least another decade.

This should also answer the worry about significant maintenance.

What I'm less confident about is edge cases as I highlighted earlier
(are there cases where systemd's safe_atoi is necessary compared to
strtol ? Is it fine if LISTEN_FDS is set but LISTEN_PID unset, or
should be check for LISTEN_PID definition too ?)

>
>>  configure.ac     |  19 +------
>>  lib/Makefile.in  |   2 +-
>>  lib/gnulib.mk.in |   2 -
>>  lib/sd-socket.c  | 137 +++++++++++++++++++++++++++++++++++++++++++++++
>>  lib/sd-socket.h  |  57 ++++++++++++++++++++
>>  msdos/sed1v2.inp |   3 --
>>  src/Makefile.in  |   9 ++--
>>  src/deps.mk      |   2 +-
>>  src/emacs.c      |  50 ++++++++---------
>>  9 files changed, 226 insertions(+), 55 deletions(-)
>>  create mode 100644 lib/sd-socket.c
>>  create mode 100644 lib/sd-socket.h
>
> Your code is not from Gnulib, so it is wrong to place it in lib/.  It
> should be in src/, and probably just an addition to sysdep.c.  Then
> many of the changes to the build infrastructure will not be needed.

Thanks, I'll place that there.

>
>> -HAVE_LIBSYSTEMD=no
>> -if test "${with_libsystemd}" = "yes" ; then
>> -  dnl This code has been tested with libsystemd 222 and later.
>> -  dnl FIXME: Find the earliest version number for which Emacs should work,
>> -  dnl and change '222' to that number.
>> -  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
>> -    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
>> -  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
>> -    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
>> -  fi
>> -fi
>> -
>> -AC_SUBST([LIBSYSTEMD_LIBS])
>> -AC_SUBST([LIBSYSTEMD_CFLAGS])
>
> This test should be replaced by a test to determine whether the
> systemd interface should be compiled into Emacs.  Not all of the
> supported platform can use it, so we need to determine that at
> configure time.
>
Thanks, will do.

>> -
>>  HAVE_JSON=no
>>  JSON_OBJ=
>>  
>> @@ -6652,7 +6636,7 @@ AC_DEFUN
>>  optsep=
>>  emacs_config_features=
>>  for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
>> - HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
>> + HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
>>   M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
>>   SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
>>   UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
>> @@ -6721,7 +6705,6 @@ AC_DEFUN
>>    Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
>>    Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
>>    Does Emacs use -lxft?                                   ${HAVE_XFT}
>> -  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
>>    Does Emacs use -ljansson?                               ${HAVE_JSON}
>>    Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
>>    Does Emacs use the GMP library?                         ${HAVE_GMP}
>
> The above summary of the configuration should still call out the
> result of the configure test for systemd interface, and I think the
> list of features should include some string which tells us whether
> systemd interface is supported.

Understood. 
>
>> -#ifdef HAVE_LIBSYSTEMD
>>        /* Read the number of sockets passed through by systemd.  */
>> -      int systemd_socket = sd_listen_fds (1);
>> -
>> -      if (systemd_socket > 1)
>> -        fputs (("\n"
>> -		"Warning: systemd passed more than one socket to Emacs.\n"
>> -		"Try 'Accept=false' in the Emacs socket unit file.\n"),
>> -	       stderr);
>> -      else if (systemd_socket == 1
>> -	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
>> -				     AF_UNSPEC, SOCK_STREAM, 1)))
>> -	sockfd = SD_LISTEN_FDS_START;
>> -#endif /* HAVE_LIBSYSTEMD */
>> +      const char *fds = getenv("LISTEN_FDS");
>> +      if (fds) {
>> +	int systemd_socket = strtol(fds, NULL, 0);
>> +	if (systemd_socket > 1)
>> +	  fputs (("\n"
>> +		  "Warning: systemd passed more than one socket to Emacs.\n"
>> +		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
>> +		 stderr);
>> +	else if (systemd_socket == 1
>> +		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
>> +	  sockfd = SD_LISTEN_FDS_START;
>> +      }
>
> The new code to interface with systemd cannot be compiled
> unconditionally, it should have some #ifdef condition, determined by
> the configure script, because not all the platforms we support can use
> systemd.

Same point.
>
>> @@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
>>      }
>>  #endif
>>  
>> -#ifdef HAVE_LIBSYSTEMD
>>    /* Notify systemd we are shutting down, but only if we have notified
>>       it about startup.  */
>> -  if (daemon_type == -1)
>> -    sd_notify(0, "STOPPING=1");
>> -#endif /* HAVE_LIBSYSTEMD */
>> +  if (daemon_type == -1){
>> +    int r = sd_notify_stopping();
>> +    if (r < 0) {
>> +      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
>> +      exit (EXIT_FAILURE);
>> +    }
>> +  }
>
> Same here.
>
>>    /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
>>       set.  */
>> @@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
>>  
>>    if (daemon_type == 1)
>>      {
>> -#ifdef HAVE_LIBSYSTEMD
>> -      sd_notify(0, "READY=1");
>> -#endif /* HAVE_LIBSYSTEMD */
>> +      int r = sd_notify_ready();
>> +      if (r < 0) {
>> +	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
>> +	exit (EXIT_FAILURE);
>> +      }
>>      }
>
> And here.
>
>> > Seems to work properly on my side with the following patch, on Guix with
>> > the shepherd service I sent a few weeks ago. The patch is actually
>> > pretty simple.
>
> How sure we are that the code you wrote will not need any significant
> maintenance due to changes in how systemd works?  The significant
> advantage of using libsystemd is that the systemd developers take care
> of updating it as needed.  With these changes, that burden is now on
> us.  I don't think we'd be happy maintaining system-level code that
> have very little relevance to Emacs.
>
>> > - The sd_notify code is taken from
>> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
>
> I'm not sure what would be the legal aspects of such code borrowing.

The code is given as MIT-0, hence also the two different licenses for
the two functions sd_notify and sd_is_socket. Not an expert on licenses
either, but with a proper flag about what this function's license is, I
guess it should be fine, since other projects also do that.

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

On 2024-05-12 09:50, Nicolas Graves wrote:

> On 2024-05-12 09:29, Eli Zaretskii wrote:
>
>>> Cc: Ludovic Courtès <ludo@gnu.org>, emacs-devel@gnu.org,
>>>  Andrew Tropin <andrew@trop.in>, guix-devel@gnu.org, bjorn.bidar@thaodan.de,
>>>  rudi@constantly.at, felix.lechner@lease-up.com
>>> Date: Sun, 12 May 2024 01:07:51 +0200
>>> From:  Nicolas Graves via "Emacs development discussions." <emacs-devel@gnu.org>
>>> 
>>> A lightly cleaned-up version attached.
>>
>> Thanks.  What was the motivation for this, again?
>
> A light summary (all is in the preceding exchanges in the mailing list):
>
> - Ludovic Courtès suggested this change because linking with systemd is
> unnecessary (very light usage), and increases the attack surface.
>
> - Rudolf Schlatte highlights that Lennart Poettering says that the
> notify protocol is stable and independent of libsystemd.
>           https://mastodon.social/@pid_eins/112202687764571433
>           https://mastodon.social/@pid_eins/112202696589971319
>   This mastondon thread itself contains a lot of info/answers about
>   this, including examples of similar work on other projects:
>   - https://github.com/FRRouting/frr/pull/8508
>   - https://github.com/OISF/suricata/pull/10757
>   Lennart Poettering also says that the protocol has been stable for a
>   decade and will surely be for at least another decade.

A bit more on the stability promise :
 This protocol is guaranteed to be stable as per:
 https://systemd.io/PORTABILITY_AND_STABILITY/
 
 sd_notify is definitely given as a stable interface. 

>
> This should also answer the worry about significant maintenance.
>
> What I'm less confident about is edge cases as I highlighted earlier
> (are there cases where systemd's safe_atoi is necessary compared to
> strtol ? Is it fine if LISTEN_FDS is set but LISTEN_PID unset, or
> should be check for LISTEN_PID definition too ?)
>
>>
>>>  configure.ac     |  19 +------
>>>  lib/Makefile.in  |   2 +-
>>>  lib/gnulib.mk.in |   2 -
>>>  lib/sd-socket.c  | 137 +++++++++++++++++++++++++++++++++++++++++++++++
>>>  lib/sd-socket.h  |  57 ++++++++++++++++++++
>>>  msdos/sed1v2.inp |   3 --
>>>  src/Makefile.in  |   9 ++--
>>>  src/deps.mk      |   2 +-
>>>  src/emacs.c      |  50 ++++++++---------
>>>  9 files changed, 226 insertions(+), 55 deletions(-)
>>>  create mode 100644 lib/sd-socket.c
>>>  create mode 100644 lib/sd-socket.h
>>
>> Your code is not from Gnulib, so it is wrong to place it in lib/.  It
>> should be in src/, and probably just an addition to sysdep.c.  Then
>> many of the changes to the build infrastructure will not be needed.
>
> Thanks, I'll place that there.
>
>>
>>> -HAVE_LIBSYSTEMD=no
>>> -if test "${with_libsystemd}" = "yes" ; then
>>> -  dnl This code has been tested with libsystemd 222 and later.
>>> -  dnl FIXME: Find the earliest version number for which Emacs should work,
>>> -  dnl and change '222' to that number.
>>> -  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
>>> -    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
>>> -  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
>>> -    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
>>> -  fi
>>> -fi
>>> -
>>> -AC_SUBST([LIBSYSTEMD_LIBS])
>>> -AC_SUBST([LIBSYSTEMD_CFLAGS])
>>
>> This test should be replaced by a test to determine whether the
>> systemd interface should be compiled into Emacs.  Not all of the
>> supported platform can use it, so we need to determine that at
>> configure time.
>>
> Thanks, will do.
>
>>> -
>>>  HAVE_JSON=no
>>>  JSON_OBJ=
>>>  
>>> @@ -6652,7 +6636,7 @@ AC_DEFUN
>>>  optsep=
>>>  emacs_config_features=
>>>  for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
>>> - HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
>>> + HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
>>>   M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
>>>   SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
>>>   UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
>>> @@ -6721,7 +6705,6 @@ AC_DEFUN
>>>    Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
>>>    Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
>>>    Does Emacs use -lxft?                                   ${HAVE_XFT}
>>> -  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
>>>    Does Emacs use -ljansson?                               ${HAVE_JSON}
>>>    Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
>>>    Does Emacs use the GMP library?                         ${HAVE_GMP}
>>
>> The above summary of the configuration should still call out the
>> result of the configure test for systemd interface, and I think the
>> list of features should include some string which tells us whether
>> systemd interface is supported.
>
> Understood. 
>>
>>> -#ifdef HAVE_LIBSYSTEMD
>>>        /* Read the number of sockets passed through by systemd.  */
>>> -      int systemd_socket = sd_listen_fds (1);
>>> -
>>> -      if (systemd_socket > 1)
>>> -        fputs (("\n"
>>> -		"Warning: systemd passed more than one socket to Emacs.\n"
>>> -		"Try 'Accept=false' in the Emacs socket unit file.\n"),
>>> -	       stderr);
>>> -      else if (systemd_socket == 1
>>> -	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
>>> -				     AF_UNSPEC, SOCK_STREAM, 1)))
>>> -	sockfd = SD_LISTEN_FDS_START;
>>> -#endif /* HAVE_LIBSYSTEMD */
>>> +      const char *fds = getenv("LISTEN_FDS");
>>> +      if (fds) {
>>> +	int systemd_socket = strtol(fds, NULL, 0);
>>> +	if (systemd_socket > 1)
>>> +	  fputs (("\n"
>>> +		  "Warning: systemd passed more than one socket to Emacs.\n"
>>> +		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
>>> +		 stderr);
>>> +	else if (systemd_socket == 1
>>> +		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
>>> +	  sockfd = SD_LISTEN_FDS_START;
>>> +      }
>>
>> The new code to interface with systemd cannot be compiled
>> unconditionally, it should have some #ifdef condition, determined by
>> the configure script, because not all the platforms we support can use
>> systemd.
>
> Same point.
>>
>>> @@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
>>>      }
>>>  #endif
>>>  
>>> -#ifdef HAVE_LIBSYSTEMD
>>>    /* Notify systemd we are shutting down, but only if we have notified
>>>       it about startup.  */
>>> -  if (daemon_type == -1)
>>> -    sd_notify(0, "STOPPING=1");
>>> -#endif /* HAVE_LIBSYSTEMD */
>>> +  if (daemon_type == -1){
>>> +    int r = sd_notify_stopping();
>>> +    if (r < 0) {
>>> +      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
>>> +      exit (EXIT_FAILURE);
>>> +    }
>>> +  }
>>
>> Same here.
>>
>>>    /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
>>>       set.  */
>>> @@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
>>>  
>>>    if (daemon_type == 1)
>>>      {
>>> -#ifdef HAVE_LIBSYSTEMD
>>> -      sd_notify(0, "READY=1");
>>> -#endif /* HAVE_LIBSYSTEMD */
>>> +      int r = sd_notify_ready();
>>> +      if (r < 0) {
>>> +	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
>>> +	exit (EXIT_FAILURE);
>>> +      }
>>>      }
>>
>> And here.
>>
>>> > Seems to work properly on my side with the following patch, on Guix with
>>> > the shepherd service I sent a few weeks ago. The patch is actually
>>> > pretty simple.
>>
>> How sure we are that the code you wrote will not need any significant
>> maintenance due to changes in how systemd works?  The significant
>> advantage of using libsystemd is that the systemd developers take care
>> of updating it as needed.  With these changes, that burden is now on
>> us.  I don't think we'd be happy maintaining system-level code that
>> have very little relevance to Emacs.
>>
>>> > - The sd_notify code is taken from
>>> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
>>
>> I'm not sure what would be the legal aspects of such code borrowing.
>
> The code is given as MIT-0, hence also the two different licenses for
> the two functions sd_notify and sd_is_socket. Not an expert on licenses
> either, but with a proper flag about what this function's license is, I
> guess it should be fine, since other projects also do that.

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Eli Zaretskii]

> From: Nicolas Graves <ngraves@ngraves.fr>
> Cc: monnier@iro.umontreal.ca, ludo@gnu.org, emacs-devel@gnu.org,
>  andrew@trop.in, guix-devel@gnu.org, bjorn.bidar@thaodan.de,
>  rudi@constantly.at, felix.lechner@lease-up.com
> Date: Sun, 12 May 2024 09:50:06 +0200
> 
> On 2024-05-12 09:29, Eli Zaretskii wrote:
> 
> > Thanks.  What was the motivation for this, again?
> 
> A light summary (all is in the preceding exchanges in the mailing list):
> 
> - Ludovic Courtès suggested this change because linking with systemd is
> unnecessary (very light usage), and increases the attack surface.
> 
> - Rudolf Schlatte highlights that Lennart Poettering says that the
> notify protocol is stable and independent of libsystemd.
>           https://mastodon.social/@pid_eins/112202687764571433
>           https://mastodon.social/@pid_eins/112202696589971319
>   This mastondon thread itself contains a lot of info/answers about
>   this, including examples of similar work on other projects:
>   - https://github.com/FRRouting/frr/pull/8508
>   - https://github.com/OISF/suricata/pull/10757
>   Lennart Poettering also says that the protocol has been stable for a
>   decade and will surely be for at least another decade.
> 
> This should also answer the worry about significant maintenance.

I'm sorry, but I'm wary of believing such assertions, especially when
systemd is involved.  E.g., what's to prevent people from asking us to
support the various forks of systemd as well?

Reimplementing everything in-house doesn't scale, definitely not in a
project as large as Emacs.  So the argument about smaller attack
surface doesn't really convince me.  Emacs uses quite a lot of
external libraries to the benefit of our users, and that will not
change any time soon.

> What I'm less confident about is edge cases as I highlighted earlier
> (are there cases where systemd's safe_atoi is necessary compared to
> strtol ? Is it fine if LISTEN_FDS is set but LISTEN_PID unset, or
> should be check for LISTEN_PID definition too ?)

This is exactly the kind of maintenance burden I'm concerned about:
who will help us answer these questions, now and in the future?  We
cannot rely on having systemd experts on board at all times.

> >> > - The sd_notify code is taken from
> >> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
> >
> > I'm not sure what would be the legal aspects of such code borrowing.
> 
> The code is given as MIT-0, hence also the two different licenses for
> the two functions sd_notify and sd_is_socket. Not an expert on licenses
> either, but with a proper flag about what this function's license is, I
> guess it should be fine, since other projects also do that.

The license is only half of the problem.  Every non-trivial
contribution to Emacs must have its copyright assigned to the FSF,
because the FSF is in charge of protecting the Emacs sources,
something that only the copyright holder can do, at least in some
countries.  You will need to assign the copyright as well (a
relatively simple procedure of filling a form and emailing it), but if
the code is not yours, you cannot assign its copyright.

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

On 2024-05-12 12:36, Eli Zaretskii wrote:

>> From: Nicolas Graves <ngraves@ngraves.fr>
>> Cc: monnier@iro.umontreal.ca, ludo@gnu.org, emacs-devel@gnu.org,
>>  andrew@trop.in, guix-devel@gnu.org, bjorn.bidar@thaodan.de,
>>  rudi@constantly.at, felix.lechner@lease-up.com
>> Date: Sun, 12 May 2024 09:50:06 +0200
>> 
>> On 2024-05-12 09:29, Eli Zaretskii wrote:
>> 
>> > Thanks.  What was the motivation for this, again?
>> 
>> A light summary (all is in the preceding exchanges in the mailing list):
>> 
>> - Ludovic Courtès suggested this change because linking with systemd is
>> unnecessary (very light usage), and increases the attack surface.
>> 
>> - Rudolf Schlatte highlights that Lennart Poettering says that the
>> notify protocol is stable and independent of libsystemd.
>>           https://mastodon.social/@pid_eins/112202687764571433
>>           https://mastodon.social/@pid_eins/112202696589971319
>>   This mastondon thread itself contains a lot of info/answers about
>>   this, including examples of similar work on other projects:
>>   - https://github.com/FRRouting/frr/pull/8508
>>   - https://github.com/OISF/suricata/pull/10757
>>   Lennart Poettering also says that the protocol has been stable for a
>>   decade and will surely be for at least another decade.
>> 
>> This should also answer the worry about significant maintenance.
>
> I'm sorry, but I'm wary of believing such assertions, especially when
> systemd is involved.  E.g., what's to prevent people from asking us to
> support the various forks of systemd as well?

Don't they also support this precise protocol?

Originally this thread was precisely about some limitations about
emacs's integration with GNU shepherd, exposing a "manual" pid-file
solution I found, and Ludovic answering "great, but look GNU shepherd
can emulate notify protocol's server side and GNU emacs can do it on the
client side". What I get from that is that systemd is so ubiquitously
used that even GNU Shepherd needed to emulate systemd's notify protocol
to properly manage some services, this protocol is probably already
emulated in most init system (is it?). In my own experience, it's indeed
simpler to rely on it rather than manually implementing proper
emacs-shepherd integration.

> Reimplementing everything in-house doesn't scale, definitely not in a
> project as large as Emacs.  So the argument about smaller attack
> surface doesn't really convince me.  Emacs uses quite a lot of
> external libraries to the benefit of our users, and that will not
> change any time soon.

Lennart himself wrote "if all you want is sd_notify() then don't bother
linking to libsystemd, since the protocol is stable and should be
considered the API", I'm (with my biases) more worried about "if all you
want" and what happens if users ask for deeper integration with systemd
than these two functions than about systemd's stability promise.

One example is what I did with my pid-file emacs-shepherd integration :
be able to notify (in the sense of libnotify, not systemd) when server
has started but is not ready yet. This could be done with some smart use
of sd_notify_reloading, and would require the vendoring of this other
function (although this one is provided as such by Lennart too ; but any
deeper use (I can't find an example though) might be harder to implement).

I'm not going to push much for this, currently rewriting the patch, will
make that working again with recommended changes. If maintainers get
convinced and licensing is ok, use it. If Guix or some other distro want
to use it as a vendored patch, fine (indeed it doesn't make sense on
Guix to pass elogind as an input for this). I just wrote that after
Stefan's suggestion, and after seeing that the actual code is basically
two C functions, I'm not actively pushing for it.

>> What I'm less confident about is edge cases as I highlighted earlier
>> (are there cases where systemd's safe_atoi is necessary compared to
>> strtol ? Is it fine if LISTEN_FDS is set but LISTEN_PID unset, or
>> should be check for LISTEN_PID definition too ?)
>
> This is exactly the kind of maintenance burden I'm concerned about:
> who will help us answer these questions, now and in the future?  We
> cannot rely on having systemd experts on board at all times.

I'll add a tiny check to also verify that LISTEN_PID is set.

The first question is the kind of questions that come up when adapting
foreign code, but when strictly reading the protocol, it should be
fine, the point is to parse LISTEN_FDS.

>> >> > - The sd_notify code is taken from
>> >> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
>> >
>> > I'm not sure what would be the legal aspects of such code borrowing.
>> 
>> The code is given as MIT-0, hence also the two different licenses for
>> the two functions sd_notify and sd_is_socket. Not an expert on licenses
>> either, but with a proper flag about what this function's license is, I
>> guess it should be fine, since other projects also do that.
>
> The license is only half of the problem.  Every non-trivial
> contribution to Emacs must have its copyright assigned to the FSF,
> because the FSF is in charge of protecting the Emacs sources,
> something that only the copyright holder can do, at least in some
> countries.  You will need to assign the copyright as well (a
> relatively simple procedure of filling a form and emailing it), but if
> the code is not yours, you cannot assign its copyright.

Don't emacs have some C functions coming mainly from other codebases?
How is it handled in this case?




I have a development question. Where should be the functions
declaration header? There are specific to gnu-linux/aix/hpux. Thanks.




NB. In emacs's configure.ac, the note about finding which libsystemd
version is the first supported, with 222 truly working, has an answer in 
https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
Interfaces used have been introduced in 217. Requires a test, but that
is almost certainly the answer.

-- 
Best regards,
Nicolas Graves

Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file

[Nicolas Graves via Development of GNU Guix and the GNU System distribution.]

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

On 2024-05-12 13:11, Nicolas Graves wrote:

> On 2024-05-12 12:36, Eli Zaretskii wrote:
>
>>> From: Nicolas Graves <ngraves@ngraves.fr>
>>> Cc: monnier@iro.umontreal.ca, ludo@gnu.org, emacs-devel@gnu.org,
>>>  andrew@trop.in, guix-devel@gnu.org, bjorn.bidar@thaodan.de,
>>>  rudi@constantly.at, felix.lechner@lease-up.com
>>> Date: Sun, 12 May 2024 09:50:06 +0200
>>> 
>>> On 2024-05-12 09:29, Eli Zaretskii wrote:
>>> 
>>> > Thanks.  What was the motivation for this, again?
>>> 
>>> A light summary (all is in the preceding exchanges in the mailing list):
>>> 
>>> - Ludovic Courtès suggested this change because linking with systemd is
>>> unnecessary (very light usage), and increases the attack surface.
>>> 
>>> - Rudolf Schlatte highlights that Lennart Poettering says that the
>>> notify protocol is stable and independent of libsystemd.
>>>           https://mastodon.social/@pid_eins/112202687764571433
>>>           https://mastodon.social/@pid_eins/112202696589971319
>>>   This mastondon thread itself contains a lot of info/answers about
>>>   this, including examples of similar work on other projects:
>>>   - https://github.com/FRRouting/frr/pull/8508
>>>   - https://github.com/OISF/suricata/pull/10757
>>>   Lennart Poettering also says that the protocol has been stable for a
>>>   decade and will surely be for at least another decade.
>>> 
>>> This should also answer the worry about significant maintenance.
>>
>> I'm sorry, but I'm wary of believing such assertions, especially when
>> systemd is involved.  E.g., what's to prevent people from asking us to
>> support the various forks of systemd as well?
>
> Don't they also support this precise protocol?
>
> Originally this thread was precisely about some limitations about
> emacs's integration with GNU shepherd, exposing a "manual" pid-file
> solution I found, and Ludovic answering "great, but look GNU shepherd
> can emulate notify protocol's server side and GNU emacs can do it on the
> client side". What I get from that is that systemd is so ubiquitously
> used that even GNU Shepherd needed to emulate systemd's notify protocol
> to properly manage some services, this protocol is probably already
> emulated in most init system (is it?). In my own experience, it's indeed
> simpler to rely on it rather than manually implementing proper
> emacs-shepherd integration.
>
>> Reimplementing everything in-house doesn't scale, definitely not in a
>> project as large as Emacs.  So the argument about smaller attack
>> surface doesn't really convince me.  Emacs uses quite a lot of
>> external libraries to the benefit of our users, and that will not
>> change any time soon.
>
> Lennart himself wrote "if all you want is sd_notify() then don't bother
> linking to libsystemd, since the protocol is stable and should be
> considered the API", I'm (with my biases) more worried about "if all you
> want" and what happens if users ask for deeper integration with systemd
> than these two functions than about systemd's stability promise.
>
> One example is what I did with my pid-file emacs-shepherd integration :
> be able to notify (in the sense of libnotify, not systemd) when server
> has started but is not ready yet. This could be done with some smart use
> of sd_notify_reloading, and would require the vendoring of this other
> function (although this one is provided as such by Lennart too ; but any
> deeper use (I can't find an example though) might be harder to
> implement).

I'm not sure any use outside this protocol is not far-fetched for emacs,
can't find an example that would take advantage of libsystemd outside of
the notify protocol and what's already there.

Here another working patch taking into account your remarks Eli.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Implement-systemd-socket.patch --]
[-- Type: text/x-patch, Size: 17592 bytes --]

From e18f0e4035c71524be96826b543b64487ace922d Mon Sep 17 00:00:00 2001
From: Nicolas Graves <ngraves@ngraves.fr>
Date: Sat, 13 Apr 2024 19:37:34 +0200
Subject: [PATCH] Implement systemd notify protocol and is_socket function.

---
 configure.ac     |  26 +++-----
 lib/gnulib.mk.in |   2 -
 msdos/sed1v2.inp |   3 -
 src/Makefile.in  |   9 +--
 src/deps.mk      |   8 +--
 src/emacs.c      |  60 +++++++++++--------
 src/sysdep.c     | 150 +++++++++++++++++++++++++++++++++++++++++++++++
 src/syssocket.h  |  52 ++++++++++++++++
 8 files changed, 253 insertions(+), 57 deletions(-)
 create mode 100644 src/syssocket.h

diff --git a/configure.ac b/configure.ac
index 29b71ea2730..ab1eeef4efe 100644
--- a/configure.ac
+++ b/configure.ac
@@ -457,7 +457,6 @@ AC_DEFUN
 OPTION_DEFAULT_ON([webp],[don't compile with WebP image support])
 OPTION_DEFAULT_ON([sqlite3],[don't compile with sqlite3 support])
 OPTION_DEFAULT_ON([lcms2],[don't compile with Little CMS support])
-OPTION_DEFAULT_ON([libsystemd],[don't compile with libsystemd support])
 OPTION_DEFAULT_ON([cairo],[don't compile with Cairo drawing])
 OPTION_DEFAULT_OFF([cairo-xcb], [use XCB surfaces for Cairo support])
 OPTION_DEFAULT_ON([xml2],[don't compile with XML parsing support])
@@ -3203,20 +3202,13 @@ AC_DEFUN
 AC_SUBST([LIBGNUTLS_LIBS])
 AC_SUBST([LIBGNUTLS_CFLAGS])
 
-HAVE_LIBSYSTEMD=no
-if test "${with_libsystemd}" = "yes" ; then
-  dnl This code has been tested with libsystemd 222 and later.
-  dnl FIXME: Find the earliest version number for which Emacs should work,
-  dnl and change '222' to that number.
-  EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >= 222],
-    [HAVE_LIBSYSTEMD=yes], [HAVE_LIBSYSTEMD=no])
-  if test "${HAVE_LIBSYSTEMD}" = "yes"; then
-    AC_DEFINE([HAVE_LIBSYSTEMD], [1], [Define if using libsystemd.])
-  fi
-fi
-
-AC_SUBST([LIBSYSTEMD_LIBS])
-AC_SUBST([LIBSYSTEMD_CFLAGS])
+dnl Systemd is developped for GNU/linux.
+dnl It seems that hpux and aix systems support it too.
+case $opsys in
+    gnu* | hpux* | aix* )
+    AC_DEFINE([USE_SYSTEMD_NOTIFY], [1], [Define if the systemd notify interface can be supported.])
+    ;;
+esac
 
 HAVE_JSON=no
 JSON_OBJ=
@@ -6652,7 +6644,7 @@ AC_DEFUN
 optsep=
 emacs_config_features=
 for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GPM GSETTINGS \
- HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 \
+ HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBXML2 \
  M17N_FLT MODULES NATIVE_COMP NOTIFY NS OLDXMENU PDUMPER PGTK PNG RSVG SECCOMP \
  SOUND SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER \
  UNEXEC WEBP X11 XAW3D XDBE XFT XIM XINPUT2 XPM XWIDGETS X_TOOLKIT \
@@ -6721,7 +6713,7 @@ AC_DEFUN
   Does Emacs use -lm17n-flt?                              ${HAVE_M17N_FLT}
   Does Emacs use -lotf?                                   ${HAVE_LIBOTF}
   Does Emacs use -lxft?                                   ${HAVE_XFT}
-  Does Emacs use -lsystemd?                               ${HAVE_LIBSYSTEMD}
+  Does Emacs use systemd notify interface ?               ${USE_SYSTEMD_NOTIFY}
   Does Emacs use -ljansson?                               ${HAVE_JSON}
   Does Emacs use -ltree-sitter?                           ${HAVE_TREE_SITTER}
   Does Emacs use the GMP library?                         ${HAVE_GMP}
diff --git a/lib/gnulib.mk.in b/lib/gnulib.mk.in
index 9ab4b741595..f9ed4a4cb23 100644
--- a/lib/gnulib.mk.in
+++ b/lib/gnulib.mk.in
@@ -912,8 +912,6 @@ LIBSECCOMP_CFLAGS = @LIBSECCOMP_CFLAGS@
 LIBSECCOMP_LIBS = @LIBSECCOMP_LIBS@
 LIBSELINUX_LIBS = @LIBSELINUX_LIBS@
 LIBSOUND = @LIBSOUND@
-LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
-LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
 LIBS_ECLIENT = @LIBS_ECLIENT@
 LIBS_GNUSTEP = @LIBS_GNUSTEP@
 LIBS_MAIL = @LIBS_MAIL@
diff --git a/msdos/sed1v2.inp b/msdos/sed1v2.inp
index 4d4c80a6b1a..d6d80ff809c 100644
--- a/msdos/sed1v2.inp
+++ b/msdos/sed1v2.inp
@@ -138,8 +138,6 @@ s/ *@WEBP_LIBS@//
 /^LIBMODULES *=/s/@LIBMODULES@//
 /^MODULES_OBJ *=/s/@MODULES_OBJ@//
 /^LIBSELINUX_LIBS *=/s/@LIBSELINUX_LIBS@//
-/^LIBSYSTEMD_LIBS *=/s/@LIBSYSTEMD_LIBS@//
-/^LIBSYSTEMD_CFLAGS *=/s/@LIBSYSTEMD_CFLAGS@//
 /^LIB_CLOCK_GETTIME *=/s/@[^@\n]*@//g
 /^LIB_TIMER_TIME *=/s/@[^@\n]*@//g
 /^LIB_EXECINFO *=/s/@[^@\n]*@//g
@@ -269,7 +267,6 @@ s/echo.*buildobj.lst/dj&/
 # Make the GCC command line fit one screen line
 /^[ 	][ 	]*\$(GNUSTEP_CFLAGS)/d
 /^[ 	][ 	]*\$(LIBGNUTLS_CFLAGS)/d
-/^[ 	][ 	]*\$(LIBSYSTEMD_CFLAGS)/d
 /^[ 	][ 	]*\$(XRANDR_CFLAGS)/d
 /^[ 	][ 	]*\$(WEBKIT_CFLAGS)/d
 /^[ 	][ 	]*\$(SETTINGS_CFLAGS)/d
diff --git a/src/Makefile.in b/src/Makefile.in
index 9bc53c072ea..d3a7f432ea8 100644
--- a/src/Makefile.in
+++ b/src/Makefile.in
@@ -336,9 +336,6 @@ LIBSELINUX_LIBS =
 LIBGNUTLS_LIBS = @LIBGNUTLS_LIBS@
 LIBGNUTLS_CFLAGS = @LIBGNUTLS_CFLAGS@
 
-LIBSYSTEMD_LIBS = @LIBSYSTEMD_LIBS@
-LIBSYSTEMD_CFLAGS = @LIBSYSTEMD_CFLAGS@
-
 JSON_LIBS = @JSON_LIBS@
 JSON_CFLAGS = @JSON_CFLAGS@
 JSON_OBJ = @JSON_OBJ@
@@ -405,11 +402,11 @@ EMACS_CFLAGS=
   $(C_SWITCH_MACHINE) $(C_SWITCH_SYSTEM) $(C_SWITCH_X_SITE) \
   $(GNUSTEP_CFLAGS) $(CFLAGS_SOUND) $(RSVG_CFLAGS) $(IMAGEMAGICK_CFLAGS) \
   $(PNG_CFLAGS) $(LIBXML2_CFLAGS) $(LIBGCCJIT_CFLAGS) $(DBUS_CFLAGS) \
-  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFLAGS) \
+  $(XRANDR_CFLAGS) $(XINERAMA_CFLAGS) $(XFIXES_CFLAGS) $(XDBE_CFAGS) \
   $(XINPUT_CFLAGS) $(WEBP_CFLAGS) $(WEBKIT_CFLAGS) $(LCMS2_CFLAGS) \
   $(SETTINGS_CFLAGS) $(FREETYPE_CFLAGS) $(FONTCONFIG_CFLAGS) \
   $(HARFBUZZ_CFLAGS) $(LIBOTF_CFLAGS) $(M17N_FLT_CFLAGS) $(DEPFLAGS) \
-  $(LIBSYSTEMD_CFLAGS) $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
+  $(JSON_CFLAGS) $(XSYNC_CFLAGS) $(TREE_SITTER_CFLAGS) \
   $(LIBGNUTLS_CFLAGS) $(NOTIFY_CFLAGS) $(CAIRO_CFLAGS) \
   $(WERROR_CFLAGS) $(HAIKU_CFLAGS) $(XCOMPOSITE_CFLAGS) $(XSHAPE_CFLAGS)
 ALL_CFLAGS = $(EMACS_CFLAGS) $(WARN_CFLAGS) $(CFLAGS)
@@ -567,7 +564,7 @@ LIBES =
    $(LIBS_TERMCAP) $(GETLOADAVG_LIBS) $(SETTINGS_LIBS) $(LIBSELINUX_LIBS) \
    $(FREETYPE_LIBS) $(FONTCONFIG_LIBS) $(HARFBUZZ_LIBS) $(LIBOTF_LIBS) $(M17N_FLT_LIBS) \
    $(LIBGNUTLS_LIBS) $(LIB_PTHREAD) $(GETADDRINFO_A_LIBS) $(LCMS2_LIBS) \
-   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) $(LIBSYSTEMD_LIBS) \
+   $(NOTIFY_LIBS) $(LIB_MATH) $(LIBZ) $(LIBMODULES) \
    $(JSON_LIBS) $(LIBGMP) $(LIBGCCJIT_LIBS) $(XINPUT_LIBS) $(HAIKU_LIBS) \
    $(TREE_SITTER_LIBS) $(SQLITE3_LIBS) $(XCOMPOSITE_LIBS) $(XSHAPE_LIBS)
 
diff --git a/src/deps.mk b/src/deps.mk
index a7c8ae11f72..f5a25e8497e 100644
--- a/src/deps.mk
+++ b/src/deps.mk
@@ -92,7 +92,7 @@ editfns.o:
 emacs.o: emacs.c commands.h systty.h syssignal.h blockinput.h process.h \
    termhooks.h buffer.h atimer.h systime.h $(INTERVALS_H) lisp.h $(config_h) \
    globals.h ../lib/unistd.h window.h dispextern.h keyboard.h keymap.h \
-   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h
+   frame.h coding.h gnutls.h msdos.h dosfns.h unexec.h syssocket.h
 fileio.o: fileio.c window.h buffer.h systime.h $(INTERVALS_H) character.h \
    coding.h msdos.h blockinput.h atimer.h lisp.h $(config_h) frame.h \
    commands.h globals.h ../lib/unistd.h
@@ -184,9 +184,9 @@ sound.o:
    atimer.h systime.h ../lib/unistd.h msdos.h
 syntax.o: syntax.c syntax.h buffer.h commands.h category.h character.h \
    keymap.h regex-emacs.h $(INTERVALS_H) lisp.h globals.h $(config_h)
-sysdep.o: sysdep.c syssignal.h systty.h systime.h syswait.h blockinput.h \
-   process.h dispextern.h termhooks.h termchar.h termopts.h coding.h \
-   frame.h atimer.h window.h msdos.h dosfns.h keyboard.h cm.h lisp.h \
+sysdep.o: sysdep.c syssignal.h syssocket.h systty.h systime.h syswait.h \
+   blockinput.h process.h dispextern.h termhooks.h termchar.h termopts.h \
+   coding.h frame.h atimer.h window.h msdos.h dosfns.h keyboard.h cm.h lisp.h \
    globals.h $(config_h) composite.h sysselect.h gnutls.h \
    ../lib/allocator.h ../lib/careadlinkat.h \
    ../lib/unistd.h
diff --git a/src/emacs.c b/src/emacs.c
index dde305edbc2..5a0ead6ed27 100644
--- a/src/emacs.c
+++ b/src/emacs.c
@@ -57,11 +57,6 @@ #define MAIN_PROGRAM
 #include "dosfns.h"
 #endif
 
-#ifdef HAVE_LIBSYSTEMD
-# include <systemd/sd-daemon.h>
-# include <sys/socket.h>
-#endif
-
 #if defined HAVE_LINUX_SECCOMP_H && defined HAVE_LINUX_FILTER_H \
   && HAVE_DECL_SECCOMP_SET_MODE_FILTER                          \
   && HAVE_DECL_SECCOMP_FILTER_FLAG_TSYNC
@@ -137,6 +132,10 @@ #define MAIN_PROGRAM
 #include <sys/resource.h>
 #endif
 
+#ifdef USE_SYSTEMD_NOTIFY
+#include "syssocket.h"
+#endif
+
 /* We don't guard this with HAVE_TREE_SITTER because treesit.o is
    always compiled (to provide treesit-available-p).  */
 #include "treesit.h"
@@ -1719,20 +1718,22 @@ main (int argc, char **argv)
             }
         } /* daemon_type == 2 */
 
-#ifdef HAVE_LIBSYSTEMD
+#ifdef USE_SYSTEMD_NOTIFY
       /* Read the number of sockets passed through by systemd.  */
-      int systemd_socket = sd_listen_fds (1);
-
-      if (systemd_socket > 1)
-        fputs (("\n"
-		"Warning: systemd passed more than one socket to Emacs.\n"
-		"Try 'Accept=false' in the Emacs socket unit file.\n"),
-	       stderr);
-      else if (systemd_socket == 1
-	       && (0 < sd_is_socket (SD_LISTEN_FDS_START,
-				     AF_UNSPEC, SOCK_STREAM, 1)))
-	sockfd = SD_LISTEN_FDS_START;
-#endif /* HAVE_LIBSYSTEMD */
+      const char *sd_pid = getenv("LISTEN_PID");
+      const char *fds = getenv("LISTEN_FDS");
+      if (sd_pid && fds) {
+	int systemd_socket = strtol(fds, NULL, 0);
+	if (systemd_socket > 1)
+	  fputs (("\n"
+		  "Warning: systemd passed more than one socket to Emacs.\n"
+		  "Try 'Accept=false' in the Emacs socket unit file.\n"),
+		 stderr);
+	else if (systemd_socket == 1
+		 && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1)))
+	  sockfd = SD_LISTEN_FDS_START;
+      }
+#endif /* USE_SYSTEMD_NOTIFY */
 
       /* On X, the bug happens because we call abort to avoid GLib
 	 crashes upon a longjmp in our X error handler.
@@ -2857,12 +2858,17 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, 0, 2, "P",
     }
 #endif
 
-#ifdef HAVE_LIBSYSTEMD
+#ifdef USE_SYSTEMD_NOTIFY
   /* Notify systemd we are shutting down, but only if we have notified
      it about startup.  */
-  if (daemon_type == -1)
-    sd_notify(0, "STOPPING=1");
-#endif /* HAVE_LIBSYSTEMD */
+  if (daemon_type == -1){
+    int r = sd_notify_stopping();
+    if (r < 0) {
+      fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET: %s\n", strerror(-r));
+      exit (EXIT_FAILURE);
+    }
+  }
+#endif /* USE_SYSTEMD_NOTIFY */
 
   /* Fsignal calls emacs_abort () if it sees that waiting_for_input is
      set.  */
@@ -3382,9 +3388,13 @@ DEFUN ("daemon-initialized", Fdaemon_initialized, Sdaemon_initialized, 0, 0, 0,
 
   if (daemon_type == 1)
     {
-#ifdef HAVE_LIBSYSTEMD
-      sd_notify(0, "READY=1");
-#endif /* HAVE_LIBSYSTEMD */
+#ifdef USE_SYSTEMD_NOTIFY
+      int r = sd_notify_ready();
+      if (r < 0) {
+	fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n", strerror(-r));
+	exit (EXIT_FAILURE);
+      }
+#endif /* USE_SYSTEMD_NOTIFY */
     }
 
   if (daemon_type == 2)
diff --git a/src/sysdep.c b/src/sysdep.c
index 7bac3d8935a..8476d0cff93 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -40,6 +40,10 @@
 #include "sysselect.h"
 #include "blockinput.h"
 
+#ifdef USE_SYSTEMD_NOTIFY
+#include "syssocket.h"
+#endif
+
 #ifdef HAVE_LINUX_FS_H
 # include <linux/fs.h>
 # include <sys/syscall.h>
@@ -4513,3 +4517,149 @@ syms_of_sysdep (void)
 {
   defsubr (&Sget_internal_run_time);
 }
+
+#ifdef USE_SYSTEMD_NOTIFY
+#define _cleanup_(f) __attribute__((cleanup(f)))
+
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/* Copied and adapted from systemd source code. */
+/* This is the sd_is_socket_internal function from sd-daemon.c */
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is a socket of type (SOCK_DGRAM, SOCK_STREAM,
+  ...), 0 otherwise. If type is 0 a socket type check will not be done
+  and the call only verifies if the file descriptor refers to a
+  socket. If listening is > 0 it is verified that the socket is in
+  listening mode. (i.e. listen() has been called) If listening is == 0
+  it is verified that the socket is not in listening mode. If
+  listening is < 0 no listening mode check is done. Returns a negative
+  errno style error code on failure.
+*/
+int sd_is_socket(int fd, int type, int listening) {
+        struct stat st_fd;
+
+        assert(fd >= 0);
+        assert(type >= 0);
+
+        if (fstat(fd, &st_fd) < 0)
+                return -errno;
+
+        if (!S_ISSOCK(st_fd.st_mode))
+                return 0;
+
+        if (type != 0) {
+                int other_type = 0;
+                socklen_t l = sizeof(other_type);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_TYPE, &other_type, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(other_type))
+                        return -EINVAL;
+
+                if (other_type != type)
+                        return 0;
+        }
+
+        if (listening >= 0) {
+                int accepting = 0;
+                socklen_t l = sizeof(accepting);
+
+                if (getsockopt(fd, SOL_SOCKET, SO_ACCEPTCONN, &accepting, &l) < 0)
+                        return -errno;
+
+                if (l != sizeof(accepting))
+                        return -EINVAL;
+
+                if (!accepting != !listening)
+                        return 0;
+        }
+
+        return 1;
+}
+
+/* SPDX-License-Identifier: MIT-0 */
+
+/* Implement the systemd notify protocol without external dependencies.
+ * Supports both readiness notification on startup and on reloading,
+ * according to the protocol defined at:
+ * https://www.freedesktop.org/software/systemd/man/latest/sd_notify.html
+ * sd_notify function is directly copied from this link.
+ * This protocol is guaranteed to be stable as per:
+ * https://systemd.io/PORTABILITY_AND_STABILITY/ */
+
+static void closep(int *fd) {
+  if (!fd || *fd < 0)
+    return;
+
+  close(*fd);
+  *fd = -1;
+}
+
+static int sd_notify(const char *message) {
+  union sockaddr_union {
+    struct sockaddr sa;
+    struct sockaddr_un sun;
+  } socket_addr = {
+    .sun.sun_family = AF_UNIX,
+  };
+  size_t path_length, message_length;
+  _cleanup_(closep) int fd = -1;
+  const char *socket_path;
+
+  /* Verify the argument first */
+  if (!message)
+    return -EINVAL;
+
+  message_length = strlen(message);
+  if (message_length == 0)
+    return -EINVAL;
+
+  /* If the variable is not set, the protocol is a noop */
+  socket_path = getenv("NOTIFY_SOCKET");
+  if (!socket_path)
+    return 0; /* Not set? Nothing to do */
+
+  /* Only AF_UNIX is supported, with path or abstract sockets */
+  if (socket_path[0] != '/' && socket_path[0] != '@')
+    return -EAFNOSUPPORT;
+
+  path_length = strlen(socket_path);
+  /* Ensure there is room for NUL byte */
+  if (path_length >= sizeof(socket_addr.sun.sun_path))
+    return -E2BIG;
+
+  memcpy(socket_addr.sun.sun_path, socket_path, path_length);
+
+  /* Support for abstract socket */
+  if (socket_addr.sun.sun_path[0] == '@')
+    socket_addr.sun.sun_path[0] = 0;
+
+  fd = socket(AF_UNIX, SOCK_DGRAM|SOCK_CLOEXEC, 0);
+  if (fd < 0)
+    return -errno;
+
+  if (connect(fd, &socket_addr.sa, offsetof(struct sockaddr_un, sun_path) + path_length) != 0)
+    return -errno;
+
+  ssize_t written = write(fd, message, message_length);
+  if (written != (ssize_t) message_length)
+    return written < 0 ? -errno : -EPROTO;
+
+  return 1; /* Notified! */
+}
+
+/*
+  Tells systemd that daemon startup or daemon reload is finished.
+*/
+int sd_notify_ready(void) {
+  return sd_notify("READY=1");
+}
+
+/*
+  Tells systemd that the daemon is about to go down.
+ */
+int sd_notify_stopping(void) {
+  return sd_notify("STOPPING=1");
+}
+#endif /* USE_SYSTEMD_NOTIFY */
diff --git a/src/syssocket.h b/src/syssocket.h
new file mode 100644
index 00000000000..1f1da2a34a6
--- /dev/null
+++ b/src/syssocket.h
@@ -0,0 +1,52 @@
+#ifndef _EMACS_SOCKET_H
+#define _EMACS_SOCKET_H 1
+
+#include <assert.h>
+#include <errno.h>
+#include <signal.h>
+#include <stddef.h>
+#include <stdlib.h>
+#include <sys/stat.h>
+#include <sys/un.h>
+
+#include <inttypes.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+
+/* Systemd's first passed file descriptor */
+#define SD_LISTEN_FDS_START 3
+
+/*
+  Helper call for identifying a passed file descriptor. Returns 1 if
+  the file descriptor is a socket of type (SOCK_DGRAM, SOCK_STREAM,
+  ...), 0 otherwise. If type is 0 a socket type check will not be done
+  and the call only verifies if the file descriptor refers to a
+  socket. If listening is > 0 it is verified that the socket is in
+  listening mode. (i.e. listen() has been called) If listening is == 0
+  it is verified that the socket is not in listening mode. If
+  listening is < 0 no listening mode check is done. Returns a negative
+  errno style error code on failure.
+
+  See https://www.freedesktop.org/software/systemd/man/devel/sd_is_socket.html
+  for more information.
+*/
+int sd_is_socket(int fd, int type, int listening);
+
+/*
+  Tells systemd that daemon startup or daemon reload is finished.
+
+  See https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
+  for more information.
+*/
+int sd_notify_ready(void);
+
+/*
+  Tells systemd that the daemon is about to go down.
+
+  See https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
+  for more information.
+
+ */
+int sd_notify_stopping(void);
+
+#endif
-- 
2.41.0


[-- Attachment #3: Type: text/plain, Size: 2935 bytes --]


Best regards,
Nicolas

>
> I'm not going to push much for this, currently rewriting the patch, will
> make that working again with recommended changes. If maintainers get
> convinced and licensing is ok, use it. If Guix or some other distro want
> to use it as a vendored patch, fine (indeed it doesn't make sense on
> Guix to pass elogind as an input for this). I just wrote that after
> Stefan's suggestion, and after seeing that the actual code is basically
> two C functions, I'm not actively pushing for it.
>
>>> What I'm less confident about is edge cases as I highlighted earlier
>>> (are there cases where systemd's safe_atoi is necessary compared to
>>> strtol ? Is it fine if LISTEN_FDS is set but LISTEN_PID unset, or
>>> should be check for LISTEN_PID definition too ?)
>>
>> This is exactly the kind of maintenance burden I'm concerned about:
>> who will help us answer these questions, now and in the future?  We
>> cannot rely on having systemd experts on board at all times.
>
> I'll add a tiny check to also verify that LISTEN_PID is set.
>
> The first question is the kind of questions that come up when adapting
> foreign code, but when strictly reading the protocol, it should be
> fine, the point is to parse LISTEN_FDS.
>
>>> >> > - The sd_notify code is taken from
>>> >> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
>>> >
>>> > I'm not sure what would be the legal aspects of such code borrowing.
>>> 
>>> The code is given as MIT-0, hence also the two different licenses for
>>> the two functions sd_notify and sd_is_socket. Not an expert on licenses
>>> either, but with a proper flag about what this function's license is, I
>>> guess it should be fine, since other projects also do that.
>>
>> The license is only half of the problem.  Every non-trivial
>> contribution to Emacs must have its copyright assigned to the FSF,
>> because the FSF is in charge of protecting the Emacs sources,
>> something that only the copyright holder can do, at least in some
>> countries.  You will need to assign the copyright as well (a
>> relatively simple procedure of filling a form and emailing it), but if
>> the code is not yours, you cannot assign its copyright.
>
> Don't emacs have some C functions coming mainly from other codebases?
> How is it handled in this case?
>
>
>
>
> I have a development question. Where should be the functions
> declaration header? There are specific to gnu-linux/aix/hpux. Thanks.

In the end I kept that in its own header file, couldn't find a nice
place. Named syssocket.h.

>
>
> NB. In emacs's configure.ac, the note about finding which libsystemd
> version is the first supported, with 222 truly working, has an answer in 
> https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html#Notes
> Interfaces used have been introduced in 217. Requires a test, but that
> is almost certainly the answer.

-- 
Best regards,
Nicolas Graves