Re: [PATCH v2] cherry-pick: make sure all input objects are commits
- From
Ramkumar Ramachandra <artagnon@gmail.com>
- Date
- Apr 11, 2013, 10:22 UTC
- Message-ID
- <CALkWK0n6FjGbXTqiOT_O6NbB5h0DLaNWKCCTQAFSO_BL-pPdBA@mail.gmail.com>
- In-Reply-To
- <20130411092638.GA12770@suse.cz>
Miklos Vajna wrote:
Show 13 quoted lines
> 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.
Show 18 quoted lines
> 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?
Show 13 quoted lines
> 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.