[PATCH] Improved logging for filemerge, making it easier to debug problems with merge configuration ... and refac

Mads Kiilerich mads at kiilerich.com
Thu Nov 6 16:15:59 CST 2008


# HG changeset patch
# User Mads Kiilerich <mads at kiilerich.com>
# Date 1226009733 -3600
# Node ID 6d68d6778484a077f4b9f163f3be015e533cb644
# Parent  eae1767cc6a8002a1d75b3af40e11782efc288ba
Improved logging for filemerge, making it easier to debug problems with merge configuration ... and refac

Previously it was very hard to debug situations where the expected merge tool
wasn't used - especially on windows. This patch tried to improve that.

The patch started out very simple but ended up refactoring a lot of things. It
is mostly moving code around and cleaning up. The intention was to not change
the semantics.

The patch as it is probably does too much and can't be reviewed as patch. And
it probably contains some unacceptable changes. Perhaps it would be better if
someone with the power to review and apply took a look at the code and tried to
do something similar instead ...

Benefits:
* Better logging
* Less tense code
* Quoting with util function

Notes:
* Should behaviour be preserved in all corner cases? Or is some cleanup ok?
* Should pattern-matched tools be checked for symlink but not binary capabilities?
* How should configured but bogus registry settings be handled? Just ignoring seems wrong ...

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -5,7 +5,7 @@
 # This software may be used and distributed according to the terms
 # of the GNU General Public License, incorporated herein by reference.
 
-from node import nullrev, short
+from node import short
 from i18n import _
 import util, os, tempfile, simplemerge, re, filecmp
 
@@ -15,67 +15,90 @@
 def _toolbool(ui, tool, part, default=False):
     return ui.configbool("merge-tools", tool + "." + part, default)
 
-def _findtool(ui, tool):
-    if tool in ("internal:fail", "internal:local", "internal:other"):
+def _checktool(ui, tool, tooldesc, symlink, binary):
+    if symlink and not _toolbool(ui, tool, "symlink"):
+        ui.warn(_("%s can't handle symlinks\n") % tooldesc)
+    elif binary and not _toolbool(ui, tool, "binary"):
+        ui.warn(_("%s can't handle binary\n") % tooldesc)
+    elif not util.gui() and _toolbool(ui, tool, "gui"):
+        ui.warn(_("%s requires a GUI\n") % tooldesc)
+    else:
+        return True
+    return False
+
+def _findtool(ui, tool, tooldesc):
+    if tool in ("internal:fail", "internal:local", "internal:other", "internal:merge"):
         return tool
+    exename = exe = None
     k = _toolstr(ui, tool, "regkey")
     if k:
-        p = util.lookup_reg(k, _toolstr(ui, tool, "regname"))
+        n = _toolstr(ui, tool, "regname")
+        p = util.lookup_reg(k, n)
         if p:
-            p = util.find_exe(p + _toolstr(ui, tool, "regappend"))
-            if p:
-                return p
-    return util.find_exe(_toolstr(ui, tool, "executable", tool))
+            a = _toolstr(ui, tool, "regappend")
+            exename = p + a
+            exe = util.find_exe(exename)
+            if not exe:
+                ui.warn(_("couldn't find merge tool '%s' (from regkey '%s' regname %r regappend %r) for %s\n") % (exename, k, n, a, tooldesc))
+        else:
+            ui.warn(_("couldn't find regkey '%s' regname %r for %s\n") % (k, n, tooldesc))
+    if not exe:
+        exename = _toolstr(ui, tool, "executable", tool)
+        exe = util.find_exe(exename)
+    if exe:
+        return util.shellquote(exe)
+    ui.warn(_("couldn't find merge tool '%s' for %s\n") % (exename, tooldesc))
 
 def _picktool(repo, ui, path, binary, symlink):
-    def check(tool, pat, symlink, binary):
-        tmsg = tool
-        if pat:
-            tmsg += " specified for " + pat
-        if pat and not _findtool(ui, tool): # skip search if not matching
-            ui.warn(_("couldn't find merge tool %s\n") % tmsg)
-        elif symlink and not _toolbool(ui, tool, "symlink"):
-            ui.warn(_("tool %s can't handle symlinks\n") % tmsg)
-        elif binary and not _toolbool(ui, tool, "binary"):
-            ui.warn(_("tool %s can't handle binary\n") % tmsg)
-        elif not util.gui() and _toolbool(ui, tool, "gui"):
-            ui.warn(_("tool %s requires a GUI\n") % tmsg)
-        else:
-            return True
-        return False
+    
+    def candidates():
+        # HGMERGE takes precedence
+        hgmerge = os.environ.get("HGMERGE")
+        if hgmerge:
+            yield (True, hgmerge, None)
+    
+        # then patterns
+        for pat, tool in ui.configitems("merge-patterns"):
+            mf = util.matcher(repo.root, "", [pat], [], [])[1]
+            if mf(path):
+                yield (False, tool, _("%r for pattern '%s'") % (tool, pat))
+    
+        # read configured merge-tools and their priorities
+        toolpriorities = {}
+        for k,v in ui.configitems("merge-tools"):
+            t = k.split('.')[0]
+            if t not in toolpriorities:
+                toolpriorities[t] = int(_toolstr(ui, t, "priority", "0"))
+    
+        # uimerge 
+        uimerge = ui.config("ui", "merge")
+        if uimerge:
+            if toolpriorities.has_key(uimerge):
+                yield (False, uimerge, _("%r from ui.merge") % (uimerge, ))
+            else:
+                yield (True, uimerge, None)
+    
+        for p, t in util.sort([(-p, t) for (t, p) in toolpriorities.items()]):
+            yield (False, t, _("%r with priority %s") % (t, p))
+    
+        # then old default hgmerge
+        yield (False, "hgmerge", _("%r (old default)") % 'hgmerge')
 
-    # HGMERGE takes precedence
-    hgmerge = os.environ.get("HGMERGE")
-    if hgmerge:
-        return (hgmerge, hgmerge)
+        # internal merge as last resort
+        if not symlink and not binary:
+            yield (True, "internal:merge", None)
+        
+    # choose first tool assumed or checked ok 
+    for ok, tool, tooldesc in candidates():
+        if ok:
+            return (tool, tool)
+        if _checktool(ui, tool, tooldesc, symlink, binary):
+            toolpath = _findtool(ui, tool, tooldesc)
+            if toolpath:
+                return (tool, toolpath)
 
-    # then patterns
-    for pat, tool in ui.configitems("merge-patterns"):
-        mf = util.matcher(repo.root, "", [pat], [], [])[1]
-        if mf(path) and check(tool, pat, symlink, False):
-                toolpath = _findtool(ui, tool)
-                return (tool, '"' + toolpath + '"')
-
-    # then merge tools
-    tools = {}
-    for k,v in ui.configitems("merge-tools"):
-        t = k.split('.')[0]
-        if t not in tools:
-            tools[t] = int(_toolstr(ui, t, "priority", "0"))
-    names = tools.keys()
-    tools = util.sort([(-p,t) for t,p in tools.items()])
-    uimerge = ui.config("ui", "merge")
-    if uimerge:
-        if uimerge not in names:
-            return (uimerge, uimerge)
-        tools.insert(0, (None, uimerge)) # highest priority
-    tools.append((None, "hgmerge")) # the old default, if found
-    for p,t in tools:
-        toolpath = _findtool(ui, t)
-        if toolpath and check(t, None, symlink, binary):
-            return (t, '"' + toolpath + '"')
-    # internal merge as last resort
-    return (not (symlink or binary) and "internal:merge" or None, None)
+    # asking user is last last resort
+    return (None, None)
 
 def _eoltype(data):
     "Guess the EOL type of a file"
@@ -89,16 +112,16 @@
         return '\n'
     return None # unknown
 
-def _matcheol(file, origfile):
+def _matcheol(fname, origfile):
     "Convert EOL markers in a file to match origfile"
     tostyle = _eoltype(open(origfile, "rb").read())
     if tostyle:
-        data = open(file, "rb").read()
+        data = open(fname, "rb").read()
         style = _eoltype(data)
         if style:
             newdata = data.replace(style, tostyle)
             if newdata != data:
-                open(file, "wb").write(newdata)
+                open(fname, "wb").write(newdata)
 
 def filemerge(repo, mynode, orig, fcd, fco, fca):
     """perform a 3-way merge in the working directory
@@ -169,7 +192,7 @@
     if _toolbool(ui, tool, "premerge", not (binary or symlink)):
         r = simplemerge.simplemerge(a, b, c, quiet=True)
         if not r:
-            ui.debug(_(" premerge successful\n"))
+            ui.debug(_(" premerge successful and sufficient\n"))
             os.unlink(back)
             os.unlink(b)
             os.unlink(c)
diff --git a/tests/test-copy-move-merge.out b/tests/test-copy-move-merge.out
--- a/tests/test-copy-move-merge.out
+++ b/tests/test-copy-move-merge.out
@@ -18,11 +18,11 @@
 picked tool 'internal:merge' for b (binary False symlink False)
 merging a and b to b
 my b at fb3948d97f07+ other b at 40da226db0f0 ancestor a at 583c7b748052
- premerge successful
+ premerge successful and sufficient
 picked tool 'internal:merge' for c (binary False symlink False)
 merging a and c to c
 my c at fb3948d97f07+ other c at 40da226db0f0 ancestor a at 583c7b748052
- premerge successful
+ premerge successful and sufficient
 0 files updated, 2 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
 -- b --
diff --git a/tests/test-double-merge.out b/tests/test-double-merge.out
--- a/tests/test-double-merge.out
+++ b/tests/test-double-merge.out
@@ -15,11 +15,11 @@
 picked tool 'internal:merge' for bar (binary False symlink False)
 merging foo and bar to bar
 my bar at 2092631ce82b+ other bar at 7731dad1c2b9 ancestor foo at 310fd17130da
- premerge successful
+ premerge successful and sufficient
 picked tool 'internal:merge' for foo (binary False symlink False)
 merging foo
 my foo at 2092631ce82b+ other foo at 7731dad1c2b9 ancestor foo at 310fd17130da
- premerge successful
+ premerge successful and sufficient
 0 files updated, 2 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
 -- foo --
diff --git a/tests/test-issue672.out b/tests/test-issue672.out
--- a/tests/test-issue672.out
+++ b/tests/test-issue672.out
@@ -34,7 +34,7 @@
 picked tool 'internal:merge' for 1a (binary False symlink False)
 merging 1a and 1 to 1a
 my 1a at ac7575e3c052+ other 1 at 746e9549ea96 ancestor 1 at 81f4b099af3d
- premerge successful
+ premerge successful and sufficient
 0 files updated, 1 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
 1 files updated, 0 files merged, 1 files removed, 0 files unresolved
@@ -53,6 +53,6 @@
 picked tool 'internal:merge' for 1a (binary False symlink False)
 merging 1 and 1a to 1a
 my 1a at 746e9549ea96+ other 1a at ac7575e3c052 ancestor 1 at 81f4b099af3d
- premerge successful
+ premerge successful and sufficient
 0 files updated, 1 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
diff --git a/tests/test-merge-commit.out b/tests/test-merge-commit.out
--- a/tests/test-merge-commit.out
+++ b/tests/test-merge-commit.out
@@ -31,7 +31,7 @@
 picked tool 'internal:merge' for bar (binary False symlink False)
 merging bar
 my bar at 2d2f9a22c82b+ other bar at 7d3b554bfdf1 ancestor bar at 0a3ab4856510
- premerge successful
+ premerge successful and sufficient
 0 files updated, 1 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
 % contents of bar should be line1 line2
@@ -81,7 +81,7 @@
 picked tool 'internal:merge' for bar (binary False symlink False)
 merging bar
 my bar at 2d2f9a22c82b+ other bar at 96ab80c60897 ancestor bar at 0a3ab4856510
- premerge successful
+ premerge successful and sufficient
 0 files updated, 1 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
 % contents of bar should be line1 line2
diff --git a/tests/test-merge-internal-tools-pattern b/tests/test-merge-internal-tools-pattern
--- a/tests/test-merge-internal-tools-pattern
+++ b/tests/test-merge-internal-tools-pattern
@@ -50,7 +50,7 @@
 echo "# merge using default tool"
 hg update -C 2
 rm .hg/hgrc
-hg merge
+/usr/bin/hg merge
 cat f
 hg stat
 
diff --git a/tests/test-merge-internal-tools-pattern.out b/tests/test-merge-internal-tools-pattern.out
--- a/tests/test-merge-internal-tools-pattern.out
+++ b/tests/test-merge-internal-tools-pattern.out
@@ -32,6 +32,7 @@
 M f
 # merge using default tool
 1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+couldn't find merge tool 'hgmerge' for 'hgmerge' (old default)
 merging f
 0 files updated, 1 files merged, 0 files removed, 0 files unresolved
 (branch merge, don't forget to commit)
diff --git a/tests/test-rename-merge1.out b/tests/test-rename-merge1.out
--- a/tests/test-rename-merge1.out
+++ b/tests/test-rename-merge1.out
@@ -24,7 +24,7 @@
 picked tool 'internal:merge' for b (binary False symlink False)
 merging a and b to b
 my b at f26ec4fc3fa3+ other b at 8e765a822af2 ancestor a at af1939970a1c
- premerge successful
+ premerge successful and sufficient
 warning: detected divergent renames of a2 to:
  c2
  b2
diff --git a/tests/test-rename-merge2.out b/tests/test-rename-merge2.out
--- a/tests/test-rename-merge2.out
+++ b/tests/test-rename-merge2.out
@@ -18,7 +18,7 @@
 picked tool 'python ../merge' for b (binary False symlink False)
 merging a and b to b
 my b at e300d1c794ec+ other b at 735846fee2d7 ancestor a at 924404dff337
- premerge successful
+ premerge successful and sufficient
 picked tool 'python ../merge' for rev (binary False symlink False)
 merging rev
 my rev at e300d1c794ec+ other rev at 735846fee2d7 ancestor rev at 924404dff337
@@ -52,7 +52,7 @@
 picked tool 'python ../merge' for b (binary False symlink False)
 merging b and a to b
 my b at ac809aeed39a+ other a at f4db7e329e71 ancestor a at 924404dff337
- premerge successful
+ premerge successful and sufficient
 picked tool 'python ../merge' for rev (binary False symlink False)
 merging rev
 my rev at ac809aeed39a+ other rev at f4db7e329e71 ancestor rev at 924404dff337
@@ -85,7 +85,7 @@
 picked tool 'python ../merge' for b (binary False symlink False)
 merging a and b to b
 my b at e300d1c794ec+ other b at e03727d2d66b ancestor a at 924404dff337
- premerge successful
+ premerge successful and sufficient
 picked tool 'python ../merge' for rev (binary False symlink False)
 merging rev
 my rev at e300d1c794ec+ other rev at e03727d2d66b ancestor rev at 924404dff337
@@ -116,7 +116,7 @@
 picked tool 'python ../merge' for b (binary False symlink False)
 merging b and a to b
 my b at ecf3cb2a4219+ other a at f4db7e329e71 ancestor a at 924404dff337
- premerge successful
+ premerge successful and sufficient
 picked tool 'python ../merge' for rev (binary False symlink False)
 merging rev
 my rev at ecf3cb2a4219+ other rev at f4db7e329e71 ancestor rev at 924404dff337
@@ -581,7 +581,7 @@
 picked tool 'python ../merge' for b (binary False symlink False)
 merging b and a to b
 my b at ecf3cb2a4219+ other a at 2b958612230f ancestor a at 924404dff337
- premerge successful
+ premerge successful and sufficient
 getting c
 picked tool 'python ../merge' for rev (binary False symlink False)
 merging rev


More information about the Mercurial-devel mailing list