GNU bug report logs - #26084
24.5; epatch save wrong backups in Git multipatches

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 26084 in the body.
You can then email your comments to 26084 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.5; epatch save wrong backups in Git multipatches
Date: Mon, 13 Mar 2017 23:24:09 +0900

emacs -Q /tmp
;; Eval following form:
(let ((cmd "
mkdir -p foo
cd foo
echo 'qux here' > qux.txt
echo 'bar here' > bar.txt
git init
git add . && git commit -m 'Test repository.'
echo 'foo here' > qux.txt
echo 'foo here' > bar.txt
git diff > ../foo.diff
git reset --hard HEAD
"))
  (call-process-shell-command cmd))

M-x epatch RET n /tmp/foo.diff RET /tmp/foo RET n RET q y RET q y q y
;; Eval this form:
(string=
 (with-temp-buffer
   (insert-file-contents "/tmp/foo/bar.txt")
   (buffer-string))
(with-temp-buffer
  (insert-file-contents
   (concat "/tmp/foo/bar.txt" ediff-backup-extension))
  (buffer-string)))
=> t ; Should be nil.

--8<-----------------------------cut here---------------start------------->8---
From 0cb7eb3d3d7cb44051ea0739bf04e34fd8ec80e7 Mon Sep 17 00:00:00 2001
From: Tino Calancha <tino.calancha <at> gmail.com>
Date: Mon, 13 Mar 2017 23:21:43 +0900
Subject: [PATCH] epatch: Save right backups in Git multipatches

Multipatches on N Git files save wrong backups for
N-1 files; only the last one has a proper backup (Bug#26084).
* lisp/vc/diff-mode.el (diff-file-junk-re): Add 'Prereq: '
* lisp/vc/ediff-ptch.el (ediff-map-patch-buffer): Use 'diff-file-junk-re'.
* test/lisp/vc/ediff-ptch-tests.el (ediff-ptch-test-bug25010):
Rename from ibuffer-test-bug25010.
(ediff-ptch-test-bug26084): New test.
---
 lisp/vc/diff-mode.el             |  2 +-
 lisp/vc/ediff-ptch.el            | 13 ++++-----
 test/lisp/vc/ediff-ptch-tests.el | 59 +++++++++++++++++++++++++++++++++++++++-
 3 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index 31c33e6a72..aa8d77882e 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -504,7 +504,7 @@ diff-end-of-hunk
 ;; "index ", "old mode", "new mode", "new file mode" and
 ;; "deleted file mode" are output by git-diff.
 (defconst diff-file-junk-re
-  (concat "Index: \\|=\\{20,\\}\\|" ; SVN
+  (concat "Index: \\|Prereq: \\|=\\{20,\\}\\|" ; SVN
           "diff \\|index \\|\\(?:deleted file\\|new\\(?: file\\)?\\|old\\) mode\\|=== modified file"))
 
 ;; If point is in a diff header, then return beginning
diff --git a/lisp/vc/ediff-ptch.el b/lisp/vc/ediff-ptch.el
index 9d2ec51b59..36aebf4aed 100644
--- a/lisp/vc/ediff-ptch.el
+++ b/lisp/vc/ediff-ptch.el
@@ -25,6 +25,8 @@
 ;;; Code:
 
 
+(require 'diff-mode) ; For `diff-file-junk-re'.
+
 (provide 'ediff-ptch)
 
 (defgroup ediff-ptch nil
@@ -225,14 +227,11 @@ ediff-map-patch-buffer
 			(if (and beg2 end2)
 			    (buffer-substring beg2 end2)
 			  "/dev/null")))
-	    ;; check for any `Index:' or `Prereq:' lines, but don't use them
-	    (if (re-search-backward "^Index:" mark1-end 'noerror)
-		(move-marker mark2 (match-beginning 0)))
-	    (if (re-search-backward "^Prereq:" mark1-end 'noerror)
-		(move-marker mark2 (match-beginning 0)))
-
+            ;; Remove file junk (Bug#26084).
+            (while (re-search-backward
+                    (concat "^" diff-file-junk-re) mark1-end t)
+                (move-marker mark2 (match-beginning 0)))
 	    (goto-char mark2-end)
-
 	    (if filenames
 		(setq patch-map
 		      (cons (ediff-make-new-meta-list-element
diff --git a/test/lisp/vc/ediff-ptch-tests.el b/test/lisp/vc/ediff-ptch-tests.el
index 912c6b1e81..9aacb6bd20 100644
--- a/test/lisp/vc/ediff-ptch-tests.el
+++ b/test/lisp/vc/ediff-ptch-tests.el
@@ -22,7 +22,7 @@
 (require 'ert)
 (require 'ediff-ptch)
 
-(ert-deftest ibuffer-test-bug25010 ()
+(ert-deftest ediff-ptch-test-bug25010 ()
   "Test for http://debbugs.gnu.org/25010 ."
   (with-temp-buffer
     (insert "diff --git a/lisp/vc/ediff-ptch.el b/lisp/vc/ediff-ptch.el
@@ -38,5 +38,62 @@
              (match-string 1))))
       (should-not (string-suffix-p "@@" filename)))))
 
+
+(ert-deftest ediff-ptch-test-bug26084 ()
+  "Test for http://debbugs.gnu.org/26084 ."
+  (let* ((tmpdir temporary-file-directory)
+         (foo (expand-file-name "foo" tmpdir))
+         (patch (expand-file-name "foo.diff" tmpdir))
+         (qux (expand-file-name "qux.txt" foo))
+         (bar (expand-file-name "bar.txt" foo))
+         (cmd "
+mkdir -p foo
+cd foo
+echo 'qux here' > qux.txt
+echo 'bar here' > bar.txt
+git init
+git add . && git commit -m 'Test repository.'
+echo 'foo here' > qux.txt
+echo 'foo here' > bar.txt
+git diff > ../foo.diff
+git reset --hard HEAD
+"))
+    (setq default-directory tmpdir)
+    (call-process-shell-command cmd)
+    (find-file patch)
+    (unwind-protect
+        (let* ((info
+                (progn (ediff-map-patch-buffer (current-buffer)) ediff-patch-map))
+               (patch1
+                (buffer-substring-no-properties
+                 (car (nth 3 (car info)))
+                 (car (nth 4 (car info)))))
+               (patch2
+                (buffer-substring-no-properties
+                 (car (nth 3 (cadr info)))
+                 (car (nth 4 (cadr info))))))
+          ;; Apply both patches.
+          (dolist (x (list (cons patch1 bar) (cons patch2 qux)))
+            (with-temp-buffer
+              (insert (car x))
+              (call-shell-region (point-min)
+                                 (point-max)
+                                 (format "%s %s %s %s"
+                                         ediff-patch-program
+                                         ediff-patch-options
+                                         ediff-backup-specs
+                                         (cdr x)))))
+          ;; Check backup files were saved correctly.
+          (dolist (x (list qux bar))
+            (should-not (string= (with-temp-buffer
+                                   (insert-file-contents x)
+                                   (buffer-string))
+                                 (with-temp-buffer
+                                   (insert-file-contents (concat x ediff-backup-extension))
+                                   (buffer-string))))))
+      (delete-directory foo 'recursive)
+      (delete-file patch))))
+
+
 (provide 'ediff-ptch-tests)
 ;;; ediff-ptch-tests.el ends here
-- 
2.11.0

--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.8)
 of 2017-03-13
Repository revision: 7a50abee22581e02f0d822c3d9684c0985cdecb2

Message #10 received at 26084-done <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: 26084-done <at> debbugs.gnu.org
Subject: Re: bug#26084: 24.5; epatch save wrong backups in Git multipatches
Date: Tue, 21 Mar 2017 17:29:29 +0900

Tino Calancha <tino.calancha <at> gmail.com> writes:

> emacs -Q /tmp
> ;; Eval following form:
> (let ((cmd "
> mkdir -p foo
> cd foo
> echo 'qux here' > qux.txt
> echo 'bar here' > bar.txt
> git init
> git add . && git commit -m 'Test repository.'
> echo 'foo here' > qux.txt
> echo 'foo here' > bar.txt
> git diff > ../foo.diff
> git reset --hard HEAD
> "))
>   (call-process-shell-command cmd))
>
> M-x epatch RET n /tmp/foo.diff RET /tmp/foo RET n RET q y RET q y q y
> ;; Eval this form:
> (string=
>  (with-temp-buffer
>    (insert-file-contents "/tmp/foo/bar.txt")
>    (buffer-string))
> (with-temp-buffer
>   (insert-file-contents
>    (concat "/tmp/foo/bar.txt" ediff-backup-extension))
>   (buffer-string)))
> => t ; Should be nil.
Pushed fix into master branch as commit:
abe731eef0fcbc59ef4836e06f6099ea17042b90

Message #13 received at 26084 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 26084 <at> debbugs.gnu.org
Cc: Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#26084: closed (Re: bug#26084: 24.5;
 epatch save wrong backups in Git multipatches)
Date: Fri, 31 Mar 2017 18:54:43 -0400

FYI, the test for this has been failing on hydra since it was added.
Sadly Hydra is deleting build products very quickly at the moment,
but I managed to catch the latest coverage evaluation while it was in
progress. The not very informative failure message is:

Running 2 tests (2017-03-31 22:21:13+0000)
   passed  1/2  ediff-ptch-test-bug25010
Test ediff-ptch-test-bug26084 backtrace:
  (if (not (unwind-protect (setq value-17 (apply fn-15 args-16)) (setq
  (let (form-description-19) (if (not (unwind-protect (setq value-17 (
  (let ((value-17 (quote ert-form-evaluation-aborted-18))) (let (form-
  (let ((fn-15 (function string=)) (args-16 (list (let ((temp-buffer (
  (while --dolist-tail-- (setq x (car --dolist-tail--)) (let ((fn-15 (
  (let ((--dolist-tail-- (list qux bar)) x) (while --dolist-tail-- (se
  (let* ((info (progn (ediff-map-patch-buffer (current-buffer)) ediff-
  (unwind-protect (let* ((info (progn (ediff-map-patch-buffer (current
  (let* ((tmpdir (make-temp-file "ediff-ptch-test" t)) (default-direct
  (lambda nil (let ((fn-5 (function executable-find)) (args-6 (list "g
  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
  ert-run-test([cl-struct-ert-test ediff-ptch-test-bug26084 "Test for 
  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test e
  ert-run-tests(t #[385 "\306\307\"\203G\211\211G\310U\203\211@\20
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit(nil)
  eval((ert-run-tests-batch-and-exit nil))
  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/vc/ediff-ptch-tests.
  command-line()
  normal-top-level()
Test ediff-ptch-test-bug26084 condition:
    (ert-test-failed
     ((should-not
       (string=
	(with-temp-buffer ... ...)
	(with-temp-buffer ... ...)))
      :form
      (string= "foo here
" "foo here
")
      :value t))
   FAILED  2/2  ediff-ptch-test-bug26084

Ran 2 tests, 1 results as expected, 1 unexpected (2017-03-31 22:21:14+0000)

1 unexpected results:
   FAILED  ediff-ptch-test-bug26084

ERROR: lisp/vc/ediff-ptch-tests.log

Message #16 received at 26084 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 26084 <at> debbugs.gnu.org, Tino Calancha <tino.calancha <at> gmail.com>
Subject: Re: bug#26084: closed (Re: bug#26084: 24.5; epatch save wrong backups
 in Git multipatches)
Date: Sat, 1 Apr 2017 12:33:16 +0900 (JST)

Thank you for fixing this.
From now on i will look on Hydra whenver i write a new test.

On Fri, 31 Mar 2017, Glenn Morris wrote:

>
> FYI, the test for this has been failing on hydra since it was added.
> Sadly Hydra is deleting build products very quickly at the moment,
> but I managed to catch the latest coverage evaluation while it was in
> progress. The not very informative failure message is:
>
> Running 2 tests (2017-03-31 22:21:13+0000)
>   passed  1/2  ediff-ptch-test-bug25010
> Test ediff-ptch-test-bug26084 backtrace:
>  (if (not (unwind-protect (setq value-17 (apply fn-15 args-16)) (setq
>  (let (form-description-19) (if (not (unwind-protect (setq value-17 (
>  (let ((value-17 (quote ert-form-evaluation-aborted-18))) (let (form-
>  (let ((fn-15 (function string=)) (args-16 (list (let ((temp-buffer (
>  (while --dolist-tail-- (setq x (car --dolist-tail--)) (let ((fn-15 (
>  (let ((--dolist-tail-- (list qux bar)) x) (while --dolist-tail-- (se
>  (let* ((info (progn (ediff-map-patch-buffer (current-buffer)) ediff-
>  (unwind-protect (let* ((info (progn (ediff-map-patch-buffer (current
>  (let* ((tmpdir (make-temp-file "ediff-ptch-test" t)) (default-direct
>  (lambda nil (let ((fn-5 (function executable-find)) (args-6 (list "g
>  ert--run-test-internal([cl-struct-ert--test-execution-info [cl-struc
>  ert-run-test([cl-struct-ert-test ediff-ptch-test-bug26084 "Test for
>  ert-run-or-rerun-test([cl-struct-ert--stats t [[cl-struct-ert-test e
>  ert-run-tests(t #[385 "\306\307\"\203G\211\211G\310U\203\211@\20
>  ert-run-tests-batch(nil)
>  ert-run-tests-batch-and-exit(nil)
>  eval((ert-run-tests-batch-and-exit nil))
>  command-line-1(("-L" ":." "-l" "ert" "-l" "lisp/vc/ediff-ptch-tests.
>  command-line()
>  normal-top-level()
> Test ediff-ptch-test-bug26084 condition:
>    (ert-test-failed
>     ((should-not
>       (string=
> 	(with-temp-buffer ... ...)
> 	(with-temp-buffer ... ...)))
>      :form
>      (string= "foo here
> " "foo here
> ")
>      :value t))
>   FAILED  2/2  ediff-ptch-test-bug26084
>
> Ran 2 tests, 1 results as expected, 1 unexpected (2017-03-31 22:21:14+0000)
>
> 1 unexpected results:
>   FAILED  ediff-ptch-test-bug26084
>
> ERROR: lisp/vc/ediff-ptch-tests.log
>

Message #19 received at 26084 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Tino Calancha <tino.calancha <at> gmail.com>
Cc: 26084 <at> debbugs.gnu.org
Subject: Re: bug#26084: closed (Re: bug#26084: 24.5;
 epatch save wrong backups in Git multipatches)
Date: Sun, 02 Apr 2017 11:51:46 -0400

Tino Calancha wrote:

> Thank you for fixing this.

I didn't think I had, but perhaps that was because the new test failure
introduced just before in 1f5b4ed was masking things.

> From now on i will look on Hydra whenver i write a new test.

It's a nice system, when it's working right. Hopefully
https://github.com/NixOS/hydra/issues/456 will get some attention.

It mails the emacs-buildstatus list when failures occur, but sometimes
there is a delay (not sure why, list moderation?).

BTW, in 226cafd, call-process already does a PATH lookup when needed.
Also, the hard-coding of "-z.orig" means this test will fail with some
of the older "patch" versions that ediff supports (cf ediff-backup-specs).

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.