Re: [PATCH 1/2] sequencer: handle empty-set cases consistently
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 9, 2018, 20:20 UTC
- Message-ID
- <xmqqlgakcga6.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20180709194819.GA7980@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 23 quoted lines
> 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 exited before then (and which > continues to issue an error).
Other than s/exited/existed/, the above looks perfect ;-) I wish all analysis part of proposed log messages were written like this.
Show 53 quoted lines
>
> 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 &&