Re: [PATCH RFC v2 06/19] rebase -i: Stop on root commits with empty log messages

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

Fabian Ruch <bafain@gmail.com> writes:

> The command line used to recreate root commits specifies the
> erroneous option `--allow-empty-message`. If the root commit has an
> empty log message, the replay of this commit should fail and the
> rebase should be interrupted like for any other commit that is on the
> to-do list and has an empty commit message. Remove the option.
>
> The option might have been introduced by copy-and-paste of the first
> part of the command line which initializes the authorship of the
> sentinel commit. Indeed, the sentinel commit has an empty log message
> and this should not trigger a failure, which is why the option
> `--allow-empty-message` is correctly specified here.

The first "commit --amend" uses -C "$1" to give the amended result
not just the authorship but also the log message taken from "$1".
If we are allowing a commit without any message to be used as "$1",
I think --allow-empty-message needs to be there.  If "$1" requires
the option here, why doesn't the second one, that records the
updated tree with the metainformation taken from the same commit
"$1" can successfully commit without the option?

Puzzled...

> Add test.
>
> Signed-off-by: Fabian Ruch <bafain@gmail.com>
> ---
>  git-rebase--interactive.sh |  2 +-
>  t/t3412-rebase-root.sh     | 39 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 4c875d5..0af96f2 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -510,7 +510,7 @@ do_pick () {
>  		git commit --allow-empty --allow-empty-message --amend \
>  			   --no-post-rewrite -n -q -C $1 &&
>  			pick_one -n $1 &&
> -			git commit --allow-empty --allow-empty-message \
> +			git commit --allow-empty \
>  				   --amend --no-post-rewrite -n -q -C $1 \
>  				   ${gpg_sign_opt:+"$gpg_sign_opt"} ||
>  			die_with_patch $1 "Could not apply $1... $2"
> diff --git a/t/t3412-rebase-root.sh b/t/t3412-rebase-root.sh
> index 0b52105..9867705 100755
> --- a/t/t3412-rebase-root.sh
> +++ b/t/t3412-rebase-root.sh
> @@ -278,4 +278,43 @@ test_expect_success 'rebase -i -p --root with conflict (second part)' '
>  	test_cmp expect-conflict-p out
>  '
>  
> +test_expect_success 'stop rebase --root on empty root log message' '
> +	# create a root commit with a non-empty tree so that rebase does
> +	# not fail because of an empty commit, and an empty log message
> +	echo root-commit >file &&
> +	git add file &&
> +	tree=$(git write-tree) &&
> +	root=$(git commit-tree $tree </dev/null) &&
> +	git checkout -b no-message-root-commit $root &&
> +	# do not ff because otherwise neither the patch nor the message
> +	# are looked at and checked for emptiness
> +	test_when_finished git rebase --abort &&
> +	test_must_fail env EDITOR=true git rebase -i --force-rebase --root &&
> +	echo root-commit >file.expected &&
> +	test_cmp file.expected file
> +'
> +
> +test_expect_success 'stop rebase --root on empty child log message' '
> +	# create a root commit with a non-empty tree and provide a log
> +	# message so that rebase does not fail until the root commit is
> +	# successfully replayed
> +	echo root-commit >file &&
> +	git add file &&
> +	tree=$(git write-tree) &&
> +	root=$(git commit-tree $tree -m root-commit) &&
> +	git checkout -b no-message-child-commit $root &&
> +	# create a child commit with a non-empty patch so that rebase
> +	# does not fail because of an empty commit, but an empty log
> +	# message
> +	echo child-commit >file &&
> +	git add file &&
> +	git commit --allow-empty-message --no-edit &&
> +	# do not ff because otherwise neither the patch nor the message
> +	# are looked at and checked for emptiness
> +	test_when_finished git rebase --abort &&
> +	test_must_fail env EDITOR=true git rebase -i --force-rebase --root &&
> +	echo child-commit >file.expected &&
> +	test_cmp file.expected file
> +'
> +
>  test_done