<div dir="ltr"><div dir="ltr">On Wed, Apr 17, 2019 at 7:54 AM Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org">pierre-yves.david@ens-lyon.org</a>> wrote:<br></div><div class="gmail_quote"><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 4/17/19 3:15 PM, Gregory Szorc wrote:<br>
> <br>
> <br>
>> On Apr 17, 2019, at 01:38, Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a>> 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>
>> # Date 1555456341 -7200<br>
>> # Wed Apr 17 01:12:21 2019 +0200<br>
>> # Node ID 55bd98999c25b10e220477fd4cc446a7c9c1f8ca<br>
>> # Parent f233cb63bc077267d8571378350d9563cbabcf3d<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 <a href="https://bitbucket.org/octobus/mercurial-devel/" rel="noreferrer" target="_blank">https://bitbucket.org/octobus/mercurial-devel/</a> -r 55bd98999c25<br>
>> verify: introduce a notion of "level"<br>
>><br>
>> Some checks are slower than others, to help the user to run the checks he needs,<br>
>> we are about to introduce new flag to select faster vs deeper runs. This put<br>
>> the scaffolding in place to do this.<br>
> <br>
> I’m in favor of customizing verify behavior: it is an overdue feature IMO.<br>
> <br>
> Experience tells me that shoehorning things into a numeric level will be fragile and won’t scale well. I’m wondering if we should introduce individual “feature flags” / arguments to control what is verified. That will make the code a bit cleaner and easier to separate IMO. If we want to map a number to a set of verify options, we can do that too.<br>
> <br>
> This idea is scope bloat. But something tells me we’ll regret the limitations of numeric levels in the future. I’d rather pass in a set of things to verify. This is also more extensible.<br>
<br>
I agree with you on the general principle (being able to select what we <br>
check for. I also agree that it is outside the scope of this small <br>
series. (I documented the checks earlier this cycle, so we will have <br>
something to start from).<br>
<br>
I am not sure if you are requesting changes here or not. The internal <br>
implementation can be changed later and the flag is experimental, so <br>
everything in this series can be adjusted later.<br>
<br>
Do you have specific changes in mind to get this series in ?<br></blockquote><div><br></div><div>I have no major objections to using a numeric level for now. But if any more complexity is added, it might be best to do something more flexible sooner rather than later.<br></div><div> </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>
> <br>
>><br>
>> diff --git a/mercurial/hg.py b/mercurial/hg.py<br>
>> --- a/mercurial/hg.py<br>
>> +++ b/mercurial/hg.py<br>
>> @@ -1092,9 +1092,9 @@ def outgoing(ui, repo, dest, opts):<br>
>> recurse()<br>
>> return 0 # exit code is zero since we found outgoing changes<br>
>><br>
>> -def verify(repo):<br>
>> +def verify(repo, level=None):<br>
>> """verify the consistency of a repository"""<br>
>> - ret = verifymod.verify(repo)<br>
>> + ret = verifymod.verify(repo, level=level)<br>
>><br>
>> # Broken subrepo references in hidden csets don't seem worth worrying about,<br>
>> # since they can't be pushed/pulled, and --hidden can be used if they are a<br>
>> diff --git a/mercurial/verify.py b/mercurial/verify.py<br>
>> --- a/mercurial/verify.py<br>
>> +++ b/mercurial/verify.py<br>
>> @@ -22,9 +22,12 @@ from . import (<br>
>> util,<br>
>> )<br>
>><br>
>> -def verify(repo):<br>
>> +VERIFY_DEFAULT = 0<br>
>> +<br>
>> +def verify(repo, level=None):<br>
>> with repo.lock():<br>
>> - return verifier(repo).verify()<br>
>> + v = verifier(repo, level)<br>
>> + return v.verify()<br>
>><br>
>> def _normpath(f):<br>
>> # under hg < 2.4, convert didn't sanitize paths properly, so a<br>
>> @@ -34,10 +37,13 @@ def _normpath(f):<br>
>> return f<br>
>><br>
>> class verifier(object):<br>
>> - def __init__(self, repo):<br>
>> + def __init__(self, repo, level=None):<br>
>> self.repo = repo.unfiltered()<br>
>> self.ui = repo.ui<br>
>> self.match = repo.narrowmatch()<br>
>> + if level is None:<br>
>> + level = VERIFY_DEFAULT<br>
>> + self._level = level<br>
>> self.badrevs = set()<br>
>> self.errors = 0<br>
>> self.warnings = 0<br>
>> _______________________________________________<br>
>> Mercurial-devel mailing list<br>
>> <a href="mailto:Mercurial-devel@mercurial-scm.org" target="_blank">Mercurial-devel@mercurial-scm.org</a><br>
>> <a href="https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel" rel="noreferrer" target="_blank">https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel</a><br>
<br>
-- <br>
Pierre-Yves David<br>
</blockquote></div></div>