Re: [PATCH v2] cherry-pick: make sure all input objects are commits
From: Ramkumar Ramachandra <hidden>
Date: 2016-06-15 22:56:47
Miklos Vajna wrote:
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. Instead, now make sure all arguments are commits, and for
the first non-commit, error out with:
fatal: <name>: Can't cherry-pick a <type>Thanks. This is worth fixing.
quoted hunk ↗ jump to hunk
diff --git a/sequencer.c b/sequencer.c index baa0310..eb25101 100644 --- a/sequencer.c +++ b/sequencer.c@@ -1067,6 +1068,18 @@ 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; + + 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? What happens if get_sha1() fails?
quoted hunk ↗ jump to hunk
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: +'
So you're testing just the third case (where commit objects are mixed with non-commit objects), which is arguably a bug. Okay.