Subject: bug#3348: 23.0.92.1; Ediff: Creating a multi-file patch

From: Mauro Aranda <[email-masked]>
Date: Sat, 1 Mar 2025 08:02:57 -0300

Mauro Aranda <[email-masked]> writes:

 > Noam Postavsky <[email-masked]> writes:
 >
 >> tag 3348 + confirmed
 >> severity 3348 minor
 >> found 3348 25.0.94
 >> quit
 >>
 >> Confirmed this is still an issue. Here is an elisp script for easier
 >> reproduction (run as emacs -Q -l bug-3348.el):
 >>
 >> (defconst bug-3348-test-dir
 >>   (expand-file-name "bug-3348-testdir" temporary-file-directory))
 >> (make-directory bug-3348-test-dir t)
 >> (cd bug-3348-test-dir)
 >>
 >> (make-directory "dir-a" t)
 >> (make-directory "dir-b" t)
 >>
 >> (with-temp-file "dir-a/file"
 >>   (insert "aaa"))
 >> (with-temp-file "dir-b/file"
 >>   (insert "bbb"))
 >>
 >> (ediff-directories "dir-a" "dir-b" nil)
 >> (switch-to-buffer "*Ediff Session Group Panel*")
 >>
 >> (ediff-next-meta-item 1)
 >> (ediff-mark-for-operation-at-pos nil)
 >> (ediff-collect-custom-diffs)
 >>
 >> (with-current-buffer "*Ediff Multifile Diffs*"
 >>   (write-file "foo.patch"))
 >>
 >> (with-temp-file "dir-b/file"
 >>   (insert "BBB"))
 >>
 >> (ediff-collect-custom-diffs)
 >>
 >> (switch-to-buffer "foo.patch")
 >> (diff-buffer-with-file)
 >> (message (concat "Observe foo.patch buffer is different from its 
file,\n"
 >>                  "but is marked read-only, not modified"))
 >
 > This still happens, but I'm not really sure if this should be called a
 > bug.  ediff-collect-custom-diffs explicitly marks this buffer as not
 > modified with (set-buffer-modified-p nil).  I can't find the reasons for
 > that, but that doesn't mean there isn't a good one.  And it is
 > consistent about doing that.
 >
 > The first call to ediff-collect-custom-diffs creates the buffer
 > *Ediff Multifile Diffs* and also marks it as unmodified, even though it
 > is.  So, why is that not a problem? Why is the second time a problem?
 >
 > Using write-file again is possible, so what are the expectations of the
 > second time of typing `P' (or calling ediff-collect-custom-diffs) and
 > why do they differ from the expectations for the 1st call?

After a year and a half, I think I got it :-)

After the call to write-file, ediff-meta-diff-buffer is visting a file,
and ediff assumes it never visits a file: that's why it erases, refills
it, and marks it as not modified.

So I think the attached patch fixes it.
From e1bc504704f4be71db28e355c72a5916854c7f35 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <[email-masked]>
Date: Sat, 1 Mar 2025 07:42:24 -0300
Subject: [PATCH] Guard against user saving ediff-meta-diff-buffer

* lisp/vc/ediff-mult.el (ediff-collect-custom-diffs): If the
diff buffer is visiting a file, create a new buffer.  (Bug#3348)

* test/lisp/vc/ediff-mult-tests.el: New test file.
---
 lisp/vc/ediff-mult.el            |  9 +++++-
 test/lisp/vc/ediff-mult-tests.el | 54 ++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+), 1 deletion(-)
 create mode 100644 test/lisp/vc/ediff-mult-tests.el

diff --git a/lisp/vc/ediff-mult.el b/lisp/vc/ediff-mult.el
index 493e98f76b7..ade20a4a299 100644
--- a/lisp/vc/ediff-mult.el
+++ b/lisp/vc/ediff-mult.el
@@ -1719,7 +1719,14 @@ ediff-collect-custom-diffs
 all marked sessions must be active."
   (interactive)
   (let ((coding-system-for-read ediff-coding-system-for-read))
-    (or (ediff-buffer-live-p ediff-meta-diff-buffer)
+    (unless (and (ediff-buffer-live-p ediff-meta-diff-buffer)
+                 ;; We assume `ediff-meta-diff-buffer' doesn't
+                 ;; visit any file.  But if the user saves the
+                 ;; `ediff-meta-diff-buffer' to a file, that
+                 ;; assumption isn't right anymore.  (Bug#3348)
+                 ;; So, if `ediff-meta-diff-buffer' is visiting some
+                 ;; file, create a new buffer rather than reusing it.
+                 (not (buffer-file-name ediff-meta-diff-buffer)))
 	(setq ediff-meta-diff-buffer
 	      (get-buffer-create
 	       (ediff-unique-buffer-name "*Ediff Multifile Diffs" "*"))))
diff --git a/test/lisp/vc/ediff-mult-tests.el b/test/lisp/vc/ediff-mult-tests.el
new file mode 100644
index 00000000000..525088bf856
--- /dev/null
+++ b/test/lisp/vc/ediff-mult-tests.el
@@ -0,0 +1,54 @@
+;;; ediff-mult-tests.el --- Tests for ediff-mult.el  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2025 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 <https://www.gnu.org/licenses/>.
+
+;;; Code:
+
+(require 'ert)
+(require 'ediff-mult)
+
+(ert-deftest ediff-test-bug3348 ()
+  "After saving `ediff-meta-diff-buffer' to a file, we should not reuse it."
+  (let ((test-dir
+         (expand-file-name "bug-3348-testdir" temporary-file-directory)))
+    (make-directory test-dir t)
+    (cd test-dir)
+
+    (make-directory "dir-a" t)
+    (make-directory "dir-b" t)
+
+    (with-temp-file "dir-a/file"
+      (insert "aaa\n"))
+    (with-temp-file "dir-b/file"
+      (insert "bbb\n"))
+
+    (ediff-directories "dir-a" "dir-b" nil)
+    (switch-to-buffer "*Ediff Session Group Panel*")
+
+    (ediff-next-meta-item 1)
+    (ediff-mark-for-operation-at-pos nil)
+    (ediff-collect-custom-diffs)
+
+    (with-current-buffer "*Ediff Multifile Diffs*"
+      (write-file "foo.patch"))
+
+    (with-temp-file "dir-b/file"
+      (insert "BBB\n"))
+    (ediff-collect-custom-diffs)
+
+    (should-not (equal ediff-meta-diff-buffer (get-buffer "foo.patch")))))
-- 
2.34.1