changegroup: do not delta lfs revisions stable
authorJun Wu <quark@fb.com>
Tue, 06 Feb 2018 19:08:25 -0800
branchstable
changeset 36743 d031609b3cb7
parent 36742 4e41b59633fa
child 36744 33275ab5e837
changegroup: do not delta lfs revisions 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. Differential Revision: https://phab.mercurial-scm.org/D2067
mercurial/changegroup.py
mercurial/revlog.py
tests/test-lfs-bundle.t
tests/test-lfs.t
--- a/mercurial/changegroup.py	Tue Feb 06 16:08:57 2018 -0800
+++ b/mercurial/changegroup.py	Tue Feb 06 19:08:25 2018 -0800
@@ -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
--- a/mercurial/revlog.py	Tue Feb 06 16:08:57 2018 -0800
+++ b/mercurial/revlog.py	Tue Feb 06 19:08:25 2018 -0800
@@ -77,6 +77,8 @@
     REVIDX_EXTSTORED,
 ]
 REVIDX_KNOWN_FLAGS = util.bitsfrom(REVIDX_FLAGS_ORDER)
+# bitmark for flags that could cause rawdata content change
+REVIDX_RAWTEXT_CHANGING_FLAGS = REVIDX_ISCENSORED | REVIDX_EXTSTORED
 
 # max size of revlog with inline data
 _maxinline = 131072
@@ -96,7 +98,8 @@
     """Register a flag processor on a revision data flag.
 
     Invariant:
-    - Flags need to be defined in REVIDX_KNOWN_FLAGS and REVIDX_FLAGS_ORDER.
+    - Flags need to be defined in REVIDX_KNOWN_FLAGS and REVIDX_FLAGS_ORDER,
+      and REVIDX_RAWTEXT_CHANGING_FLAGS if they can alter rawtext.
     - Only one flag processor can be registered on a specific flag.
     - flagprocessors must be 3-tuples of functions (read, write, raw) with the
       following signatures:
@@ -713,6 +716,18 @@
         except KeyError:
             return False
 
+    def candelta(self, baserev, rev):
+        """whether two revisions (baserev, rev) can be delta-ed or not"""
+        # Disable delta if either rev requires a content-changing flag
+        # processor (ex. LFS). This is because such flag processor can alter
+        # the rawtext content that the delta will be based on, and two clients
+        # could have a same revlog node with different flags (i.e. different
+        # rawtext contents) and the delta could be incompatible.
+        if ((self.flags(baserev) & REVIDX_RAWTEXT_CHANGING_FLAGS)
+            or (self.flags(rev) & REVIDX_RAWTEXT_CHANGING_FLAGS)):
+            return False
+        return True
+
     def clearcaches(self):
         self._cache = None
         self._chainbasecache.clear()
--- a/tests/test-lfs-bundle.t	Tue Feb 06 16:08:57 2018 -0800
+++ b/tests/test-lfs-bundle.t	Tue Feb 06 19:08:25 2018 -0800
@@ -95,6 +95,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
--- a/tests/test-lfs.t	Tue Feb 06 16:08:57 2018 -0800
+++ b/tests/test-lfs.t	Tue Feb 06 19:08:25 2018 -0800
@@ -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