This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4714

lfs: access revlog directly
ClosedPublic

Authored by indygreg on Sep 24 2018, 12:12 PM.

Details

Reviewers
mharbison72
Group Reviewers
hg-reviewers
Commits
rHG62a532045e71: lfs: access revlog directly
Summary

LFS is monkeypatching filelog.filelog and is then accessing
various filelog attributes in the monkeypatched function. This is all
fine.

But some of the attributes being accessed by LFS are revlog centric
and shouldn't be exposed on the file storage interface.

This commit changes the monkeypatched functions to access proxied
attributes on self._revlog instead of self.

This should be safe to do because non-revlog repositories should not
be using filelog instances: instead they should have a separate class
to represent file storage. So it is reasonable for LFS to assume the
_revlog attribute exists and points to a revlog.

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 24 2018, 12:12 PM
mharbison72 accepted this revision.Sep 25 2018, 12:36 AM
mharbison72 added a subscriber: mharbison72.

Seems reasonable to me.

Closed by commit rHG62a532045e71: lfs: access revlog directly (authored by indygreg). · Explain WhySep 26 2018, 1:24 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mhgext/lfs/wrapper.py (12 lines)
Mmercurial/filelog.py (4 lines)
Mtests/test-lfs.t (2 lines)

Diff 11386

hgext/lfs/wrapper.py

rawtext = metadata.serialize() rawtext = metadata.serialize()
return (rawtext, False) return (rawtext, False)
def _islfs(rlog, node=None, rev=None): def _islfs(rlog, node=None, rev=None):
if rev is None: if rev is None:
if node is None: if node is None:
# both None - likely working copy content where node is not ready # both None - likely working copy content where node is not ready
return False return False
rev = rlog.rev(node) rev = rlog._revlog.rev(node)
else: else:
node = rlog.node(rev) node = rlog._revlog.node(rev)
if node == nullid: if node == nullid:
return False return False
flags = rlog.flags(rev) flags = rlog._revlog.flags(rev)
return bool(flags & revlog.REVIDX_EXTSTORED) return bool(flags & revlog.REVIDX_EXTSTORED)
def filelogaddrevision(orig, self, text, transaction, link, p1, p2, def filelogaddrevision(orig, self, text, transaction, link, p1, p2,
cachedelta=None, node=None, cachedelta=None, node=None,
flags=revlog.REVIDX_DEFAULT_FLAGS, **kwds): flags=revlog.REVIDX_DEFAULT_FLAGS, **kwds):
# The matcher isn't available if reposetup() wasn't called. # The matcher isn't available if reposetup() wasn't called.
lfstrack = self.opener.options.get('lfstrack') lfstrack = self._revlog.opener.options.get('lfstrack')
if lfstrack: if lfstrack:
textlen = len(text) textlen = len(text)
# exclude hg rename meta from file size # exclude hg rename meta from file size
meta, offset = revlog.parsemeta(text) meta, offset = revlog.parsemeta(text)
if offset: if offset:
textlen -= offset textlen -= offset
if lfstrack(self.filename, textlen): if lfstrack(self.filename, textlen):
flags |= revlog.REVIDX_EXTSTORED flags |= revlog.REVIDX_EXTSTORED
return orig(self, text, transaction, link, p1, p2, cachedelta=cachedelta, return orig(self, text, transaction, link, p1, p2, cachedelta=cachedelta,
node=node, flags=flags, **kwds) node=node, flags=flags, **kwds)
def filelogrenamed(orig, self, node): def filelogrenamed(orig, self, node):
if _islfs(self, node): if _islfs(self, node):
rawtext = self.revision(node, raw=True) rawtext = self._revlog.revision(node, raw=True)
if not rawtext: if not rawtext:
return False return False
metadata = pointer.deserialize(rawtext) metadata = pointer.deserialize(rawtext)
if 'x-hg-copy' in metadata and 'x-hg-copyrev' in metadata: if 'x-hg-copy' in metadata and 'x-hg-copyrev' in metadata:
return metadata['x-hg-copy'], bin(metadata['x-hg-copyrev']) return metadata['x-hg-copy'], bin(metadata['x-hg-copyrev'])
else: else:
return False return False
return orig(self, node) return orig(self, node)
def filelogsize(orig, self, rev): def filelogsize(orig, self, rev):
if _islfs(self, rev=rev): if _islfs(self, rev=rev):
# fast path: use lfs metadata to answer size # fast path: use lfs metadata to answer size
rawtext = self.revision(rev, raw=True) rawtext = self._revlog.revision(rev, raw=True)
metadata = pointer.deserialize(rawtext) metadata = pointer.deserialize(rawtext)
return int(metadata['size']) return int(metadata['size'])
return orig(self, rev) return orig(self, rev)
def filectxcmp(orig, self, fctx): def filectxcmp(orig, self, fctx):
"""returns True if text is different than fctx""" """returns True if text is different than fctx"""
# some fctx (ex. hg-git) is not based on basefilectx and do not have islfs # some fctx (ex. hg-git) is not based on basefilectx and do not have islfs
if self.islfs() and getattr(fctx, 'islfs', lambda: False)(): if self.islfs() and getattr(fctx, 'islfs', lambda: False)():

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)
# Used by LFS, verify. # Used by verify.
def flags(self, rev): def flags(self, rev):
return self._revlog.flags(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):
@property @property
def indexfile(self): def indexfile(self):
return self._revlog.indexfile return self._revlog.indexfile
@indexfile.setter @indexfile.setter
def indexfile(self, value): def indexfile(self, value):
self._revlog.indexfile = value self._revlog.indexfile = value
# Used by LFS, repo upgrade. # Used by repo upgrade.
@property @property
def opener(self): def opener(self):
return self._revlog.opener return self._revlog.opener
# Used by repo upgrade. # Used by repo upgrade.
def clone(self, tr, destrevlog, **kwargs): def clone(self, tr, destrevlog, **kwargs):
if not isinstance(destrevlog, filelog): if not isinstance(destrevlog, filelog):
raise error.ProgrammingError('expected filelog to clone()') raise error.ProgrammingError('expected filelog to clone()')

tests/test-lfs.t

> def reposetup(ui, repo): > def reposetup(ui, repo):
> # these 2 files are interesting > # these 2 files are interesting
> for name in ['l', 's']: > for name in ['l', 's']:
> fl = repo.file(name) > fl = repo.file(name)
> if len(fl) == 0: > if len(fl) == 0:
> continue > continue
> sizes = [fl.rawsize(i) for i in fl] > sizes = [fl.rawsize(i) for i in fl]
> texts = [fl.revision(i, raw=True) for i in fl] > texts = [fl.revision(i, raw=True) for i in fl]
> flags = [int(fl.flags(i)) for i in fl] > flags = [int(fl._revlog.flags(i)) for i in fl]
> hashes = [hash(t) for t in texts] > hashes = [hash(t) for t in texts]
> print(' %s: rawsizes=%r flags=%r hashes=%r' > print(' %s: rawsizes=%r flags=%r hashes=%r'
> % (name, sizes, flags, hashes)) > % (name, sizes, flags, hashes))
> EOF > EOF
$ for i in client client2 server repo3 repo4 repo5 repo6 repo7 repo8 repo9 \ $ for i in client client2 server repo3 repo4 repo5 repo6 repo7 repo8 repo9 \
> repo10; do > repo10; do
> echo 'repo:' $i > echo 'repo:' $i