Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
On 2024-05-12 13:11, Nicolas Graves wrote: > On 2024-05-12 12:36, Eli Zaretskii wrote: > >>> From: Nicolas Graves >>> Cc: monn...@iro.umontreal.ca, l...@gnu.org, emacs-de...@gnu.org, >>> and...@trop.in, guix-devel@gnu.org, bjorn.bi...@thaodan.de, >>> r...@constantly.at, felix.lech...@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. >From e18f0e4035c71524be96826b543b64487ace922d Mon Sep 17 00:00:00 2001 From: Nicolas Graves 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_SUBS
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
On 2024-05-12 12:36, Eli Zaretskii wrote: >> From: Nicolas Graves >> Cc: monn...@iro.umontreal.ca, l...@gnu.org, emacs-de...@gnu.org, >> and...@trop.in, guix-devel@gnu.org, bjorn.bi...@thaodan.de, >> r...@constantly.at, felix.lech...@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. > >
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
> From: Nicolas Graves > Cc: monn...@iro.umontreal.ca, l...@gnu.org, emacs-de...@gnu.org, > and...@trop.in, guix-devel@gnu.org, bjorn.bi...@thaodan.de, > r...@constantly.at, felix.lech...@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
On 2024-05-12 09:50, Nicolas Graves wrote:
> On 2024-05-12 09:29, Eli Zaretskii wrote:
>
>>> Cc: Ludovic Courtès , emacs-de...@gnu.org,
>>> Andrew Tropin , guix-devel@gnu.org, bjorn.bi...@thaodan.de,
>>> r...@constantly.at, felix.lech...@lease-up.com
>>> Date: Sun, 12 May 2024 01:07:51 +0200
>>> From: Nicolas Graves via "Emacs development discussions."
>>>
>>>
>>> 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
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
On 2024-05-12 09:29, Eli Zaretskii wrote:
>> Cc: Ludovic Courtès , emacs-de...@gnu.org,
>> Andrew Tropin , guix-devel@gnu.org, bjorn.bi...@thaodan.de,
>> r...@constantly.at, felix.lech...@lease-up.com
>> Date: Sun, 12 May 2024 01:07:51 +0200
>> From: Nicolas Graves via "Emacs development discussions."
>>
>>
>> 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_LISTE
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
> Cc: Ludovic Courtès , emacs-de...@gnu.org,
> Andrew Tropin , guix-devel@gnu.org, bjorn.bi...@thaodan.de,
> r...@constantly.at, felix.lech...@lease-up.com
> Date: Sun, 12 May 2024 01:07:51 +0200
> From: Nicolas Graves via "Emacs development discussions."
>
>
> 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 termi
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
A lightly cleaned-up version attached.
>From 8ba972aec7026878bfbbe5cfb387c5c723672380 Mon Sep 17 00:00:00 2001
From: Nicolas Graves
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 000..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
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#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
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
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
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,$(
Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
> 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
Ludovic Courtès writes: > Stefan Monnier 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
Hi,
Nicolas Graves 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
Stefan Monnier 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
Hi, Björn Bidar skribis: > Stefan Monnier 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
Stefan Monnier 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
> 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
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
> 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
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
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
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
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’.
