Re: [RFC/PATCH 0/2] merge-base: add --merge-child option

From: Kevin Bracey <kevin@bracey.fi>
Date: May 12, 2013, 17:14 UTC
Message-ID: <518FCDDE.9040707@bracey.fi>
In-Reply-To: <20130512163317.GL2299@serenity.lan>

On 12/05/2013 19:33, John Keeping wrote:

Show 18 quoted lines

> On Sun, May 12, 2013 at 05:28:24PM +0100, John Keeping wrote:
>> You're right (and I was wrong in my reply to Junio's parallel message)
>> ancestry path does seem to be what I want:
>>
>>      $ git rev-list --ancestry-path --left-right --count \
>>              master...git-gui/master
>>      2056    5
>>
>> However, this doesn't seem to make a difference to the time taken when I
>> add in --cherry-mark (which is why I was partially correct in the
>> parallel thread - it doesn't have the effect on cherry-mark that I want
>> it to):
>>
>> This seems to be caused by the code in revision.c::limit_list() which
>> does the cherry detection then limits to left/right and only then
>> applies the ancestry path.  I haven't looked further than that, but is
>> there any reason not to apply the ancestry path restriction before
>> looking for patch-identical commits?

That certainly sounds like it's doing it the wrong way round. At first sight, it seems obviously suboptimal.

> No I didn't.  I forgot to update my $PATH when I built on master - those
> results are from pu.  master says:
>
>      fatal: --ancestry-path given but there are no bottom commits

Well, then it looks like we have a user for that particular syntax. Seemed a bit esoteric to me :) Although I realised after sending my mail you could also use

    git log --ancestry-path --left-right E...F --not $(git merge-base 
--all E F)

which looks like we're having to repeat ourselves because it's not paying attention...

I hit it because one of my optimisations relies on knowing the bottom commits, and I made absolutely sure I was using the exactly same definition of "bottom" as --ancestry-path. And then I found that my optimisations didn't work properly with "..."

I suggest we pull my patch out from the more complex optimisation series so it can proceed to next faster. It shouldn't have to wait for all my new fancy stuff - it's fixing something that appears to be a clear bug.

Although Junio did have a comment about the implementation - I'll revisit it tomorrow and send a revised version separately, if everyone thinks that's sensible.

Show 12 quoted lines

>
>>> On this subject, is there any way to exclude a path from a log query? Is
>>> there a "not" operator for paths? Might be another way of doing this -
>>> disjoint histories probably have disjoint paths...
>> That relates to another idea I had about optimizing the detection of
>> patch-identical commits.  If the smaller side of a symmetric difference
>> is quite small (as it is likely to be if it's a topic branch), would it
>> be a good idea to calculate the set of paths touched by commits on that
>> side and then skip calculating the patch ID for any commits that touch
>> paths outside that set.  The tree comparison is a lot cheaper than doing
>> a diff on all of the files.
>>

Sounds cute. Go for it :)

Kevin