git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/4] trailer: add "move" configuration for trailer.ifExists

From
Paolo Bonzini <pbonzini@redhat.com>
Date
Oct 5, 2017, 13:22 UTC
Message-ID
<20171005132243.27058-5-pbonzini@redhat.com>
In-Reply-To
<20171005132243.27058-1-pbonzini@redhat.com>

In some cases, people apply patches to a queue branch immediately with "git am -3 -s", and later collect Reviewed-by or Acked-by trailers as they come in from the mailing list.

In this case, "where=after" does not have the desired behavior, because it will add the trailer in an unorthodox position, after the committer's Signed-off-by line. The "move" configuration is intended to be applied in such a case to the Signed-off-by header, like

    git interpret-trailers \
	--where end --if-missing doNothing --if-exists move
	"Signed-off-by: A U Thor <au@thor.example.org>"
or perhaps with an automated configuration
    [trailer "move-sob"]
    command = "'echo \"$(git config user.name) <$(git config user.email)>\"'"
    key = Signed-off-by
    where = end
    ifMissing = doNothing
    ifExists = move

Though this of course makes the most sense if the last Signed-off-by is from the committer itself, and thus is not necessarily a good idea for everyone.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Documentation/git-interpret-trailers.txt | 13 ++++++++---
 t/t7513-interpret-trailers.sh            | 37 ++++++++++++++++++++++++++++++++
 trailer.c                                | 26 +++++++++++++++++-----
 trailer.h                                |  1 +
 4 files changed, 69 insertions(+), 8 deletions(-)
diff --git a/Documentation/git-interpret-trailers.txt b/Documentation/git-interpret-trailers.txt
index 9dd19a1dd..1cdde492c 100644
--- a/Documentation/git-interpret-trailers.txt
+++ b/Documentation/git-interpret-trailers.txt
@@ -165,7 +165,7 @@ trailer.ifexists::
 	same <token> in the message.
 +
 The valid values for this option are: `addIfDifferentNeighbor` (this
-is the default), `addIfDifferent`, `add`, `replace` or `doNothing`.
+is the default), `addIfDifferent`, `add`, `replace`, `move` or `doNothing`.
 +
 With `addIfDifferentNeighbor`, a new trailer will be added only if no
 trailer with the same (<token>, <value>) pair is above or below the line
@@ -182,13 +182,20 @@ deleted and the new trailer will be added. The deleted trailer will be
 the closest one (with the same <token>) to the place where the new one
 will be added.
 +
+With `move`, an existing trailer with the same (<token>, <value>) will be
+deleted and the new trailer will be added.  If more equal pairs exists,
+the deleted trailer will be the closest one to the place where the new one
+will be added.
++
 With `doNothing`, nothing will be done; that is no new trailer will be
 added if there is already one with the same <token> in the message.
 
 trailer.ifmissing::
 	This option makes it possible to choose what action will be
-	performed when there is not yet any trailer with the same
-	<token> in the message.
+	performed when the `ifexists` action fails.  This means that
+	there is not yet any trailer with the same <token> in the message
+	or, for the `move` action only, there is no identical trailer
+	in the message.
 +
 The valid values for this option are: `add` (this is the default) and
 `doNothing`.
diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh
index 164719d1c..a0f21fefd 100755
--- a/t/t7513-interpret-trailers.sh
+++ b/t/t7513-interpret-trailers.sh
@@ -1052,6 +1052,43 @@ test_expect_success 'using "ifExists = doNothing"' '
 	test_cmp expected actual
 '
 
+test_expect_success 'require token and value match when "ifExists = move"' '
+	git config trailer.fix.ifExists "move" &&
+	git config trailer.fix.where "start" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: 22
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Fixes: 53
+	EOF
+	(cat complex_message; echo Fixes: 53) | \
+	git interpret-trailers \
+		--trailer "fix=22" \
+		>actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'look for all trailers when "ifExists = move"' '
+	git config trailer.fix.ifMissing "doNothing" &&
+	cat complex_message_body >expected &&
+	sed -e "s/ Z\$/ /" >>expected <<-\EOF &&
+		Fixes: 22
+		Fixes: Z
+		Acked-by= Z
+		Reviewed-by:
+		Signed-off-by: Z
+		Fixes: 53
+	EOF
+	(cat complex_message; echo Fixes: 22; echo Fixes: 53) | \
+	git interpret-trailers \
+		--trailer "fix=22" \
+		>actual &&
+	test_cmp expected actual
+'
+
 test_expect_success 'the default is "ifMissing = add"' '
 	git config trailer.cc.key "Cc: " &&
 	git config trailer.cc.where "before" &&
diff --git a/trailer.c b/trailer.c
index ce0d94074..4915bda8f 100644
--- a/trailer.c
+++ b/trailer.c
@@ -202,6 +202,14 @@ static int check_if_different(struct trailer_item *in_tok,
 	return 0;
 }
 
+static int check_if_same(struct trailer_item *in_tok,
+			 struct trailer_item *neighbor,
+			 struct arg_item *arg_tok,
+			 struct list_head *head)
+{
+	return same_trailer(in_tok, arg_tok);
+}
+
 static int check_if_different_neighbor(struct trailer_item *in_tok,
 				       struct trailer_item *neighbor,
 				       struct arg_item *arg_tok,
@@ -255,6 +263,8 @@ static void apply_item_command(struct trailer_item *in_tok, struct arg_item *arg
 		}
 		arg_tok->value = apply_command(arg_tok->conf.command, arg);
 		free((char *)arg);
+		free(arg_tok->conf.command);
+		arg_tok->conf.command = NULL;
 	}
 }
 
@@ -314,9 +324,9 @@ static int apply_arg(struct trailer_item *in_tok,
 	return 1;
 }
 
-static void apply_arg_if_exists(struct trailer_item *in_tok,
-				struct arg_item *arg_tok,
-				struct list_head *head)
+static int apply_arg_if_exists(struct trailer_item *in_tok,
+			       struct arg_item *arg_tok,
+			       struct list_head *head)
 {
 	switch (arg_tok->conf.if_exists) {
 	case EXISTS_DO_NOTHING:
@@ -324,6 +334,9 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
 	case EXISTS_REPLACE:
 		apply_arg(in_tok, arg_tok, head, NULL, 1);
 		break;
+	case EXISTS_MOVE:
+		/* Look for another trailer if the match fails.  */
+		return apply_arg(in_tok, arg_tok, head, check_if_same, 1);
 	case EXISTS_ADD:
 		apply_arg(in_tok, arg_tok, head, NULL, 0);
 		break;
@@ -337,6 +350,7 @@ static void apply_arg_if_exists(struct trailer_item *in_tok,
 		die("BUG: trailer.c: unhandled value %d",
 		    arg_tok->conf.if_exists);
 	}
+	return 1;
 }
 
 static void apply_arg_if_missing(struct list_head *head,
@@ -367,8 +381,8 @@ static int find_same_and_apply_arg(struct list_head *head,
 		in_tok = list_entry(pos, struct trailer_item, list);
 		if (!same_token(in_tok, arg_tok))
 			continue;
-		apply_arg_if_exists(in_tok, arg_tok, head);
-		return 1;
+		if (apply_arg_if_exists(in_tok, arg_tok, head))
+			return 1;
 	}
 	return 0;
 }
@@ -423,6 +437,8 @@ int trailer_set_if_exists(enum trailer_if_exists *item, const char *value)
 		*item = EXISTS_ADD;
 	else if (!strcasecmp("replace", value))
 		*item = EXISTS_REPLACE;
+	else if (!strcasecmp("move", value))
+		*item = EXISTS_MOVE;
 	else if (!strcasecmp("doNothing", value))
 		*item = EXISTS_DO_NOTHING;
 	else
diff --git a/trailer.h b/trailer.h
index 6d7f8c2a5..0fbdab38d 100644
--- a/trailer.h
+++ b/trailer.h
@@ -16,6 +16,7 @@ enum trailer_if_exists {
 	EXISTS_ADD_IF_DIFFERENT,
 	EXISTS_ADD,
 	EXISTS_REPLACE,
+	EXISTS_MOVE,
 	EXISTS_DO_NOTHING
 };
 enum trailer_if_missing {

-- 
2.14.2
Previous: Paolo BonziniNext: Paolo Bonzini
Message 3 of 14 in “interpret-trailers: introduce "move" action”
  1. 0/4 interpret-trailers: introduce "move" actionPaolo Bonzini, Oct 5, 2017
  2. 3/4 trailer: create a new function to handle adding trailersPaolo Bonzini, Oct 5, 2017
  3. 4/4 trailer: add "move" configuration for trailer.ifExistsPaolo Bonzini, Oct 5, 2017
  4. 1/4 trailer: push free_arg_item upPaolo Bonzini, Oct 5, 2017
  5. 2/4 trailer: simplify check_if_differentPaolo Bonzini, Oct 5, 2017
  6. Junio C HamanoOct 6, 2017
  7. Paolo BonziniOct 6, 2017
  8. Christian CouderOct 6, 2017
  9. Paolo BonziniOct 6, 2017
  10. Christian CouderOct 6, 2017
  11. Paolo BonziniOct 6, 2017
  12. Christian CouderOct 6, 2017
  13. Paolo BonziniOct 6, 2017
  14. Junio C HamanoOct 7, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.