GNU bug report logs - #29366
gitmerge to handle NEWS better

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 29366 in the body.
You can then email your comments to 29366 AT debbugs.gnu.org in the normal way.

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

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

From: Glenn Morris <rgm <at> gnu.org>
To: submit <at> debbugs.gnu.org
Subject: gitmerge to handle NEWS better
Date: Mon, 20 Nov 2017 13:16:41 -0500

Package: emacs
Version: 25.3
Severity: wishlist

Hi,

Thanks for gitmerge.el, it's a great help.
It would be nice if it could handle NEWS better.
Currently etc/NEWS in the emacs-26 branch has been renamed to
etc/NEWS.26 in master. Running M-x gitmerge, this is not detected.
emacs-26:etc/NEWS changes are merged to master:etc/NEWS, resulting in a
merge conflict every time NEWS is touched in emacs-26.
Can gitmerge do something to help here?

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

From: Glenn Morris <rgm <at> gnu.org>
To: 29366 <at> debbugs.gnu.org
Cc: monnier <at> iro.umontreal.ca, deng <at> randomsample.de
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Mon, 11 Dec 2017 14:11:50 -0500

Glenn Morris wrote:

> Currently etc/NEWS in the emacs-26 branch has been renamed to
> etc/NEWS.26 in master. Running M-x gitmerge, this is not detected.
> emacs-26:etc/NEWS changes are merged to master:etc/NEWS, resulting in a
> merge conflict every time NEWS is touched in emacs-26.
> Can gitmerge do something to help here?

I added some code for this in 0b6f4f2 that I thought worked.
There's pre-existing gitmerge code that uses smerge to try and
auto-resolve conflicts. I just added a special-case for NEWS at the same
place.

However, it's missing a step that will automatically continue the merge
(ie, do the equivalent of running M-x gitmerge after manually fixing a
conflict). I cannot see how this would work for the "smerge fixed a
conflict automatically" case either. I don't know how to simulate an
smerge-fixable conflict to test.

Am I overlooking something, or is the code missing something like the
following:

--- a/admin/gitmerge.el
+++ b/admin/gitmerge.el
@@ -431,7 +431,8 @@ gitmerge-resolve-unmerged
 	      (setq conflicted t)
 	    ;; Mark as resolved
 	    (call-process "git" nil t nil "add" file)))
-	(when conflicted
+	(if (not conflicted)
+	    (if files (gitmerge-maybe-resume 'noask))
 	  (with-current-buffer (get-buffer-create gitmerge-warning-buffer)
 	    (erase-buffer)
 	    (insert "For the following files, conflicts could\n"
@@ -457,7 +458,7 @@ gitmerge-repo-clean
 		    "diff" "--name-only")
       (zerop (buffer-size))))
 
-(defun gitmerge-maybe-resume ()
+(defun gitmerge-maybe-resume (&optional noask)
   "Check if we have to resume a merge.
 If so, add no longer conflicted files and commit."
   (let ((mergehead (file-exists-p
@@ -469,7 +470,7 @@ gitmerge-maybe-resume
 	       (not (gitmerge-repo-clean)))
       (user-error "Repository is not clean"))
     (when statusexist
-      (if (not (y-or-n-p "Resume merge? "))
+      (if (not (or noask (y-or-n-p "Resume merge? ")))
 	  (progn
 	    (delete-file gitmerge-status-file)
 	    ;; No resume.

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

From: David Engster <deng <at> randomsample.de>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 29366 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Tue, 12 Dec 2017 00:18:33 +0100

Hi Glenn,

I'm not sure about that smerge stuff either, that was something I took
over from bzrmerge. It may be that it is not functional. I hope I can
make time to take a look at this in the coming days.

-David

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

From: Glenn Morris <rgm <at> gnu.org>
To: David Engster <deng <at> randomsample.de>
Cc: 29366 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Mon, 11 Dec 2017 18:33:32 -0500

David Engster wrote:

> I'm not sure about that smerge stuff either, that was something I took
> over from bzrmerge. It may be that it is not functional. I hope I can
> make time to take a look at this in the coming days.

Thanks! Here's my revised patch for this. It just does a commit after a
successful auto-conflict resolution. It seems to work...

--- a/admin/gitmerge.el
+++ b/admin/gitmerge.el
@@ -316,11 +316,7 @@ gitmerge-resolve
                                    (gitmerge-emacs-version gitmerge--from))))
                    (file-exists-p temp)
                    (or noninteractive
-                       (and
-                        (y-or-n-p "Try to fix NEWS conflict? ")
-                        ;; FIXME
-                        (y-or-n-p "This is buggy, really try? ")
-                        )))
+                       (y-or-n-p "Try to fix NEWS conflict? ")))
               (let ((relfile (file-name-nondirectory file))
                     (tempfile (make-temp-file "gitmerge")))
                 (unwind-protect
@@ -431,7 +427,9 @@ gitmerge-resolve-unmerged
 	      (setq conflicted t)
 	    ;; Mark as resolved
 	    (call-process "git" nil t nil "add" file)))
-	(when conflicted
+	(if (not conflicted)
+	    (and files (not (gitmerge-commit))
+		 (error "Error committing resolution - fix it manually"))
 	  (with-current-buffer (get-buffer-create gitmerge-warning-buffer)
 	    (erase-buffer)
 	    (insert "For the following files, conflicts could\n"
@@ -457,6 +455,12 @@ gitmerge-repo-clean
 		    "diff" "--name-only")
       (zerop (buffer-size))))
 
+(defun gitmerge-commit ()
+  "Commit, and return non-nil if it succeeds."
+  (with-current-buffer (get-buffer-create gitmerge-output-buffer)
+    (erase-buffer)
+    (eq 0 (call-process "git" nil t nil "commit" "--no-edit"))))
+
 (defun gitmerge-maybe-resume ()
   "Check if we have to resume a merge.
 If so, add no longer conflicted files and commit."
@@ -478,11 +482,8 @@ gitmerge-maybe-resume
 	(gitmerge-resolve-unmerged)
 	;; Commit the merge.
 	(when mergehead
-	  (with-current-buffer (get-buffer-create gitmerge-output-buffer)
-	    (erase-buffer)
-	    (unless (zerop (call-process "git" nil t nil
-					 "commit" "--no-edit"))
-	      (error "Git error during merge - fix it manually"))))
+	  (or (gitmerge-commit)
+	      (error "Git error during merge - fix it manually")))
 	;; Successfully resumed.
 	t))))
 

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

From: David Engster <deng <at> randomsample.de>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 29366 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Tue, 12 Dec 2017 21:10:42 +0100

Glenn Morris writes:
> David Engster wrote:
>
>> I'm not sure about that smerge stuff either, that was something I took
>> over from bzrmerge. It may be that it is not functional. I hope I can
>> make time to take a look at this in the coming days.
>
> Thanks! Here's my revised patch for this. It just does a commit after a
> successful auto-conflict resolution. It seems to work...

Indeed, that final commit was missing. It works for me as well.

-David

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

From: Glenn Morris <rgm <at> gnu.org>
To: David Engster <deng <at> randomsample.de>
Cc: 29366 <at> debbugs.gnu.org, monnier <at> iro.umontreal.ca
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Tue, 12 Dec 2017 22:06:33 -0500

David Engster wrote:

> Indeed, that final commit was missing. It works for me as well.

Thanks for checking.

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

From: Glenn Morris <rgm <at> gnu.org>
To: 29366 <at> debbugs.gnu.org
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Tue, 23 Oct 2018 13:10:09 -0400

Today I noticed that this still goes wrong sometimes.
See the obvious mistake in

http://lists.gnu.org/rl/emacs-diffs/2018-10/msg00163.html

where a change ends up in etc/NEWS when it should have gone to NEWS.26.
(Presumably this means git somehow merged it without a conflict?
I have no idea how it could merge it to that position.)

See also the other corrections I then found to be needed in
http://lists.gnu.org/r/emacs-diffs/2018-10/msg00165.html

I guess I repeat my comments from
http://lists.gnu.org/r/emacs-devel/2017-12/msg00340.html

1) It's a shame there's no proper rename tracking
2) Perhaps etc/NEWS should always be called etc/NEWS.MAJORVERSION right
from the start, with etc/NEWS being just a symlink.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 29366 <at> debbugs.gnu.org
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Tue, 23 Oct 2018 20:23:36 +0300

> From: Glenn Morris <rgm <at> gnu.org>
> Date: Tue, 23 Oct 2018 13:10:09 -0400
> 
> 2) Perhaps etc/NEWS should always be called etc/NEWS.MAJORVERSION right
> from the start, with etc/NEWS being just a symlink.

I cannot use symlinks, sorry.

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: rgm <at> gnu.org
Cc: 29366 <at> debbugs.gnu.org
Subject: Re: bug#29366: gitmerge to handle NEWS better
Date: Tue, 23 Oct 2018 20:35:22 +0300

> Date: Tue, 23 Oct 2018 20:23:36 +0300
> From: Eli Zaretskii <eliz <at> gnu.org>
> Cc: 29366 <at> debbugs.gnu.org
> 
> > 2) Perhaps etc/NEWS should always be called etc/NEWS.MAJORVERSION right
> > from the start, with etc/NEWS being just a symlink.
> 
> I cannot use symlinks, sorry.

But maybe we could keep a NEWS.XYZ file in the repository, and rename
it as part of make-tarball.

Previous Next

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