This is an archive of the discontinued Mercurial Phabricator instance.

Differential D7713

verify: allow the storage to signal when renames can be tested on `skipread`
ClosedPublic

Authored by mharbison72 on Dec 23 2019, 2:47 AM.

Details

Reviewers
indygreg
pulkit
Group Reviewers
hg-reviewers
Commits
rHGb9e174d4ed11: verify: allow the storage to signal when renames can be tested on `skipread`
rHG2efe3d3ec28e: verify: allow the storage to signal when renames can be tested on `skipread`
rHG8d0974108fcf: verify: allow the storage to signal when renames can be tested on `skipread`
Summary

This applies the new marker in the lfs handler to show it in action, and adds
the test mentioned at the beginning of the series to show that fulltext isn't
necessary in the LFS case.

The existing skipread isn't enough, because it is also set if an error occurs
reading the revlog data, or the data is censored. It could probably be cleared,
but then it technically violates the interface contract. That wouldn't matter
for the existing verify algorithm, but it isn't clear how that will change as
alternate storage support is added.

The flag is probably pretty revlog specific, given the comments in verify.py.
But there's already filelog specific stuff in there and I'm not sure what future
storage will bring, so I don't want to over-engineer this. Likewise, I'm not
sure that we want the verify method for each storage type to completely drive
the bus when it comes to detecting renames, so I don't want to go down the
rabbithole of having verifyintegrity() return metadata hints at this point.

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.

Revision Contents
Changeset List

Diff 19214

hgext/lfs/wrapper.py

rawtext = rl.rawdata(node) rawtext = rl.rawdata(node)
metadata = pointer.deserialize(rawtext) metadata = pointer.deserialize(rawtext)
# Don't skip blobs that are stored locally, as local verification is # Don't skip blobs that are stored locally, as local verification is
# relatively cheap and there's no other way to verify the raw data in # relatively cheap and there's no other way to verify the raw data in
# the revlog. # the revlog.
if rl.opener.lfslocalblobstore.has(metadata.oid()): if rl.opener.lfslocalblobstore.has(metadata.oid()):
skipflags &= ~revlog.REVIDX_EXTSTORED skipflags &= ~revlog.REVIDX_EXTSTORED
elif skipflags & revlog.REVIDX_EXTSTORED:
# The wrapped method will set `skipread`, but there's enough local
# info to check renames.
state[b'safe_renamed'].add(node)
orig(rl, skipflags, state, node) orig(rl, skipflags, state, node)
@eh.wrapfunction(context.basefilectx, b'cmp') @eh.wrapfunction(context.basefilectx, b'cmp')
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

mercurial/interfaces/repository.py

"""Verifies the integrity of file storage. """Verifies the integrity of file storage.
``state`` is a dict holding state of the verifier process. It can be ``state`` is a dict holding state of the verifier process. It can be
used to communicate data between invocations of multiple storage used to communicate data between invocations of multiple storage
primitives. primitives.
If individual revisions cannot have their revision content resolved, If individual revisions cannot have their revision content resolved,
the method is expected to set the ``skipread`` key to a set of nodes the method is expected to set the ``skipread`` key to a set of nodes
that encountered problems. that encountered problems. If set, the method can also add the node(s)
to ``safe_renamed`` in order to indicate nodes that may perform the
rename checks with currently accessible data.
The method yields objects conforming to the ``iverifyproblem`` The method yields objects conforming to the ``iverifyproblem``
interface. interface.
""" """
class idirs(interfaceutil.Interface): class idirs(interfaceutil.Interface):
"""Interface representing a collection of directories from paths. """Interface representing a collection of directories from paths.

mercurial/revlog.py

# The verifier tells us what version revlog we should be. # The verifier tells us what version revlog we should be.
if version != state[b'expectedversion']: if version != state[b'expectedversion']:
yield revlogproblem( yield revlogproblem(
warning=_(b"warning: '%s' uses revlog format %d; expected %d") warning=_(b"warning: '%s' uses revlog format %d; expected %d")
% (self.indexfile, version, state[b'expectedversion']) % (self.indexfile, version, state[b'expectedversion'])
) )
state[b'skipread'] = set() state[b'skipread'] = set()
state[b'safe_renamed'] = set()
for rev in self: for rev in self:
node = self.node(rev) node = self.node(rev)
# Verify contents. 4 cases to care about: # Verify contents. 4 cases to care about:
# #
# common: the most common case # common: the most common case
# rename: with a rename # rename: with a rename

mercurial/verify.py

) )
self.fncachewarned = True self.fncachewarned = True
if not len(fl) and (self.havecl or self.havemf): if not len(fl) and (self.havecl or self.havemf):
self._err(lr, _(b"empty or missing %s") % f) self._err(lr, _(b"empty or missing %s") % f)
else: else:
# Guard against implementations not setting this. # Guard against implementations not setting this.
state[b'skipread'] = set() state[b'skipread'] = set()
state[b'safe_renamed'] = set()
for problem in fl.verifyintegrity(state): for problem in fl.verifyintegrity(state):
if problem.node is not None: if problem.node is not None:
linkrev = fl.linkrev(fl.rev(problem.node)) linkrev = fl.linkrev(fl.rev(problem.node))
else: else:
linkrev = None linkrev = None
if problem.warning: if problem.warning:
self._warn(problem.warning) self._warn(problem.warning)
n = fl.node(i) n = fl.node(i)
lr = self._checkentry(fl, i, n, seen, linkrevs, f) lr = self._checkentry(fl, i, n, seen, linkrevs, f)
if f in filenodes: if f in filenodes:
if havemf and n not in filenodes[f]: if havemf and n not in filenodes[f]:
self._err(lr, _(b"%s not in manifests") % (short(n)), f) self._err(lr, _(b"%s not in manifests") % (short(n)), f)
else: else:
del filenodes[f][n] del filenodes[f][n]
if n in state[b'skipread']: if n in state[b'skipread'] and n not in state[b'safe_renamed']:
continue continue
# check renames # check renames
try: try:
# This requires resolving fulltext (at least on revlogs, # This requires resolving fulltext (at least on revlogs,
# though not with LFS revisions). We may want # though not with LFS revisions). We may want
# ``verifyintegrity()`` to pass a set of nodes with # ``verifyintegrity()`` to pass a set of nodes with
# rename metadata as an optimization. # rename metadata as an optimization.

tests/test-lfs.t

$ hg mv small s $ hg mv small s
$ hg status 'set:removed()' $ hg status 'set:removed()'
R large R large
R small R small
$ hg status 'set:removed() & lfs()' $ hg status 'set:removed() & lfs()'
R large R large
$ hg commit -m 'renames' $ hg commit -m 'renames'
$ hg cat -r . l -T '{rawdata}\n'
version https://git-lfs.github.com/spec/v1
oid sha256:66100b384bf761271b407d79fc30cdd0554f3b2c5d944836e936d584b88ce88e
size 39
x-hg-copy large
x-hg-copyrev 2c531e0992ff3107c511b53cb82a91b6436de8b2
x-is-binary 0
$ hg files -r . 'set:copied()' $ hg files -r . 'set:copied()'
l l
s s
$ hg files -r . 'set:copied() & lfs()' $ hg files -r . 'set:copied() & lfs()'
l l
$ hg status --change . 'set:removed()' $ hg status --change . 'set:removed()'
R large R large
R small R small
resolving manifests resolving manifests
getting l getting l
lfs: found 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b in the usercache lfs: found 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b in the usercache
getting s getting s
2 files updated, 0 files merged, 0 files removed, 0 files unresolved 2 files updated, 0 files merged, 0 files removed, 0 files unresolved
$ test -f fromcorrupt/.hg/store/lfs/objects/66/100b384bf761271b407d79fc30cdd0554f3b2c5d944836e936d584b88ce88e $ test -f fromcorrupt/.hg/store/lfs/objects/66/100b384bf761271b407d79fc30cdd0554f3b2c5d944836e936d584b88ce88e
[1] [1]
Verify will not try to download lfs blobs, if told not to process lfs content Verify will not try to download lfs blobs, if told not to process lfs content.
The extension makes sure that the filelog.renamed() path is taken on a missing
blob, and the output shows that it isn't fetched.
$ cat > $TESTTMP/lfsrename.py <<EOF
> from mercurial import (
> exthelper,
> )
>
> from hgext.lfs import (
> pointer,
> wrapper,
> )
>
> eh = exthelper.exthelper()
> uisetup = eh.finaluisetup
>
> @eh.wrapfunction(wrapper, b'filelogrenamed')
> def filelogrenamed(orig, orig1, self, node):
> ret = orig(orig1, self, node)
> if wrapper._islfs(self._revlog, node) and ret:
> rawtext = self._revlog.rawdata(node)
> metadata = pointer.deserialize(rawtext)
> print('lfs blob %s renamed %s -> %s'
> % (metadata[b'oid'], ret[0], self._revlog.filename))
> return ret
> EOF
$ hg -R fromcorrupt --config lfs.usercache=emptycache verify -v --no-lfs $ hg -R fromcorrupt --config lfs.usercache=emptycache verify -v --no-lfs \
> --config extensions.x=$TESTTMP/lfsrename.py
repository uses revlog format 1 repository uses revlog format 1
checking changesets checking changesets
checking manifests checking manifests
crosschecking files in changesets and manifests crosschecking files in changesets and manifests
checking files checking files
lfs: found 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b in the local lfs store lfs: found 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b in the local lfs store
lfs blob sha256:66100b384bf761271b407d79fc30cdd0554f3b2c5d944836e936d584b88ce88e renamed large -> l
checked 5 changesets with 10 changes to 4 files checked 5 changesets with 10 changes to 4 files
Verify will not try to download lfs blobs, if told not to by the config option Verify will not try to download lfs blobs, if told not to by the config option
$ hg -R fromcorrupt --config lfs.usercache=emptycache verify -v \ $ hg -R fromcorrupt --config lfs.usercache=emptycache verify -v \
> --config verify.skipflags=8192 > --config verify.skipflags=8192 \
> --config extensions.x=$TESTTMP/lfsrename.py
repository uses revlog format 1 repository uses revlog format 1
checking changesets checking changesets
checking manifests checking manifests
crosschecking files in changesets and manifests crosschecking files in changesets and manifests
checking files checking files
lfs: found 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b in the local lfs store lfs: found 22f66a3fc0b9bf3f012c814303995ec07099b3a9ce02a7af84b5970811074a3b in the local lfs store
lfs blob sha256:66100b384bf761271b407d79fc30cdd0554f3b2c5d944836e936d584b88ce88e renamed large -> l
checked 5 changesets with 10 changes to 4 files checked 5 changesets with 10 changes to 4 files
Verify will copy/link all lfs objects into the local store that aren't already Verify will copy/link all lfs objects into the local store that aren't already
present. Bypass the corrupted usercache to show that verify works when fed by present. Bypass the corrupted usercache to show that verify works when fed by
the (uncorrupted) remote store. the (uncorrupted) remote store.
$ hg -R fromcorrupt --config lfs.usercache=emptycache verify -v $ hg -R fromcorrupt --config lfs.usercache=emptycache verify -v
repository uses revlog format 1 repository uses revlog format 1