D2068: revlog: do not use delta for lfs revisions

[quark (Jun Wu)]

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

REVISION SUMMARY
  This is similar to what we have done for changegroups. It is needed to make
  sure the delta application code path can assume deltas are always against
  vanilla (ex. non-LFS) rawtext so the next fix becomes possible.

REPOSITORY
  rHG Mercurial

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

AFFECTED FILES
  mercurial/revlog.py
  tests/test-lfs-bundle.t
  tests/test-revlog-raw.py

CHANGE DETAILS

diff --git a/tests/test-revlog-raw.py b/tests/test-revlog-raw.py
--- a/tests/test-revlog-raw.py
+++ b/tests/test-revlog-raw.py
@@ -114,6 +114,8 @@
             else:
                 # suboptimal deltaparent
                 deltaparent = min(0, parentrev)
+            if not rlog.candelta(deltaparent, r):
+                deltaparent = -1
             return {'node': rlog.node(r), 'p1': pnode, 'p2': node.nullid,
                     'cs': rlog.node(rlog.linkrev(r)), 'flags': rlog.flags(r),
                     'deltabase': rlog.node(deltaparent),
@@ -151,12 +153,12 @@
     for r in rlog:
         p1 = rlog.node(r - 1)
         p2 = node.nullid
-        if r == 0:
+        if r == 0 or rlog.flags(r):
             text = rlog.revision(r, raw=True)
             cachedelta = None
         else:
-            # deltaparent is more interesting if it has the EXTSTORED flag.
-            deltaparent = max([0] + [p for p in range(r - 2) if rlog.flags(p)])
+            # deltaparent cannot have EXTSTORED flag.
+            deltaparent = max([-1] + [p for p in range(r) if not rlog.flags(p)])
             text = None
             cachedelta = (deltaparent, rlog.revdiff(deltaparent, r))
         flags = rlog.flags(r)
@@ -262,8 +264,9 @@
         result.append((text, rawtext))
 
         # Verify flags like isdelta, isext work as expected
-        if bool(rlog.deltaparent(rev) > -1) != isdelta:
-            abort('rev %d: isdelta is ineffective' % rev)
+        # isdelta can be overridden to False if this or p1 has isext set
+        if bool(rlog.deltaparent(rev) > -1) and not isdelta:
+            abort('rev %d: isdelta is unexpected' % rev)
         if bool(rlog.flags(rev)) != isext:
             abort('rev %d: isext is ineffective' % rev)
     return result
diff --git a/tests/test-lfs-bundle.t b/tests/test-lfs-bundle.t
--- a/tests/test-lfs-bundle.t
+++ b/tests/test-lfs-bundle.t
@@ -91,10 +91,7 @@
   ---- Applying src-normal.bundle to dst-normal ----
   OK
   ---- Applying src-normal.bundle to dst-lfs ----
-   X at 2: unpacking 0609652b7877: integrity check failed on data/X.i:2
-   Y at 2: unpacking e384812cdeb9: integrity check failed on data/Y.i:2
-  2 integrity errors encountered!
-  (first damaged changeset appears to be 2)
+  CRASHED
   ---- Applying src-lfs.bundle to dst-normal ----
   OK
   ---- Applying src-lfs.bundle to dst-lfs ----
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -404,6 +404,9 @@
         for candidaterevs in self._getcandidaterevs(p1, p2, cachedelta):
             nominateddeltas = []
             for candidaterev in candidaterevs:
+                # do not use flags != 0 (ex. LFS) revision as delta base
+                if revlog.flags(candidaterev) != REVIDX_DEFAULT_FLAGS:
+                    continue
                 candidatedelta = self._builddeltainfo(revinfo, candidaterev, fh)
                 if revlog._isgooddeltainfo(candidatedelta, revinfo.textlen):
                     nominateddeltas.append(candidatedelta)
@@ -2082,7 +2085,12 @@
             deltacomputer = _deltacomputer(self)
 
         revinfo = _revisioninfo(node, p1, p2, btext, textlen, cachedelta, flags)
-        deltainfo = deltacomputer.finddeltainfo(revinfo, fh)
+
+        # do not use delta for flags != 0 (ex. LFS) revisions
+        if flags == REVIDX_DEFAULT_FLAGS:
+            deltainfo = deltacomputer.finddeltainfo(revinfo, fh)
+        else:
+            deltainfo = None
 
         if deltainfo is not None:
             base = deltainfo.base



To: quark, indygreg, #hg-reviewers
Cc: mercurial-devel