D1165: arbitraryfilecontext: skip the cmp fast path if any side is a symlink

phillco (Phil Cohen) phabricator at mercurial-scm.org
Tue Oct 17 19:42:31 UTC 2017


phillco created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  `filecmp` follows symlinks by default, which a `filectx.cmp()` call should not
  be doing as it should only compare the  requested entry. After this patch, only
  the contexts' data are compared, which is the correct contract.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/context.py
  tests/test-arbitraryfilectx.t

CHANGE DETAILS

diff --git a/tests/test-arbitraryfilectx.t b/tests/test-arbitraryfilectx.t
new file mode 100644
--- /dev/null
+++ b/tests/test-arbitraryfilectx.t
@@ -0,0 +1,57 @@
+Setup:
+  $ cat > eval.py <<EOF
+  > from __future__ import absolute_import
+  > from mercurial import context, commands, registrar
+  > import filecmp
+  > cmdtable = {}
+  > command = registrar.command(cmdtable)
+  > @command(b'eval', [], 'hg eval CMD')
+  > def eval_(ui, repo, *cmds, **opts):
+  >     cmd = " ".join(cmds)
+  >     res = str(eval(cmd, globals(), locals()))
+  >     ui.warn("%s" % res)
+  > EOF
+
+  $ echo "[extensions]" >> $HGRCPATH
+  $ echo "eval=`pwd`/eval.py" >> $HGRCPATH
+
+Arbitraryfilectx.cmp does not follow symlinks:
+  $ mkdir case1
+  $ cd case1
+  $ hg init
+  $ printf "A" > real_A
+  $ printf "foo" > A
+  $ printf "foo" > B
+  $ ln -s A sym_A
+  $ hg add .
+  adding A
+  adding B
+  adding real_A
+  adding sym_A
+  $ hg commit -m "base"
+
+These files are different and should return True (different):
+(Note that filecmp.cmp's return semantics are inverted from ours, so we invert
+for simplicity):
+  $ hg eval "context.arbitraryfilectx('A', repo).cmp(repo[None]['real_A'])"
+  True (no-eol)
+  $ hg eval "not filecmp.cmp('A', 'real_A')"
+  True (no-eol)
+
+These files are identical and should return False (same):
+  $ hg eval "context.arbitraryfilectx('A', repo).cmp(repo[None]['A'])"
+  False (no-eol)
+  $ hg eval "context.arbitraryfilectx('A', repo).cmp(repo[None]['B'])"
+  False (no-eol)
+  $ hg eval "not filecmp.cmp('A', 'B')"
+  False (no-eol)
+
+This comparison should also return False, since A and sym_A are substantially
+the same in the eyes of ``filectx.cmp``, which looks at data only.
+  $ hg eval "context.arbitraryfilectx('real_A', repo).cmp(repo[None]['sym_A'])"
+  False (no-eol)
+
+A naive use of filecmp on those two would wrongly return True, since it follows
+the symlink to "A", which has different contents.
+  $ hg eval "not filecmp.cmp('real_A', 'sym_A')"
+  True (no-eol)
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -2569,9 +2569,13 @@
         self._path = path
 
     def cmp(self, fctx):
-        if isinstance(fctx, workingfilectx) and self._repo:
+        # filecmp follows symlinks whereas `cmp` should not, so skip the fast
+        # path if either side is a symlink.
+        symlinks = ('l' in self.flags() or 'l' in fctx.flags())
+        if isinstance(fctx, workingfilectx) and self._repo and not symlinks:
             # Add a fast-path for merge if both sides are disk-backed.
-            # Note that filecmp uses the opposite return values as cmp.
+            # Note that filecmp uses the opposite return values (True if same)
+            # as our ``cmp`` functions (True if different).
             return not filecmp.cmp(self.path(), self._repo.wjoin(fctx.path()))
         return self.data() != fctx.data()
 



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


More information about the Mercurial-devel mailing list