git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC PATCH] Option to allow cherry-pick to skip empty commits

From
Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
Date
Jan 23, 2017, 10:18 UTC
Message-ID
<CAOxFTczut_CGGxmbrVFzhn_o4=StTOY6N1mEAw75Ro2Q4tzWgQ@mail.gmail.com>
In-Reply-To
<xmqqpojeu14h.fsf@gitster.mtv.corp.google.com>
On Mon, Jan 23, 2017 at 12:27 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
Show 33 quoted lines
>> +static int allow_or_skip_empty(struct replay_opts *opts, struct commit *commit)
>>  {
>>       int index_unchanged, empty_commit;
>>
>>       /*
>> -      * Three cases:
>> +      * Four cases:
>>        *
>> -      * (1) we do not allow empty at all and error out.
>> +      * (1) we do not allow empty at all and error out;
>>        *
>> -      * (2) we allow ones that were initially empty, but
>> +      * (2) we skip empty commits altogether;
>> +      *
>> +      * (3) we allow ones that were initially empty, but
>>        * forbid the ones that become empty;
>>        *
>> -      * (3) we allow both.
>> +      * (4) we allow both.
>>        */
>
> The original gave callers the choice to tell two cases (a commit was
> empty in the original history, and a commit that was not empty in
> the original history turns out to be redundant) apart and handle
> them differently.  I tend to agree that skipping the former should
> be the norm, and also I think it is sensible to drop the latter, and
> that is what your updated (2) gives us, I think.
>
> But I would suspect that it would rather be common to have a
> deliberately empty commit in the original as a marker in a history
> and want to keep that across cherry-picking a series, while wanting
> to discard/skip patches that are already applied in an updated base.
> Shouldn't that be supported as the fifth case?

I was actually wondering about that. I guess the best approach (symmetric wrt to the --allow) would be to intro introduce --skip-empty _and_ --skip-redundant, with the former implying the latter.

By the way, I noticed going over the code that the -allow options are not stored, so that in case of interruption they will be reset, is this intentional or a bug?

-- 
Giuseppe "Oblomov" Bilotta
Previous: Junio C HamanoNext: Junio C Hamano
Message 3 of 7 in “Option to allow cherry-pick to skip empty commits”
  1. Option to allow cherry-pick to skip empty commitsGiuseppe Bilotta, Jan 21, 2017
  2. Junio C HamanoJan 22, 2017
  3. Giuseppe BilottaJan 23, 2017
  4. Junio C HamanoJan 23, 2017
  5. Giuseppe BilottaJan 23, 2017
  6. Junio C HamanoJan 23, 2017
  7. Giuseppe BilottaJan 23, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.