Patchwork D789: merge: add option to abort merge process on failure

login
register
mail settings
Submitter phabricator
Date Oct. 2, 2017, 3:13 p.m.
Message ID <ce119859db94a5ec12504841f044c65f@localhost.localdomain>
Download mbox | patch
Permalink /patch/24396/
State Not Applicable
Headers show

Comments

phabricator - Oct. 2, 2017, 3:13 p.m.
ryanmce updated this revision to Diff 2332.
ryanmce marked an inline comment as done.
ryanmce added a comment.


  Followed yuja's advice and moved the abort later.
  
  This has many advantages as shown in the updated test, which shows this abort
  also helps with merge tool post-checks, which are also tested.
  
  It also gives me ideas for the prompts that we can improve to allow this to
  be an option at runtime rather than only a config-time option as it is now.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D789?vs=2007&id=2332

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D789

AFFECTED FILES
  mercurial/configitems.py
  mercurial/filemerge.py
  mercurial/help/config.txt
  tests/test-merge-tool-abort.t

CHANGE DETAILS




To: ryanmce, #hg-reviewers, quark, yuja, mbthomas
Cc: mbthomas, yuja, quark, mercurial-devel

Patch

diff --git a/tests/test-merge-tool-abort.t b/tests/test-merge-tool-abort.t
new file mode 100644
--- /dev/null
+++ b/tests/test-merge-tool-abort.t
@@ -0,0 +1,70 @@ 
+  $ cat >> $HGRCPATH <<EOF
+  > [extensions]
+  > rebase=
+  > [phases]
+  > publish=False
+  > [merge]
+  > abortonfailure=True
+  > EOF
+
+  $ hg init repo
+  $ cd repo
+  $ echo a > a
+  $ echo b > b
+  $ hg commit -qAm ab
+  $ echo c >> a
+  $ echo c >> b
+  $ hg commit -qAm c
+  $ hg up -q .^
+  $ echo d >> a
+  $ echo d >> b
+  $ hg commit -qAm d
+
+Check that failing external tool aborts the merge
+  $ hg rebase -s 1 -d 2 --tool false
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  abort: merge aborted due to merge tool failure
+  [255]
+  $ hg rebase --abort
+  rebase aborted
+
+Check that successful tool with failed post-check aborts the merge
+  $ hg rebase -s 1 -d 2 --tool true --config merge-tools.true.check=changed
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+   output file a appears unchanged
+  was merge successful (yn)? n
+  abort: merge aborted due to merge tool failure
+  [255]
+  $ hg rebase --abort
+  rebase aborted
+
+  $ hg rebase -s 1 -d 2 --tool true --config merge-tools.true.check=conflicts --config merge-tools.true.premerge=keep
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  abort: merge aborted due to merge tool failure
+  [255]
+  $ hg rebase --abort
+  rebase aborted
+  $ hg rebase -s 1 -d 2 --tool true --config merge-tools.true.check=prompt
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  was merge of 'a' successful (yn)? n
+  abort: merge aborted due to merge tool failure
+  [255]
+  $ hg rebase --abort
+  rebase aborted
+
+Check that successful tool otherwise allows the merge to continue
+  $ hg rebase -s 1 -d 2 --tool echo --keep --config merge-tools.echo.premerge=keep
+  rebasing 1:1f28a51c3c9b "c"
+  merging a
+  merging b
+  $TESTTMP/repo/a *a~base* *a~other* (glob)
+  $TESTTMP/repo/b *b~base* *b~other* (glob)
+
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1239,6 +1239,14 @@ 
    different contents. Similar to ``merge.checkignored``, except for files that
    are not ignored. (default: ``abort``)
 
+``abortonfailure``
+   Traditionally, the merge process attempts to merge all unresolved files
+   using the merge tool of choice, regardless of whether previous file merge
+   attempts succeeded or failed. Setting the ``merge.abortonfailure`` option to
+   True changes this behavior so that a failed merge will immediately abort
+   the merge operation, leaving the user in a normal unresolved merge state.
+   (default: ``False``)
+
 ``merge-patterns``
 ------------------
 
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -521,6 +521,14 @@ 
         util.unlink(b)
         util.unlink(c)
 
+def getmergeerrorhandler(repo):
+    onerr = None
+    if repo.ui.configbool('merge', 'abortonfailure'):
+        def onerr(*args):
+            msg = _('merge aborted due to nonzero mergetool return code')
+            return error.Abort(msg)
+    return onerr
+
 def _formatconflictmarker(repo, ctx, template, label, pad):
     """Applies the given template to the ctx, prefixed by the label.
 
@@ -713,6 +721,9 @@ 
             r = _check(repo, r, ui, tool, fcd, files)
 
         if r:
+            if repo.ui.configbool('merge', 'abortonfailure'):
+                msg = _('merge aborted due to merge tool failure')
+                raise error.Abort(msg)
             if onfailure:
                 ui.warn(onfailure % fd)
 
diff --git a/mercurial/configitems.py b/mercurial/configitems.py
--- a/mercurial/configitems.py
+++ b/mercurial/configitems.py
@@ -299,6 +299,9 @@ 
 coreconfigitem('merge', 'followcopies',
     default=True,
 )
+coreconfigitem('merge', 'abortonfailure',
+    default=False,
+)
 coreconfigitem('pager', 'ignore',
     default=list,
 )