Hi,
On Wed, Dec 29, 2010 at 11:31 PM, Martin von Zweigbergk
<martin.von.zweigbergk@xxxxxxxxx> wrote:
> On Tue, 28 Dec 2010, Thomas Rast wrote:
>
>> Martin von Zweigbergk wrote:
>> > @@ -168,11 +168,6 @@ pick_one () {
>> > output git rev-parse --verify $sha1 || die "Invalid commit name: $sha1"
>> > test -d "$REWRITTEN" &&
>> > pick_one_preserving_merges "$@" && return
>> > - if test -n "$rebase_root"
>> > - then
>> > - output git cherry-pick "$@"
>> > - return
>> > - fi
>> > output git cherry-pick $ff "$@"
>> > }
>> [...]
>> > While factoring out the state writing code a few patches back, I went
>> > through each of the pieces of state that was written. I was a bit
>> > hesitant to include this patch since I'm not quite sure why the code
>> > was introduced, but I thought I would include it anyway to hear what
>> > you have to say.
>> >
>> > There used to be bug when using --ff when rebasing a root commit. This
>> > was fixed in 6355e50 (builtin/revert.c: don't dereference a NULL
>> > pointer, 2010-09-27). Could this have been the reason for the check?
>> > Thomas, do you remember?
>>
>> I think this just ended up being such a strange test because of the
>> following hunk in 8e75abf (rebase -i: use new --ff cherry-pick option,
>> 2010-03-06):
>>
>> @@ -232,16 +232,7 @@ pick_one () {
>> output git cherry-pick "$@"
>> return
>> fi
>> - parent_sha1=$(git rev-parse --verify $sha1^) ||
>> - die "Could not get the parent of $sha1"
>> - current_sha1=$(git rev-parse --verify HEAD)
>> - if test -z "$no_ff" && test "$current_sha1" = "$parent_sha1"
>> - then
>> - output git reset --hard $sha1
>> - output warn Fast-forward to $(git rev-parse --short $sha1)
>> - else
>> - output git cherry-pick "$@"
>> - fi
>> + output git cherry-pick $ff "$@"
>> }
>> --
>
> Yes, I saw that one as well, so it would probably have made more sense
> to ask Christian instead (the author of 8e75abf). So do you remember,
> Christian?
Yeah, I must say that I did not try to understand why there was a
special case for $rebase_root above the code I was changing.
Perhaps I should have, and I would probably have sent a patch like yours...
> Anyway, thanks for your input, Thomas. That makes me feel a little
> less worried about this patch.
Yeah I think it's a good one.
Thanks,
Christian.
--
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