bug#18336: 24.4.50; When editing externally changed file, Emacs asks too many questions

[Noam Postavsky]

tags 18336 + patch
quit

Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> I'm not sure.  Why exactly are we calling lock_file from write_region?
>
> Because write-region can be called from an unmodified buffer, I
> suppose, and from a buffer whose buffer-file-name is not the same as
> the file being written to.

So maybe we could just check those conditions?  Here's a patch,
tentatively suggested for master.

>From ae6a82e1b10ce850731fbc7135611108cd2bd147 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Sat, 21 Mar 2020 21:00:08 -0400
Subject: [PATCH] Avoid extra "changed on disk" prompt in save-buffer
 (Bug#18336)

* src/fileio.c (write_region): Only call lock_file is the buffer is
unmodified or writing to some file other than the buffer's file.  As
mentioned in lock_file's commentary, calling it for modified buffers
is redundant (this reasoning only applies when writing to the buffer's
file).
---
 src/fileio.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/fileio.c b/src/fileio.c
index 482f88627a..323faa2e77 100644
--- a/src/fileio.c
+++ b/src/fileio.c
@@ -5123,7 +5123,12 @@ write_region (Lisp_Object start, Lisp_Object end, 
Lisp_Object filename,
     = choose_write_coding_system (start, end, filename,
                                  append, visit, lockname, &coding);
 
-  if (open_and_close_file && !auto_saving)
+  if (open_and_close_file && !auto_saving &&
+      /* Only call lock_file on unmodifed buffer (see lock_file
+         commentary).  */
+      (NILP (Fbuffer_modified_p (Qnil)) ||
+       /* Or if we're not writing to the buffer's file.  */
+       NILP (Fstring_equal (visit_file, BVAR (current_buffer, filename)))))
     {
       lock_file (lockname);
       file_locked = 1;
-- 
2.11.0

> Evidently, when we disabled backups in /tmp, we missed the fact that
> lock_file called from write-region will be affected.  When backups are
> not disabled, this works correctly.

To me, the fact that it works with backups enabled seems like an
accident.  The connection between locking and backups isn't very
obvious, and I don't see any commentary about it (which I would expect
for such a subtle interaction if it was done on purpose).