Patchwork D2067: changegroup: do not delta lfs revisions

login
register
mail settings
Submitter phabricator
Date March 3, 2018, 8:28 p.m.
Message ID <a225c71e748d2af93e635f490b2392a3@localhost.localdomain>
Download mbox | patch
Permalink /patch/28834/
State Not Applicable
Headers show

Comments

phabricator - March 3, 2018, 8:28 p.m.
quark updated this revision to Diff 6469.

REPOSITORY
  rHG Mercurial

CHANGES SINCE LAST UPDATE
  https://phab.mercurial-scm.org/D2067?vs=5650&id=6469

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




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

Patch

diff --git a/tests/test-lfs.t b/tests/test-lfs.t
--- a/tests/test-lfs.t
+++ b/tests/test-lfs.t
@@ -371,7 +371,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
@@ -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
diff --git a/mercurial/revlog.py b/mercurial/revlog.py
--- a/mercurial/revlog.py
+++ b/mercurial/revlog.py
@@ -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:
@@ -738,6 +741,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()
diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -774,6 +774,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):
@@ -833,16 +835,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