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

["Santi Béjar" <santi@agolina.net>]

2009/7/18 Junio C Hamano <gitster@pobox.com>:
> Santi Béjar <santi@agolina.net> writes:
>
>> Changes since v2:
>>   - Hopefully enhance the commit log
>>   - Use a 'for' loop for the reflog entries
>>   - provide a default value in case there is no reflog
>> diff --git a/git-pull.sh b/git-pull.sh
>> index 4b78a0c..c8f1674 100755
>> --- a/git-pull.sh
>> +++ b/git-pull.sh
>> @@ -125,9 +125,16 @@ test true = "$rebase" && {
>>       die "refusing to pull with rebase: your working tree is not up-to-date"
>>
>>       . git-parse-remote &&
>> -     reflist="$(get_remote_merge_branch "$@" 2>/dev/null)" &&
>> +     remoteref="$(get_remote_merge_branch "$@" 2>/dev/null)" &&
>> +     oldremoteref= &&
>> +     for reflog in $(git rev-list -g $remoteref 2>/dev/null)
>> +     do
>> +             test $reflog = $(git merge-base $reflog $curr_branch) &&
>> +             oldremoteref=$reflog && break
>> +     done
>> +     [ -z "$oldremoteref" ] &&
>>       oldremoteref="$(git rev-parse -q --verify \
>> -             "$reflist")"
>> +             "$remoteref")"
>>  }
>
> If get_remote_merge_branch fails, oldremoteref is not initialized to empty
> string, the for loop is skipped and then the last step (by the way, please
> write that as 'test -z "$oldremoteref"') may not kick in, using whatever
> random value the variable originally had in the environment.

Not a justification but it also happens with the original code.

>
> It probably makes more sense to do it in a slightly different order:
>
>        . git-parse-remote &&
>        oldremoteref="$(get_remote_merge...)" &&

oldremotref must be a sha1, because it changes during the fetch. But
I'll use more o less this order.

>        remoteref=$oldremoteref &&
>        for old in $(git rev-list -g "$remoteref" 2>/dev/null)
>        do
>                if test "$old" = "$(git merge-base "$old" "$current_branch")
>                then
>                        oldremoteref="$old"
>                        break
>                fi
>        done
>        # and you do not need 'if test -z "$oldremoteref"' anymore...
>
> But other than that, I agree that this is the most straightforward
> algorithm to express what you wanted to do.  I guess another possibility
> is to instead look in the reflog of the _current_ branch to check how the
> previous rebase was done, iow, find out onto which commit the recent part
> of the current branch was rebased to, and rebase onto the current remote
> tip using that as the base.

It supposes that it was rebased already, and you have to interpret the
reflog history. And there are situations where you don't get the same
answer, i.e you create a new local branch using another local branch
as the starting point but with the same upstream.

Santi