( )⚙ D3029 tests: add test extension implementing custom filelog storage

This is an archive of the discontinued Mercurial Phabricator instance.

Differential D3029

tests: add test extension implementing custom filelog storage
ClosedPublic

Authored by indygreg on Apr 2 2018, 11:53 PM.

Details

Reviewers
durin42
Group Reviewers
hg-reviewers
Commits
rHGd257c5f2a940: tests: add test extension implementing custom filelog storage
Summary

In order to better support partial clones, we'll need alternate
repository storage mechanisms that aren't based on revlogs.

Today, the interface for repository storage isn't very well defined.
And there are various layering violations and assumptions made
throughout the code that storage is backed by revlogs.

In order to support alternate storage mechanisms, we'll need to
formally declare and adhere to interfaces for storage. This will
be a long, arduous process.

This commit creates an extension that implements non-revlog storage
for files. It defines a custom type that quacks like the existing
revlog/filelog API but isn't backed by a revlog. The backing storage
is - for simplicity reasons - a CBOR index and per-node files
representing fulltext data.

The localrepository class is modified so file(f) returns instances of
this class instead of filelog instances.

The purpose of this extension is to tease out what the actual filelog
interface is - based on running the test harness - so we can formalize
that interface and then implement a *real* alternate storage backend.

Using run-tests.py --extra-config-opt to run the test harness
with this extension enabled yields 83 failures out of 634 ran
tests.

The most common test failures are due to:

  • Issues with hg verify
  • LFS and largefiles (probably flags processing related)
  • Narrow.
  • Any test touching or inspecting individual filelog paths.
  • help and error output that is confused by the presence of an extension.
  • hg debug* commands doing low-level, revlog-y things.

An 88% pass rate is pretty good for an initial implementation if you
ask me!

There is a bit of duplicate code in the new extension. That's by
design: a point of this code is to tease out dependencies on revlog.
That being said, there is opportunity to consolidate code by moving
things out of the revlog API. For example, DAG traversal operations
don't necessarily need to be implemented at the storage level. (Although
for performance reasons they probably do.) Once we have a more
well-defined interface, we could probably define the default
implementations in terms of the base interface, pull those in via
class inheritance, and have implementations override with faster
versions if they so choose. (Or something like that.) But for now,
the duplicate code should be acceptable.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

indygreg created this revision.Apr 2 2018, 11:53 PM
indygreg edited the summary of this revision. (Show Details)Apr 3 2018, 1:53 PM
indygreg updated this revision to Diff 7563.
durin42 requested changes to this revision.Apr 4 2018, 2:03 PM
durin42 added a subscriber: durin42.

One thing that I'm 99% sure is a bug. The "ew" comment is not actionable, it just feels bad.

tests/simplestorerepo.py
36–45

ew

216–217

should one of these be baserev?

This revision now requires changes to proceed.Apr 4 2018, 2:03 PM
indygreg added inline comments.Apr 4 2018, 2:15 PM
tests/simplestorerepo.py
36–45

Why?

The point of these functions is to help validate the interface. There are surprisingly few failures in core due to this type checking.

Manifests and changelog, however, failure in several places. But our use of revisions and nodes in filelogs is surprisingly consistent.

indygreg updated this revision to Diff 7634.Apr 4 2018, 2:37 PM
durin42 added inline comments.Apr 4 2018, 2:57 PM
tests/simplestorerepo.py
36–45

I just wish this could be done with a typechecker instead of these functions.

durin42 accepted this revision.Apr 4 2018, 2:57 PM
This revision is now accepted and ready to land.Apr 4 2018, 2:57 PM
Closed by commit rHGd257c5f2a940: tests: add test extension implementing custom filelog storage (authored by indygreg). · Explain WhyApr 4 2018, 3:06 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
AMtests/simplestorerepo.py (589 lines)

Diff 7638

tests/simplestorerepo.py

  • This file was added.
# simplestorerepo.py - Extension that swaps in alternate repository storage.
#
# Copyright 2018 Gregory Szorc <gregory.szorc@gmail.com>
#
# This software may be used and distributed according to the terms of the
# GNU General Public License version 2 or any later version.
from __future__ import absolute_import
from mercurial.i18n import _
from mercurial.node import (
bin,
hex,
nullid,
nullrev,
)
from mercurial.thirdparty import (
cbor,
)
from mercurial import (
ancestor,
error,
filelog,
mdiff,
pycompat,
revlog,
)
# Note for extension authors: ONLY specify testedwith = 'ships-with-hg-core' for
# extensions which SHIP WITH MERCURIAL. Non-mainline extensions should
# be specifying the version(s) of Mercurial they are tested with, or
# leave the attribute unspecified.
testedwith = 'ships-with-hg-core'
def validatenode(node):
if isinstance(node, int):
raise ValueError('expected node; got int')
if len(node) != 20:
raise ValueError('expected 20 byte node')
def validaterev(rev):
if not isinstance(rev, int):
raise ValueError('expected int')
durin42Unsubmitted
Not Done

ew

durin42: ew
indygregAuthorUnsubmitted
Not Done

Why?

The point of these functions is to help validate the interface. There are surprisingly few failures in core due to this type checking.

Manifests and changelog, however, failure in several places. But our use of revisions and nodes in filelogs is surprisingly consistent.

indygreg: Why? The point of these functions is to help validate the interface. There are surprisingly…
durin42Unsubmitted
Not Done

I just wish this could be done with a typechecker instead of these functions.

durin42: I just wish this could be done with a typechecker instead of these functions.
class filestorage(object):
"""Implements storage for a tracked path.
Data is stored in the VFS in a directory corresponding to the tracked
path.
Index data is stored in an ``index`` file using CBOR.
Fulltext data is stored in files having names of the node.
"""
def __init__(self, svfs, path):
self._svfs = svfs
self._path = path
self._storepath = b'/'.join([b'data', path])
self._indexpath = b'/'.join([self._storepath, b'index'])
indexdata = self._svfs.tryread(self._indexpath)
if indexdata:
indexdata = cbor.loads(indexdata)
self._indexdata = indexdata or []
self._indexbynode = {}
self._indexbyrev = {}
self.index = []
self._refreshindex()
# This is used by changegroup code :/
self._generaldelta = True
self.storedeltachains = False
self.version = 1
def _refreshindex(self):
self._indexbynode.clear()
self._indexbyrev.clear()
self.index = []
for i, entry in enumerate(self._indexdata):
self._indexbynode[entry[b'node']] = entry
self._indexbyrev[i] = entry
self._indexbynode[nullid] = {
b'node': nullid,
b'p1': nullid,
b'p2': nullid,
b'linkrev': nullrev,
b'flags': 0,
}
self._indexbyrev[nullrev] = {
b'node': nullid,
b'p1': nullid,
b'p2': nullid,
b'linkrev': nullrev,
b'flags': 0,
}
for i, entry in enumerate(self._indexdata):
p1rev, p2rev = self.parentrevs(self.rev(entry[b'node']))
# start, length, rawsize, chainbase, linkrev, p1, p2, node
self.index.append((0, 0, 0, -1, entry[b'linkrev'], p1rev, p2rev,
entry[b'node']))
self.index.append((0, 0, 0, -1, -1, -1, -1, nullid))
def __len__(self):
return len(self._indexdata)
def __iter__(self):
return iter(range(len(self)))
def revs(self, start=0, stop=None):
step = 1
if stop is not None:
if start > stop:
step = -1
stop += step
else:
stop = len(self)
return range(start, stop, step)
def parents(self, node):
validatenode(node)
if node not in self._indexbynode:
raise KeyError('unknown node')
entry = self._indexbynode[node]
return entry[b'p1'], entry[b'p2']
def parentrevs(self, rev):
p1, p2 = self.parents(self._indexbyrev[rev][b'node'])
return self.rev(p1), self.rev(p2)
def rev(self, node):
validatenode(node)
# Will raise KeyError.
self._indexbynode[node]
for rev, entry in self._indexbyrev.items():
if entry[b'node'] == node:
return rev
raise error.ProgrammingError('this should not occur')
def node(self, rev):
validaterev(rev)
return self._indexbyrev[rev][b'node']
def lookup(self, node):
if isinstance(node, int):
return self.node(node)
if len(node) == 20:
try:
self.rev(node)
return node
except LookupError:
pass
try:
rev = int(node)
if '%d' % rev != node:
raise ValueError
if rev < 0:
rev = len(self) + rev
if rev < 0 or rev >= len(self):
raise ValueError
return self.node(rev)
except (ValueError, OverflowError):
pass
if len(node) == 40:
try:
rawnode = bin(node)
self.rev(rawnode)
return rawnode
except (TypeError, LookupError):
pass
raise LookupError(node, self._path, _('invalid lookup input'))
def linkrev(self, rev):
validaterev(rev)
return self._indexbyrev[rev][b'linkrev']
def flags(self, rev):
validaterev(rev)
return self._indexbyrev[rev][b'flags']
def deltaparent(self, rev):
validaterev(rev)
p1node = self.parents(self.node(rev))[0]
return self.rev(p1node)
def candelta(self, baserev, rev):
validaterev(baserev)
validaterev(rev)
durin42Unsubmitted
Not Done

should one of these be baserev?

durin42: should one of these be baserev?
if ((self.flags(baserev) & revlog.REVIDX_RAWTEXT_CHANGING_FLAGS)
or (self.flags(rev) & revlog.REVIDX_RAWTEXT_CHANGING_FLAGS)):
return False
return True
def rawsize(self, rev):
validaterev(rev)
node = self.node(rev)
return len(self.revision(node, raw=True))
def _processflags(self, text, flags, operation, raw=False):
if flags == 0:
return text, True
validatehash = True
# Depending on the operation (read or write), the order might be
# reversed due to non-commutative transforms.
orderedflags = revlog.REVIDX_FLAGS_ORDER
if operation == 'write':
orderedflags = reversed(orderedflags)
for flag in orderedflags:
# If a flagprocessor has been registered for a known flag, apply the
# related operation transform and update result tuple.
if flag & flags:
vhash = True
if flag not in revlog._flagprocessors:
message = _("missing processor for flag '%#x'") % (flag)
raise revlog.RevlogError(message)
processor = revlog._flagprocessors[flag]
if processor is not None:
readtransform, writetransform, rawtransform = processor
if raw:
vhash = rawtransform(self, text)
elif operation == 'read':
text, vhash = readtransform(self, text)
else: # write operation
text, vhash = writetransform(self, text)
validatehash = validatehash and vhash
return text, validatehash
def checkhash(self, text, node, p1=None, p2=None, rev=None):
if p1 is None and p2 is None:
p1, p2 = self.parents(node)
if node != revlog.hash(text, p1, p2):
raise error.RevlogError(_("integrity check failed on %s") %
self._path)
def revision(self, node, raw=False):
validatenode(node)
if node == nullid:
return b''
self._indexbynode[node]
rev = self.rev(node)
flags = self.flags(rev)
path = b'/'.join([self._storepath, hex(node)])
rawtext = self._svfs.read(path)
text, validatehash = self._processflags(rawtext, flags, 'read', raw=raw)
if validatehash:
self.checkhash(text, node, rev=rev)
return text
def read(self, node):
validatenode(node)
revision = self.revision(node)
if not revision.startswith(b'\1\n'):
return revision
start = revision.index(b'\1\n', 2)
return revision[start + 2:]
def renamed(self, node):
validatenode(node)
if self.parents(node)[0] != nullid:
return False
fulltext = self.revision(node)
m = filelog.parsemeta(fulltext)[0]
if m and 'copy' in m:
return m['copy'], bin(m['copyrev'])
return False
def cmp(self, node, text):
validatenode(node)
t = text
if text.startswith(b'\1\n'):
t = b'\1\n\1\n' + text
p1, p2 = self.parents(node)
if revlog.hash(t, p1, p2) == node:
return False
if self.iscensored(self.rev(node)):
return text != b''
if self.renamed(node):
t2 = self.read(node)
return t2 != text
return True
def size(self, rev):
validaterev(rev)
node = self._indexbyrev[rev][b'node']
if self.renamed(node):
return len(self.read(node))
if self.iscensored(rev):
return 0
return len(self.revision(node))
def iscensored(self, rev):
validaterev(rev)
return self.flags(rev) & revlog.REVIDX_ISCENSORED
def commonancestorsheads(self, a, b):
validatenode(a)
validatenode(b)
a = self.rev(a)
b = self.rev(b)
ancestors = ancestor.commonancestorsheads(self.parentrevs, a, b)
return pycompat.maplist(self.node, ancestors)
def descendants(self, revs):
# This is a copy of revlog.descendants()
first = min(revs)
if first == nullrev:
for i in self:
yield i
return
seen = set(revs)
for i in self.revs(start=first + 1):
for x in self.parentrevs(i):
if x != nullrev and x in seen:
seen.add(i)
yield i
break
# Required by verify.
def files(self):
entries = self._svfs.listdir(self._storepath)
# Strip out undo.backup.* files created as part of transaction
# recording.
entries = [f for f in entries if not f.startswith('undo.backup.')]
return [b'/'.join((self._storepath, f)) for f in entries]
# Required by verify.
def checksize(self):
return 0, 0
def add(self, text, meta, transaction, linkrev, p1, p2):
if meta or text.startswith(b'\1\n'):
text = filelog.packmeta(meta, text)
return self.addrevision(text, transaction, linkrev, p1, p2)
def addrevision(self, text, transaction, linkrev, p1, p2, node=None,
flags=0):
validatenode(p1)
validatenode(p2)
if flags:
node = node or revlog.hash(text, p1, p2)
rawtext, validatehash = self._processflags(text, flags, 'write')
node = node or revlog.hash(text, p1, p2)
if node in self._indexbynode:
return node
if validatehash:
self.checkhash(rawtext, node, p1=p1, p2=p2)
path = b'/'.join([self._storepath, hex(node)])
self._svfs.write(path, text)
self._indexdata.append({
b'node': node,
b'p1': p1,
b'p2': p2,
b'linkrev': linkrev,
b'flags': flags,
})
self._reflectindexupdate()
return node
def _reflectindexupdate(self):
self._refreshindex()
self._svfs.write(self._indexpath, cbor.dumps(self._indexdata))
def addgroup(self, deltas, linkmapper, transaction, addrevisioncb=None):
nodes = []
transaction.addbackup(self._indexpath)
for node, p1, p2, linknode, deltabase, delta, flags in deltas:
linkrev = linkmapper(linknode)
nodes.append(node)
if node in self._indexbynode:
continue
# Need to resolve the fulltext from the delta base.
if deltabase == nullid:
text = mdiff.patch(b'', delta)
else:
text = mdiff.patch(self.revision(deltabase), delta)
self.addrevision(text, transaction, linkrev, p1, p2, flags)
if addrevisioncb:
addrevisioncb(self, node)
return nodes
def revdiff(self, rev1, rev2):
validaterev(rev1)
validaterev(rev2)
node1 = self.node(rev1)
node2 = self.node(rev2)
return mdiff.textdiff(self.revision(node1, raw=True),
self.revision(node2, raw=True))
def headrevs(self):
# Assume all revisions are heads by default.
ishead = {rev: True for rev in self._indexbyrev}
for rev, entry in self._indexbyrev.items():
# Unset head flag for all seen parents.
ishead[self.rev(entry[b'p1'])] = False
ishead[self.rev(entry[b'p2'])] = False
return [rev for rev, ishead in sorted(ishead.items())
if ishead]
def heads(self, start=None, stop=None):
# This is copied from revlog.py.
if start is None and stop is None:
if not len(self):
return [nullid]
return [self.node(r) for r in self.headrevs()]
if start is None:
start = nullid
if stop is None:
stop = []
stoprevs = set([self.rev(n) for n in stop])
startrev = self.rev(start)
reachable = {startrev}
heads = {startrev}
parentrevs = self.parentrevs
for r in self.revs(start=startrev + 1):
for p in parentrevs(r):
if p in reachable:
if r not in stoprevs:
reachable.add(r)
heads.add(r)
if p in heads and p not in stoprevs:
heads.remove(p)
return [self.node(r) for r in heads]
def children(self, node):
validatenode(node)
# This is a copy of revlog.children().
c = []
p = self.rev(node)
for r in self.revs(start=p + 1):
prevs = [pr for pr in self.parentrevs(r) if pr != nullrev]
if prevs:
for pr in prevs:
if pr == p:
c.append(self.node(r))
elif p == nullrev:
c.append(self.node(r))
return c
def getstrippoint(self, minlink):
# This is largely a copy of revlog.getstrippoint().
brokenrevs = set()
strippoint = len(self)
heads = {}
futurelargelinkrevs = set()
for head in self.headrevs():
headlinkrev = self.linkrev(head)
heads[head] = headlinkrev
if headlinkrev >= minlink:
futurelargelinkrevs.add(headlinkrev)
# This algorithm involves walking down the rev graph, starting at the
# heads. Since the revs are topologically sorted according to linkrev,
# once all head linkrevs are below the minlink, we know there are
# no more revs that could have a linkrev greater than minlink.
# So we can stop walking.
while futurelargelinkrevs:
strippoint -= 1
linkrev = heads.pop(strippoint)
if linkrev < minlink:
brokenrevs.add(strippoint)
else:
futurelargelinkrevs.remove(linkrev)
for p in self.parentrevs(strippoint):
if p != nullrev:
plinkrev = self.linkrev(p)
heads[p] = plinkrev
if plinkrev >= minlink:
futurelargelinkrevs.add(plinkrev)
return strippoint, brokenrevs
def strip(self, minlink, transaction):
if not len(self):
return
rev, _ignored = self.getstrippoint(minlink)
if rev == len(self):
return
# Purge index data starting at the requested revision.
self._indexdata[rev:] = []
self._reflectindexupdate()
def reposetup(ui, repo):
if not repo.local():
return
class simplestorerepo(repo.__class__):
def file(self, f):
return filestorage(self.svfs, f)
repo.__class__ = simplestorerepo