Re: [PATCH v3] contrib/subtree: fix "subtree split" skipped-merge bug

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 9, 2015 at 10:23 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Dave Ware <davidw@xxxxxxxxxxxxxxxxxxxx> writes:
>
>> A bug occurs in 'git-subtree split' where a merge is skipped even when
>> both parents act on the subtree, provided the merge results in a tree
>> identical to one of the parents. Fix by copying the merge if at least
>> one parent is non-identical, and the non-identical parent is not an
>> ancestor of the identical parent.
>>
>> Also, add a test case which checks that a descendant can be pushed to
>> its ancestor in this case.
>>
>> Signed-off-by: Dave Ware <davidw@xxxxxxxxxxxxxxxxxxxx>
>> ---
>
> The first sentence may be made clearer if you rephrased the early
> part of the sentence this way:
>
>         'git subtree split' can incorrectly skip a merge even when
>         both parents ...
>

Noted.

>> diff --git a/contrib/subtree/git-subtree.sh b/contrib/subtree/git-subtree.sh
>> index 9f06571..b837531 100755
>> --- a/contrib/subtree/git-subtree.sh
>> +++ b/contrib/subtree/git-subtree.sh
>> @@ -479,8 +479,16 @@ copy_or_skip()
>>                       p="$p -p $parent"
>>               fi
>>       done
>> -
>> -     if [ -n "$identical" ]; then
>> +
>> +     copycommit=
>> +     if [ -n "$identical" ] && [ -n "$nonidentical" ]; then
>> +             extras=$(git rev-list --boundary $identical..$nonidentical)
>> +             if [ -n "$extras" ]; then
>> +                     # we need to preserve history along the other branch
>> +                     copycommit=1
>> +             fi
>
> What is the significance of "--boundary" here?  I think for the
> purpose of "is the identical one part of the nonidentical one?" you
> do not need it, but there may be something subtle I missed.  I am
> asking this because use of "rev-list --boundary" in scripts is
> almost always a bug.
>

The other way around actually I'm trying to determine if nonidentical
contains any commits
 not in identical.  I'll confess I don't actually know specifically
what the --boundary option
does, this probably came from a stack overflow example while we were
looking up how to
best do the check. Further experimentation with the option suggests
that it does not do what
I want, so I will remove it. Thank you.

> Also, depending on how huge the output from the rev-list could be,
> you might want to use "rev-list --count $i..$n" and compare it with
> 0 instead--that way, you would not have to be worried about having
> to carry around a huge string that you would otherwise not use, only
> to see if that string is empty.

Thanks, I didn't know about that option.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]