Re: [PATCH 1/2] sequencer: handle empty-set cases consistently

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> 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.



>
> 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 &&