Patchwork [V2] verify: add a config option to skip certain flag processors

login
register
mail settings
Submitter Jun Wu
Date May 14, 2017, 4:41 p.m.
Message ID <c3d89aaf46bf50343dd0.1494780114@x1c>
Download mbox | patch
Permalink /patch/20619/
State Accepted
Headers show

Comments

Jun Wu - May 14, 2017, 4:41 p.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1494779886 25200
#      Sun May 14 09:38:06 2017 -0700
# Node ID c3d89aaf46bf50343dd06ff4e59dbadb0563b464
# Parent  78496ac300255e9996b3e282086661afc08af37c
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r c3d89aaf46bf
verify: add a config option to skip certain flag processors

Previously, "hg verify" verifies everything, which could be undesirable when
there are expensive flag processor contents.

This patch adds a "verify.skipflags" developer config. A flag processor will
be skipped if (flag & verify.skipflags) == 0.

In the LFS usecase, that means "hg verify --config verify.skipflags=8192"
will not download all LFS blobs, which could be too large to be stored
locally.

Note: "renamed" is also skipped since its default implementation may call
filelog.data() which will trigger the flag processor.
Sean Farley - May 14, 2017, 5:53 p.m.
Jun Wu <quark@fb.com> writes:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1494779886 25200
> #      Sun May 14 09:38:06 2017 -0700
> # Node ID c3d89aaf46bf50343dd06ff4e59dbadb0563b464
> # Parent  78496ac300255e9996b3e282086661afc08af37c
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r c3d89aaf46bf
> verify: add a config option to skip certain flag processors
>
> Previously, "hg verify" verifies everything, which could be undesirable when
> there are expensive flag processor contents.
>
> This patch adds a "verify.skipflags" developer config. A flag processor will
> be skipped if (flag & verify.skipflags) == 0.
>
> In the LFS usecase, that means "hg verify --config verify.skipflags=8192"
> will not download all LFS blobs, which could be too large to be stored
> locally.
>
> Note: "renamed" is also skipped since its default implementation may call
> filelog.data() which will trigger the flag processor.

Maybe I didn't follow this from before (apologies if so), but since this
is a user-facing option for LFS, why doesn't the LFS monkey patch this
in?
Jun Wu - May 14, 2017, 6:34 p.m.
Excerpts from Sean Farley's message of 2017-05-14 10:53:25 -0700:
> Maybe I didn't follow this from before (apologies if so), but since this
> is a user-facing option for LFS, why doesn't the LFS monkey patch this
> in?

The flag processor framework is in revlog.py, this config option is not only
for LFS but possible other flag procesors.
Sean Farley - May 15, 2017, 4:33 a.m.
Jun Wu <quark@fb.com> writes:

> Excerpts from Sean Farley's message of 2017-05-14 10:53:25 -0700:
>> Maybe I didn't follow this from before (apologies if so), but since this
>> is a user-facing option for LFS, why doesn't the LFS monkey patch this
>> in?
>
> The flag processor framework is in revlog.py, this config option is not only
> for LFS but possible other flag procesors.

I see. So, multiple extensions could use this. Ok, seems fine to me.
Augie Fackler - May 15, 2017, 5:41 p.m.
On Sun, May 14, 2017 at 09:41:54AM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1494779886 25200
> #      Sun May 14 09:38:06 2017 -0700
> # Node ID c3d89aaf46bf50343dd06ff4e59dbadb0563b464
> # Parent  78496ac300255e9996b3e282086661afc08af37c
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r c3d89aaf46bf
> verify: add a config option to skip certain flag processors

queued, thanks
Yuya Nishihara - May 16, 2017, 1:17 p.m.
On Sun, 14 May 2017 09:41:54 -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1494779886 25200
> #      Sun May 14 09:38:06 2017 -0700
> # Node ID c3d89aaf46bf50343dd06ff4e59dbadb0563b464
> # Parent  78496ac300255e9996b3e282086661afc08af37c
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r c3d89aaf46bf
> verify: add a config option to skip certain flag processors
> 
> Previously, "hg verify" verifies everything, which could be undesirable when
> there are expensive flag processor contents.
> 
> This patch adds a "verify.skipflags" developer config. A flag processor will
> be skipped if (flag & verify.skipflags) == 0.

Nit: maybe this should be "if (flag & verify.skipflags) != 0" ?
Jun Wu - May 16, 2017, 4:27 p.m.
Excerpts from Yuya Nishihara's message of 2017-05-16 22:17:57 +0900:
> > This patch adds a "verify.skipflags" developer config. A flag processor will
> > be skipped if (flag & verify.skipflags) == 0.
> 
> Nit: maybe this should be "if (flag & verify.skipflags) != 0" ?

You're right. I'm bad at writing logic expressions.
via Mercurial-devel - May 16, 2017, 4:31 p.m.
On Tue, May 16, 2017 at 9:27 AM, Jun Wu <quark@fb.com> wrote:
> Excerpts from Yuya Nishihara's message of 2017-05-16 22:17:57 +0900:
>> > This patch adds a "verify.skipflags" developer config. A flag processor will
>> > be skipped if (flag & verify.skipflags) == 0.
>>
>> Nit: maybe this should be "if (flag & verify.skipflags) != 0" ?
>
> You're right. I'm bad at writing logic expressions.

I'm sorry but I missed Yuya's email before accepting the patch, so
it's too late to change it :-(

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - May 16, 2017, 6:26 p.m.
On Tue, May 16, 2017 at 09:31:17AM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> On Tue, May 16, 2017 at 9:27 AM, Jun Wu <quark@fb.com> wrote:
> > Excerpts from Yuya Nishihara's message of 2017-05-16 22:17:57 +0900:
> >> > This patch adds a "verify.skipflags" developer config. A flag processor will
> >> > be skipped if (flag & verify.skipflags) == 0.
> >>
> >> Nit: maybe this should be "if (flag & verify.skipflags) != 0" ?
> >
> > You're right. I'm bad at writing logic expressions.
>
> I'm sorry but I missed Yuya's email before accepting the patch, so
> it's too late to change it :-(

Ugh, I'm sorry too. Jun, can you send a quick followup for us?

Thanks!

>
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
via Mercurial-devel - May 16, 2017, 8:44 p.m.
On Tue, May 16, 2017 at 11:26 AM, Augie Fackler <raf@durin42.com> wrote:
> On Tue, May 16, 2017 at 09:31:17AM -0700, Martin von Zweigbergk via Mercurial-devel wrote:
>> On Tue, May 16, 2017 at 9:27 AM, Jun Wu <quark@fb.com> wrote:
>> > Excerpts from Yuya Nishihara's message of 2017-05-16 22:17:57 +0900:
>> >> > This patch adds a "verify.skipflags" developer config. A flag processor will
>> >> > be skipped if (flag & verify.skipflags) == 0.
>> >>
>> >> Nit: maybe this should be "if (flag & verify.skipflags) != 0" ?
>> >
>> > You're right. I'm bad at writing logic expressions.
>>
>> I'm sorry but I missed Yuya's email before accepting the patch, so
>> it's too late to change it :-(
>
> Ugh, I'm sorry too. Jun, can you send a quick followup for us?

I think it was just in the commit message, so can't do a followup.

>
> Thanks!
>
>>
>> > _______________________________________________
>> > Mercurial-devel mailing list
>> > Mercurial-devel@mercurial-scm.org
>> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/verify.py b/mercurial/verify.py
--- a/mercurial/verify.py
+++ b/mercurial/verify.py
@@ -50,4 +50,6 @@  class verifier(object):
         self.refersmf = False
         self.fncachewarned = False
+        # developer config: verify.skipflags
+        self.skipflags = repo.ui.configint('verify', 'skipflags')
 
     def warn(self, msg):
@@ -428,6 +430,10 @@  class verifier(object):
                 #     use either "text" (external), or "rawtext" (in revlog).
                 try:
-                    fl.read(n) # side effect: read content and do checkhash
-                    rp = fl.renamed(n)
+                    skipflags = self.skipflags
+                    if skipflags:
+                        skipflags &= fl.flags(i)
+                    if not skipflags:
+                        fl.read(n) # side effect: read content and do checkhash
+                        rp = fl.renamed(n)
                     # the "L1 == L2" check
                     l1 = fl.rawsize(i)
diff --git a/tests/test-verify.t b/tests/test-verify.t
--- a/tests/test-verify.t
+++ b/tests/test-verify.t
@@ -318,2 +318,46 @@  test revlog format 0
   1 files, 1 changesets, 1 total revisions
   $ cd ..
+
+test flag processor and skipflags
+
+  $ hg init skipflags
+  $ cd skipflags
+  $ cat >> .hg/hgrc <<EOF
+  > [extensions]
+  > flagprocesor=$RUNTESTDIR/flagprocessorext.py
+  > EOF
+  $ echo '[BASE64]content' > base64
+  $ hg commit -Aqm 'flag processor content' base64
+  $ hg verify
+  checking changesets
+  checking manifests
+  crosschecking files in changesets and manifests
+  checking files
+  1 files, 1 changesets, 1 total revisions
+
+  $ cat >> $TESTTMP/break-base64.py <<EOF
+  > from __future__ import absolute_import
+  > import base64
+  > base64.b64decode=lambda x: x
+  > EOF
+  $ cat >> .hg/hgrc <<EOF
+  > breakbase64=$TESTTMP/break-base64.py
+  > EOF
+
+  $ hg verify
+  checking changesets
+  checking manifests
+  crosschecking files in changesets and manifests
+  checking files
+   base64@0: unpacking 794cee7777cb: integrity check failed on data/base64.i:0
+  1 files, 1 changesets, 1 total revisions
+  1 integrity errors encountered!
+  (first damaged changeset appears to be 0)
+  [1]
+  $ hg verify --config verify.skipflags=2147483647
+  checking changesets
+  checking manifests
+  crosschecking files in changesets and manifests
+  checking files
+  1 files, 1 changesets, 1 total revisions
+