This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4746

filelog: stop proxying flags() (API)
ClosedPublic

Authored by indygreg on Sep 26 2018, 2:28 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG2ac4f3e97813: filelog: stop proxying flags() (API)
Summary

Per-revision storage flags are kinda a revlog-centric API. (Except for
the fact that changegroup uses the same integer flags as revlog does
and there's minimal verification that the server's flags map to the
client's storage flags - but that's another problem.)

The last user of flags() was in verify.py and that code was just moved
into revlog.py and is accessed behind the verifyintegrity() file
storage API.

Since there are no more consumers, let's drop the proxy and remove
the method from the file storage interface.

This commit only drops the dedicated API for reading a single
revision's storage flags: we still support reading and writing flags
through the bulk data retrieval and add revision APIs. And since
changegroups encode revlog integer flags over the wire, we'll always
need to support flags at some level. The removal of individual storage
flags may be too premature. But since flags() is now unused, I'd like
to see how far we can get without that dedicated API - especially
since it uses revision numbers instead of nodes.

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.Sep 26 2018, 2:28 PM
Closed by commit rHG2ac4f3e97813: filelog: stop proxying flags() (API) (authored by indygreg). · Explain WhySep 26 2018, 5:05 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Diff 11424

mercurial/filelog.py

return self._revlog.node(rev) return self._revlog.node(rev)
def lookup(self, node): def lookup(self, node):
return self._revlog.lookup(node) return self._revlog.lookup(node)
def linkrev(self, rev): def linkrev(self, rev):
return self._revlog.linkrev(rev) return self._revlog.linkrev(rev)
# Unused.
def flags(self, rev):
return self._revlog.flags(rev)
def commonancestorsheads(self, node1, node2): def commonancestorsheads(self, node1, node2):
return self._revlog.commonancestorsheads(node1, node2) return self._revlog.commonancestorsheads(node1, node2)
# Used by dagop.blockdescendants(). # Used by dagop.blockdescendants().
def descendants(self, revs): def descendants(self, revs):
return self._revlog.descendants(revs) return self._revlog.descendants(revs)
def heads(self, start=None, stop=None): def heads(self, start=None, stop=None):

mercurial/repository.py

that can be converted to an integer. that can be converted to an integer.
Raises ``error.LookupError`` if a node could not be resolved. Raises ``error.LookupError`` if a node could not be resolved.
""" """
def linkrev(rev): def linkrev(rev):
"""Obtain the changeset revision number a revision is linked to.""" """Obtain the changeset revision number a revision is linked to."""
def flags(rev):
"""Obtain flags used to affect storage of a revision."""
def iscensored(rev): def iscensored(rev):
"""Return whether a revision's content has been censored.""" """Return whether a revision's content has been censored."""
def commonancestorsheads(node1, node2): def commonancestorsheads(node1, node2):
"""Obtain an iterable of nodes containing heads of common ancestors. """Obtain an iterable of nodes containing heads of common ancestors.
See ``ancestor.commonancestorsheads()``. See ``ancestor.commonancestorsheads()``.
""" """

mercurial/testing/storage.py

for i in range(-5, 5): for i in range(-5, 5):
if i == nullrev: if i == nullrev:
continue continue
with self.assertRaises(IndexError): with self.assertRaises(IndexError):
f.linkrev(i) f.linkrev(i)
self.assertEqual(f.flags(nullrev), 0)
for i in range(-5, 5):
if i == nullrev:
continue
with self.assertRaises(IndexError):
f.flags(i)
self.assertFalse(f.iscensored(nullrev)) self.assertFalse(f.iscensored(nullrev))
for i in range(-5, 5): for i in range(-5, 5):
if i == nullrev: if i == nullrev:
continue continue
with self.assertRaises(IndexError): with self.assertRaises(IndexError):
f.iscensored(i) f.iscensored(i)
self.assertEqual(f.lookup(b'0'), node) self.assertEqual(f.lookup(b'0'), node)
self.assertEqual(f.lookup(hex(node)), node) self.assertEqual(f.lookup(hex(node)), node)
self.assertEqual(f.linkrev(0), 0) self.assertEqual(f.linkrev(0), 0)
with self.assertRaises(IndexError): with self.assertRaises(IndexError):
f.linkrev(1) f.linkrev(1)
self.assertEqual(f.flags(0), 0)
with self.assertRaises(IndexError):
f.flags(1)
self.assertFalse(f.iscensored(0)) self.assertFalse(f.iscensored(0))
with self.assertRaises(IndexError): with self.assertRaises(IndexError):
f.iscensored(1) f.iscensored(1)
self.assertEqual(list(f.descendants([0])), []) self.assertEqual(list(f.descendants([0])), [])
self.assertEqual(f.heads(), [node]) self.assertEqual(f.heads(), [node])
self.assertEqual(f.linkrev(0), 0) self.assertEqual(f.linkrev(0), 0)
self.assertEqual(f.linkrev(1), 1) self.assertEqual(f.linkrev(1), 1)
self.assertEqual(f.linkrev(2), 3) self.assertEqual(f.linkrev(2), 3)
with self.assertRaises(IndexError): with self.assertRaises(IndexError):
f.linkrev(3) f.linkrev(3)
self.assertEqual(f.flags(0), 0)
self.assertEqual(f.flags(1), 0)
self.assertEqual(f.flags(2), 0)
with self.assertRaises(IndexError):
f.flags(3)
self.assertFalse(f.iscensored(0)) self.assertFalse(f.iscensored(0))
self.assertFalse(f.iscensored(1)) self.assertFalse(f.iscensored(1))
self.assertFalse(f.iscensored(2)) self.assertFalse(f.iscensored(2))
with self.assertRaises(IndexError): with self.assertRaises(IndexError):
f.iscensored(3) f.iscensored(3)
self.assertEqual(f.commonancestorsheads(node1, nullid), []) self.assertEqual(f.commonancestorsheads(node1, nullid), [])
# TODO tests are incomplete because we need the node to be # TODO tests are incomplete because we need the node to be
# different due to presence of censor metadata. But we can't # different due to presence of censor metadata. But we can't
# do this with addrevision(). # do this with addrevision().
with self._maketransactionfn() as tr: with self._maketransactionfn() as tr:
node0 = f.add(b'foo', None, tr, 0, nullid, nullid) node0 = f.add(b'foo', None, tr, 0, nullid, nullid)
f.addrevision(stored1, tr, 1, node0, nullid, f.addrevision(stored1, tr, 1, node0, nullid,
flags=revlog.REVIDX_ISCENSORED) flags=revlog.REVIDX_ISCENSORED)
self.assertEqual(f.flags(1), revlog.REVIDX_ISCENSORED)
self.assertTrue(f.iscensored(1)) self.assertTrue(f.iscensored(1))
self.assertEqual(f.revision(1), stored1) self.assertEqual(f.revision(1), stored1)
self.assertEqual(f.revision(1, raw=True), stored1) self.assertEqual(f.revision(1, raw=True), stored1)
self.assertEqual(f.read(1), b'') self.assertEqual(f.read(1), b'')
class ifilemutationtests(basetestcase): class ifilemutationtests(basetestcase):

tests/simplestorerepo.py

raise error.LookupError(node, self._path, _('invalid lookup input')) raise error.LookupError(node, self._path, _('invalid lookup input'))
def linkrev(self, rev): def linkrev(self, rev):
validaterev(rev) validaterev(rev)
return self._indexbyrev[rev][b'linkrev'] return self._indexbyrev[rev][b'linkrev']
def flags(self, rev): def _flags(self, rev):
validaterev(rev) validaterev(rev)
return self._indexbyrev[rev][b'flags'] return self._indexbyrev[rev][b'flags']
def deltaparent(self, rev): def deltaparent(self, rev):
validaterev(rev) validaterev(rev)
p1node = self.parents(self.node(rev))[0] p1node = self.parents(self.node(rev))[0]
return self.rev(p1node) return self.rev(p1node)
def _candelta(self, baserev, rev): def _candelta(self, baserev, rev):
validaterev(baserev) validaterev(baserev)
validaterev(rev) validaterev(rev)
if ((self.flags(baserev) & revlog.REVIDX_RAWTEXT_CHANGING_FLAGS) if ((self._flags(baserev) & revlog.REVIDX_RAWTEXT_CHANGING_FLAGS)
or (self.flags(rev) & revlog.REVIDX_RAWTEXT_CHANGING_FLAGS)): or (self._flags(rev) & revlog.REVIDX_RAWTEXT_CHANGING_FLAGS)):
return False return False
return True return True
def rawsize(self, rev): def rawsize(self, rev):
validaterev(rev) validaterev(rev)
node = self.node(rev) node = self.node(rev)
return len(self.revision(node, raw=True)) return len(self.revision(node, raw=True))
def revision(self, node, raw=False): def revision(self, node, raw=False):
validatenode(node) validatenode(node)
if node == nullid: if node == nullid:
return b'' return b''
rev = self.rev(node) rev = self.rev(node)
flags = self.flags(rev) flags = self._flags(rev)
path = b'/'.join([self._storepath, hex(node)]) path = b'/'.join([self._storepath, hex(node)])
rawtext = self._svfs.read(path) rawtext = self._svfs.read(path)
text, validatehash = self._processflags(rawtext, flags, 'read', raw=raw) text, validatehash = self._processflags(rawtext, flags, 'read', raw=raw)
if validatehash: if validatehash:
self.checkhash(text, node, rev=rev) self.checkhash(text, node, rev=rev)
if self.iscensored(rev): if self.iscensored(rev):
return 0 return 0
return len(self.revision(node)) return len(self.revision(node))
def iscensored(self, rev): def iscensored(self, rev):
validaterev(rev) validaterev(rev)
return self.flags(rev) & revlog.REVIDX_ISCENSORED return self._flags(rev) & revlog.REVIDX_ISCENSORED
def commonancestorsheads(self, a, b): def commonancestorsheads(self, a, b):
validatenode(a) validatenode(a)
validatenode(b) validatenode(b)
a = self.rev(a) a = self.rev(a)
b = self.rev(b) b = self.rev(b)