all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Nicolas Graves via "Development of GNU Guix and the GNU System distribution." <guix-devel@gnu.org>
To: Eli Zaretskii <eliz@gnu.org>
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
Subject: Re: [Nicolas Graves] [PATCH v6 01/10] rde: emacs: Start emacs in --daemon mode, with shepherd and pid-file
Date: Sun, 12 May 2024 17:01:24 +0200	[thread overview]
Message-ID: <87eda7ula3.fsf@ngraves.fr> (raw)
In-Reply-To: <8734qn9te1.fsf@ngraves.fr>

[-- 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

  reply	other threads:[~2024-05-12 15:02 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20240410234923.29319-2-ngraves@ngraves.fr>
     [not found] ` <875xwotg35.fsf@trop.in>
2024-04-11 11:15   ` [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.
2024-04-12 20:38     ` Ludovic Courtès
2024-04-13 14:20       ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-04-13 15:09         ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-04-13 15:16         ` Stefan Monnier
2024-04-14 19:11           ` Björn Bidar
2024-04-14 20:52             ` Stefan Monnier
2024-04-19 14:19               ` Ludovic Courtès
2024-04-19 14:36                 ` Rudolf Schlatte
2024-04-20  2:31                   ` Stefan Monnier
2024-04-19 14:17             ` Ludovic Courtès
2024-05-11 20:15           ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-05-11 23:07             ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-05-12  6:29               ` Eli Zaretskii
2024-05-12  7:50                 ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-05-12  7:54                   ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-05-12  9:36                   ` Eli Zaretskii
2024-05-12 11:11                     ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-05-12 15:01                       ` Nicolas Graves via Development of GNU Guix and the GNU System distribution. [this message]
2024-04-13 16:50       ` Nicolas Graves via Development of GNU Guix and the GNU System distribution.
2024-04-19 14:25         ` Ludovic Courtès
2024-04-14 16:51       ` Felix Lechner via Development of GNU Guix and the GNU System distribution.

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87eda7ula3.fsf@ngraves.fr \
    --to=guix-devel@gnu.org \
    --cc=andrew@trop.in \
    --cc=bjorn.bidar@thaodan.de \
    --cc=eliz@gnu.org \
    --cc=emacs-devel@gnu.org \
    --cc=felix.lechner@lease-up.com \
    --cc=ludo@gnu.org \
    --cc=monnier@iro.umontreal.ca \
    --cc=ngraves@ngraves.fr \
    --cc=rudi@constantly.at \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/guix.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.