[PATCH 2 of 6] filemerge: turn filemerge into a generator

Siddharth Agarwal sid0 at fb.com
Fri Oct 9 13:46:09 CDT 2015


# HG changeset patch
# User Siddharth Agarwal <sid0 at fb.com>
# Date 1444371736 25200
#      Thu Oct 08 23:22:16 2015 -0700
# Node ID 66ed0fddfd25ea6070c578d0c58e4d198e9236d1
# Parent  f32ecec8697102c6daeda1f3a98711a8305f928d
filemerge: turn filemerge into a generator

As described in the previous patch, we separate out premerge and merge into
separate steps. We pause the file merge operation in between the premerge and
merge steps while retaining all the state.

We don't take advantage of this yet in the merge state, but will soon.

Why a generator and not an explicit state machine?

- The filemerge operation is a linear state machine with no retries or anything
  of that sort. That is the sort of operation that a generator is best at
  expressing.

- There is a huge amount of state shared between steps, and a lot of state is
  initialized in one step before being used in the next. A state machine would
  have to either have all those initializations as explicit steps (verbose), use
  memoized getters (slightly less verbose but harder to follow execution flow),
  or just have earlier states dump fields onto the object that later states
  access (error-prone).

  In contrast, the generator allows us to follow execution flow linearly, with
  a single 'yield' statement inserted in between.

- The state machine would have to have an explicit cleanup state. That's much
  more naturally handled by the try/finally in the generator.

I actually did try writing an explicit state machine, but found that it was
significantly more complicated, with five total states, and had more
confusing transitions due to the need to capture the cleanup step in the state
machine. It was also much longer (~220 lines vs ~130).

The API for this is kind of crappy, but we'll improve on it in an upcoming
patch.

diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -539,7 +539,10 @@ def mergerecordupdates(orig, repo, actio
 def overridefilemerge(origfn, repo, mynode, orig, fcd, fco, fca, labels=None):
     if not lfutil.isstandin(orig):
         return origfn(repo, mynode, orig, fcd, fco, fca, labels=labels)
+    return _filemergegen(repo, mynode, orig, fcd, fco, fca, labels=labels)
 
+def _filemergegen(repo, mynode, orig, fcd, fco, fca, labels=None):
+    '''fake generator that looks similar to filemerge.filemerge'''
     ahash = fca.data().strip().lower()
     dhash = fcd.data().strip().lower()
     ohash = fco.data().strip().lower()
@@ -553,7 +556,8 @@ def overridefilemerge(origfn, repo, myno
                (lfutil.splitstandin(orig), ahash, dhash, ohash),
              0) == 1)):
         repo.wwrite(fcd.path(), fco.data(), fco.flags())
-    return 0
+    raise StopIteration(0)
+    yield  # make this a pretend generator
 
 def copiespathcopies(orig, ctx1, ctx2, match=None):
     copies = orig(ctx1, ctx2, match=match)
diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -438,6 +438,16 @@ def _formatlabels(repo, fcd, fco, fca, l
 def filemerge(repo, mynode, orig, fcd, fco, fca, labels=None):
     """perform a 3-way merge in the working directory
 
+    This is a coroutine with the following protocol:
+    - The first time this yields, a 'premerge' step has happened.
+    - On continuing it, a 'merge' step will be performed and the iteration will
+      complete.
+    - This may create temporary files. These files will be cleaned up when
+      close() is called on the coroutine.
+
+    A StopIteration exception may be thrown at any time. The return value of the
+    function is args[0] of the exception.
+
     mynode = parent node before merge
     orig = original local filename before merge
     fco = other file context
@@ -456,7 +466,7 @@ def filemerge(repo, mynode, orig, fcd, f
             return name
 
         if not fco.cmp(fcd): # files identical?
-            return None
+            raise StopIteration(None)
 
         ui = repo.ui
         fd = fcd.path()
@@ -483,7 +493,8 @@ def filemerge(repo, mynode, orig, fcd, f
         toolconf = tool, toolpath, binary, symlink
 
         if mergetype == nomerge:
-            return func(repo, mynode, orig, fcd, fco, fca, toolconf)
+            r = func(repo, mynode, orig, fcd, fco, fca, toolconf)
+            raise StopIteration(r)
 
         if orig != fco.path():
             ui.status(_("merging %s and %s to %s\n") % (orig, fco.path(), fd))
@@ -496,7 +507,7 @@ def filemerge(repo, mynode, orig, fcd, f
                                      toolconf):
             if onfailure:
                 ui.warn(onfailure % fd)
-            return 1
+            raise StopIteration(1)
 
         a = repo.wjoin(fd)
         b = temp("base", fca)
@@ -516,6 +527,8 @@ def filemerge(repo, mynode, orig, fcd, f
         if mergetype == fullmerge:
             r = _premerge(repo, toolconf, files, labels=labels)
 
+        yield
+
         if not r:  # premerge successfully merged the file
             needcheck = False
         else:
@@ -529,7 +542,8 @@ def filemerge(repo, mynode, orig, fcd, f
             if onfailure:
                 ui.warn(onfailure % fd)
 
-        return r
+        raise StopIteration(r)
+
     finally:
         if not r:
             util.unlink(back)
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -309,8 +309,14 @@ class mergestate(object):
         f = self._repo.vfs('merge/' + hash)
         self._repo.wwrite(dfile, f.read(), flags)
         f.close()
-        r = filemerge.filemerge(self._repo, self._local, lfile, fcd, fco, fca,
-                                labels=labels)
+        mergecr = filemerge.filemerge(self._repo, self._local, lfile, fcd, fco,
+                                      fca, labels=labels)
+        try:
+            next(mergecr)  # premerge
+            next(mergecr)  # merge
+        except StopIteration as e:
+            r = e.args[0]
+
         if r is None:
             # no real conflict
             del self._state[dfile]


More information about the Mercurial-devel mailing list