[PATCH 4 of 4] verify: also check full manifest validity during verify runs
Pierre-Yves David
pierre-yves.david at ens-lyon.org
Wed Apr 17 09:51:59 EDT 2019
On 4/17/19 3:49 PM, Martin von Zweigbergk wrote:
>
>
> On Wed, Apr 17, 2019, 06:47 Pierre-Yves David
> <pierre-yves.david at ens-lyon.org <mailto:pierre-yves.david at ens-lyon.org>>
> wrote:
>
>
>
> On 4/17/19 3:06 PM, Martin von Zweigbergk wrote:
> >
> >
> > On Wed, Apr 17, 2019, 03:34 Pierre-Yves David
> > <pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>
> <mailto:pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>>>
> > wrote:
> >
> > On 4/17/19 4:59 AM, Martin von Zweigbergk wrote:
> > >
> > >
> > > On Tue, Apr 16, 2019, 16:46 Pierre-Yves David
> > > <pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>
> > <mailto:pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>>
> > <mailto:pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>
> > <mailto:pierre-yves.david at ens-lyon.org
> <mailto:pierre-yves.david at ens-lyon.org>>>>
> > > wrote:
> > >
> > > # HG changeset patch
> > > # User Pierre-Yves David
> <pierre-yves.david at octobus.net <mailto:pierre-yves.david at octobus.net>
> > <mailto:pierre-yves.david at octobus.net
> <mailto:pierre-yves.david at octobus.net>>
> > > <mailto:pierre-yves.david at octobus.net
> <mailto:pierre-yves.david at octobus.net>
> > <mailto:pierre-yves.david at octobus.net
> <mailto:pierre-yves.david at octobus.net>>>>
> > > # Date 1551881213 -3600
> > > # Wed Mar 06 15:06:53 2019 +0100
> > > # Node ID ed796867a06764cd78a57b2ed0249353f5809319
> > > # Parent 9bec7491e9b4cabdfa4d264e5213b1f416ec2607
> > > # EXP-Topic verify
> > > # Available At
> https://bitbucket.org/octobus/mercurial-devel/
> > > # hg pull
> > > https://bitbucket.org/octobus/mercurial-devel/ -r ed796867a067
> > > verify: also check full manifest validity during
> verify runs
> > >
> > > Before this changes, `hg verify` only checked if a
> manifest
> > revision
> > > existed and
> > > referenced the proper files. However it never checked the
> > manifest
> > > revision
> > > content itself.
> > >
> > > Mercurial is expecting manifest entries to be sorted
> and will
> > crash
> > > otherwise.
> > > Since `hg verify` did not attempted a full restoration of
> > manifest
> > > entry, it
> > > could ignore this kind of corruption.
> > >
> > > This new check significantly increases the cost of a
> `hg verify`
> > > run. This
> > > especially affects large repository not using
> > `sparse-revlog`. For
> > > now, this is
> > > hidden behind the `--full` experimental flag.
> > >
> > > diff --git a/mercurial/verify.py b/mercurial/verify.py
> > > --- a/mercurial/verify.py
> > > +++ b/mercurial/verify.py
> > > @@ -337,6 +337,16 @@ class verifier(object):
> > > filenodes.setdefault(fullpath,
> > > {}).setdefault(fn, lr)
> > > except Exception as inst:
> > > self._exc(lr, _("reading delta %s")
> % short(n),
> > > inst, label)
> > > + if not dir and self._level >= VERIFY_FULL:
> > >
> > >
> > > What does the "not dir" mean? I guess it's to do this
> check only
> > for the
> > > root directory when using tree manifests. Should we do it
> for all
> > > directories?
> >
> > That is used earlier in the same function to denote "the root
> > manifest".
> > I think check the root manifest will trigger checks of the
> sub manifest
> > but I am not sure, I am not too familiar tree manifest.
> >
> >
> > I'm pretty sure it's about tree manifests. I asked just to make sure
> > since it surprised me that you used that as part of the condition
> here.
>
> I know it is about tree manifest, I am reusing a pattern used by
> earlier
> code.
>
> > Can we double
> > check/fix this as a follow up ?
> >
> >
> > It should be easy to check (just add a print statement and run
> tests,
> > for example), so I don't see much reason to fix such a simple
> thing in a
> > follow-up.
>
> The question here is "Does reading the top level manifest will
> validated
> the content of the sub manifest too". I don't know how reliably to
> check
> it in a reasonable amount of time. I am not exposed to any tree
> manifest
> user myself.
>
>
> Ah, no, it won't read sub manifests. You're simply skipping sub
> manifests by including the "not dir" check.
so should we just drop the "not dir" ?
--
Pierre-Yves David
More information about the Mercurial-devel
mailing list