Re: [PATCH 2/2] sequencer: unset GIT_CHERRY_PICK_HELP for 'exec' commands

[Phillip Wood <phillip.wood123@gmail.com>]

Hi Junio

On 08/02/2024 17:20, Junio C Hamano wrote:
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> I think that typically for small suggestions like that we just add a
>> Helped-by: trailer but feel free to add my SOB if you want.
> 
> Thanks, both.  Here is what I assembled from the pieces.
> 
> ----- >8 --------- >8 --------- >8 --------- >8 -----
> From: Vegard Nossum <vegard.nossum@oracle.com>
> Date: Fri, 2 Feb 2024 10:18:50 +0100
> Subject: [PATCH] sequencer: unset GIT_CHERRY_PICK_HELP for 'exec' commands
> 
> Running "git cherry-pick" as an x-command in the rebase plan loses
> the original authorship information.

It might be worth explaining why this happens

This is because rebase sets the GIT_CHERRY_PICK_HELP environment 
variable to customize the advice given to users when there are conflicts 
which causes the sequencer to remove CHERRY_PICK_HEAD.

> To fix this, unset GIT_CHERRY_PICK_HELP for 'exec' commands.

The patch itself looks fine

Best Wishes

Phillip

> Helped-by: Phillip Wood <phillip.wood123@gmail.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>   sequencer.c                   |  1 +
>   t/t3404-rebase-interactive.sh | 12 ++++++++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index d584cac8ed..ed30ceaf8b 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -3647,6 +3647,7 @@ static int do_exec(struct repository *r, const char *command_line)
>   	fprintf(stderr, _("Executing: %s\n"), command_line);
>   	cmd.use_shell = 1;
>   	strvec_push(&cmd.args, command_line);
> +	strvec_push(&cmd.env, "GIT_CHERRY_PICK_HELP");
>   	status = run_command(&cmd);
>   
>   	/* force re-reading of the cache */
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index c5f30554c6..84a92d6da0 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -153,6 +153,18 @@ test_expect_success 'rebase -i with the exec command checks tree cleanness' '
>   	git rebase --continue
>   '
>   
> +test_expect_success 'cherry-pick works with rebase --exec' '
> +	test_when_finished "git cherry-pick --abort; \
> +			    git rebase --abort; \
> +			    git checkout primary" &&
> +	echo "exec git cherry-pick G" >todo &&
> +	(
> +		set_replace_editor todo &&
> +		test_must_fail git rebase -i D D
> +	) &&
> +	test_cmp_rev G CHERRY_PICK_HEAD
> +'
> +
>   test_expect_success 'rebase -x with empty command fails' '
>   	test_when_finished "git rebase --abort ||:" &&
>   	test_must_fail env git rebase -x "" @ 2>actual &&