Patchwork D674: filemerge: use arbitraryfilectx for backup files

login
register
mail settings
Submitter phabricator
Date Sept. 26, 2017, 2:06 p.m.
Message ID <a9ef4d088d20d65ec97b60c08b4af007@localhost.localdomain>
Download mbox | patch
Permalink /patch/24163/
State Not Applicable
Headers show

Comments

phabricator - Sept. 26, 2017, 2:06 p.m.
phillco updated this revision to Diff 2088.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D674?vs=1989&id=2088

REVISION DETAIL
  https://phab.mercurial-scm.org/D674

AFFECTED FILES
  mercurial/context.py
  mercurial/filemerge.py

CHANGE DETAILS




To: phillco, #hg-reviewers
Cc: sid0, martinvonz, mercurial-devel

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -7,7 +7,6 @@ 
 
 from __future__ import absolute_import
 
-import filecmp
 import os
 import re
 import tempfile
@@ -226,9 +225,9 @@ 
         return '\n'
     return None # unknown
 
-def _matcheol(file, origfile):
+def _matcheol(file, back):
     "Convert EOL markers in a file to match origfile"
-    tostyle = _eoltype(util.readfile(origfile))
+    tostyle = _eoltype(back.data()) # No repo.wread filters?
     if tostyle:
         data = util.readfile(file)
         style = _eoltype(data)
@@ -468,6 +467,12 @@ 
     a = _workingpath(repo, fcd)
     fd = fcd.path()
 
+    # Run ``flushall()`` to make any missing folders the following wwrite
+    # calls might be depending on.
+    from . import context
+    if isinstance(fcd, context.overlayworkingfilectx):
+        fcd.ctx().flushall()
+
     util.writefile(a + ".local", fcd.decodeddata())
     repo.wwrite(fd + ".other", fco.data(), fco.flags())
     repo.wwrite(fd + ".base", fca.data(), fca.flags())
@@ -505,7 +510,9 @@ 
 
         args = _toolstr(ui, tool, "args", '$local $base $other')
         if "$output" in args:
-            out, a = a, back # read input from backup, write to original
+            # read input from backup, write to original
+            out = a
+            a = repo.wvfs.join(back.path())
         replace = {'local': a, 'base': b, 'other': c, 'output': out}
         args = util.interpolate(r'\$', replace, args,
                                 lambda s: util.shellquote(util.localpath(s)))
@@ -588,24 +595,40 @@ 
 def _restorebackup(fcd, back):
     # TODO: Add a workingfilectx.write(otherfilectx) path so we can use
     # util.copy here instead.
-    fcd.write(util.readfile(back), fcd.flags())
+    fcd.write(back.data(), fcd.flags())
 
-def _makebackup(repo, ui, fcd, premerge):
-    """Makes a backup of the local `fcd` file prior to merging.
+def _makebackup(repo, ui, wctx, fcd, premerge):
+    """Makes and returns a filectx-like object for ``fcd``'s backup file.
 
     In addition to preserving the user's pre-existing modifications to `fcd`
     (if any), the backup is used to undo certain premerges, confirm whether a
     merge changed anything, and determine what line endings the new file should
     have.
     """
     if fcd.isabsent():
         return None
-
+    from . import context
     a = _workingpath(repo, fcd)
     back = scmutil.origpath(ui, repo, a)
-    if premerge:
-        util.copyfile(a, back)
-    return back
+
+    inworkingdir = (back.startswith(repo.wvfs.base) and not
+        back.startswith(repo.vfs.base))
+
+    if isinstance(fcd, context.overlayworkingfilectx) and inworkingdir:
+        # If the backup file is to be in the working directory, and we're
+        # merging in-memory, we must redirect the backup to the memory context
+        # so we don't disturb the working directory.
+        relpath = back[len(repo.wvfs.base) + 1:]
+        wctx[relpath].write(fcd.data(), fcd.flags())
+        return wctx[relpath]
+    else:
+        # Otherwise, write to wherever the user specified the backups should go.
+        #
+        # A arbitraryfilectx is returned, so we can run the same functions on
+        # the backup context regardless of where it lives.
+        if premerge:
+            util.copyfile(a, back)
+        return context.arbitraryfilectx(back, repo=repo)
 
 def _maketempfiles(repo, fco, fca):
     """Writes out `fco` and `fca` as temporary files, so an external merge
@@ -691,7 +714,7 @@ 
             ui.warn(onfailure % fd)
         return True, 1, False
 
-    back = _makebackup(repo, ui, fcd, premerge)
+    back = _makebackup(repo, ui, wctx, fcd, premerge)
     files = (None, None, None, back)
     r = 1
     try:
@@ -719,7 +742,7 @@ 
         return True, r, deleted
     finally:
         if not r and back is not None:
-            util.unlink(back)
+            back.remove()
 
 def _check(repo, r, ui, tool, fcd, files):
     fd = fcd.path()
@@ -741,7 +764,7 @@ 
     if not r and not checked and (_toolbool(ui, tool, "checkchanged") or
                                   'changed' in
                                   _toollist(ui, tool, "check")):
-        if back is not None and filecmp.cmp(_workingpath(repo, fcd), back):
+        if back is not None and not fcd.cmp(back):
             if ui.promptchoice(_(" output file %s appears unchanged\n"
                                  "was merge successful (yn)?"
                                  "$$ &Yes $$ &No") % fd, 1):
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -8,6 +8,7 @@ 
 from __future__ import absolute_import
 
 import errno
+import filecmp
 import os
 import re
 import stat
@@ -2121,6 +2122,9 @@ 
         self._parent = parent
         self._path = path
 
+    def cmp(self, fctx):
+        return self.data() != fctx.data()
+
     def ctx(self):
         return self._parent
 
@@ -2578,11 +2582,17 @@ 
     """Allows you to use filectx-like functions on a file in an arbitrary
     location on disk, possibly not in the working directory.
     """
-    def __init__(self, path):
+    def __init__(self, path, repo=None):
+        # Repo is optional because contrib/simplemerge uses this class.
+        self._repo = repo
         self._path = path
 
-    def cmp(self, otherfilectx):
-        return self.data() != otherfilectx.data()
+    def cmp(self, fctx):
+        if isinstance(fctx, workingfilectx) and self._repo:
+            # Add a fast-path for merge if both sides are disk-backed.
+            # Note that filecmp uses the opposite return values as cmp.
+            return not filecmp.cmp(self.path(), self._repo.wjoin(fctx.path()))
+        return self.data() != fctx.data()
 
     def path(self):
         return self._path