D2067: changegroup: do not delta 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
  There is no way to distinguish whether a delta base is LFS or non-LFS.
  
  If the delta is against LFS rawtext, and the client trying to apply it has
  the base revision stored as fulltext, the delta (aka. bundle) will fail to
  apply.
  
  This patch forbids using delta for LFS revisions in changegroup so bad
  deltas won't be transmitted.
  
  Note: this does not solve the problem entirely. It solves LFS delta applying
  to non-LFS base. But the other direction: non-LFS delta applying to LFS base
  is not solved yet.

REPOSITORY
  rHG Mercurial

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

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

CHANGE DETAILS

diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -349,7 +349,7 @@
   uncompressed size of bundle content:
        * (changelog) (glob)
        * (manifests) (glob)
-       *  a (glob)
+      * a (glob)
   $ hg --config extensions.strip= strip -r 2 --no-backup --force -q
   $ hg -R bundle.hg log -p -T '{rev} {desc}\n' a
   5 branching
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
@@ -96,6 +96,6 @@
   2 integrity errors encountered!
   (first damaged changeset appears to be 2)
   ---- Applying src-lfs.bundle to dst-normal ----
-  CRASHED
+  OK
   ---- Applying src-lfs.bundle to dst-lfs ----
   OK
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -713,6 +713,13 @@
         except KeyError:
             return False
 
+    def candelta(self, baserev, rev):
+        """whether two revisions (prev, rev) can be delta-ed or not"""
+        # disable delta if either rev uses non-default flag (ex. LFS)
+        if self.flags(baserev) or self.flags(rev):
+            return False
+        return True
+
     def clearcaches(self):
         self._cache = None
         self._chainbasecache.clear()
diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -770,6 +770,8 @@
         progress(msgbundling, None)
 
     def deltaparent(self, revlog, rev, p1, p2, prev):
+        if not revlog.candelta(prev, rev):
+            raise error.ProgrammingError('cg1 should not be used in this case')
         return prev
 
     def revchunk(self, revlog, rev, prev, linknode):
@@ -829,16 +831,19 @@
             # expensive. The revlog caches should have prev cached, meaning
             # less CPU for changegroup generation. There is likely room to add
             # a flag and/or config option to control this behavior.
-            return prev
+            base = prev
         elif dp == nullrev:
             # revlog is configured to use full snapshot for a reason,
             # stick to full snapshot.
-            return nullrev
+            base = nullrev
         elif dp not in (p1, p2, prev):
             # Pick prev when we can't be sure remote has the base revision.
             return prev
         else:
-            return dp
+            base = dp
+        if base != nullrev and not revlog.candelta(base, rev):
+            base = nullrev
+        return base
 
     def builddeltaheader(self, node, p1n, p2n, basenode, linknode, flags):
         # Do nothing with flags, it is implicitly 0 in cg1 and cg2



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