Re: [PATCH 3/3] commit: give correct advice for empty commit during a rebase

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Nov 11, 2019, 16:13 UTC
Message-ID: <64bd1151-d2cd-4e8c-51f6-21e779d333a3@gmail.com>
In-Reply-To: <nycvar.QRO.7.76.6.1911081508560.46@tvgsbejvaqbjf.bet>

Hi Dscho & Junio

On 08/11/2019 14:09, Johannes Schindelin wrote:

Show 22 quoted lines

> Hi,
> 
> On Fri, 8 Nov 2019, Junio C Hamano wrote:
> 
>> Phillip Wood <phillip.wood123@gmail.com> writes:
>>
>>>>> I don't this patch can distinguish between an empty cherry-pick
>>>>> performed by the user while a rebase is in progress and an empty pick
>>>>> performed by rebase as both create CHERRY_PICK_HEAD while
>>>>> .git/rebase-merge exists. It seems to assume that CHERRY_PICK_HEAD was
>>>>> created by rebase and prints advise based on that which may or may not
>>>>> be the correct. I think we could distinguish the two by checking if
>>>>> CHERRY_PICK_HEAD matches .git/rebase-merge/stopped-sha or REBASE_HEAD.
>>>>
>>>> I guess we could, but then, I would rather worry about that in the next
>>>> cycle. In this cycle, I would rather fix the common case, which is that
>>>> a `git rebase -i` fails and tells me to `git cherry-pick --skip` instead
>>>> of `git rebase --skip`.
>>>>
>>>> And even if I performed a `git cherry-pick` during a `git rebase` and
>>>> the result would be an empty commit, I'd rather be told to `git rebase
>>>> --skip` to continue...

It depends if you want to continue immediately or do something else in which case running reset is probably better advice. I'm not sure that there's an easy solution for all possible scenarios though.

Show 14 quoted lines

>>>>
>>>> But if you feel strongly that this should be fixed differently, I'll
>>>> gladly leave it to you ;-)
>>>
>>> I'm happy to wait until the next cycle once we've decided what to do
>>> about CHERRY_PICK_HEAD during rebases.
>>
>> So, is that agreed between the two?
>>
>> Should I eject js/advise-rebase-skip topic out of my tree and wait
>> for the decision wrt CHERRY_PICK_HEAD?
> 
> Phillip, if you have some time to spend on that, I'd be very grateful, I
> am a bit under the weather and in dear need for an offline weekend.

I'm happy to have a look at it but it probably wont be for a couple of weeks. I've been thinking about keeping CHERRY_PICK_HEAD and it's not totally straight forward. CHERRY_PICK_HEAD needs to be removed when committing fixups to keep the author data from the commit that's being amended (also I think `git commit --amend` errors out if CHERRY_PICK_HEAD is present). It's further complicated by --reset-date and --committer-date-is-author-date as we want commit to respect those when they've been passed to the rebase command.

Junio, if you drop this series for now I'll rework it with an enum as discussed elsewhere in this thread.

Best Wishes

Phillip

> Thanks,
> Dscho
>