From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.ciao.gmane.io!not-for-mail From: Stefan Kangas Newsgroups: gmane.emacs.bugs Subject: bug#1474: 23.0.60; desktop.el don't check if pid in his lock file is always in use Date: Fri, 24 Jan 2020 17:47:29 +0100 Message-ID: <87v9p094ym.fsf@marxist.se> References: <87bpvufe28.fsf@tux.homenetwork> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="ciao.gmane.io:159.69.161.202"; logging-data="16348"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.0.50 (gnu/linux) Cc: 1474@debbugs.gnu.org, emacs@gentoo.org To: Thierry Volpiatto Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Fri Jan 24 17:48:15 2020 Return-path: Envelope-to: geb-bug-gnu-emacs@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 1iv27z-0004CT-6x for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 24 Jan 2020 17:48:15 +0100 Original-Received: from localhost ([::1]:44794 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iv27y-00086L-Ar for geb-bug-gnu-emacs@m.gmane-mx.org; Fri, 24 Jan 2020 11:48:14 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:43685) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1iv27o-00081O-Gv for bug-gnu-emacs@gnu.org; Fri, 24 Jan 2020 11:48:06 -0500 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1iv27m-0005z8-FI for bug-gnu-emacs@gnu.org; Fri, 24 Jan 2020 11:48:04 -0500 Original-Received: from debbugs.gnu.org ([209.51.188.43]:48720) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1iv27m-0005yc-2j for bug-gnu-emacs@gnu.org; Fri, 24 Jan 2020 11:48:02 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1iv27m-0001cF-1G for bug-gnu-emacs@gnu.org; Fri, 24 Jan 2020 11:48:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Kangas Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 24 Jan 2020 16:48:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 1474 X-GNU-PR-Package: emacs Original-Received: via spool by 1474-submit@debbugs.gnu.org id=B1474.15798844656174 (code B ref 1474); Fri, 24 Jan 2020 16:48:01 +0000 Original-Received: (at 1474) by debbugs.gnu.org; 24 Jan 2020 16:47:45 +0000 Original-Received: from localhost ([127.0.0.1]:54690 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iv27U-0001bV-Cv for submit@debbugs.gnu.org; Fri, 24 Jan 2020 11:47:44 -0500 Original-Received: from ted.gofardesign.uk ([67.225.143.91]:38552) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1iv27R-0001bE-Qj; Fri, 24 Jan 2020 11:47:42 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=marxist.se; s=default; h=Content-Type:MIME-Version:Message-ID:Date:References: In-Reply-To:Subject:Cc:To:From:Sender:Reply-To:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=vQCxccyoBSC9odqKHWEXMc64G+61xn3ztDHApE2MLZU=; b=Xe2esNmpfRCfW6fAl7YIpi3xTV uLmDK2bbAXGLmlW8izorHSvuNJyDUNU+vszIZUok3kDN0v3ofh+gtwczxNgiFbtwW0chGbKIuOlrU 7sw9CcZXv2sZW10DYGg05n0bvqw6gKkRtbJPwhXXp4QwBX7bqDqcaSMnYvOfxvpm4ha+iUwoJLa2j TlOYF4ShtLTZbHDJIZFrP4a+D7jZ8UjvsdBHxAzO+cu+fLwoGwl0a84g4xmVrXzo5JhLHFUcozvug KxCJqoA/kmUUSlkNAnHhZD3JVVunEJD7zpQNFPVZYZDpptrolE/Do+iaVmEOpldRTK+l99thX5bJK nducRPZQ==; Original-Received: from h-70-69.a785.priv.bahnhof.se ([155.4.70.69]:33248 helo=localhost) by ted.gofardesign.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES256-GCM-SHA384:256) (Exim 4.92) (envelope-from ) id 1iv27H-0015oc-2s; Fri, 24 Jan 2020 11:47:31 -0500 In-Reply-To: <87bpvufe28.fsf@tux.homenetwork> (Thierry Volpiatto's message of "Tue, 02 Dec 2008 22:43:43 +0100") X-AntiAbuse: This header was added to track abuse, please include it with any abuse report X-AntiAbuse: Primary Hostname - ted.gofardesign.uk X-AntiAbuse: Original Domain - debbugs.gnu.org X-AntiAbuse: Originator/Caller UID/GID - [47 12] / [47 12] X-AntiAbuse: Sender Address Domain - marxist.se X-Get-Message-Sender-Via: ted.gofardesign.uk: authenticated_id: stefan@marxist.se X-Authenticated-Sender: ted.gofardesign.uk: stefan@marxist.se X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.io gmane.emacs.bugs:175235 Archived-At: --=-=-= Content-Type: text/plain tags 1474 + patch thanks Thierry Volpiatto writes: > When killing emacs --daemon or emacs brutally with killall for example, > when reloading emacs or emacs --daemon, emacs ask if we want to load > desktop file or not (pid is already in use...). > I think emacs should check if the PID that is in his lock file is always > in use. I agree with the proposal, and have written up a suggested patch. This would work as follows (taken from my NEWS entry, see the suggested documentation for more): ** Emacs Sessions (Desktop) +++ *** New option to load if locking Emacs not running locally. The option 'desktop-load-locked-desktop' can now be set to value 'check', which means to load the desktop only if the locking Emacs process is not running on the local machine. See the "(emacs) Saving Emacs Sessions" node in the Emacs manual for details. The concerns with this proposal was that this breaks if Emacs is running on a remote machine. The user could have the lock file in a remoted directory (e.g. on an NFS mount). I have therefore documented in the manual and the doc string that this value should be avoided under such circumstances. Another idea suggested in this thread was to change the lock file to also include (system-name). That could be done, if it's deemed to be better, but has the drawback that a lock file from a recent Emacs would not be recognized by an old Emacs. (The relevant code reads the whole buffer.) A possible work-around is to add the backwards incompatible system name in a comment instead. Maybe that is much more desirable than simply documenting the limitation; it would be good to hear other opinions on that. Any comments or suggestions? Best regards, Stefan Kangas --=-=-= Content-Type: text/x-diff Content-Disposition: inline; filename=0001-Load-desktop-without-prompting-if-process-is-dead.patch >From 04c2d26df4f73be675cc9ea6aa2ce10a474ecd18 Mon Sep 17 00:00:00 2001 From: Stefan Kangas Date: Fri, 24 Jan 2020 05:12:20 +0100 Subject: [PATCH] Load desktop without prompting if process is dead * lisp/desktop.el (desktop-load-locked-desktop): Add new value 'check' to load desktop file without prompting if locking Emacs process does not exist on the local machine. (Bug#1474) (desktop-read): Extract function from here... (desktop--load-locked-desktop-p): ...to here. New function handles the semantics of 'desktop-load-locked-desktop', including above new value 'check'. (desktop--emacs-pid-running-p): New function. * test/lisp/desktop-tests.el: New file with tests for the above. * doc/emacs/misc.texi (Saving Emacs Sessions): Document the new 'check' value. * etc/NEWS: Announce the change. --- doc/emacs/misc.texi | 7 +++++- etc/NEWS | 9 +++++++ lisp/desktop.el | 48 +++++++++++++++++++++++++++--------- test/lisp/desktop-tests.el | 50 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 102 insertions(+), 12 deletions(-) create mode 100644 test/lisp/desktop-tests.el diff --git a/doc/emacs/misc.texi b/doc/emacs/misc.texi index 6b95b12a84..bedbfb7abe 100644 --- a/doc/emacs/misc.texi +++ b/doc/emacs/misc.texi @@ -2653,7 +2653,12 @@ Saving Emacs Sessions can avoid the question by customizing the variable @code{desktop-load-locked-desktop} to either @code{nil}, which means never load the desktop in this case, or @code{t}, which means load the -desktop without asking. +desktop without asking. Finally, the @code{check} value means to load +the file if the Emacs process that has locked the desktop is not +running on the local machine. This should not be used in +circumstances where the locking Emacs might still be running on +another machine. This could be the case in multi-user environments +where your home directory is mounted remotely using NFS or similar. @cindex desktop restore in daemon mode When Emacs starts in daemon mode, it cannot ask you any questions, diff --git a/etc/NEWS b/etc/NEWS index 11ef31b2c8..de39912e90 100644 --- a/etc/NEWS +++ b/etc/NEWS @@ -90,6 +90,15 @@ supplied error message. *** New connection method "media", which allows accessing media devices like cell phones, tablets or cameras. +** Emacs Sessions (Desktop) + ++++ +*** New option to load if locking Emacs not running locally. +The option 'desktop-load-locked-desktop' can now be set to value +'check', which means to load the desktop only if the locking Emacs +process is not running on the local machine. See the "(emacs) Saving +Emacs Sessions" node in the Emacs manual for details. + * New Modes and Packages in Emacs 28.1 diff --git a/lisp/desktop.el b/lisp/desktop.el index 9538bb4a34..27f6c80531 100644 --- a/lisp/desktop.el +++ b/lisp/desktop.el @@ -230,16 +230,25 @@ desktop-auto-save-timeout (defcustom desktop-load-locked-desktop 'ask "Specifies whether the desktop should be loaded if locked. Possible values are: - t -- load anyway. - nil -- don't load. - ask -- ask the user. -If the value is nil, or `ask' and the user chooses not to load the desktop, -the normal hook `desktop-not-loaded-hook' is run." + t -- load anyway. + nil -- don't load. + ask -- ask the user. + check -- load if locking Emacs process is missing locally. + +If the value is nil, or `ask' and the user chooses not to load +the desktop, the normal hook `desktop-not-loaded-hook' is run. + +If the value is `check', load the desktop if the Emacs process +that has locked it is not running on the local machine. This +should not be used in circumstances where the locking Emacs might +still be running on another machine. That could be the case if +you have remotely mounted (NFS) paths in `desktop-dirname'." :type '(choice (const :tag "Load anyway" t) (const :tag "Don't load" nil) - (const :tag "Ask the user" ask)) + (const :tag "Ask the user" ask) + (const :tag "Load if no local process" check)) :group 'desktop :version "22.2") @@ -662,6 +671,27 @@ desktop-owner (integerp owner))) owner))) +(defun desktop--emacs-pid-running-p (pid) + "Return t if an Emacs process with PID exists." + (when-let ((attr (process-attributes pid))) + (string-match "^emacs$" (alist-get 'comm attr)))) + +(defun desktop--load-locked-desktop-p (owner) + "Return t if a locked desktop should be loaded. +OWNER is the pid in the lock file. +The return value of this function depends on the value of +`desktop-load-locked-desktop'." + (pcase desktop-load-locked-desktop + ('ask + (unless (daemonp) + (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\ +Using it may cause conflicts. Use it anyway? " owner)))) + ('check + (or (eq (emacs-pid) owner) + (not (desktop--emacs-pid-running-p owner)))) + ('nil nil) + (_ t))) + (defun desktop-claim-lock (&optional dirname) "Record this Emacs process as the owner of the desktop file in DIRNAME. DIRNAME omitted or nil means use `desktop-dirname'." @@ -1238,11 +1268,7 @@ desktop-read (desktop-save nil) (desktop-autosave-was-enabled)) (if (and owner - (memq desktop-load-locked-desktop '(nil ask)) - (or (null desktop-load-locked-desktop) - (daemonp) - (not (y-or-n-p (format "Warning: desktop file appears to be in use by PID %s.\n\ -Using it may cause conflicts. Use it anyway? " owner))))) + (not (desktop--load-locked-desktop-p owner))) (let ((default-directory desktop-dirname)) (setq desktop-dirname nil) (run-hooks 'desktop-not-loaded-hook) diff --git a/test/lisp/desktop-tests.el b/test/lisp/desktop-tests.el new file mode 100644 index 0000000000..7483bb8adb --- /dev/null +++ b/test/lisp/desktop-tests.el @@ -0,0 +1,50 @@ +;;; desktop-tests.el --- Tests for desktop.el -*- lexical-binding: t -*- + +;; Copyright (C) 2020 Free Software Foundation, Inc. + +;; This file is part of GNU Emacs. + +;; GNU Emacs is free software: you can redistribute it and/or modify +;; it under the terms of the GNU General Public License as published by +;; the Free Software Foundation, either version 3 of the License, or +;; (at your option) any later version. + +;; GNU Emacs is distributed in the hope that it will be useful, +;; but WITHOUT ANY WARRANTY; without even the implied warranty of +;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +;; GNU General Public License for more details. + +;; You should have received a copy of the GNU General Public License +;; along with GNU Emacs. If not, see . + +;;; Commentary: + +;;; Code: + +(require 'ert) +(require 'desktop) + +(ert-deftest desktop-tests--emacs-pid-running-p () + (should (desktop--emacs-pid-running-p (emacs-pid))) + (should-not (desktop--emacs-pid-running-p 1))) + +(ert-deftest desktop-tests--load-locked-desktop-p () + (let ((desktop-load-locked-desktop t)) + (should (desktop--load-locked-desktop-p (emacs-pid))))) + +(ert-deftest desktop-tests--load-locked-desktop-p-nil () + (let ((desktop-load-locked-desktop nil)) + (should-not (desktop--load-locked-desktop-p (emacs-pid))))) + +(ert-deftest desktop-tests--load-locked-desktop-p-ask () + (let ((desktop-load-locked-desktop 'ask)) + (cl-letf (((symbol-function 'y-or-n-p) (lambda (&rest _) t))) + (should (desktop--load-locked-desktop-p (emacs-pid)))) + (cl-letf (((symbol-function 'y-or-n-p) (lambda (&rest _) nil))) + (should-not (desktop--load-locked-desktop-p (emacs-pid)))))) + +(ert-deftest desktop-tests--load-locked-desktop-p-check () + (let ((desktop-load-locked-desktop 'check)) + (desktop--load-locked-desktop-p (emacs-pid)))) + +(provide 'desktop-tests) -- 2.20.1 --=-=-=--