cleanup: say goodbye to manifestv2 format

This experiment was a bust: we'd hoped for smaller repository sizes,
but things got larger. Google ended up rolling out tree manifests in a
format that's compatible with the original manifest format, and I
believe Facebook is doing the same. This code was never implemented as
native speedups, so I'm pretty comfortable saying nobody is using the
experimental feature. Let's rip it out.

I noticed this code still kicking around because I was investigating a
repo corruption issue for timeless.

.. bc::

   Support for the experimental manifestv2 format has been removed, as
   it was never completed and failed to meet expectations.

Differential Revision: https://phab.mercurial-scm.org/D2393

mercurial/configitems.py

@@ -538,9 +538,6 b" coreconfigitem('experimental', 'hook-tra"
 coreconfigitem('experimental', 'httppostargs',
     default=False,
 )
-coreconfigitem('experimental', 'manifestv2',
-    default=False,
-)
 coreconfigitem('experimental', 'mergedriver',
     default=None,
 )

mercurial/help/internals/requirements.txt

@@ -1,4 +1,3 b''
-
 Repositories contain a file (``.hg/requires``) containing a list of
 features/capabilities that are *required* for clients to interface
 with the repository. This file has been present in Mercurial since
@@ -105,8 +104,10 b' manifestv2'
 Denotes that version 2 of manifests are being used.
 
 Support for this requirement was added in Mercurial 3.4 (released
-May 2015). The requirement is currently experimental and is disabled
-by default.
+May 2015). The new format failed to meet expectations and support
+for the format and requirement were removed in Mercurial 4.6
+(released May 2018) since the feature never graduated frome experiment
+status.
 
 treemanifest
 ============

mercurial/localrepo.py

@@ -303,11 +303,15 b" REVLOGV2_REQUIREMENT = 'exp-revlogv2.0'"
 
 class localrepository(object):
 
+    # obsolete experimental requirements:
+    #  - manifestv2: An experimental new manifest format that allowed
+    #    for stem compression of long paths. Experiment ended up not
+    #    being successful (repository sizes went up due to worse delta
+    #    chains), and the code was deleted in 4.6.
     supportedformats = {
         'revlogv1',
         'generaldelta',
         'treemanifest',
-        'manifestv2',
         REVLOGV2_REQUIREMENT,
     }
     _basesupported = supportedformats | {
@@ -322,7 +326,6 b' class localrepository(object):'
         'revlogv1',
         'generaldelta',
         'treemanifest',
-        'manifestv2',
     }
 
     # a list of (ui, featureset) functions.
@@ -2261,8 +2264,6 b' def newreporequirements(repo):'
         requirements.add('generaldelta')
     if ui.configbool('experimental', 'treemanifest'):
         requirements.add('treemanifest')
-    if ui.configbool('experimental', 'manifestv2'):
-        requirements.add('manifestv2')
 
     revlogv2 = ui.config('experimental', 'revlogv2')
     if revlogv2 == 'enable-unstable-format-and-corrupt-my-data':

mercurial/manifest.py

@@ -9,7 +9,6 b' from __future__ import absolute_import'
 
 import heapq
 import itertools
-import os
 import struct
 
 from .i18n import _
@@ -28,7 +27,7 b' from . import ('
 parsers = policy.importmod(r'parsers')
 propertycache = util.propertycache
 
-def _parsev1(data):
+def _parse(data):
     # This method does a little bit of excessive-looking
     # precondition checking. This is so that the behavior of this
     # class exactly matches its C counterpart to try and help
@@ -47,43 +46,7 b' def _parsev1(data):'
         else:
             yield f, bin(n), ''
 
-def _parsev2(data):
-    metadataend = data.find('\n')
-    # Just ignore metadata for now
-    pos = metadataend + 1
-    prevf = ''
-    while pos < len(data):
-        end = data.find('\n', pos + 1) # +1 to skip stem length byte
-        if end == -1:
-            raise ValueError('Manifest ended with incomplete file entry.')
-        stemlen = ord(data[pos:pos + 1])
-        items = data[pos + 1:end].split('\0')
-        f = prevf[:stemlen] + items[0]
-        if prevf > f:
-            raise ValueError('Manifest entries not in sorted order.')
-        fl = items[1]
-        # Just ignore metadata (items[2:] for now)
-        n = data[end + 1:end + 21]
-        yield f, n, fl
-        pos = end + 22
-        prevf = f
-
-def _parse(data):
-    """Generates (path, node, flags) tuples from a manifest text"""
-    if data.startswith('\0'):
-        return iter(_parsev2(data))
-    else:
-        return iter(_parsev1(data))
-
-def _text(it, usemanifestv2):
-    """Given an iterator over (path, node, flags) tuples, returns a manifest
-    text"""
-    if usemanifestv2:
-        return _textv2(it)
-    else:
-        return _textv1(it)
-
-def _textv1(it):
+def _text(it):
     files = []
     lines = []
     _hex = revlog.hex
@@ -96,19 +59,6 b' def _textv1(it):'
     _checkforbidden(files)
     return ''.join(lines)
 
-def _textv2(it):
-    files = []
-    lines = ['\0\n']
-    prevf = ''
-    for f, n, fl in it:
-        files.append(f)
-        stem = os.path.commonprefix([prevf, f])
-        stemlen = min(len(stem), 255)
-        lines.append("%c%s\0%s\n%s\n" % (stemlen, f[stemlen:], fl, n))
-        prevf = f
-    _checkforbidden(files)
-    return ''.join(lines)
-
 class lazymanifestiter(object):
     def __init__(self, lm):
         self.pos = 0
@@ -414,13 +364,7 b' except AttributeError:'
 
 class manifestdict(object):
     def __init__(self, data=''):
-        if data.startswith('\0'):
-            #_lazymanifest can not parse v2
-            self._lm = _lazymanifest('')
-            for f, n, fl in _parsev2(data):
-                self._lm[f] = n, fl
-        else:
-            self._lm = _lazymanifest(data)
+        self._lm = _lazymanifest(data)
 
     def __getitem__(self, key):
         return self._lm[key][0]
@@ -589,12 +533,9 b' class manifestdict(object):'
     def iterentries(self):
         return self._lm.iterentries()
 
-    def text(self, usemanifestv2=False):
-        if usemanifestv2:
-            return _textv2(self._lm.iterentries())
-        else:
-            # use (probably) native version for v1
-            return self._lm.text()
+    def text(self):
+        # most likely uses native version
+        return self._lm.text()
 
     def fastdelta(self, base, changes):
         """Given a base manifest text as a bytearray and a list of changes
@@ -1138,12 +1079,12 b' class treemanifest(object):'
                 if fl:
                     self._flags[f] = fl
 
-    def text(self, usemanifestv2=False):
+    def text(self):
         """Get the full data of this manifest as a bytestring."""
         self._load()
-        return _text(self.iterentries(), usemanifestv2)
+        return _text(self.iterentries())
 
-    def dirtext(self, usemanifestv2=False):
+    def dirtext(self):
         """Get the full data of this directory as a bytestring. Make sure that
         any submanifests have been written first, so their nodeids are correct.
         """
@@ -1151,7 +1092,7 b' class treemanifest(object):'
         flags = self.flags
         dirs = [(d[:-1], self._dirs[d]._node, 't') for d in self._dirs]
         files = [(f, self._files[f], flags(f)) for f in self._files]
-        return _text(sorted(dirs + files), usemanifestv2)
+        return _text(sorted(dirs + files))
 
     def read(self, gettext, readsubtree):
         def _load_for_read(s):
@@ -1208,15 +1149,12 b' class manifestrevlog(revlog.revlog):'
         # stacks of commits, the number can go up, hence the config knob below.
         cachesize = 4
         optiontreemanifest = False
-        usemanifestv2 = False
         opts = getattr(opener, 'options', None)
         if opts is not None:
             cachesize = opts.get('manifestcachesize', cachesize)
             optiontreemanifest = opts.get('treemanifest', False)
-            usemanifestv2 = opts.get('manifestv2', usemanifestv2)
 
         self._treeondisk = optiontreemanifest or treemanifest
-        self._usemanifestv2 = usemanifestv2
 
         self._fulltextcache = util.lrucachedict(cachesize)
 
@@ -1262,8 +1200,7 b' class manifestrevlog(revlog.revlog):'
         return self._dirlogcache[d]
 
     def add(self, m, transaction, link, p1, p2, added, removed, readtree=None):
-        if (p1 in self.fulltextcache and util.safehasattr(m, 'fastdelta')
-            and not self._usemanifestv2):
+        if p1 in self.fulltextcache and util.safehasattr(m, 'fastdelta'):
             # If our first parent is in the manifest cache, we can
             # compute a delta here using properties we know about the
             # manifest up-front, which may save time later for the
@@ -1290,7 +1227,7 b' class manifestrevlog(revlog.revlog):'
                 n = self._addtree(m, transaction, link, m1, m2, readtree)
                 arraytext = None
             else:
-                text = m.text(self._usemanifestv2)
+                text = m.text()
                 n = self.addrevision(text, transaction, link, p1, p2)
                 arraytext = bytearray(text)
 
@@ -1309,13 +1246,13 b' class manifestrevlog(revlog.revlog):'
             sublog.add(subm, transaction, link, subp1, subp2, None, None,
                        readtree=readtree)
         m.writesubtrees(m1, m2, writesubtree)
-        text = m.dirtext(self._usemanifestv2)
+        text = m.dirtext()
         n = None
         if self._dir != '':
             # Double-check whether contents are unchanged to one parent
-            if text == m1.dirtext(self._usemanifestv2):
+            if text == m1.dirtext():
                 n = m1.node()
-            elif text == m2.dirtext(self._usemanifestv2):
+            elif text == m2.dirtext():
                 n = m2.node()
 
         if not n:
@@ -1493,19 +1430,6 b' class manifestctx(object):'
         Changing the value of `shallow` has no effect on flat manifests.
         '''
         revlog = self._revlog()
-        if revlog._usemanifestv2:
-            # Need to perform a slow delta
-            r0 = revlog.deltaparent(revlog.rev(self._node))
-            m0 = self._manifestlog[revlog.node(r0)].read()
-            m1 = self.read()
-            md = manifestdict()
-            for f, ((n0, fl0), (n1, fl1)) in m0.diff(m1).iteritems():
-                if n1:
-                    md[f] = n1
-                    if fl1:
-                        md.setflag(f, fl1)
-            return md
-
         r = revlog.rev(self._node)
         d = mdiff.patchtext(revlog.revdiff(revlog.deltaparent(r), r))
         return manifestdict(d)
@@ -1608,7 +1532,7 b' class treemanifestctx(object):'
         its 't' flag.
         '''
         revlog = self._revlog()
-        if shallow and not revlog._usemanifestv2:
+        if shallow:
             r = revlog.rev(self._node)
             d = mdiff.patchtext(revlog.revdiff(revlog.deltaparent(r), r))
             return manifestdict(d)

mercurial/upgrade.py

@@ -46,7 +46,6 b' def blocksourcerequirements(repo):'
     return {
         # The upgrade code does not yet support these experimental features.
         # This is an artificial limitation.
-        'manifestv2',
         'treemanifest',
         # This was a precursor to generaldelta and was never enabled by default.
         # It should (hopefully) not exist in the wild.

tests/test-manifest.py

@@ -11,7 +11,6 b' from mercurial import ('
 )
 
 EMTPY_MANIFEST = b''
-EMTPY_MANIFEST_V2 = b'\0\n'
 
 HASH_1 = b'1' * 40
 BIN_HASH_1 = binascii.unhexlify(HASH_1)
@@ -28,42 +27,6 b' A_SHORT_MANIFEST = ('
          b'flag2': b'l',
          }
 
-# Same data as A_SHORT_MANIFEST
-A_SHORT_MANIFEST_V2 = (
-    b'\0\n'
-    b'\x00bar/baz/qux.py\0%(flag2)s\n%(hash2)s\n'
-    b'\x00foo\0%(flag1)s\n%(hash1)s\n'
-    ) % {b'hash1': BIN_HASH_1,
-         b'flag1': b'',
-         b'hash2': BIN_HASH_2,
-         b'flag2': b'l',
-         }
-
-# Same data as A_SHORT_MANIFEST
-A_METADATA_MANIFEST = (
-    b'\0foo\0bar\n'
-    b'\x00bar/baz/qux.py\0%(flag2)s\0foo\0bar\n%(hash2)s\n' # flag and metadata
-    b'\x00foo\0%(flag1)s\0foo\n%(hash1)s\n' # no flag, but metadata
-    ) % {b'hash1': BIN_HASH_1,
-         b'flag1': b'',
-         b'hash2': BIN_HASH_2,
-         b'flag2': b'l',
-         }
-
-A_STEM_COMPRESSED_MANIFEST = (
-    b'\0\n'
-    b'\x00bar/baz/qux.py\0%(flag2)s\n%(hash2)s\n'
-    b'\x04qux/foo.py\0%(flag1)s\n%(hash1)s\n' # simple case of 4 stem chars
-    b'\x0az.py\0%(flag1)s\n%(hash1)s\n' # tricky newline = 10 stem characters
-    b'\x00%(verylongdir)sx/x\0\n%(hash1)s\n'
-    b'\xffx/y\0\n%(hash2)s\n' # more than 255 stem chars
-    ) % {b'hash1': BIN_HASH_1,
-         b'flag1': b'',
-         b'hash2': BIN_HASH_2,
-         b'flag2': b'l',
-         b'verylongdir': 255 * b'x',
-         }
-
 A_DEEPER_MANIFEST = (
     b'a/b/c/bar.py\0%(hash3)s%(flag1)s\n'
     b'a/b/c/bar.txt\0%(hash1)s%(flag1)s\n'
@@ -111,11 +74,6 b' class basemanifesttests(object):'
         self.assertEqual(0, len(m))
         self.assertEqual([], list(m))
 
-    def testEmptyManifestv2(self):
-        m = self.parsemanifest(EMTPY_MANIFEST_V2)
-        self.assertEqual(0, len(m))
-        self.assertEqual([], list(m))
-
     def testManifest(self):
         m = self.parsemanifest(A_SHORT_MANIFEST)
         self.assertEqual([b'bar/baz/qux.py', b'foo'], list(m))
@@ -126,31 +84,6 b' class basemanifesttests(object):'
         with self.assertRaises(KeyError):
             m[b'wat']
 
-    def testParseManifestV2(self):
-        m1 = self.parsemanifest(A_SHORT_MANIFEST)
-        m2 = self.parsemanifest(A_SHORT_MANIFEST_V2)
-        # Should have same content as A_SHORT_MANIFEST
-        self.assertEqual(m1.text(), m2.text())
-
-    def testParseManifestMetadata(self):
-        # Metadata is for future-proofing and should be accepted but ignored
-        m = self.parsemanifest(A_METADATA_MANIFEST)
-        self.assertEqual(A_SHORT_MANIFEST, m.text())
-
-    def testParseManifestStemCompression(self):
-        m = self.parsemanifest(A_STEM_COMPRESSED_MANIFEST)
-        self.assertIn(b'bar/baz/qux.py', m)
-        self.assertIn(b'bar/qux/foo.py', m)
-        self.assertIn(b'bar/qux/foz.py', m)
-        self.assertIn(256 * b'x' + b'/x', m)
-        self.assertIn(256 * b'x' + b'/y', m)
-        self.assertEqual(A_STEM_COMPRESSED_MANIFEST, m.text(usemanifestv2=True))
-
-    def testTextV2(self):
-        m1 = self.parsemanifest(A_SHORT_MANIFEST)
-        v2text = m1.text(usemanifestv2=True)
-        self.assertEqual(A_SHORT_MANIFEST_V2, v2text)
-
     def testSetItem(self):
         want = BIN_HASH_1
 

tests/test-upgrade-repo.t

@@ -31,23 +31,18 b' Cannot upgrade shared repositories'
   abort: cannot upgrade repository; unsupported source requirement: shared
   [255]
 
-Do not yet support upgrading manifestv2 and treemanifest repos
-
-  $ hg --config experimental.manifestv2=true init manifestv2
-  $ hg -R manifestv2 debugupgraderepo
-  abort: cannot upgrade repository; unsupported source requirement: manifestv2
-  [255]
+Do not yet support upgrading treemanifest repos
 
   $ hg --config experimental.treemanifest=true init treemanifest
   $ hg -R treemanifest debugupgraderepo
   abort: cannot upgrade repository; unsupported source requirement: treemanifest
   [255]
 
-Cannot add manifestv2 or treemanifest requirement during upgrade
+Cannot add treemanifest requirement during upgrade
 
   $ hg init disallowaddedreq
-  $ hg -R disallowaddedreq --config experimental.manifestv2=true --config experimental.treemanifest=true debugupgraderepo
-  abort: cannot upgrade repository; do not support adding requirement: manifestv2, treemanifest
+  $ hg -R disallowaddedreq --config experimental.treemanifest=true debugupgraderepo
+  abort: cannot upgrade repository; do not support adding requirement: treemanifest
   [255]
 
 An upgrade of a repository created with recommended settings only suggests optimizations