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

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

From
Jeff King <peff@peff.net>
Date
Jul 10, 2018, 04:31 UTC
Message-ID
<20180710043142.GA1909@sigill.intra.peff.net>
In-Reply-To
<20180710043120.GA1330@sigill.intra.peff.net>

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 existed before then (and which continues to issue an error).

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 &&
-- 
2.18.0.400.g702e398724
Previous: Jeff KingNext: Jeff King
Message 12 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.