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

Re: [PATCH 1/2] sequencer: handle empty-set cases consistently

From
Junio C Hamano <gitster@pobox.com>
Date
Jul 9, 2018, 20:20 UTC
Message-ID
<xmqqlgakcga6.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<20180709194819.GA7980@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 23 quoted lines
> If the user gives us a set that prepare_revision_walk()
> takes to be empty, like:
>
>   git cherry-pick base..base
>
> then we report an error. It's nonsense, and there's nothing
> to pick.
>
> But if they use revision options that later cull the list,
> like:
>
>   git cherry-pick --author=nobody base~2..base
>
> then we quietly create an empty todo list and return
> success.
>
> Arguably either behavior is acceptable, but we should
> definitely be consistent about it. Reporting an error
> seems to match the original intent, which dates all the way
> back to 7e2bfd3f99 (revert: allow cherry-picking more than
> one commit, 2010-06-02). That in turn was trying to match
> the single-commit case that exited before then (and which
> continues to issue an error).

Other than s/exited/existed/, the above looks perfect ;-) I wish all analysis part of proposed log messages were written like this.

Show 53 quoted lines
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  sequencer.c                     | 6 ++++--
>  t/t3510-cherry-pick-sequence.sh | 7 ++++++-
>  2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 5354d4d51e..f692b2ef44 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -1863,8 +1863,6 @@ static int prepare_revs(struct replay_opts *opts)
>  	if (prepare_revision_walk(opts->revs))
>  		return error(_("revision walk setup failed"));
>  
> -	if (!opts->revs->commits)
> -		return error(_("empty commit set passed"));
>  	return 0;
>  }
>  
> @@ -2317,6 +2315,10 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
>  			short_commit_name(commit), subject_len, subject);
>  		unuse_commit_buffer(commit, commit_buffer);
>  	}
> +
> +	if (!todo_list->nr)
> +		return error(_("empty commit set passed"));
> +
>  	return 0;
>  }
>  
> diff --git a/t/t3510-cherry-pick-sequence.sh b/t/t3510-cherry-pick-sequence.sh
> index b42cd66d3a..3505b6aa14 100755
> --- a/t/t3510-cherry-pick-sequence.sh
> +++ b/t/t3510-cherry-pick-sequence.sh
> @@ -480,11 +480,16 @@ test_expect_success 'malformed instruction sheet 2' '
>  	test_expect_code 128 git cherry-pick --continue
>  '
>  
> -test_expect_success 'empty commit set' '
> +test_expect_success 'empty commit set (no commits to walk)' '
>  	pristine_detach initial &&
>  	test_expect_code 128 git cherry-pick base..base
>  '
>  
> +test_expect_success 'empty commit set (culled during walk)' '
> +	pristine_detach initial &&
> +	test_expect_code 128 git cherry-pick -2 --author=no.such.author base
> +'
> +
>  test_expect_success 'malformed instruction sheet 3' '
>  	pristine_detach initial &&
>  	test_expect_code 1 git cherry-pick base..anotherpick &&
Previous: Jeff KingNext: Johannes Schindelin
Message 5 of 15 in “[BUG] git cherry-pick does not complain about unknown options”
  1. Andrei RybakJul 9, 2018
  2. Jeff KingJul 9, 2018
  3. 0/2 de-confuse git cherry-pick --author behaviorJeff King, Jul 9, 2018
  4. 1/2 sequencer: handle empty-set cases consistentlyJeff King, Jul 9, 2018
  5. Junio C HamanoJul 9, 2018
  6. Johannes SchindelinJul 9, 2018
  7. 2/2 sequencer: don't say BUG on bogus inputJeff King, Jul 9, 2018
  8. Johannes SchindelinJul 9, 2018
  9. Junio C HamanoJul 9, 2018
  10. Jeff KingJul 10, 2018
  11. 0/2 de-confuse git cherry-pick --authorJeff King, Jul 10, 2018
  12. 1/2 sequencer: handle empty-set cases consistentlyJeff King, Jul 10, 2018
  13. 2/2 sequencer: don't say BUG on bogus inputJeff King, Jul 10, 2018
  14. Johannes SchindelinJul 11, 2018
  15. Junio C HamanoJul 11, 2018

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.