Re: [PATCH v2 6/8] cherry-pick: decouple `--allow-empty` and `--keep-redundant-commits`

From: Junio C Hamano <gitster@pobox.com>
To: Phillip Wood <phillip.wood123@gmail.com>
Cc: Brian Lyles <brianmlyles@gmail.com>,
	 git@vger.kernel.org, newren@gmail.com,  me@ttaylorr.com
Subject: Re: [PATCH v2 6/8] cherry-pick: decouple `--allow-empty` and `--keep-redundant-commits`
Date: Thu, 22 Feb 2024 10:41:16 -0800	[thread overview]
Message-ID: <xmqqwmqwcpf7.fsf@gitster.g> (raw)
In-Reply-To: <3f276e10-7b03-4480-a157-47a7648e7f19@gmail.com> (Phillip Wood's message of "Thu, 22 Feb 2024 16:35:29 +0000")

Phillip Wood <phillip.wood123@gmail.com> writes:

> ... I still tend to think the practical effect of implying
> --allow-empty with --keep-redundant-commits is largely beneficial as
> I'm skeptical that users want to keep commits that become empty but
> not the ones that started empty.

I share that feeling exactly.

There are good reasons to keep a commit that starts as empty (as
much as creating an empty commit in the first place), so if
anything, a more common workflow element would be to drop the ones
that have become unnecessary (e.g. because the upstream already
added a change that is equivalent to what is being picked) while
keeping the ones that are empty from the start (e.g. in some
workflows an empty commit can be used as a container of
metainfo---you can imagine that in an N-commit chain leading to the
tip of a topic branch forked from the master branch, the topmost
commit is an empty one with the cover letter being prepared, so that
the resulting topic branch can be either (1) made into a patch
series with an advanced version of "git format-patch" that knows how
to use such an empty top commit in the cover letter message, or (2)
merged to the mainline via a pull request, with an advanced version
of "git merge" that notices the empty commit at the tip, and makes a
merge with the commit topic~1 while using the empty top commit to
write the message for the merge commit.

I do not quite see a good reason to do the opposite, dropping
commits that started out as empty but keeping the ones that have
become empty.  Such a behaviour has additional downside that after
such a cherry-pick, when you cherry-pick the resulting history onto
yet another base, your precious "were not empty but have become so
during the initial cherry-pick" commits will appear as commits that
were empty from the start.  So I do not see much reason to allow the
decoupling, even with the new "empty=keep" thing that does not imply
"allow-empty".

Thanks.