Patchwork [1,of,5,mergedriver] merge.mergestate: add support for persisting a custom merge driver

login
register
mail settings
Submitter Siddharth Agarwal
Date Oct. 13, 2015, 4:50 a.m.
Message ID <cc121f7aecfb330d82b6.1444711802@dev6666.prn1.facebook.com>
Download mbox | patch
Permalink /patch/11002/
State Superseded
Commit f618b6aa8cdde436e9b4cf23299bcd688683433c
Headers show

Comments

Siddharth Agarwal - Oct. 13, 2015, 4:50 a.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1443674572 25200
#      Wed Sep 30 21:42:52 2015 -0700
# Node ID cc121f7aecfb330d82b6c1037a60110ea2a8738d
# Parent  36383507a6f8adb39df38fd20ca3bf5bc9d8ac25
merge.mergestate: add support for persisting a custom merge driver

A 'merge driver' is a coordinator for the overall merge process. It will be
able to control:

- tools for individual files, much like the merge-patterns configuration does
  today
- tools that can work across groups of files
- the ordering of file resolution
- resolution of automatically generated files
- adding and removing additional files to and from the dirstate

Since it is a critical part of the merge process, it really is part of the
merge state.

This is a lowercase character (i.e. optional) because ignoring this is fine for
older versions of Mercurial -- however, if there are any files that are
specially treated by the driver, we should abort. That will happen in upcoming
patches.
Pierre-Yves David - Oct. 13, 2015, 8:35 a.m.
On 10/12/2015 09:50 PM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1443674572 25200
> #      Wed Sep 30 21:42:52 2015 -0700
> # Node ID cc121f7aecfb330d82b6c1037a60110ea2a8738d
> # Parent  36383507a6f8adb39df38fd20ca3bf5bc9d8ac25
> merge.mergestate: add support for persisting a custom merge driver

I should probably have spointed at that earlier, but… what do you intend 
to store in the merge driver. We can probably not point to any binary 
here for security reason but we still have to detect and fail on change 
in the driver configuration.

The rest of the series looks okay to me (I'm a bit afraid of the lack of 
test but I assume they come with a later changesets) and I'll likely 
push them tomorrow morning if Augie did not.

(the mergedriver behing unused mean that there is no security risk yet.

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -61,6 +61,14 @@  class mergestate(object):
     L: the node of the "local" part of the merge (hexified version)
     O: the node of the "other" part of the merge (hexified version)
     F: a file to be merged entry
+    m: the external merge driver defined for this merge plus its run state
+       (experimental)
+
+    Merge driver run states (experimental):
+    u: driver-resolved files unmarked -- needs to be run next time we're about
+       to resolve or commit
+    m: driver-resolved files marked -- only needs to be run before commit
+    s: success/skipped -- does not need to be run any more
     '''
     statepathv1 = 'merge/state'
     statepathv2 = 'merge/state2'
@@ -70,13 +78,15 @@  class mergestate(object):
         self._dirty = False
         self._read()
 
-    def reset(self, node=None, other=None):
+    def reset(self, node=None, other=None, mergedriver=None):
         self._state = {}
         self._local = None
         self._other = None
         if node:
             self._local = node
             self._other = other
+        self._mergedriver = mergedriver
+        self._mdstate = 'u'
         shutil.rmtree(self._repo.join('merge'), True)
         self._dirty = False
 
@@ -89,12 +99,23 @@  class mergestate(object):
         self._state = {}
         self._local = None
         self._other = None
+        self._mergedriver = None
+        self._mdstate = 'u'
         records = self._readrecords()
         for rtype, record in records:
             if rtype == 'L':
                 self._local = bin(record)
             elif rtype == 'O':
                 self._other = bin(record)
+            elif rtype == 'm':
+                bits = record.split('\0', 1)
+                mdstate = bits[1]
+                if len(mdstate) != 1 or mdstate not in 'ums':
+                    # the merge driver should be idempotent, so just rerun it
+                    mdstate = 'u'
+
+                self._mergedriver = bits[0]
+                self._mdstate = mdstate
             elif rtype == 'F':
                 bits = record.split('\0')
                 self._state[bits[0]] = bits[1:]
@@ -216,6 +237,9 @@  class mergestate(object):
             records = []
             records.append(('L', hex(self._local)))
             records.append(('O', hex(self._other)))
+            if self._mergedriver:
+                records.append(('m', '\0'.join([
+                    self._mergedriver, self._mdstate])))
             for d, v in self._state.iteritems():
                 records.append(('F', '\0'.join([d] + v)))
             self._writerecords(records)