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

From: Phillip Wood <phillip.wood123@gmail.com>
To: Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Junio C Hamano <gitster@pobox.com>
Cc: Johannes Schindelin via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Subject: Re: [PATCH 3/3] commit: give correct advice for empty commit during a rebase
Date: Mon, 11 Nov 2019 16:13:37 +0000	[thread overview]
Message-ID: <64bd1151-d2cd-4e8c-51f6-21e779d333a3@gmail.com> (raw)
In-Reply-To: <nycvar.QRO.7.76.6.1911081508560.46@tvgsbejvaqbjf.bet>

Hi Dscho & Junio

On 08/11/2019 14:09, Johannes Schindelin wrote:
> 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.

>>>>
>>>> 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
> 