Re: [PATCH] branch & tag: Add a --no-contains option

From: Jeff King <peff@peff.net>
Date: Mar 9, 2017, 10:46 UTC
Message-ID: <20170309104657.7pwreyozxo2tdhk4@sigill.intra.peff.net>
In-Reply-To: <CACBZZX53rMiB5-cA_7-SeU2Dt7d_Cr7_GgyC0rjQQPPf4qyCqw@mail.gmail.com>

On Thu, Mar 09, 2017 at 11:41:59AM +0100, Ævar Arnfjörð Bjarmason wrote:

Show 13 quoted lines

> > I almost suggested that there simply be an option to invert the match
> > (like --invert-contains or something).  But what you have here is more
> > flexible, if somebody ever wanted to do:
> >
> >   git tag --contains X --no-contains Y
> 
> Yeah that's really useful. E.g. this shows the branches I branched off
> (or have locally) from 2.6..2.8:
> 
>     $ ./git branch --contains v2.6.0 --no-contains v2.8.0
>       avar/monkeypatch-untracked-cache-disabled
>       avar/uc-notifs21
>       dturner/pclouds-watchman-noshm

Oh, that's a clever application.

Show 9 quoted lines

> But I'd expect this to show all the tags between the two:
> 
>     $ ./git tag --contains v2.6.0 --no-contains v2.8.0
>     $
> 
> But it just returns an empty list. Manually disabling the
> contains_tag_algo() path (i.e. effectively locally reverting your
> ffc4b8012d) makes it "work", but of course it's much slower now. I
> haven't dug into why it's not working yet.

I'm almost certain this is because the contains_tag_algo one doesn't clean up the flag bits it sets on the commit objects. So running it twice in the same process is going to give you nonsense results.

Coincidentally, I've been looking into resurrecting the cleaner approach that I sent long ago:

  http://public-inbox.org/git/20140625233429.GA20457@sigill.intra.peff.net/

But it's sufficiently complex that it's probably worth fixing the existing algorithm to clean up its bits in the meantime.

Show 6 quoted lines

> Also I wonder if this should be an error:
> 
>     $ ./git [tag|branch|for-each-ref] --contains A --no-contains A
> 
> I.e. when you give the same argument to both, this can never return
> anything for obvious reasons.

It's clearly nonsense, but I don't think there's any need for it to be an error. GIGO.

-Peff