Re: [PATCH 2/2] pull: support rebased upstream + fetch + pull --rebase

From: Santi Béjar <santi@agolina.net>
Date: Jul 16, 2009, 06:29 UTC
Message-ID: <adf1fd3d0907152329v7f49999u42b0d0fc4d39f5e9@mail.gmail.com>
In-Reply-To: <7vab35cw2e.fsf@alter.siamese.dyndns.org>

2009/7/16 Junio C Hamano <gitster@pobox.com>:

Show 11 quoted lines

> Santi Béjar <santi@agolina.net> writes:
>
>>       reflist="$(get_remote_merge_branch "$@" 2>/dev/null)" &&
>> -     oldremoteref="$(git rev-parse -q --verify \
>> -             "$reflist")"
>> +     num=0 &&
>> +     while oldremoteref="$(git rev-parse -q --verify "$reflist@{$num}")"
>
> Applying @{nth} reflog notation to something that identifies itself as a
> "list" made me go "Huh?".  Why is this variable called refLIST?  Shouldn't
> it be simply called something like "remoteref" or even "ref"?

It used to be a list, before my patch 97af7ff (parse-remote: function to get the tracking branch to be merge, 2009-06-12). I'll change it.

Show 8 quoted lines

>
>> +     do
>> +             test $oldremoteref = $(git merge-base $oldremoteref $curr_branch) &&
>> +             break
>> +             num=$((num+1))
>
> I think we always write "num=$(( $num + 1 ))" for portability; notice the
> lack of $ in your version.

Oops, you are right. I somehow missed, I even did "git grep "((" *.sh" to check it.

>
>> +     done
>
> Does this loop ever give up?  Should it?

When remote/$origin/$branch@{nth} does not exist. I don't think we need another way to give up (nth<10?) because normally nth is small, it does not harm the normal case and it can help when nth is large.

>
> What happens in the subsequent code outside of the patch context, when
> this loop does not find any suitable "old" value?

Then the $oldremoteref is empty and in the git-rebase command it is used as ${oldremoteref:-$merge_head} so it get replaced by $merge_head.

Thanks, Santi