mergedriver: delete it

The merge driver code was added in late 2015. I think
'406a654b::6f045b56 & user("sid0")' is a reasonable revset for finding
the relevant commits, including preparation for it. The code is very
poorly tested, which makes it very hard to maintain. It seems it's
only used by FB and they don't use this code base anymore, so let's
remove the code to make it easier for us to maintain our product.

Differential Revision: https://phab.mercurial-scm.org/D9041

mercurial/commands.py

@@ -5986,10 +5986,6 def resolve(ui, repo, *pats, **opts):
                 b'resolve.resolved',
                 b'R',
             ),
-            mergestatemod.MERGE_RECORD_DRIVER_RESOLVED: (
-                b'resolve.driverresolved',
-                b'D',
-            ),
         }
 
         for f in ms:
@@ -6014,21 +6010,9 def resolve(ui, repo, *pats, **opts):
             )
 
         wctx = repo[None]
-
-        if (
-            ms.mergedriver
-            and ms.mdstate() == mergestatemod.MERGE_DRIVER_STATE_UNMARKED
-        ):
-            proceed = mergemod.driverpreprocess(repo, ms, wctx)
-            ms.commit()
-            # allow mark and unmark to go through
-            if not mark and not unmark and not proceed:
-                return 1
-
         m = scmutil.match(wctx, pats, opts)
         ret = 0
         didwork = False
-        runconclude = False
 
         tocomplete = []
         hasconflictmarkers = []
@@ -6043,26 +6027,6 def resolve(ui, repo, *pats, **opts):
 
             didwork = True
 
-            # don't let driver-resolved files be marked, and run the conclude
-            # step if asked to resolve
-            if ms[f] == mergestatemod.MERGE_RECORD_DRIVER_RESOLVED:
-                exact = m.exact(f)
-                if mark:
-                    if exact:
-                        ui.warn(
-                            _(b'not marking %s as it is driver-resolved\n')
-                            % uipathfn(f)
-                        )
-                elif unmark:
-                    if exact:
-                        ui.warn(
-                            _(b'not unmarking %s as it is driver-resolved\n')
-                            % uipathfn(f)
-                        )
-                else:
-                    runconclude = True
-                continue
-
             # path conflicts must be resolved manually
             if ms[f] in (
                 mergestatemod.MERGE_RECORD_UNRESOLVED_PATH,
@@ -6184,32 +6148,11 def resolve(ui, repo, *pats, **opts):
             ui.warn(_(b"arguments do not match paths that need resolving\n"))
             if hint:
                 ui.warn(hint)
-        elif ms.mergedriver and ms.mdstate() != b's':
-            # run conclude step when either a driver-resolved file is requested
-            # or there are no driver-resolved files
-            # we can't use 'ret' to determine whether any files are unresolved
-            # because we might not have tried to resolve some
-            if (runconclude or not list(ms.driverresolved())) and not list(
-                ms.unresolved()
-            ):
-                proceed = mergemod.driverconclude(repo, ms, wctx)
-                ms.commit()
-                if not proceed:
-                    return 1
-
-    # Nudge users into finishing an unfinished operation
+
     unresolvedf = list(ms.unresolved())
-    driverresolvedf = list(ms.driverresolved())
-    if not unresolvedf and not driverresolvedf:
+    if not unresolvedf:
         ui.status(_(b'(no more unresolved files)\n'))
         cmdutil.checkafterresolved(repo)
-    elif not unresolvedf:
-        ui.status(
-            _(
-                b'(no more unresolved files -- '
-                b'run "hg resolve --all" to conclude)\n'
-            )
-        )
 
     return ret
 

mercurial/configitems.py

@@ -635,9 +635,6 coreconfigitem(
 coreconfigitem(
     b'experimental', b'httppostargs', default=False,
 )
-coreconfigitem(
-    b'experimental', b'mergedriver', default=None,
-)
 coreconfigitem(b'experimental', b'nointerrupt', default=False)
 coreconfigitem(b'experimental', b'nointerrupt-interactiveonly', default=True)
 

mercurial/helptext/internals/mergestate.txt

@@ -37,30 +37,18 Currently known records:
 | * O: the node of the "other" part of the merge (hexified version)
 | * F: a file to be merged entry
 | * C: a change/delete or delete/change conflict
-| * D: a file that the external merge driver will merge internally
-|      (experimental)
 | * P: a path conflict (file vs directory)
-| * m: the external merge driver defined for this merge plus its run state
-|      (experimental)
 | * f: a (filename, dictionary) tuple of optional values for a given file
 | * X: unsupported mandatory record type (used in tests)
 | * x: unsupported advisory record type (used in tests)
 | * l: the labels for the parts of the merge.
 
-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
-
 Merge record states (indexed by filename):
 
 | * u: unresolved conflict
 | * r: resolved conflict
 | * pu: unresolved path conflict (file conflicts with directory)
 | * pr: resolved path conflict
-| * d: driver-resolved conflict
 
 The resolve command transitions between 'u' and 'r' for conflicts and
 'pu' and 'pr' for path conflicts.

mercurial/merge.py

@@ -355,20 +355,6 def _checkcollision(repo, wmf, mresult):
         lastfull = f
 
 
-def driverpreprocess(repo, ms, wctx, labels=None):
-    """run the preprocess step of the merge driver, if any
-
-    This is currently not implemented -- it's an extension point."""
-    return True
-
-
-def driverconclude(repo, ms, wctx, labels=None):
-    """run the conclude step of the merge driver, if any
-
-    This is currently not implemented -- it's an extension point."""
-    return True
-
-
 def _filesindirs(repo, manifest, dirs):
     """
     Generator that yields pairs of all the files in the manifest that are found
@@ -1605,27 +1591,6 def applyupdates(
             mergestatemod.ACTION_DIR_RENAME_MOVE_LOCAL,
         )
     )
-    # the ordering is important here -- ms.mergedriver will raise if the merge
-    # driver has changed, and we want to be able to bypass it when overwrite is
-    # True
-    usemergedriver = not overwrite and mergeactions and ms.mergedriver
-
-    if usemergedriver:
-        ms.commit()
-        proceed = driverpreprocess(repo, ms, wctx, labels=labels)
-        # the driver might leave some files unresolved
-        unresolvedf = set(ms.unresolved())
-        if not proceed:
-            # XXX setting unresolved to at least 1 is a hack to make sure we
-            # error out
-            return updateresult(
-                updated, merged, removed, max(len(unresolvedf), 1)
-            )
-        newactions = []
-        for f, args, msg in mergeactions:
-            if f in unresolvedf:
-                newactions.append((f, args, msg))
-        mergeactions = newactions
 
     try:
         # premerge
@@ -1655,18 +1620,6 def applyupdates(
 
     unresolved = ms.unresolvedcount()
 
-    if (
-        usemergedriver
-        and not unresolved
-        and ms.mdstate() != mergestatemod.MERGE_DRIVER_STATE_SUCCESS
-    ):
-        if not driverconclude(repo, ms, wctx, labels=labels):
-            # XXX setting unresolved to at least 1 is a hack to make sure we
-            # error out
-            unresolved = max(unresolved, 1)
-
-        ms.commit()
-
     msupdated, msmerged, msremoved = ms.counts()
     updated += msupdated
     merged += msmerged
@@ -1674,9 +1627,6 def applyupdates(
 
     extraactions = ms.actions()
     if extraactions:
-        mfiles = {
-            a[0] for a in mresult.getactions((mergestatemod.ACTION_MERGE,))
-        }
         for k, acts in pycompat.iteritems(extraactions):
             for a in acts:
                 mresult.addfile(a[0], k, *a[1:])
@@ -1684,27 +1634,6 def applyupdates(
                 # no filedata until mergestate is updated to provide it
                 for a in acts:
                     getfiledata[a[0]] = None
-            # Remove these files from actions[ACTION_MERGE] as well. This is
-            # important because in recordupdates, files in actions[ACTION_MERGE]
-            # are processed after files in other actions, and the merge driver
-            # might add files to those actions via extraactions above. This can
-            # lead to a file being recorded twice, with poor results. This is
-            # especially problematic for actions[ACTION_REMOVE] (currently only
-            # possible with the merge driver in the initial merge process;
-            # interrupted merges don't go through this flow).
-            #
-            # The real fix here is to have indexes by both file and action so
-            # that when the action for a file is changed it is automatically
-            # reflected in the other action lists. But that involves a more
-            # complex data structure, so this will do for now.
-            #
-            # We don't need to do the same operation for 'dc' and 'cd' because
-            # those lists aren't consulted again.
-            mfiles.difference_update(a[0] for a in acts)
-
-        for a in list(mresult.getactions((mergestatemod.ACTION_MERGE,))):
-            if a[0] not in mfiles:
-                mresult.removefile(a[0])
 
     progress.complete()
     assert len(getfiledata) == (

mercurial/mergestate.py

@@ -48,8 +48,6 RECORD_LOCAL = b'L'
 RECORD_OTHER = b'O'
 # record merge labels
 RECORD_LABELS = b'l'
-# store info about merge driver used and it's state
-RECORD_MERGE_DRIVER_STATE = b'm'
 
 #####
 # record extra information about files, with one entry containing info about one
@@ -65,7 +63,6 RECORD_FILE_VALUES = b'f'
 #####
 RECORD_MERGED = b'F'
 RECORD_CHANGEDELETE_CONFLICT = b'C'
-RECORD_MERGE_DRIVER_MERGE = b'D'
 # the path was dir on one side of merge and file on another
 RECORD_PATH_CONFLICT = b'P'
 
@@ -77,7 +74,6 MERGE_RECORD_UNRESOLVED = b'u'
 MERGE_RECORD_RESOLVED = b'r'
 MERGE_RECORD_UNRESOLVED_PATH = b'pu'
 MERGE_RECORD_RESOLVED_PATH = b'pr'
-MERGE_RECORD_DRIVER_RESOLVED = b'd'
 # represents that the file was automatically merged in favor
 # of other version. This info is used on commit.
 # This is now deprecated and commit related information is now
@@ -91,18 +87,16 MERGE_RECORD_MERGED_OTHER = b'o'
 RECORD_OVERRIDE = b't'
 
 #####
-# possible states which a merge driver can have. These are stored inside a
-# RECORD_MERGE_DRIVER_STATE entry
-#####
-MERGE_DRIVER_STATE_UNMARKED = b'u'
-MERGE_DRIVER_STATE_MARKED = b'm'
-MERGE_DRIVER_STATE_SUCCESS = b's'
-
-#####
 # legacy records which are no longer used but kept to prevent breaking BC
 #####
 # This record was release in 5.4 and usage was removed in 5.5
 LEGACY_RECORD_RESOLVED_OTHER = b'R'
+# This record was release in 3.7 and usage was removed in 5.6
+LEGACY_RECORD_DRIVER_RESOLVED = b'd'
+# This record was release in 3.7 and usage was removed in 5.6
+LEGACY_MERGE_DRIVER_STATE = b'm'
+# This record was release in 3.7 and usage was removed in 5.6
+LEGACY_MERGE_DRIVER_MERGE = b'D'
 
 
 ACTION_FORGET = b'f'
@@ -147,26 +141,15 class _mergestate_base(object):
     O: the node of the "other" part of the merge (hexified version)
     F: a file to be merged entry
     C: a change/delete or delete/change conflict
-    D: a file that the external merge driver will merge internally
-       (experimental)
     P: a path conflict (file vs directory)
-    m: the external merge driver defined for this merge plus its run state
-       (experimental)
     f: a (filename, dictionary) tuple of optional values for a given file
     l: the labels for the parts of the merge.
 
-    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
-
     Merge record states (stored in self._state, indexed by filename):
     u: unresolved conflict
     r: resolved conflict
     pu: unresolved path conflict (file conflicts with directory)
     pr: resolved path conflict
-    d: driver-resolved conflict
 
     The resolve command transitions between 'u' and 'r' for conflicts and
     'pu' and 'pr' for path conflicts.
@@ -182,8 +165,6 class _mergestate_base(object):
         self._local = None
         self._other = None
         self._labels = None
-        self._readmergedriver = None
-        self._mdstate = MERGE_DRIVER_STATE_UNMARKED
         # contains a mapping of form:
         # {filename : (merge_return_value, action_to_be_performed}
         # these are results of re-running merge process
@@ -199,32 +180,6 class _mergestate_base(object):
         self._local = node
         self._other = other
         self._labels = labels
-        if self.mergedriver:
-            self._mdstate = MERGE_DRIVER_STATE_SUCCESS
-
-    @util.propertycache
-    def mergedriver(self):
-        # protect against the following:
-        # - A configures a malicious merge driver in their hgrc, then
-        #   pauses the merge
-        # - A edits their hgrc to remove references to the merge driver
-        # - A gives a copy of their entire repo, including .hg, to B
-        # - B inspects .hgrc and finds it to be clean
-        # - B then continues the merge and the malicious merge driver
-        #  gets invoked
-        configmergedriver = self._repo.ui.config(
-            b'experimental', b'mergedriver'
-        )
-        if (
-            self._readmergedriver is not None
-            and self._readmergedriver != configmergedriver
-        ):
-            raise error.ConfigError(
-                _(b"merge driver changed since merge started"),
-                hint=_(b"revert merge driver change or abort merge"),
-            )
-
-        return configmergedriver
 
     @util.propertycache
     def local(self):
@@ -330,9 +285,6 class _mergestate_base(object):
         self._state[dfile][0] = state
         self._dirty = True
 
-    def mdstate(self):
-        return self._mdstate
-
     def unresolved(self):
         """Obtain the paths of unresolved files."""
 
@@ -343,13 +295,6 class _mergestate_base(object):
             ):
                 yield f
 
-    def driverresolved(self):
-        """Obtain the paths of driver-resolved files."""
-
-        for f, entry in self._state.items():
-            if entry[0] == MERGE_RECORD_DRIVER_RESOLVED:
-                yield f
-
     def extras(self, filename):
         return self._stateextras[filename]
 
@@ -358,7 +303,10 class _mergestate_base(object):
         Returns whether the merge was completed and the return value of merge
         obtained from filemerge._filemerge().
         """
-        if self[dfile] in (MERGE_RECORD_RESOLVED, MERGE_RECORD_DRIVER_RESOLVED):
+        if self[dfile] in (
+            MERGE_RECORD_RESOLVED,
+            LEGACY_RECORD_DRIVER_RESOLVED,
+        ):
             return True, 0
         stateentry = self._state[dfile]
         state, localkey, lfile, afile, anode, ofile, onode, flags = stateentry
@@ -490,24 +438,6 class _mergestate_base(object):
                 actions[action].append((f, None, b"merge result"))
         return actions
 
-    def queueremove(self, f):
-        """queues a file to be removed from the dirstate
-
-        Meant for use by custom merge drivers."""
-        self._results[f] = 0, ACTION_REMOVE
-
-    def queueadd(self, f):
-        """queues a file to be added to the dirstate
-
-        Meant for use by custom merge drivers."""
-        self._results[f] = 0, ACTION_ADD
-
-    def queueget(self, f):
-        """queues a file to be marked modified in the dirstate
-
-        Meant for use by custom merge drivers."""
-        self._results[f] = 0, ACTION_GET
-
 
 class mergestate(_mergestate_base):
 
@@ -535,7 +465,6 class mergestate(_mergestate_base):
         This function process "record" entry produced by the de-serialization
         of on disk file.
         """
-        self._mdstate = MERGE_DRIVER_STATE_SUCCESS
         unsupported = set()
         records = self._readrecords()
         for rtype, record in records:
@@ -543,24 +472,13 class mergestate(_mergestate_base):
                 self._local = bin(record)
             elif rtype == RECORD_OTHER:
                 self._other = bin(record)
-            elif rtype == RECORD_MERGE_DRIVER_STATE:
-                bits = record.split(b'\0', 1)
-                mdstate = bits[1]
-                if len(mdstate) != 1 or mdstate not in (
-                    MERGE_DRIVER_STATE_UNMARKED,
-                    MERGE_DRIVER_STATE_MARKED,
-                    MERGE_DRIVER_STATE_SUCCESS,
-                ):
-                    # the merge driver should be idempotent, so just rerun it
-                    mdstate = MERGE_DRIVER_STATE_UNMARKED
-
-                self._readmergedriver = bits[0]
-                self._mdstate = mdstate
+            elif rtype == LEGACY_MERGE_DRIVER_STATE:
+                pass
             elif rtype in (
                 RECORD_MERGED,
                 RECORD_CHANGEDELETE_CONFLICT,
                 RECORD_PATH_CONFLICT,
-                RECORD_MERGE_DRIVER_MERGE,
+                LEGACY_MERGE_DRIVER_MERGE,
                 LEGACY_RECORD_RESOLVED_OTHER,
             ):
                 bits = record.split(b'\0')
@@ -710,25 +628,13 class mergestate(_mergestate_base):
         records = []
         records.append((RECORD_LOCAL, hex(self._local)))
         records.append((RECORD_OTHER, hex(self._other)))
-        if self.mergedriver:
-            records.append(
-                (
-                    RECORD_MERGE_DRIVER_STATE,
-                    b'\0'.join([self.mergedriver, self._mdstate]),
-                )
-            )
         # Write out state items. In all cases, the value of the state map entry
         # is written as the contents of the record. The record type depends on
         # the type of state that is stored, and capital-letter records are used
         # to prevent older versions of Mercurial that do not support the feature
         # from loading them.
         for filename, v in pycompat.iteritems(self._state):
-            if v[0] == MERGE_RECORD_DRIVER_RESOLVED:
-                # Driver-resolved merge. These are stored in 'D' records.
-                records.append(
-                    (RECORD_MERGE_DRIVER_MERGE, b'\0'.join([filename] + v))
-                )
-            elif v[0] in (
+            if v[0] in (
                 MERGE_RECORD_UNRESOLVED_PATH,
                 MERGE_RECORD_RESOLVED_PATH,
             ):
@@ -814,17 +720,6 class memmergestate(_mergestate_base):
     def _restore_backup(self, fctx, localkey, flags):
         fctx.write(self._backups[localkey], flags)
 
-    @util.propertycache
-    def mergedriver(self):
-        configmergedriver = self._repo.ui.config(
-            b'experimental', b'mergedriver'
-        )
-        if configmergedriver:
-            raise error.InMemoryMergeConflictsError(
-                b"in-memory merge does not support mergedriver"
-            )
-        return None
-
 
 def recordupdates(repo, actions, branchmerge, getfiledata):
     """record merge actions to the dirstate"""

mercurial/mergeutil.py

@@ -17,8 +17,3 def checkunresolved(ms):
         raise error.Abort(
             _(b"unresolved merge conflicts (see 'hg help resolve')")
         )
-    if ms.mdstate() != b's' or list(ms.driverresolved()):
-        raise error.Abort(
-            _(b'driver-resolved merge conflicts'),
-            hint=_(b'run "hg resolve --all" to resolve'),
-        )

tests/test-resolve.t

@@ -87,64 +87,6 cleanup
   $ cd ..
   $ rmdir nested
 
-don't allow marking or unmarking driver-resolved files
-
-  $ cat > $TESTTMP/markdriver.py << EOF
-  > '''mark and unmark files as driver-resolved'''
-  > from mercurial import (
-  >    mergestate,
-  >    pycompat,
-  >    registrar,
-  >    scmutil,
-  > )
-  > cmdtable = {}
-  > command = registrar.command(cmdtable)
-  > @command(b'markdriver',
-  >   [(b'u', b'unmark', None, b'')],
-  >   b'FILE...')
-  > def markdriver(ui, repo, *pats, **opts):
-  >     wlock = repo.wlock()
-  >     opts = pycompat.byteskwargs(opts)
-  >     try:
-  >         ms = mergestate.mergestate.read(repo)
-  >         m = scmutil.match(repo[None], pats, opts)
-  >         for f in ms:
-  >             if not m(f):
-  >                 continue
-  >             if not opts[b'unmark']:
-  >                 ms.mark(f, b'd')
-  >             else:
-  >                 ms.mark(f, b'u')
-  >         ms.commit()
-  >     finally:
-  >         wlock.release()
-  > EOF
-  $ hg --config extensions.markdriver=$TESTTMP/markdriver.py markdriver file1
-  $ hg resolve --list
-  D file1
-  U file2
-  $ hg resolve --mark file1
-  not marking file1 as it is driver-resolved
-this should not print out file1
-  $ hg resolve --mark --all
-  (no more unresolved files -- run "hg resolve --all" to conclude)
-  $ hg resolve --mark 'glob:file*'
-  (no more unresolved files -- run "hg resolve --all" to conclude)
-  $ hg resolve --list
-  D file1
-  R file2
-  $ hg resolve --unmark file1
-  not unmarking file1 as it is driver-resolved
-  (no more unresolved files -- run "hg resolve --all" to conclude)
-  $ hg resolve --unmark --all
-  $ hg resolve --list
-  D file1
-  U file2
-  $ hg --config extensions.markdriver=$TESTTMP/markdriver.py markdriver --unmark file1
-  $ hg resolve --list
-  U file1
-  U file2
-
 resolve the failure
 
   $ echo resolved > file1