Thread (1 message) 1 message, 1 author, 2016-06-15

Re: [PATCH v3] cherry-pick: make sure all input objects are commits

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:57:11

Miklos Vajna [off-list ref] writes:
When a single argument was a non-commit, the error message used to be:

	fatal: BUG: expected exactly one commit from walk

For multiple arguments, when none of the arguments was a commit, the error was:

	fatal: empty commit set passed

Finally, when some of the arguments were non-commits, we ignored those
arguments.  Fix this bug and make sure all arguments are commits, and
for the first non-commit, error out with:

	fatal: <name>: Can't cherry-pick a <type>

Signed-off-by: Miklos Vajna <redacted>
This turns out to be an irritatingly stupid change.  While I am
rebuilding a privately tagged tip of 'maint', I am seeing:

	fatal: v1.8.2.3: Can't cherry-pick a tag

You would want to reject non committish, not non commit.
quoted hunk
 sequencer.c                         | 18 ++++++++++++++++++
 t/t3508-cherry-pick-many-commits.sh |  6 ++++++
 2 files changed, 24 insertions(+)
diff --git a/sequencer.c b/sequencer.c
index baa0310..61fdb68 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1047,6 +1047,7 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 {
 	struct commit_list *todo_list = NULL;
 	unsigned char sha1[20];
+	int i;
 
 	if (opts->subcommand == REPLAY_NONE)
 		assert(opts->revs);
@@ -1067,6 +1068,23 @@ int sequencer_pick_revisions(struct replay_opts *opts)
 	if (opts->subcommand == REPLAY_CONTINUE)
 		return sequencer_continue(opts);
 
+	for (i = 0; i < opts->revs->pending.nr; i++) {
+		unsigned char sha1[20];
+		const char *name = opts->revs->pending.objects[i].name;
+
+		/* This happens when using --stdin. */
+		if (!strlen(name))
+			continue;
+
+		if (!get_sha1(name, sha1)) {
+			enum object_type type = sha1_object_info(sha1, NULL);
+
+			if (type > 0 && type != OBJ_COMMIT)
+				die(_("%s: can't cherry-pick a %s"), name, typename(type));
+		} else
+			die(_("%s: bad revision"), name);
+	}
+
 	/*
 	 * If we were called as "git cherry-pick <commit>", just
 	 * cherry-pick/revert it, set CHERRY_PICK_HEAD /
diff --git a/t/t3508-cherry-pick-many-commits.sh b/t/t3508-cherry-pick-many-commits.sh
index 4e7136b..19c99d7 100755
--- a/t/t3508-cherry-pick-many-commits.sh
+++ b/t/t3508-cherry-pick-many-commits.sh
@@ -55,6 +55,12 @@ one
 two"
 '
 
+test_expect_success 'cherry-pick three one two: fails' '
+	git checkout -f master &&
+	git reset --hard first &&
+	test_must_fail git cherry-pick three one two:
+'
+
 test_expect_success 'output to keep user entertained during multi-pick' '
 	cat <<-\EOF >expected &&
 	[master OBJID] second
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help