Re: [RFC] should `git rebase --keep-base` imply `--reapply-cherry-picks` ?

From: Denton Liu <liu.denton@gmail.com>
Date: Jul 14, 2020, 03:10 UTC
Message-ID: <20200714031017.GA15143@generichostname>
In-Reply-To: <0EA8C067-5805-40A7-857A-55C2633B8570@gmail.com>

Hi Philippe,

On Mon, Jul 13, 2020 at 10:44:06PM -0400, Philippe Blain wrote:

Show 6 quoted lines

> Hello,
> 
> I learned today that doing `git rebase --keep-base master` 
> will drop commits that were cherry-picked from master to the current branch. 
> I was simply doing a code clean up on my feature branch (the full command was
> `git rebase -i --keep-base master`), and this kind of confused me for a moment.

Glad I'm not the only one using this feature :)

Show 5 quoted lines

> Is this a sane default ? I understand that it is a good default when we are rebasing 
> *on top* of master, but here I'm just doing some squashing and fixup's and I did not
> want the commit I had cherry-picked from master to disappear (yet). In fact, because it
> was dropped, it created a modify/delete conflict because in a subsequent commit 
> in my feature branch I'm modifying files that are added in the commit I cherry-picked.

So if I'm not mistaken, if we have the following graph

	A - B - C - D (master)
	     \
	       - C' - D (feature)

and we do `git rebase --keep-base master` from feature, C' will be dropped? Indeed, I am surprised by how this interacts with the default setting of --reapply-cherry-picks.

> How would a change that made '--reapply-cherry-picks' be the default when using 'keep-base'
> be received ?

I'm somewhat surprised that --no-reapply-cherry-picks is the default. I would argue that it _shouldn't_ be the default at all. It's an optimisation for when no --onto or --keep-base are specified but it definitely can cause problems otherwise, as we've seen.

I think I would argue for the following in decreasing order of preference:

	1. Make --no-reapply-cherry-picks the default in all cases.
	   (Those who need the optimisation can enable it manually and
	   we can add a configuration option for it.)

	2. Make --no-reapply-cherry-picks only active if no --onto or
	   --keep-base are given (--keep-base is a special case of --onto
	   so we only have to handle it in one place).

> Tangential question: in any case, would it make sense to still add the "dropped because 
> already upstream" commits to the todo list, in the case of an interactive rebase ? 
> (maybe commented out, or listed as 'drop' with some kind of comment saying those 
> are dropped because they appear textually upstream?)

That would make sense to me. I don't have a preference between either.

Thanks,

Denton

> Cheers,
> Philippe.
> P.S. I CC'd those who were involved with the 'keep-base' patch or the 'reapply-cherry-picks' patch.