Patchwork [3,of,3] merge: support timeout for merge-tool (issue4841)

login
register
mail settings
Submitter timeless@mozdev.org
Date Sept. 25, 2015, 6:33 a.m.
Message ID <c20ebc3b551a8bbf0839.1443162807@waste.org>
Download mbox | patch
Permalink /patch/10628/
State Rejected
Headers show

Comments

timeless@mozdev.org - Sept. 25, 2015, 6:33 a.m.
# HG changeset patch
# User timeless@mozdev.org
# Date 1443157100 14400
#      Fri Sep 25 00:58:20 2015 -0400
# Node ID c20ebc3b551a8bbf0839771f27a2523dfa917bb4
# Parent  51d7fd39756d08699fa9ec39e4387f4558df083a
merge: support timeout for merge-tool (issue4841)
Matt Mackall - Sept. 29, 2015, 6:36 p.m.
On Fri, 2015-09-25 at 01:33 -0500, timeless@mozdev.org wrote:
> # HG changeset patch
> # User timeless@mozdev.org
> # Date 1443157100 14400
> #      Fri Sep 25 00:58:20 2015 -0400
> # Node ID c20ebc3b551a8bbf0839771f27a2523dfa917bb4
> # Parent  51d7fd39756d08699fa9ec39e4387f4558df083a
> merge: support timeout for merge-tool (issue4841)

If I'm understanding this approach correctly, this does file merges as
normal until a certain amount of time elapses and then aborts?

As explained in the BTS, I don't like this approach because it
potentially leaves a lot of trivial merges. We should refactor to have a
two-pass approach: merges we can do in memory and merges that require an
external tool. Then we can make decisions based on how many non-trivial
merges remain.

The 5 minute number is a bit too arbitrary. For people who are doing
merges they don't want to be doing, 5 minutes is way too long. For
people who are intentionally doing a real merge, it's probably too
short.

(This patch probably also fails test-check-config-hg.t which ensures
that options like this are documented and use consistent defaults.)

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5448,6 +5448,8 @@ 
         ret = 0
         didwork = False
 
+        timeout = repo.ui.configint('merge-tools', 'timeout', default=60*5)
+        stop = False
         for f in ms:
             if not m(f):
                 continue
@@ -5459,6 +5461,8 @@ 
             elif unmark:
                 ms.mark(f, "u")
             else:
+                if stop:
+                    continue
                 wctx = repo[None]
 
                 # backup pre-resolve (merge uses .orig for its own purposes)
@@ -5469,7 +5473,12 @@ 
                     # resolve file
                     ui.setconfig('ui', 'forcemerge', opts.get('tool', ''),
                                  'resolve')
-                    if ms.resolve(f, wctx):
+                    try:
+                        r = ms.resolve(f, wctx, timeout=timeout)
+                    except error.TimeoutWarning as t:
+                        r = t.result
+                        stop = True
+                    if r:
                         ret = 1
                 finally:
                     ui.setconfig('ui', 'forcemerge', '', 'resolve')
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -8,6 +8,7 @@ 
 from __future__ import absolute_import
 
 import errno
+import time
 import os
 import shutil
 import struct
@@ -21,6 +22,7 @@ 
 )
 from . import (
     copies,
+    error,
     filemerge,
     obsolete,
     subrepo,
@@ -279,7 +281,7 @@ 
             if entry[0] == 'u':
                 yield f
 
-    def resolve(self, dfile, wctx, labels=None):
+    def resolve(self, dfile, wctx, labels=None, timeout=None):
         """rerun merge process for file path `dfile`"""
         if self[dfile] == 'r':
             return 0
@@ -302,14 +304,23 @@ 
         f = self._repo.vfs('merge/' + hash)
         self._repo.wwrite(dfile, f.read(), flags)
         f.close()
+
+        if timeout:
+            start = time.time()
         r = filemerge.filemerge(self._repo, self._local, lfile, fcd, fco, fca,
                                 labels=labels)
+        if timeout:
+            elapsed = time.time() - start
         if r is None:
             # no real conflict
             del self._state[dfile]
             self._dirty = True
         elif not r:
             self.mark(dfile, 'r')
+        if timeout and elapsed > 1:
+            self._repo.ui.warn(_('warning: timeout running mergetool for %s\n') %
+                               lfile)
+            raise error.TimeoutWarning('timeout running mergetool', r, (lfile))
         return r
 
 def _checkunknownfile(repo, wctx, mctx, f, f2=None):
@@ -831,7 +842,12 @@ 
         updated += 1
 
     # merge
+    stop = False
+    timeout = repo.ui.configint('merge-tools', 'timeout', default=60*5)
     for f, args, msg in actions['m']:
+        if stop:
+            unresolved += 1
+            continue
         repo.ui.debug(" %s: %s -> m\n" % (f, msg))
         z += 1
         progress(_updating, z, item=f, total=numupdates, unit=_files)
@@ -840,7 +856,12 @@ 
                              overwrite)
             continue
         audit(f)
-        r = ms.resolve(f, wctx, labels=labels)
+        stop = False
+        try:
+            r = ms.resolve(f, wctx, labels=labels, timeout=timeout)
+        except error.TimeoutWarning as t:
+            r = t.result
+            stop = True
         if r is not None and r > 0:
             unresolved += 1
         else:
diff --git a/tests/test-merge-tools.t b/tests/test-merge-tools.t
--- a/tests/test-merge-tools.t
+++ b/tests/test-merge-tools.t
@@ -1028,3 +1028,80 @@ 
   [1]
 
 #endif
+
+merge timeouts
+
+  $ cat >> $HGRCPATH <<EOF
+  > [merge-tools]
+  > timeout = 1
+  > sleep.executable = python
+  > sleep.args = -c 'import time; time.sleep(5)'
+  > sleep.premerge = False
+  > false.executable = python
+  > false.args = -c 'quit(1)'
+  > false.premerge = False
+  > slow.executable = python
+  > slow.args = -c 'import os,sys,time;quit((time.sleep(5) or 1) if os.path.basename(sys.argv[1]) == "y" else 0)' \$local
+  > slow.premerge = False
+  > EOF
+  $ hg update -q -C 1
+  $ rm -f f.orig
+  $ echo later > z
+  $ cp z y
+  $ hg commit -Am z-later
+  adding y
+  adding z
+  created new head
+  $ hg update -q -C 2
+  $ echo sooner > z
+  $ cp z y
+  $ hg commit -Am z-sooner
+  adding y
+  adding z
+  $ hg merge 8 --tool sleep
+  merging f
+  warning: timeout running mergetool for f
+  0 files updated, 1 files merged, 0 files removed, 2 files unresolved
+  use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
+  [1]
+  $ hg resolve -l
+  R f
+  U y
+  U z
+  $ hg update -q -C 9
+  $ hg merge 8 --tool false
+  merging f
+  merging f failed!
+  merging y
+  merging y failed!
+  merging z
+  merging z failed!
+  0 files updated, 0 files merged, 0 files removed, 3 files unresolved
+  use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
+  [1]
+  $ hg resolve -l
+  U f
+  U y
+  U z
+  $ hg update -q -C 9
+  $ hg merge 8 --tool slow
+  merging f
+  merging y
+  merging y failed!
+  warning: timeout running mergetool for y
+  0 files updated, 1 files merged, 0 files removed, 2 files unresolved
+  use 'hg resolve' to retry unresolved file merges or 'hg update -C .' to abandon
+  [1]
+  $ hg resolve -l
+  R f
+  U y
+  U z
+  $ hg resolve --tool slow y z
+  merging y
+  merging y failed!
+  warning: timeout running mergetool for y
+  [1]
+  $ hg resolve -l
+  R f
+  U y
+  U z