<div dir="ltr"><div dir="ltr"><div class="gmail_default" style="font-family:arial,helvetica,sans-serif"><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Mar 7, 2019 at 9:21 PM Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org">pierre-yves.david@ens-lyon.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><br>
<br>
On 3/6/19 7:31 PM, Pulkit Goyal wrote:<br>
> <br>
> <br>
> On Wed, Mar 6, 2019 at 8:10 PM Pierre-Yves David <br>
> <<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a> <mailto:<a href="mailto:pierre-yves.david@ens-lyon.org" 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" target="_blank">pierre-yves.david@octobus.net</a><br>
>     <mailto:<a href="mailto:pierre-yves.david@octobus.net" target="_blank">pierre-yves.david@octobus.net</a>>><br>
>     # Date 1551881213 -3600<br>
>     #      Wed Mar 06 15:06:53 2019 +0100<br>
>     # Node ID 1e978412397a4fe629dfa9420b1a7773dc936b19<br>
>     # Parent  92091b3de1503bf562fe5bf2b544781f92702738<br>
>     # EXP-Topic verify<br>
>     # Available At <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a><br>
>     #              hg pull<br>
>     <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a> -r 1e978412397a<br>
>     verify: also check full manifest validity during verify runs [RFC]<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 tried a full restoration of manifest<br>
>     entry, it could<br>
>     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`. We<br>
>     might want to<br>
>     put it behind a new flag like `--full`. Another way to look at this<br>
>     would be to<br>
>     offer a `--quick` flag that disable it. In particular, since `hg<br>
>     recover` runs<br>
>     verify, this could impact users.<br>
> <br>
> <br>
> A slow `hg recover` is already painful for us whenever we run it. So we <br>
> decided whatever happen, we will never Ctrl+C a hg process.<br>
<br>
Yeah, I've experienced that too. Overall, the recover logic have been <br>
pretty solid in the case I end up using it. We should offer a way to run <br>
recover without the verify step.<br>
<br>
<br>
> That said, what are the cases which can lead to unsorted manifest <br>
> revisions? It will be worth putting them in commit message.<br>
<br>
There are a chicken and eggs issue here, I needed this command to learn <br>
more about the issue. And I now do know more:<br>
<br>
There are 2 affected manifest used in 3 changesets for Mozilla-try <br>
(6b58490b1f9b, f13fe2770423, and 58842787f756).<br>
All three changesets dates back from 2015 and are Authored by Mike <br>
Hommey. So I strongly suspect they were created by an early version of <br>
cinabar.<br>
<br>
(I'm playing with Mozilla try as a test repository for our discovery works).<br>
<br>
Even if such corruption will be very rare, I still think that running <br>
the extra check make sense by default. The following verify helps <br>
statement is currently wrong: "validating the hashes and checksums of <br>
each entry in the changelog, manifest, and tracked files". The manifest <br>
log hashes are not currently checked. This changeset fix this.<br>
<br>
However this will make the recover situation worth. Here a wider plan to <br>
deal with that<br>
<br>
1) add a --quick flag to `hg verify` that only check revlog length,<br>
2) add a --verify flag to `hg recover` (--no-verify will skip the verify <br>
step),<br>
3) make --no-verify the default,<br>
3.5) maybe deprecated the --verify flag,<br>
4) check manifest hashes (and order) by default.<br>
<br>
What do you think ?<br></blockquote><div><br></div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default">I am bit confused. In 2) we add a --verify flag and in 3.5) we deprecate that.</div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default"><br></div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default">I like the idea of `hg recover` having a `--verify` flag. For verify command, I am not sure which one is better from these:</div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default"><br></div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default">1) have a `--quick` flag having behavior similar to current verify</div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default">2) have a `--full` flag which does full verification</div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default">3) have both 1) and 2), keep the default behavior as 1) and in future change it 2)</div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default"><br></div><div style="font-family:arial,helvetica,sans-serif" class="gmail_default">I am +1 for checking manifest hashes and order by default provided we have a way to skip that both in verify and recover.<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
-- <br>
Pierre-Yves David<br>
</blockquote></div></div>