{"thread":{"id":"54119","subject":"[PATCH] commit: provide option to add Fixes tags to log","startedAt":"2020-08-24T06:12:15Z","lastAt":"2020-08-24T18:19:46Z","messageCount":2,"participants":["Edwin Peer","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"404306","messageId":"20200824061156.1929850-1-espeer@gmail.com","threadId":"54119","inReplyTo":null,"subject":"[PATCH] commit: provide option to add Fixes tags to log","fromName":"Edwin Peer","fromEmail":"espeer@gmail.com","sentAt":"2020-08-24T06:11:56Z","receivedAt":"2020-08-24T06:12:15Z","isPatch":true,"sender":{"key":"espeer@gmail.com","avatar":null},"body":"The Linux style Fixes tag has been adopted by many projects and represents\nbest practice for referring to previous commits which introduce a bug that\nhas been fixed by the present commit. Creating these tags manually can be\nerror prone and doing so using git log -1 --format='Fixes: %h (\"%s\")' is\ncumbersome. It's time the commit command learn to perform this popular\npattern natively.\n\nImplement a convenient command line option to add Fixes tags at the end of\nthe log message during commmit. The tag refers to the commit hash and\nsubject line of the given commit reference. This option may appear\nmultiple times on the command line to reference more than one commit. A\nnew tag will only be added if it does not already exist in the log message\ntrailer and will otherwise be added in option order to the beginning of\nthe trailer section. The minimum number of characters comprising the hash\nportion of the tag is 12 by default, but the `core.abbrev` configuration\nvariable will be honored if it is specified.\n\nSigned-off-by: Edwin Peer \n---\n Documentation/git-commit.txt | 14 +++-\n builtin/commit.c | 54 ++++++++++++++\n t/t7527-commit-fixes.sh | 135 +++++++++++++++++++++++++++++++++++\n 3 files changed, 202 insertions(+), 1 deletion(-)\n create mode 100755 t/t7527-commit-fixes.sh\n\ndiff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt\nindex a3baea32ae..4acb4b3ac8 100644\n--- a/Documentation/git-commit.txt\n+++ b/Documentation/git-commit.txt\n@@ -14,7 +14,7 @@ SYNOPSIS\n \t [--allow-empty-message] [--no-verify] [-e] [--author=]\n \t [--date=] [--cleanup=] [--[no-]status]\n \t [-i | -o] [--pathspec-from-file= [--pathspec-file-nul]]\n-\t [-S[]] [--] [...]\n+\t [-S[]] [(-f | --fixes) ] [--] [...]\n \n DESCRIPTION\n -----------\n@@ -172,6 +172,18 @@ The `-m` option is mutually exclusive with `-c`, `-C`, and `-F`.\n \tagrees to a Developer Certificate of Origin\n \t(see http://developercertificate.org/ for more information).\n \n+-f::\n+--fixes::\n+\tAdd Linux style Fixes tag at the end of the commit log message. The\n+\ttag refers to the commit hash and subject line of the given commit\n+\treference. This option may appear multiple times on the command\n+\tline to reference more than one commit. A new tag will only be\n+\tadded if it does not already exist in the log message trailer and\n+\twill otherwise be added in option order to the beginning of the\n+\ttrailer section. The minimum number of characters comprising the\n+\thash portion of the tag is 12 by default, but the `core.abbrev`\n+\tconfiguration variable will be honored if it is specified.\n+\n -n::\n --no-verify::\n \tThis option bypasses the pre-commit and commit-msg hooks.\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 69ac78d5e5..231abe92ff 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -34,6 +34,7 @@\n #include \"sequencer.h\"\n #include \"mailmap.h\"\n #include \"help.h\"\n+#include \"trailer.h\"\n #include \"commit-reach.h\"\n #include \"commit-graph.h\"\n \n@@ -113,6 +114,7 @@ static int config_commit_verbose = -1; /* unspecified */\n static int no_post_rewrite, allow_empty_message, pathspec_file_nul;\n static char *untracked_files_arg, *force_date, *ignore_submodule_arg, *ignored_arg;\n static char *sign_commit, *pathspec_from_file;\n+static struct string_list fixes = STRING_LIST_INIT_NODUP;\n \n /*\n * The default commit message cleanup mode will remove the lines\n@@ -651,6 +653,54 @@ static int author_date_is_interesting(void)\n \treturn author_message || force_date;\n }\n \n+static void tag_fixes(struct strbuf *msg)\n+{\n+\tconst struct process_trailer_options opts = PROCESS_TRAILER_OPTIONS_INIT;\n+\tstruct pretty_print_context pretty = { .abbrev = default_abbrev };\n+\tconst char *format = \"Fixes: %h (\\\"%s\\\")\";\n+\tconst struct string_list_item *fix;\n+\tstruct strbuf tags = STRBUF_INIT;\n+\tstruct trailer_info info;\n+\tsize_t insert_pos;\n+\n+\t/* use at least 12 hash characters unless explicitly configured */\n+\tif (pretty.abbrev == -1)\n+\t\tpretty.abbrev = 12;\n+\n+\ttrailer_info_get(&info, msg->buf, &opts);\n+\n+\tfor_each_string_list_item(fix, &fixes) {\n+\t\tstruct strbuf tag = STRBUF_INIT;\n+\t\tstruct commit *commit;\n+\t\tint i;\n+\n+\t\tcommit = lookup_commit_reference_by_name(fix->string);\n+\t\tif (!commit)\n+\t\t\tdie(_(\"could not find commit: %s\"), fix->string);\n+\n+\t\tformat_commit_message(commit, format, &tag, &pretty);\n+\n+\t\t/* skip fixes tags that are already present */\n+\t\tfor (i = 0; i < info.trailer_nr; i++)\n+\t\t\tif (!strncmp(info.trailers[i], tag.buf, tag.len))\n+\t\t\t\tgoto skip;\n+\n+\t\tstrbuf_add(&tags, tag.buf, tag.len);\n+\t\tstrbuf_addch(&tags, '\\n');\n+skip:\n+\t\tstrbuf_release(&tag);\n+\t}\n+\n+\tinsert_pos = info.trailer_start - msg->buf;\n+\tstrbuf_insert(msg, insert_pos, tags.buf, tags.len);\n+\tif (tags.len != 0 && !info.blank_line_before_trailer) {\n+\t\tif (msg->len == tags.len)\n+\t\t\tstrbuf_insert(msg, insert_pos, \"\\n\", 1);\n+\t\tstrbuf_insert(msg, insert_pos, \"\\n\", 1);\n+\t}\n+\tstrbuf_release(&tags);\n+}\n+\n static void adjust_comment_line_char(const struct strbuf *sb)\n {\n \tchar candidates[] = \"#;@!$%^&|:\";\n@@ -829,6 +879,9 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \tif (signoff)\n \t\tappend_signoff(&sb, ignore_non_trailer(sb.buf, sb.len), 0);\n \n+\tif (fixes.nr)\n+\t\ttag_fixes(&sb);\n+\n \tif (fwrite(sb.buf, 1, sb.len, s->fp) < sb.len)\n \t\tdie_errno(_(\"could not write commit template\"));\n \n@@ -1510,6 +1563,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"squash\", &squash_message, N_(\"commit\"), N_(\"use autosquash formatted message to squash specified commit\")),\n \t\tOPT_BOOL(0, \"reset-author\", &renew_authorship, N_(\"the commit is authored by me now (used with -C/-c/--amend)\")),\n \t\tOPT_BOOL('s', \"signoff\", &signoff, N_(\"add Signed-off-by:\")),\n+\t\tOPT_STRING_LIST('f', \"fixes\", &fixes, N_(\"commit\"), N_(\"add Fixes: tag referencing \")),\n \t\tOPT_FILENAME('t', \"template\", &template_file, N_(\"use specified template file\")),\n \t\tOPT_BOOL('e', \"edit\", &edit_flag, N_(\"force edit of commit\")),\n \t\tOPT_CLEANUP(&cleanup_arg),\ndiff --git a/t/t7527-commit-fixes.sh b/t/t7527-commit-fixes.sh\nnew file mode 100755\nindex 0000000000..8067cd1ed5\n--- /dev/null\n+++ b/t/t7527-commit-fixes.sh\n@@ -0,0 +1,135 @@\n+#!/bin/sh\n+#\n+# Copyright (c) 2020 Edwin Peer \n+#\n+\n+test_description=\"git commit -f\"\n+. ./test-lib.sh\n+\n+test_expect_success 'empty message' '\n+\t>foo && git add foo &&\n+\tgit commit -m \"first\" &&\n+\techo 1 >foo &&\n+\tgit commit -af 3b6336 &&\n+\tgit log -1 --format=\"%s\" >actual &&\n+\techo \"Fixes: 3b6336cd156f (\\\"first\\\")\" >expected &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'invalid commit' '\n+\techo 2>foo &&\n+\ttest_must_fail git commit -af abcdef\n+'\n+\n+test_expect_success 'blank line after message' '\n+\tgit commit --amend -m \"second commit with spaces\" -f 3b6336 &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tsecond commit with spaces\n+\t\t\n+\t\tFixes: 3b6336cd156f (\"first\")\n+\t\t\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'no duplicate' '\n+\tgit commit --amend -f 3b6336 &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'at start of trailer' '\n+\techo 3>foo &&\n+\tgit commit -asm \"third\n+\n+Reviewed-by: John Doe \" &&\n+\tgit commit --amend -f HEAD~ &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tthird\n+\t\t\n+\t\tFixes: a102cd7a0d16 (\"second commit with spaces\")\n+\t\tReviewed-by: John Doe \n+\t\tSigned-off-by: C O Mitter \n+\t\t\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'multiple' '\n+\techo 4 >foo &&\n+\tgit commit -asm \"fourth\" -f 3b6336 -f 6b4393 &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tfourth\n+\t\t\n+\t\tFixes: 3b6336cd156f (\"first\")\n+\t\tFixes: 6b4393deae7c (\"third\")\n+\t\tSigned-off-by: C O Mitter \n+\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'multiple without duplicates' '\n+\tgit commit --amend -f 6b4393 -f a102cd -f 3b6336 &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tfourth\n+\t\t\n+\t\tFixes: a102cd7a0d16 (\"second commit with spaces\")\n+\t\tFixes: 3b6336cd156f (\"first\")\n+\t\tFixes: 6b4393deae7c (\"third\")\n+\t\tSigned-off-by: C O Mitter \n+\t\t\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'honors core.abbrev' '\n+\tgit config core.abbrev 8 &&\n+\techo 5 >foo &&\n+\tgit commit -asm \"fifth\" -f b06e7571b56c3fc5ac5151ff1b0651debbdada51 &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tfifth\n+\t\t\n+\t\tFixes: b06e7571 (\"fourth\")\n+\t\tSigned-off-by: C O Mitter \n+\t\t\n+\tEOF\n+\ttest_cmp expected actual &&\n+\tgit config --unset core.abbrev\n+'\n+\n+test_expect_success 'option order' '\n+\techo 6 >foo &&\n+\tgit commit -asm \"sixth\" -f 3b6336cd -f a102cd7a -f 6b4393de &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tsixth\n+\t\t\n+\t\tFixes: 3b6336cd156f (\"first\")\n+\t\tFixes: a102cd7a0d16 (\"second commit with spaces\")\n+\t\tFixes: 6b4393deae7c (\"third\")\n+\t\tSigned-off-by: C O Mitter \n+\t\t\n+\tEOF\n+\ttest_cmp expected actual &&\n+\techo 7 >foo &&\n+\tgit commit -asm \"seventh\" -f 6b4393de -f a102cd7a -f 3b6336cd &&\n+\tgit log -1 --format=\"%B\" >actual &&\n+\tcat >expected <<-EOF &&\n+\t\tseventh\n+\t\t\n+\t\tFixes: 6b4393deae7c (\"third\")\n+\t\tFixes: a102cd7a0d16 (\"second commit with spaces\")\n+\t\tFixes: 3b6336cd156f (\"first\")\n+\t\tSigned-off-by: C O Mitter \n+\t\t\n+\tEOF\n+\ttest_cmp expected actual\n+'\n+\n+test_done\n-- \n2.28.0\n\n"},{"id":"404347","messageId":"xmqqft8b6h52.fsf@gitster.c.googlers.com","threadId":"54119","inReplyTo":"20200824061156.1929850-1-espeer@gmail.com","subject":"Re: [PATCH] commit: provide option to add Fixes tags to log","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-08-24T18:19:37Z","receivedAt":"2020-08-24T18:19:46Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Edwin Peer writes:\n\n> The Linux style Fixes tag has been adopted by many projects and represents\n> best practice for referring to previous commits which introduce a bug that\n> has been fixed by the present commit. Creating these tags manually can be\n> error prone and doing so using git log -1 --format='Fixes: %h (\"%s\")' is\n> cumbersome. It's time the commit command learn to perform this popular\n> pattern natively.\n\nSorry, but not in this form.\n\nIt is not a reasonable way forward to add a new \"--\" option\nto each and every conceivable \"Trailer:\" a random person wants to\nadd to the command line. The presence of \"-s\" (signoff) option was\nan early \"mistake\" we made, not something we would want to mimic and\nmake things worse (besides, \"Fixes\" is not necessarily used with any\nobject name---projects can use an identifier used in their bug\ntrackers).\n\nThe \"interpret-trailers\" (Christian CC'ed) subsystem was an attempt\nto create a foundation to consistently treat these lines in the\ntrailer block and the hope back when it was invented was to\neventually integrate it to these subcommands that want to process\ncommit log messages (like \"rebase\", \"commit\", etc.), but it hasn't\nhappened yet. And that may be the approach we would want to take.\n\nFor example, imagine that \"git commit\", \"git am\", etc. learns to\ntake a new option whose canonical form is to spell it like so:\n\n\t--trailer=:\n\nwhile (optionally) allowing any unrecognized command line option\n\n\t--=\n\nto be internally rewritten into the canonical form, as long as \nis what the interpret-trailers subsystem recognises as configured. That\nwould extend the command line option for \"git commit\" etc. with new\n\n\t--trailer=fixes:foo\n\t--fixes=foo\n\n(the latter is available only if there is no \"--fixes\" option used\nby the implementation of subcommands for other purposes, but the\nformer is always available) without having to add any new code. \n\nThat kind of future I'd be happy to see. Not with individual option\nwith fixed semantics tailored to a single project's convention.\n\nThanks.\n\n"}]}