[PATCH v3] cherry-pick: make sure all input objects are commits
- From
Miklos Vajna <vmiklos@suse.cz>
- Date
- Apr 11, 2013, 13:06 UTC
- Message-ID
- <20130411130652.GG12770@suse.cz>
- In-Reply-To
- <CALkWK0kb+2KZLvRJDJb_VrNNs1k4grsfyFv0HfYv0Kr9v4sChQ@mail.gmail.com>
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 <vmiklos@suse.cz> ---
On Thu, Apr 11, 2013 at 05:12:06PM +0530, Ramkumar Ramachandra <artagnon@gmail.com> wrote:
> Then why do you have an if() guarding the code? In my opinion, you > should have an else-clause that die()s with an appropriate message.
And you were right -- I actually forgot about --stdin, where the else-clause is hit. Added that for now, excluding --stdin.
> Nope, I'd never suggest that: this is fine. What I meant is: you > should clarify that you're fixing a bug and adding a test to guard it, > in the commit message.
Done.
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
-- 1.8.1.4