all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#29366: gitmerge to handle NEWS better
@ 2017-11-20 18:16 Glenn Morris
  2017-12-11 19:11 ` Glenn Morris
  0 siblings, 1 reply; 9+ messages in thread
From: Glenn Morris @ 2017-11-20 18:16 UTC (permalink / raw)
  To: 29366; +Cc: monnier, deng

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?





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2017-11-20 18:16 bug#29366: gitmerge to handle NEWS better Glenn Morris
@ 2017-12-11 19:11 ` Glenn Morris
  2017-12-11 23:18   ` David Engster
  0 siblings, 1 reply; 9+ messages in thread
From: Glenn Morris @ 2017-12-11 19:11 UTC (permalink / raw)
  To: 29366; +Cc: monnier, deng

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.







^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2017-12-11 19:11 ` Glenn Morris
@ 2017-12-11 23:18   ` David Engster
  2017-12-11 23:33     ` Glenn Morris
  0 siblings, 1 reply; 9+ messages in thread
From: David Engster @ 2017-12-11 23:18 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 29366, monnier

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2017-12-11 23:18   ` David Engster
@ 2017-12-11 23:33     ` Glenn Morris
  2017-12-12 20:10       ` David Engster
  0 siblings, 1 reply; 9+ messages in thread
From: Glenn Morris @ 2017-12-11 23:33 UTC (permalink / raw)
  To: David Engster; +Cc: 29366, monnier

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))))
 





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2017-12-11 23:33     ` Glenn Morris
@ 2017-12-12 20:10       ` David Engster
  2017-12-13  3:06         ` Glenn Morris
  0 siblings, 1 reply; 9+ messages in thread
From: David Engster @ 2017-12-12 20:10 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 29366, monnier

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2017-12-12 20:10       ` David Engster
@ 2017-12-13  3:06         ` Glenn Morris
  2018-10-23 17:10           ` Glenn Morris
  0 siblings, 1 reply; 9+ messages in thread
From: Glenn Morris @ 2017-12-13  3:06 UTC (permalink / raw)
  To: David Engster; +Cc: 29366, monnier

David Engster wrote:

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

Thanks for checking.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2017-12-13  3:06         ` Glenn Morris
@ 2018-10-23 17:10           ` Glenn Morris
  2018-10-23 17:23             ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Glenn Morris @ 2018-10-23 17:10 UTC (permalink / raw)
  To: 29366


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.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2018-10-23 17:10           ` Glenn Morris
@ 2018-10-23 17:23             ` Eli Zaretskii
  2018-10-23 17:35               ` Eli Zaretskii
  0 siblings, 1 reply; 9+ messages in thread
From: Eli Zaretskii @ 2018-10-23 17:23 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 29366

> From: Glenn Morris <rgm@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.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#29366: gitmerge to handle NEWS better
  2018-10-23 17:23             ` Eli Zaretskii
@ 2018-10-23 17:35               ` Eli Zaretskii
  0 siblings, 0 replies; 9+ messages in thread
From: Eli Zaretskii @ 2018-10-23 17:35 UTC (permalink / raw)
  To: rgm; +Cc: 29366

> Date: Tue, 23 Oct 2018 20:23:36 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 29366@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.





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-10-23 17:35 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-11-20 18:16 bug#29366: gitmerge to handle NEWS better Glenn Morris
2017-12-11 19:11 ` Glenn Morris
2017-12-11 23:18   ` David Engster
2017-12-11 23:33     ` Glenn Morris
2017-12-12 20:10       ` David Engster
2017-12-13  3:06         ` Glenn Morris
2018-10-23 17:10           ` Glenn Morris
2018-10-23 17:23             ` Eli Zaretskii
2018-10-23 17:35               ` Eli Zaretskii

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.