[PATCH 2 of 2] cat: do not instantiate subrepo if no potential match in it

[Matt Harbison]

On Sun, 26 Nov 2017 05:38:53 -0500, Yuya Nishihara <yuya at tcha.org> wrote:

> On Sat, 25 Nov 2017 23:53:45 -0500, Matt Harbison wrote:
>> On Sat, 25 Nov 2017 02:35:04 -0500, Yuya Nishihara <yuya at tcha.org>  
>> wrote:
>>
>> > # HG changeset patch
>> > # User Yuya Nishihara <yuya at tcha.org>
>> > # Date 1511593310 -32400
>> > #      Sat Nov 25 16:01:50 2017 +0900
>> > # Node ID b6e526ee5d2662ca30a555ca51264b3e371fe44e
>> > # Parent  38e952030f0c4746257280d47f0f94b6cb9ddb41
>> > cat: do not instantiate subrepo if no potential match in it
>> >
>> > This fixes the test failure in hg-git.
>> >
>> > https://bitbucket.org/durin42/hg-git/issues/227/
>> >
>> > diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> > --- a/mercurial/cmdutil.py
>> > +++ b/mercurial/cmdutil.py
>> > @@ -3068,6 +3068,8 @@ def cat(ui, repo, ctx, matcher, basefm,
>> >          err = 0
>> >     for subpath in sorted(ctx.substate):
>> > +        if not matcher.visitdir(subpath):
>> > +            continue
>>
>> The basematcher contract says the result is undefined if False is  
>> returned
>> for any parent directory.  Do we need a subrepo utility method to break  
>> up
>> the subpath and test top down?  Consider 'directory/subrepo', where
>> 'directory' is excluded.
>
> (+CC Martin)
>
> Yeah the doc says that, but not all callers seem to obey the  
> restriction. So
> I take it visitdir() won't return Falsy value if there is a possible  
> match,
> even though it might return non-False for Falsy case if any parents  
> return
> False.
>
>> It's probably beyond the scope of what you were fixing, but should all
>> subrepo recursion be guarded like this?
>>
>> >          sub = ctx.sub(subpath)
>> >          try:
>> >              submatch = matchmod.subdirmatcher(subpath, matcher)
>
> Maybe we'll need ctx.walksub(matcher) which yields (subrepo, submatcher)  
> pairs?

I like it.

I wonder if the exception handler that prints 'skipping..' can be rolled  
in too.  At least cmdutil.add() and cmdutil.cat() catch different things,  
and I have a vague recollection that there was also some uncaught  
exception in this area.  (I don't recall what it was, maybe the subrepo  
being completely missing?)