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

From: Santi Béjar <santi@agolina.net>
Date: Jul 17, 2009, 13:24 UTC
Message-ID: <adf1fd3d0907170624t7c2bf8f5w35c03788fdd82422@mail.gmail.com>
In-Reply-To: <7vr5wf67iz.fsf@alter.siamese.dyndns.org>

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

Show 50 quoted lines

> Santi Béjar <santi@agolina.net> writes:
>
>> 2009/7/17 Santi Béjar <santi@agolina.net>:
>>> 2009/7/16 Junio C Hamano <gitster@pobox.com>:
>>>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>>>
>>>>> How about
>>>>>
>>>>>       oldremoteref="$(git rev-list --boundary HEAD --not \
>>>>>                       $(git rev-list -g $remoteref | sed 's/$/^@/') |
>>>>>               sed -e '/^[^-]/d' -e q)"
>>>>>
>>>>> Explanation: the "git rev-list -g $remoteref" lists the previous commits
>>>>> the remote ref pointed to, and the ^@ appended to them means all their
>>>>> parents.  Now, the outer rev-list says to take everything in HEAD but
>>>>> _not_ in those parents, showing the boundary commits.  The "sed" call
>>>>> lists the first such boundary commit (which must, by construction, be one
>>>>> of the commits shown by the first rev-list).
>>>>
>>>> Hmm, I am not sure about that "(which must..." part.
>>
>> Unfortunatly you are right with the "(which must..." part. Even
>> without the ^@. Normally gives the right answer, but it is not
>> sure that the first commit boundary is the correct one. For
>> example:
>>
>>          o--C
>>         /
>>  A--x--y--B--o--z
>>      \      /
>>       o----o
>>
>> A, B, C are upstream@{n}
>>
>> It involves a merge with a branch forked before the fork commit
>> for the current branch, and it will not work neither with git
>> pull --rebase. We could say that it is not supported, but
>> nevertheless it gives the wrong answer.
>>
>> The right answer is B, but:
>> $ git rev-list --boundary z --not C B A
>> z
>> o
>> o
>> o
>> -x
>> -B
>
> Now a short question.  Does your original loop give a correct answer in
> this case?

Yes, it returns B. But there are other cases where there is not a single right answer if you allow merges. For the moment the more sensible thing to do is to not allow merges in the local commits. I hope nobody relies on "git pull --rebase" with local merges.

Just an example:

        E
       /
      D----a topic2
     /      \
 A--B--C--b--c--d--topic1

A, B, C, D, E are upstream@{n} (n = 4,3,2,1,0)

if you are on branch "topic1", and run "git pull --rebase" you would want to rebase only b, c and d (maybe "a" but you should not), but for sure not D. And my algorithm returns C, but a more "correct" answer would be C and D.

Another possibility could be to check that there is only one boundary commit (only one fork point for all the local commits).

Wait! Let's return to the original problem. The original problem is that you cannot do a "git pull --rebase" with a rebased upstream if you have already done "git fetch" before. And the solution would be: Try to behaved as if the "git fetch" was not run.

And this is exactly what my patch does.

All the other "problems" happens already.

Now I only have to solve the "git rev-parse -q --verify upstream@{large_n}" problem or workaround it.

Sometimes thinks aloud, Santi