Re: [PATCH v2] rebase: ignore non-branch update-refs

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

Hi Abhinav

Thanks for re-working the test - this looks great.

Phillip

On 08/05/2026 02:58, mail@abhinavg.net wrote:
> From: Abhinav Gupta <mail@abhinavg.net>
> 
> The following Git configuration breaks git rebase --update-refs:
> 
>      [rebase]
>          instructionFormat = %s%d
> 
> The '%d' format requests all available decorations for a commit,
> filling the global decoration table with all of them,
> which --update-refs then uses to populate 'update-ref' instructions
> in the rebase todo list.
> 
> Specifically, this results in the following instruction:
> 
>      update-ref HEAD
> 
> The todo parser then rejects the instruction:
> 
>      error: update-ref requires a fully qualified refname e.g. refs/heads/HEAD
>      error: invalid line 3: update-ref HEAD
> 
> To fix, ignore decorations that are not local branches
> when scanning through the table.
> 
> This matches the documented contract:
> it moves branch refs under refs/heads/
> and leaves display-only decorations (HEAD, tags, etc.) alone.
> 
> Verification:
> A regression test that fails without this fix is included.
> 
> Signed-off-by: Abhinav Gupta <mail@abhinavg.net>
> ---
> Updates:
>      v2: incorporate suggestions to simplify the test
> 
>   sequencer.c                   | 10 ++++++++++
>   t/t3404-rebase-interactive.sh | 18 ++++++++++++++++++
>   2 files changed, 28 insertions(+)
> 
> diff --git a/sequencer.c b/sequencer.c
> index b7d8dca47f..25bcfc5da0 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -6428,6 +6428,16 @@ static int add_decorations_to_list(const struct commit *commit,
>   		const char *path;
>   		size_t base_offset = ctx->buf->len;
>   
> +		/*
> +		 * The global decoration table may contain names loaded by
> +		 * a previous pretty format such as "%d".
> +		 * This will result in refs such as "HEAD" being present.
> +		 */
> +		if (decoration->type != DECORATION_REF_LOCAL) {
> +			decoration = decoration->next;
> +			continue;
> +		}
> +
>   		/*
>   		 * If the branch is the current HEAD, then it will be
>   		 * updated by the default rebase behavior.
> diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
> index 3e44562afa..58b3bb0c27 100755
> --- a/t/t3404-rebase-interactive.sh
> +++ b/t/t3404-rebase-interactive.sh
> @@ -1960,6 +1960,24 @@ test_expect_success '--update-refs adds commands with --rebase-merges' '
>   	)
>   '
>   
> +test_expect_success '--update-refs ignores non-branch decorations' '
> +	test_when_finished "git branch -D update-refs" &&
> +	test_when_finished "git checkout primary" &&
> +	git checkout -B update-refs no-conflict-branch &&
> +	(
> +		set_cat_todo_editor &&
> +
> +		# rebase.instructionFormat=%d loads normal log decorations before
> +		# --update-refs adds its branch placeholders so we must ignore
> +		# all non-local decorations.
> +		test_must_fail git -c rebase.instructionFormat="%s%d" \
> +			rebase -i --update-refs HEAD^ >todo
> +	) &&
> +	grep ^update-ref todo >actual &&
> +	test_write_lines "update-ref refs/heads/no-conflict-branch" >expect &&
> +	test_cmp expect actual
> +'
> +
>   test_expect_success '--update-refs updates refs correctly' '
>   	git checkout -B update-refs no-conflict-branch &&
>   	git branch -f base HEAD~4 &&
> 
> base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0