Re: [PATCH] sequencer.c: terminate the last line of author-script properly

From: Phillip Wood <phillip.wood@talktalk.net>
Date: Jul 18, 2018, 09:25 UTC
Message-ID: <f5b56540-d26a-044e-5f46-1d975f889d06@talktalk.net>
In-Reply-To: <86a7qwpt9g.knu@iDaemons.org>

Hi Akinori

On 12/07/18 12:18, Akinori MUSHA wrote:

Show 25 quoted lines

> 
> It looks like write_author_script() intends to write out a file in
> Bourne shell syntax, but it doesn't put a closing single quote on the
> last line.
> 
> This patch makes .git/rebase-merge/author-script actually parsable by
> sh(1) by adding a single quote and a linefeed to terminate the line
> properly.
> 
> Signed-off-by: Akinori MUSHA <knu@idaemons.org>
> ---
>   sequencer.c                   |  1 +
>   t/t3404-rebase-interactive.sh | 13 +++++++++++++
>   2 files changed, 14 insertions(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index 4034c0461..5f32b6df1 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -651,6 +651,7 @@ static int write_author_script(const char *message)
>   			strbuf_addch(&buf, *(message++));
>   		else
>   			strbuf_addf(&buf, "'\\\\%c'", *(message++));
> +	strbuf_addstr(&buf, "'\n");
>   	res = write_message(buf.buf, buf.len, rebase_path_author_script(), 1);

The third parameter here means that write_message() will append a new line (you can check this by looking at the file that's created) so strictly speaking we only need to add "'" to the end of the message, alternatively it would be more obvious to keep adding "'\n" and change 1 to 0 in the call to write_message()

Best Wishes

Phillip

Show 27 quoted lines

>   	strbuf_release(&buf);
>   	return res;
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 352a52e59..345b103eb 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -75,6 +75,19 @@ test_expect_success 'rebase --keep-empty' '
>   	test_line_count = 6 actual
>   '
>   
> +test_expect_success 'rebase -i writes out .git/rebase-merge/author-script in "edit" that sh(1) can parse' '
> +	test_when_finished "git rebase --abort ||:" &&
> +	git checkout master &&
> +	set_fake_editor &&
> +	FAKE_LINES="edit 1" git rebase -i HEAD^ &&
> +	test -f .git/rebase-merge/author-script &&
> +	unset GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE &&
> +	eval "$(cat .git/rebase-merge/author-script)" &&
> +	test "$(git show --quiet --pretty=format:%an)" = "$GIT_AUTHOR_NAME" &&
> +	test "$(git show --quiet --pretty=format:%ae)" = "$GIT_AUTHOR_EMAIL" &&
> +	test "$(git show --quiet --date=raw --pretty=format:@%ad)" = "$GIT_AUTHOR_DATE"
> +'
> +
>   test_expect_success 'rebase -i with the exec command' '
>   	git checkout master &&
>   	(
>