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 _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel