From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Nicolas Graves via "Development of GNU Guix and the GNU System distribution." Newsgroups: gmane.comp.gnu.guix.devel,gmane.emacs.devel 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 Message-ID: <87eda7ula3.fsf@ngraves.fr> References: <20240410234923.29319-2-ngraves@ngraves.fr> <875xwotg35.fsf@trop.in> <87zfu0m9ps.fsf@ngraves.fr> <87jzl22u5w.fsf@gnu.org> <87frvp4a4u.fsf@ngraves.fr> <87msow5cm8.fsf@ngraves.fr> <87msow7xrs.fsf@ngraves.fr> <86bk5b1r1i.fsf@gnu.org> <87cyprbhap.fsf@ngraves.fr> <861q671idt.fsf@gnu.org> <8734qn9te1.fsf@ngraves.fr> Reply-To: Nicolas Graves Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="28721"; mail-complaints-to="usenet@ciao.gmane.io" 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 To: Eli Zaretskii Original-X-From: guix-devel-bounces+gcggd-guix-devel=m.gmane-mx.org@gnu.org Sun May 12 17:02:25 2024 Return-path: Envelope-to: gcggd-guix-devel@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1s6AiX-0007CP-GR for gcggd-guix-devel@m.gmane-mx.org; Sun, 12 May 2024 17:02:25 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1s6Ahy-0002nj-4q; Sun, 12 May 2024 11:01:50 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1s6Ahv-0002n7-Ar for guix-devel@gnu.org; Sun, 12 May 2024 11:01:49 -0400 Original-Received: from 8.mo581.mail-out.ovh.net ([46.105.77.114]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1s6Ahl-0005mP-Pn for guix-devel@gnu.org; Sun, 12 May 2024 11:01:46 -0400 Original-Received: from director11.ghost.mail-out.ovh.net (unknown [10.108.17.154]) by mo581.mail-out.ovh.net (Postfix) with ESMTP id 4Vcm6Q1t0yz1C5P for ; Sun, 12 May 2024 15:01:34 +0000 (UTC) Original-Received: from ghost-submission-6684bf9d7b-lhpkp (unknown [10.108.54.69]) by director11.ghost.mail-out.ovh.net (Postfix) with ESMTPS id C13EE1FE33; Sun, 12 May 2024 15:01:31 +0000 (UTC) Original-Received: from ngraves.fr ([37.59.142.110]) by ghost-submission-6684bf9d7b-lhpkp with ESMTPSA id 0+S0KMvZQGbiRAAAz2Rp0A (envelope-from ); Sun, 12 May 2024 15:01:31 +0000 Authentication-Results: garm.ovh; auth=pass (GARM-110S004a5d6a040-216a-4851-8afc-683d711d8fc5, 657DB11BEA81279387F13E7D9D19D75D6CB13AAA) smtp.auth=ngraves@ngraves.fr X-OVh-ClientIp: 176.158.104.228 In-Reply-To: <8734qn9te1.fsf@ngraves.fr> X-Ovh-Tracer-Id: 15694481752420901416 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedvledrvdegvddgkeefucetufdoteggodetrfdotffvucfrrhhofhhilhgvmecuqfggjfdpvefjgfevmfevgfenuceurghilhhouhhtmecuhedttdenucesvcftvggtihhpihgvnhhtshculddquddttddmnecujfgurhephffvvefujghffffkgggtsehmtderredttdejnecuhfhrohhmpefpihgtohhlrghsucfirhgrvhgvshcuoehnghhrrghvvghssehnghhrrghvvghsrdhfrheqnecuggftrfgrthhtvghrnhepfeehffdtvefhgfetveelhfduheeuteeiudeljedttefgvefgveejledtfeegueejnecuffhomhgrihhnpehmrghsthhoughonhdrshhotghirghlpdhgihhthhhusgdrtghomhenucfkphepuddvjedrtddrtddruddpudejiedrudehkedruddtgedrvddvkedpfeejrdehledrudegvddruddutdenucevlhhushhtvghrufhiiigvpedtnecurfgrrhgrmhepihhnvghtpeduvdejrddtrddtrddupdhmrghilhhfrhhomhepnhhgrhgrvhgvshesnhhgrhgrvhgvshdrfhhrpdhnsggprhgtphhtthhopedupdhrtghpthhtohepghhuihigqdguvghvvghlsehgnhhurdhorhhgpdfovfetjfhoshht pehmohehkedupdhmohguvgepshhmthhpohhuth Received-SPF: pass client-ip=46.105.77.114; envelope-from=ngraves@ngraves.fr; helo=8.mo581.mail-out.ovh.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, RCVD_IN_DNSWL_NONE=-0.0001, RCVD_IN_MSPIKE_H4=-0.01, RCVD_IN_MSPIKE_WL=-0.01, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=unavailable autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: guix-devel-bounces+gcggd-guix-devel=m.gmane-mx.org@gnu.org Original-Sender: guix-devel-bounces+gcggd-guix-devel=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.comp.gnu.guix.devel:70201 gmane.emacs.devel:319188 Archived-At: --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 2024-05-12 13:11, Nicolas Graves wrote: > On 2024-05-12 12:36, Eli Zaretskii wrote: > >>> From: Nicolas Graves >>> 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 >>>=20 >>> On 2024-05-12 09:29, Eli Zaretskii wrote: >>>=20 >>> > Thanks. What was the motivation for this, again? >>>=20 >>> A light summary (all is in the preceding exchanges in the mailing list): >>>=20 >>> - Ludovic Court=C3=A8s suggested this change because linking with syste= md is >>> unnecessary (very light usage), and increases the attack surface. >>>=20 >>> - 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. >>>=20 >>> 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. --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Implement-systemd-socket.patch >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_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 -# include -#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 #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 # include @@ -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 +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +/* 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 --=-=-= Content-Type: text/plain 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 --=-=-=--