<div dir="auto"><div><br><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Apr 17, 2019, 03:34 Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank" rel="noreferrer">pierre-yves.david@ens-lyon.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
<br>
On 4/17/19 4:59 AM, Martin von Zweigbergk wrote:<br>
> <br>
> <br>
> On Tue, Apr 16, 2019, 16:46 Pierre-Yves David <br>
> <<a href="mailto:pierre-yves.david@ens-lyon.org" rel="noreferrer noreferrer" target="_blank">pierre-yves.david@ens-lyon.org</a> <mailto:<a href="mailto:pierre-yves.david@ens-lyon.org" rel="noreferrer noreferrer" target="_blank">pierre-yves.david@ens-lyon.org</a>>> <br>
> wrote:<br>
> <br>
> # HG changeset patch<br>
> # User Pierre-Yves David <<a href="mailto:pierre-yves.david@octobus.net" rel="noreferrer noreferrer" target="_blank">pierre-yves.david@octobus.net</a><br>
> <mailto:<a href="mailto:pierre-yves.david@octobus.net" rel="noreferrer noreferrer" target="_blank">pierre-yves.david@octobus.net</a>>><br>
> # Date 1551881213 -3600<br>
> # Wed Mar 06 15:06:53 2019 +0100<br>
> # Node ID ed796867a06764cd78a57b2ed0249353f5809319<br>
> # Parent 9bec7491e9b4cabdfa4d264e5213b1f416ec2607<br>
> # EXP-Topic verify<br>
> # Available At <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer noreferrer noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a><br>
> # hg pull<br>
> <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer noreferrer noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a> -r ed796867a067<br>
> verify: also check full manifest validity during verify runs<br>
> <br>
> Before this changes, `hg verify` only checked if a manifest revision<br>
> existed and<br>
> referenced the proper files. However it never checked the manifest<br>
> revision<br>
> content itself.<br>
> <br>
> Mercurial is expecting manifest entries to be sorted and will crash<br>
> otherwise.<br>
> Since `hg verify` did not attempted a full restoration of manifest<br>
> entry, it<br>
> could ignore this kind of corruption.<br>
> <br>
> This new check significantly increases the cost of a `hg verify`<br>
> run. This<br>
> especially affects large repository not using `sparse-revlog`. For<br>
> now, this is<br>
> hidden behind the `--full` experimental flag.<br>
> <br>
> diff --git a/mercurial/verify.py b/mercurial/verify.py<br>
> --- a/mercurial/verify.py<br>
> +++ b/mercurial/verify.py<br>
> @@ -337,6 +337,16 @@ class verifier(object):<br>
> filenodes.setdefault(fullpath,<br>
> {}).setdefault(fn, lr)<br>
> except Exception as inst:<br>
> self._exc(lr, _("reading delta %s") % short(n),<br>
> inst, label)<br>
> + if not dir and self._level >= VERIFY_FULL:<br>
> <br>
> <br>
> What does the "not dir" mean? I guess it's to do this check only for the <br>
> root directory when using tree manifests. Should we do it for all <br>
> directories?<br>
<br>
That is used earlier in the same function to denote "the root manifest". <br>
I think check the root manifest will trigger checks of the sub manifest <br>
but I am not sure, I am not too familiar tree manifest.</blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> Can we double <br>
check/fix this as a follow up ?<br></blockquote></div></div><div dir="auto"><br></div><div dir="auto">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.</div><div dir="auto"><br></div><div dir="auto">Btw, it would be nice to have tests too, but I understand that that's harder to do. Thoughts on how it could be done? Prepared bundle or some python code for writing out a bad manifest entry should work, I guess.</div><div dir="auto"><br></div><div dir="auto"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> <br>
> + try:<br>
> + # Manifest not in sorted order are invalid and<br>
> will crash<br>
> + # Mercurial. We restore each entry to make sure<br>
> they are<br>
> <br>
> <br>
> Nit: "restore" almost makes it sound like this is fixing broken entries. <br>
> Maybe something like "We read the full manifest at each revision to..."?<br>
<br>
Good point, V2, coming<br>
<br>
-- <br>
Pierre-Yves David<br>
</blockquote></div></div></div>