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

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

mail@abhinavg.net writes:

> 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.
> +		 */

Your long topic branch may have local unannotated tags that point
into the middle of it, marking strategic points in the topic.

With this change, the command no longer moves them when it rebases
the entire topic.  Isn't it a regression?

> +		if (decoration->type != DECORATION_REF_LOCAL) {
> +			decoration = decoration->next;
> +			continue;
> +		}

In other words, what you want to prevent from appearing in the insn
stream may be "HEAD", but if so, "must be DECORATION_REF_LOCAL" is
too broad a net to catch it, and causing unintended collateral damage.

As to the style, as the body of the new conditional works
identically with the existing code to exclude the current branch, I
wonder why it shouldn't read more like this?  The following
illustration still uses "must be DECORATION_REF_LOCAL" and that may
have to be corrected, of course.

 sequencer.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git i/sequencer.c w/sequencer.c
index b7d8dca47f..1ba95fbae1 100644
--- i/sequencer.c
+++ w/sequencer.c
@@ -6429,10 +6429,12 @@ static int add_decorations_to_list(const struct commit *commit,
 		size_t base_offset = ctx->buf->len;
 
 		/*
-		 * If the branch is the current HEAD, then it will be
-		 * updated by the default rebase behavior.
+		 * Exclude the "current" branch, which will be updated
+		 * by the default rebase behavior.  Exclude non-branch
+		 * decorations as well.
 		 */
-		if (head_ref && !strcmp(head_ref, decoration->name)) {
+		if ((head_ref && !strcmp(head_ref, decoration->name)) ||
+		    (decoration->type != DECORATION_REF_LOCAL)) {
 			decoration = decoration->next;
 			continue;
 		}