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 09:54:37 +0200 Message-ID: <87a5kvbh36.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> Reply-To: Nicolas Graves Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="32483"; 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 09:55:03 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 1s642x-0008CE-3V for gcggd-guix-devel@m.gmane-mx.org; Sun, 12 May 2024 09:55:03 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1s642g-0001bR-Gt; Sun, 12 May 2024 03:54:46 -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 1s642f-0001bG-Jj for guix-devel@gnu.org; Sun, 12 May 2024 03:54:45 -0400 Original-Received: from 16.mo584.mail-out.ovh.net ([188.165.55.104]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1s642d-0004fE-1U for guix-devel@gnu.org; Sun, 12 May 2024 03:54:45 -0400 Original-Received: from director3.ghost.mail-out.ovh.net (unknown [10.108.2.251]) by mo584.mail-out.ovh.net (Postfix) with ESMTP id 4VcZdr3FmDz1BQd for ; Sun, 12 May 2024 07:54:40 +0000 (UTC) Original-Received: from ghost-submission-6684bf9d7b-m7hfg (unknown [10.110.178.33]) by director3.ghost.mail-out.ovh.net (Postfix) with ESMTPS id 5AA4B1FDB0; Sun, 12 May 2024 07:54:38 +0000 (UTC) Original-Received: from ngraves.fr ([37.59.142.102]) by ghost-submission-6684bf9d7b-m7hfg with ESMTPSA id C9z6Er51QGZqFx0AbQBy3A (envelope-from ); Sun, 12 May 2024 07:54:38 +0000 Authentication-Results: garm.ovh; auth=pass (GARM-102R00468b4be3a-1760-4001-b2bf-b74521007d9d, 657DB11BEA81279387F13E7D9D19D75D6CB13AAA) smtp.auth=ngraves@ngraves.fr X-OVh-ClientIp: 176.191.136.151 In-Reply-To: <87cyprbhap.fsf@ngraves.fr> X-Ovh-Tracer-Id: 8484781698258297384 X-VR-SPAMSTATE: OK X-VR-SPAMSCORE: -100 X-VR-SPAMCAUSE: gggruggvucftvghtrhhoucdtuddrgedvledrvdeguddguddufecutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfqggfjpdevjffgvefmvefgnecuuegrihhlohhuthemucehtddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenucfjughrpefhvfevufgjfhffkfggtgfgsehtqhertddttdejnecuhfhrohhmpefpihgtohhlrghsucfirhgrvhgvshcuoehnghhrrghvvghssehnghhrrghvvghsrdhfrheqnecuggftrfgrthhtvghrnhepieevtdeftdegfefghedvjeejudffgedtgffhvdeiffettdevieeuhfdviefhteffnecuffhomhgrihhnpehmrghsthhoughonhdrshhotghirghlpdhgihhthhhusgdrtghomhdpshihshhtvghmugdrihhopdhfrhgvvgguvghskhhtohhprdhorhhgnecukfhppeduvdejrddtrddtrddupddujeeirdduledurddufeeirdduhedupdefjedrheelrddugedvrddutddvnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghmpehinhgvthepuddvjedrtddrtddruddpmhgrihhlfhhrohhmpehnghhrrghvvghssehnghhrrghvvghsrdhfrhdpnhgspghrtghpthhtohepuddprhgtphht thhopehguhhigidquggvvhgvlhesghhnuhdrohhrghdpoffvtefjohhsthepmhhoheekgedpmhhouggvpehsmhhtphhouhht Received-SPF: pass client-ip=188.165.55.104; envelope-from=ngraves@ngraves.fr; helo=16.mo584.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_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham 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:70198 gmane.emacs.devel:319178 Archived-At: On 2024-05-12 09:50, Nicolas Graves wrote: > On 2024-05-12 09:29, Eli Zaretskii wrote: > >>> Cc: Ludovic Court=C3=A8s , emacs-devel@gnu.org, >>> Andrew Tropin , guix-devel@gnu.org, bjorn.bidar@thaoda= n.de, >>> rudi@constantly.at, felix.lechner@lease-up.com >>> Date: Sun, 12 May 2024 01:07:51 +0200 >>> From: Nicolas Graves via "Emacs development discussions." >>>=20 >>> 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=C3=A8s 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/ =20 sd_notify is definitely given as a stable interface.=20 > > 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=3Dno >>> -if test "${with_libsystemd}" =3D "yes" ; then >>> - dnl This code has been tested with libsystemd 222 and later. >>> - dnl FIXME: Find the earliest version number for which Emacs should w= ork, >>> - dnl and change '222' to that number. >>> - EMACS_CHECK_MODULES([LIBSYSTEMD], [libsystemd >=3D 222], >>> - [HAVE_LIBSYSTEMD=3Dyes], [HAVE_LIBSYSTEMD=3Dno]) >>> - if test "${HAVE_LIBSYSTEMD}" =3D "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=3Dno >>> JSON_OBJ=3D >>>=20=20 >>> @@ -6652,7 +6636,7 @@ AC_DEFUN >>> optsep=3D >>> emacs_config_features=3D >>> for opt in ACL BE_APP CAIRO DBUS FREETYPE GCONF GIF GLIB GMP GNUTLS GP= M GSETTINGS \ >>> - HARFBUZZ IMAGEMAGICK JPEG JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIB= XML2 \ >>> + 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_LIBOT= F} >>> Does Emacs use -lxft? ${HAVE_XFT} >>> - Does Emacs use -lsystemd? ${HAVE_LIBSY= STEMD} >>> 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.=20 >> >>> -#ifdef HAVE_LIBSYSTEMD >>> /* Read the number of sockets passed through by systemd. */ >>> - int systemd_socket =3D sd_listen_fds (1); >>> - >>> - if (systemd_socket > 1) >>> - fputs (("\n" >>> - "Warning: systemd passed more than one socket to Emacs.\n" >>> - "Try 'Accept=3Dfalse' in the Emacs socket unit file.\n"), >>> - stderr); >>> - else if (systemd_socket =3D=3D 1 >>> - && (0 < sd_is_socket (SD_LISTEN_FDS_START, >>> - AF_UNSPEC, SOCK_STREAM, 1))) >>> - sockfd =3D SD_LISTEN_FDS_START; >>> -#endif /* HAVE_LIBSYSTEMD */ >>> + const char *fds =3D getenv("LISTEN_FDS"); >>> + if (fds) { >>> + int systemd_socket =3D strtol(fds, NULL, 0); >>> + if (systemd_socket > 1) >>> + fputs (("\n" >>> + "Warning: systemd passed more than one socket to Emacs.\n" >>> + "Try 'Accept=3Dfalse' in the Emacs socket unit file.\n"), >>> + stderr); >>> + else if (systemd_socket =3D=3D 1 >>> + && (0 < sd_is_socket (SD_LISTEN_FDS_START, SOCK_STREAM, 1))) >>> + sockfd =3D SD_LISTEN_FDS_START; >>> + } >> >> The new code to interface with systemd cannot be compiled >> unconditionally, it should have some #ifdef condition, determined by >> the configure script, because not all the platforms we support can use >> systemd. > > Same point. >> >>> @@ -2857,12 +2854,15 @@ DEFUN ("kill-emacs", Fkill_emacs, Skill_emacs, = 0, 2, "P", >>> } >>> #endif >>>=20=20 >>> -#ifdef HAVE_LIBSYSTEMD >>> /* Notify systemd we are shutting down, but only if we have notified >>> it about startup. */ >>> - if (daemon_type =3D=3D -1) >>> - sd_notify(0, "STOPPING=3D1"); >>> -#endif /* HAVE_LIBSYSTEMD */ >>> + if (daemon_type =3D=3D -1){ >>> + int r =3D sd_notify_stopping(); >>> + if (r < 0) { >>> + fprintf(stderr, "Failed to report termination to $NOTIFY_SOCKET:= %s\n", strerror(-r)); >>> + exit (EXIT_FAILURE); >>> + } >>> + } >> >> Same here. >> >>> /* Fsignal calls emacs_abort () if it sees that waiting_for_input is >>> set. */ >>> @@ -3382,9 +3382,11 @@ DEFUN ("daemon-initialized", Fdaemon_initialized= , Sdaemon_initialized, 0, 0, 0, >>>=20=20 >>> if (daemon_type =3D=3D 1) >>> { >>> -#ifdef HAVE_LIBSYSTEMD >>> - sd_notify(0, "READY=3D1"); >>> -#endif /* HAVE_LIBSYSTEMD */ >>> + int r =3D sd_notify_ready(); >>> + if (r < 0) { >>> + fprintf(stderr, "Failed to notify readiness to $NOTIFY_SOCKET: %s\n",= strerror(-r)); >>> + exit (EXIT_FAILURE); >>> + } >>> } >> >> And here. >> >>> > Seems to work properly on my side with the following patch, on Guix w= ith >>> > the shepherd service I sent a few weeks ago. The patch is actually >>> > pretty simple. >> >> How sure we are that the code you wrote will not need any significant >> maintenance due to changes in how systemd works? The significant >> advantage of using libsystemd is that the systemd developers take care >> of updating it as needed. With these changes, that burden is now on >> us. I don't think we'd be happy maintaining system-level code that >> have very little relevance to Emacs. >> >>> > - The sd_notify code is taken from >>> > https://www.freedesktop.org/software/systemd/man/devel/sd_notify.html= #Notes >> >> I'm not sure what would be the legal aspects of such code borrowing. > > The code is given as MIT-0, hence also the two different licenses for > the two functions sd_notify and sd_is_socket. Not an expert on licenses > either, but with a proper flag about what this function's license is, I > guess it should be fine, since other projects also do that. --=20 Best regards, Nicolas Graves