threads / patch / 54119

patchcommit: provide option to add Fixes tags to log

Subject: [PATCH] commit: provide option to add Fixes tags to log

## tl;dr

2 messages between Aug 24, 2020 and Aug 24, 2020. Diffs are folded; open one to read it.

replies: 1people: 2as markdown or json

Edwin Peer· Aug 24, 2020, 06:11 UTC · lore

The Linux style Fixes tag has been adopted by many projects and represents best practice for referring to previous commits which introduce a bug that has been fixed by the present commit. Creating these tags manually can be error prone and doing so using git log -1 --format='Fixes: %h ("%s")' is cumbersome. It's time the commit command learn to perform this popular pattern natively.

Implement a convenient command line option to add Fixes tags at the end of the log message during commmit. The tag refers to the commit hash and subject line of the given commit reference. This option may appear multiple times on the command line to reference more than one commit. A new tag will only be added if it does not already exist in the log message trailer and will otherwise be added in option order to the beginning of the trailer section. The minimum number of characters comprising the hash portion of the tag is 12 by default, but the `core.abbrev` configuration variable will be honored if it is specified.

Signed-off-by: Edwin Peer <espeer@gmail.com>
---
 Documentation/git-commit.txt |  14 +++-
 builtin/commit.c             |  54 ++++++++++++++
 t/t7527-commit-fixes.sh      | 135 +++++++++++++++++++++++++++++++++++
 3 files changed, 202 insertions(+), 1 deletion(-)
 create mode 100755 t/t7527-commit-fixes.sh
Show changes to 3 files +202 −1

Documentation/git-commit.txt, builtin/commit.c, t/t7527-commit-fixes.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index a3baea32ae..4acb4b3ac8 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -14,7 +14,7 @@ SYNOPSIS
 	   [--allow-empty-message] [--no-verify] [-e] [--author=<author>]
 	   [--date=<date>] [--cleanup=<mode>] [--[no-]status]
 	   [-i | -o] [--pathspec-from-file=<file> [--pathspec-file-nul]]
-	   [-S[<keyid>]] [--] [<pathspec>...]
+	   [-S[<keyid>]] [(-f | --fixes) <commit>] [--] [<pathspec>...]
 
 DESCRIPTION
 -----------
@@ -172,6 +172,18 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.
 	agrees to a Developer Certificate of Origin
 	(see http://developercertificate.org/ for more information).
 
+-f::
+--fixes::
+	Add Linux style Fixes tag at the end of the commit log message. The
+	tag refers to the commit hash and subject line of the given commit
+	reference. This option may appear multiple times on the command
+	line to reference more than one commit. A new tag will only be
+	added if it does not already exist in the log message trailer and
+	will otherwise be added in option order to the beginning of the
+	trailer section. The minimum number of characters comprising the
+	hash portion of the tag is 12 by default, but the `core.abbrev`
+	configuration variable will be honored if it is specified.
+
 -n::
 --no-verify::
 	This option bypasses the pre-commit and commit-msg hooks.
diff --git a/builtin/commit.c b/builtin/commit.c
index 69ac78d5e5..231abe92ff 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -34,6 +34,7 @@
 #include "sequencer.h"
 #include "mailmap.h"
 #include "help.h"
+#include "trailer.h"
 #include "commit-reach.h"
 #include "commit-graph.h"
 
@@ -113,6 +114,7 @@ static int config_commit_verbose = -1; /* unspecified */
 static int no_post_rewrite, allow_empty_message, pathspec_file_nul;
 static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;
 static char *sign_commit, *pathspec_from_file;
+static struct string_list fixes = STRING_LIST_INIT_NODUP;
 
 /*
  * The default commit message cleanup mode will remove the lines
@@ -651,6 +653,54 @@ static int author_date_is_interesting(void)
 	return author_message || force_date;
 }
 
+static void tag_fixes(struct strbuf *msg)
+{
+	const struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;
+	struct pretty_print_context pretty = { .abbrev = default_abbrev };
+	const char *format = "Fixes: %h (\"%s\")";
+	const struct string_list_item *fix;
+	struct strbuf tags = STRBUF_INIT;
+	struct trailer_info info;
+	size_t insert_pos;
+
+	/* use at least 12 hash characters unless explicitly configured */
+	if (pretty.abbrev == -1)
+		pretty.abbrev = 12;
+
+	trailer_info_get(&info, msg->buf, &opts);
+
+	for_each_string_list_item(fix, &fixes) {
+		struct strbuf tag = STRBUF_INIT;
+		struct commit *commit;
+		int i;
+
+		commit = lookup_commit_reference_by_name(fix->string);
+		if (!commit)
+			die(_("could not find commit: %s"), fix->string);
+
+		format_commit_message(commit, format, &tag, &pretty);
+
+		/* skip fixes tags that are already present */
+		for (i = 0; i < info.trailer_nr; i++)
+			if (!strncmp(info.trailers[i], tag.buf, tag.len))
+				goto skip;
+
+		strbuf_add(&tags, tag.buf, tag.len);
+		strbuf_addch(&tags, '\n');
+skip:
+		strbuf_release(&tag);
+	}
+
+	insert_pos = info.trailer_start - msg->buf;
+	strbuf_insert(msg, insert_pos, tags.buf, tags.len);
+	if (tags.len != 0 && !info.blank_line_before_trailer) {
+		if (msg->len == tags.len)
+			strbuf_insert(msg, insert_pos, "\n", 1);
+		strbuf_insert(msg, insert_pos, "\n", 1);
+	}
+	strbuf_release(&tags);
+}
+
 static void adjust_comment_line_char(const struct strbuf *sb)
 {
 	char candidates[] = "#;@!$%^&|:";
@@ -829,6 +879,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
 	if (signoff)
 		append_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);
 
+	if (fixes.nr)
+		tag_fixes(&sb);
+
 	if (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)
 		die_errno(_("could not write commit template"));
 
@@ -1510,6 +1563,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
 		OPT_STRING(0, "squash", &squash_message, N_("commit"), N_("use autosquash formatted message to squash specified commit")),
 		OPT_BOOL(0, "reset-author", &renew_authorship, N_("the commit is authored by me now (used with -C/-c/--amend)")),
 		OPT_BOOL('s', "signoff", &signoff, N_("add Signed-off-by:")),
+		OPT_STRING_LIST('f', "fixes", &fixes, N_("commit"), N_("add Fixes: tag referencing <commit>")),
 		OPT_FILENAME('t', "template", &template_file, N_("use specified template file")),
 		OPT_BOOL('e', "edit", &edit_flag, N_("force edit of commit")),
 		OPT_CLEANUP(&cleanup_arg),
diff --git a/t/t7527-commit-fixes.sh b/t/t7527-commit-fixes.sh
new file mode 100755
index 0000000000..8067cd1ed5
--- /dev/null
+++ b/t/t7527-commit-fixes.sh
@@ -0,0 +1,135 @@
+#!/bin/sh
+#
+# Copyright (c) 2020 Edwin Peer <espeer@gmail.com>
+#
+
+test_description="git commit -f"
+. ./test-lib.sh
+
+test_expect_success 'empty message' '
+	>foo && git add foo &&
+	git commit -m "first" &&
+	echo 1 >foo &&
+	git commit -af 3b6336 &&
+	git log -1 --format="%s" >actual &&
+	echo "Fixes: 3b6336cd156f (\"first\")" >expected &&
+	test_cmp expected actual
+'
+
+test_expect_success 'invalid commit' '
+	echo 2>foo &&
+	test_must_fail git commit -af abcdef
+'
+
+test_expect_success 'blank line after message' '
+	git commit --amend -m "second commit with spaces" -f 3b6336 &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		second commit with spaces
+		
+		Fixes: 3b6336cd156f ("first")
+		
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'no duplicate' '
+	git commit --amend -f 3b6336 &&
+	git log -1 --format="%B" >actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'at start of trailer' '
+	echo 3>foo &&
+	git commit -asm "third
+
+Reviewed-by: John Doe <john@nobody.com>" &&
+	git commit --amend -f HEAD~ &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		third
+		
+		Fixes: a102cd7a0d16 ("second commit with spaces")
+		Reviewed-by: John Doe <john@nobody.com>
+		Signed-off-by: C O Mitter <committer@example.com>
+		
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'multiple' '
+	echo 4 >foo &&
+	git commit -asm "fourth" -f 3b6336 -f 6b4393 &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		fourth
+		
+		Fixes: 3b6336cd156f ("first")
+		Fixes: 6b4393deae7c ("third")
+		Signed-off-by: C O Mitter <committer@example.com>
+
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'multiple without duplicates' '
+	git commit --amend -f 6b4393 -f a102cd -f 3b6336 &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		fourth
+		
+		Fixes: a102cd7a0d16 ("second commit with spaces")
+		Fixes: 3b6336cd156f ("first")
+		Fixes: 6b4393deae7c ("third")
+		Signed-off-by: C O Mitter <committer@example.com>
+		
+	EOF
+	test_cmp expected actual
+'
+
+test_expect_success 'honors core.abbrev' '
+	git config core.abbrev 8 &&
+	echo 5 >foo &&
+	git commit -asm "fifth" -f b06e7571b56c3fc5ac5151ff1b0651debbdada51 &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		fifth
+		
+		Fixes: b06e7571 ("fourth")
+		Signed-off-by: C O Mitter <committer@example.com>
+		
+	EOF
+	test_cmp expected actual &&
+	git config --unset core.abbrev
+'
+
+test_expect_success 'option order' '
+	echo 6 >foo &&
+	git commit -asm "sixth" -f 3b6336cd -f a102cd7a -f 6b4393de &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		sixth
+		
+		Fixes: 3b6336cd156f ("first")
+		Fixes: a102cd7a0d16 ("second commit with spaces")
+		Fixes: 6b4393deae7c ("third")
+		Signed-off-by: C O Mitter <committer@example.com>
+		
+	EOF
+	test_cmp expected actual &&
+	echo 7 >foo &&
+	git commit -asm "seventh" -f 6b4393de -f a102cd7a -f 3b6336cd &&
+	git log -1 --format="%B" >actual &&
+	cat >expected <<-EOF &&
+		seventh
+		
+		Fixes: 6b4393deae7c ("third")
+		Fixes: a102cd7a0d16 ("second commit with spaces")
+		Fixes: 3b6336cd156f ("first")
+		Signed-off-by: C O Mitter <committer@example.com>
+		
+	EOF
+	test_cmp expected actual
+'
+
+test_done
-- 
2.28.0
Junio C Hamano· Aug 24, 2020, 18:19 UTC · re: Edwin Peer · lore

Re: [PATCH] commit: provide option to add Fixes tags to log

Edwin Peer <espeer@gmail.com> writes:
Show 6 quoted lines
> The Linux style Fixes tag has been adopted by many projects and represents
> best practice for referring to previous commits which introduce a bug that
> has been fixed by the present commit. Creating these tags manually can be
> error prone and doing so using git log -1 --format='Fixes: %h ("%s")' is
> cumbersome. It's time the commit command learn to perform this popular
> pattern natively.
Sorry, but not in this form.

It is not a reasonable way forward to add a new "--<trailer>" option to each and every conceivable "Trailer:" a random person wants to add to the command line. The presence of "-s" (signoff) option was an early "mistake" we made, not something we would want to mimic and make things worse (besides, "Fixes" is not necessarily used with any object name---projects can use an identifier used in their bug trackers).

The "interpret-trailers" (Christian CC'ed) subsystem was an attempt to create a foundation to consistently treat these lines in the trailer block and the hope back when it was invented was to eventually integrate it to these subcommands that want to process commit log messages (like "rebase", "commit", etc.), but it hasn't happened yet. And that may be the approach we would want to take.

For example, imagine that "git commit", "git am", etc. learns to take a new option whose canonical form is to spell it like so:

	--trailer=<trailer>:<arg>
while (optionally) allowing any unrecognized command line option
	--<trailer>=<arg>

to be internally rewritten into the canonical form, as long as <trailer> is what the interpret-trailers subsystem recognises as configured. That would extend the command line option for "git commit" etc. with new

	--trailer=fixes:foo
	--fixes=foo

(the latter is available only if there is no "--fixes" option used by the implementation of subcommands for other purposes, but the former is always available) without having to add any new code.

That kind of future I'd be happy to see. Not with individual option with fixed semantics tailored to a single project's convention.

Thanks.

← back to recent threads