{"thread":{"id":"45054","subject":"[PATCH] tag: generate useful reflog message","startedAt":"2017-02-05T21:43:30Z","lastAt":"2017-02-09T00:11:42Z","messageCount":12,"participants":["cornelius.weig@tngtech.com","Junio C Hamano","Cornelius Weig"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"310887","messageId":"20170205214254.24560-1-cornelius.weig@tngtech.com","threadId":"45054","inReplyTo":null,"subject":"[PATCH] tag: generate useful reflog message","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-05T21:42:54Z","receivedAt":"2017-02-05T21:43:30Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig <cornelius.weig@tngtech.com>\n\nWhen tags are created with `--create-reflog` or with the option\n`core.logAllRefUpdates` set to 'always', a reflog is created for them.\nSo far, the description of reflog entries for tags was empty, making the\nreflog hard to understand. For example:\n\"6e3a7b3 refs/tags/tag_with_reflog@{0}:\"\n\nNow, a reflog message is generated when creating a tag. The message\nfollows the pattern \"commit: <subject>\" where the subject is taken from\nthe commit the tag points to. For example:\n\"6e3a7b3 refs/tags/tag_with_reflog@{0}: commit: Git 2.12-rc0\"\nIf the tag points to a tree/blob/tag object, the following static\nmessages are used instead:\n\n - \"tree object\"\n - \"blob object\"\n - \"other tag object\"\n\nSigned-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\n---\n\nNotes:\n    While playing around with tag reflogs I also found a bug that was present\n    before this patch. It manifests itself when the sha1-ref in the reflog does not\n    point to a commit object but something else.\n    \n    For example,\n    \n     - when the referenced sha1 is a tag object:\n    \t$ git tag --create-reflog -f -m'annotated tag' tag_with_reflog\n     - when the referenced sha1 is a blob object:\n    \t$ git tag --create-reflog -f tag_with_reflog HEAD:<filename>\n     - when the referenced sha1 is a tree object:\n    \t$ git tag --create-reflog -f tag_with_reflog HEAD^{tree}\n    \n    In each case, a proper reflog entry is generated, but\n    \t$ git reflog tag_with_reflog\n    will sometimes segfault (if it does, it does so consistently), or only show the\n    first few entries. The tree/blob cases are IMHO not so important, but the\n    broken reflog for annotated tags I find quite severe.\n    \n    I guess it's because the reflog is funneled through the log.c code, where every\n    reflog-entry is assumed to be a commit object? If this is the case, a fix would\n    probably be quite involved.\n\n builtin/tag.c  | 43 ++++++++++++++++++++++++++++++++++++++++++-\n t/t7004-tag.sh | 14 +++++++++++++-\n 2 files changed, 55 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a9..c0d9478 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -302,6 +302,43 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t}\n }\n \n+static void create_reflog_msg(const unsigned char *object, struct strbuf *sb)\n+{\n+\tenum object_type type;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint subject_len = 0;\n+\tconst char *subject_start;\n+\n+\ttype = sha1_object_info(object, NULL);\n+\tswitch (type) {\n+\tdefault:\n+\t\tstrbuf_addstr(sb, \"internal object\");\n+\t\tbreak;\n+\tcase OBJ_COMMIT:\n+\t\tstrbuf_addstr(sb, \"commit: \");\n+\t\tbuf = read_sha1_file(object, &type, &size);\n+\t\tif (buf) {\n+\t\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\t\tstrbuf_insert(sb, 8, subject_start, subject_len);\n+\t\t\tfree(buf);\n+\t\t} else {\n+\t\t\tdie(\"commit object %s could not be read\",\n+\t\t\t\tsha1_to_hex(repl));\n+\t\t}\n+\t\tbreak;\n+\tcase OBJ_TREE:\n+\t\tstrbuf_addstr(sb, \"tree object\");\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n+\t\tstrbuf_addstr(sb, \"blob object\");\n+\t\tbreak;\n+\tcase OBJ_TAG:\n+\t\tstrbuf_addstr(sb, \"other tag object\");\n+\t\tbreak;\n+\t}\n+}\n+\n struct msg_arg {\n \tint given;\n \tstruct strbuf buf;\n@@ -335,6 +372,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf ref = STRBUF_INIT;\n+\tstruct strbuf reflog_msg = STRBUF_INIT;\n \tunsigned char object[20], prev[20];\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n@@ -494,6 +532,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n+\tcreate_reflog_msg(object, &reflog_msg);\n+\n \tif (create_tag_object) {\n \t\tif (force_sign_annotate && !annotate)\n \t\t\topt.sign = 1;\n@@ -504,7 +544,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (!transaction ||\n \t    ref_transaction_update(transaction, ref.buf, object, prev,\n \t\t\t\t   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,\n-\t\t\t\t   NULL, &err) ||\n+\t\t\t\t   reflog_msg.buf, &err) ||\n \t    ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n@@ -514,5 +554,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&err);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&ref);\n+\tstrbuf_release(&reflog_msg);\n \treturn 0;\n }\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 072e6c6..0a92b2c 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -83,7 +83,19 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag --create-reflog tag_with_reflog &&\n-\tgit reflog exists refs/tags/tag_with_reflog\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tgit log -1 --format=\"format:commit: %s%n\" > expected &&\n+\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'annotated tag with --create-reflog has correct message' '\n+\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n+\tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tgit log -1 --format=\"format:commit: %s%n\" > expected &&\n+\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success '--create-reflog does not create reflog on failure' '\n-- \n2.10.2\n\n"},{"id":"310890","messageId":"20170205231815.19001-1-cornelius.weig@tngtech.com","threadId":"45054","inReplyTo":"20170205214254.24560-1-cornelius.weig@tngtech.com","subject":"[PATCH v2] tag: generate useful reflog message","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-05T23:18:15Z","receivedAt":"2017-02-05T23:19:03Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig <cornelius.weig@tngtech.com>\n\nWhen tags are created with `--create-reflog` or with the option\n`core.logAllRefUpdates` set to 'always', a reflog is created for them.\nSo far, the description of reflog entries for tags was empty, making the\nreflog hard to understand. For example:\n\"6e3a7b3 refs/tags/test@{0}:\"\n\nNow, a reflog message is generated when creating a tag, following the\npattern \"<action>: <description>\". Here, action is the command line with\nall arguments, or the value of GIT_REFLOG_ACTION if it is set. The\ndescription is the commit subject, if the tag points to a commit. For\nexample:\n\"6e3a7b3 refs/tags/test@{0}: tag --create-reflog test: Git 2.12-rc0\"\nIf the tag points to a tree/blob/tag object, the following static\nstrings are taken as description:\n\n - \"tree object\"\n - \"blob object\"\n - \"other tag object\"\n\nSigned-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\n---\n\nNotes:\n    *This patch supersedes the version submitted a few hours earlier.*\n    \n    Sorry for the messup, but I realized that the pattern for the reflog message\n    from my first draft did not comply to standard git behavior.\n    \n    Please also note my remarks from v1 (repeated here):\n    \n    While playing around with tag reflogs I also found a bug that was present\n    before this patch. It manifests itself when the sha1-ref in the reflog does not\n    point to a commit object but something else.\n    \n    For example,\n    \n     - when the referenced sha1 is a tag object:\n    \t$ git tag --create-reflog -f -m'annotated tag' tag_with_reflog\n     - when the referenced sha1 is a blob object:\n    \t$ git tag --create-reflog -f tag_with_reflog HEAD:<filename>\n     - when the referenced sha1 is a tree object:\n    \t$ git tag --create-reflog -f tag_with_reflog HEAD^{tree}\n    \n    In each case, a proper reflog entry is generated, but\n    \t$ git reflog tag_with_reflog\n    will sometimes segfault (if it does, it does so consistently), or only show the\n    first few entries. The tree/blob cases are IMHO not so important, but the\n    broken reflog for annotated tags I find quite severe.\n    \n    I guess it's because the reflog is funneled through the log.c code, where every\n    reflog-entry is assumed to be a commit object? If this is the case, a fix would\n    probably be quite involved.\n\n builtin/tag.c  | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--\n t/t7004-tag.sh | 16 +++++++++++++++-\n 2 files changed, 69 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a9..3d9e105 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {\n \n static unsigned int colopts;\n static int force_sign_annotate;\n+static struct strbuf default_rla = STRBUF_INIT;\n \n static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)\n {\n@@ -302,6 +303,48 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t}\n }\n \n+static void create_reflog_msg(const unsigned char *object, struct strbuf *sb)\n+{\n+\tenum object_type type;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint subject_len = 0;\n+\tconst char *subject_start;\n+\n+\tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n+\tif (!rla)\n+\t\trla = default_rla.buf;\n+\n+\tstrbuf_addf(sb, \"%s: \", rla ? rla : default_rla.buf);\n+\n+\ttype = sha1_object_info(object, NULL);\n+\tswitch (type) {\n+\tdefault:\n+\t\tstrbuf_addstr(sb, \"internal object\");\n+\t\tbreak;\n+\tcase OBJ_COMMIT:\n+\t\tbuf = read_sha1_file(object, &type, &size);\n+\t\tif (buf) {\n+\t\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n+\t\t\tfree(buf);\n+\t\t} else {\n+\t\t\tdie(\"commit object %s could not be read\",\n+\t\t\t\tsha1_to_hex(object));\n+\t\t}\n+\t\tbreak;\n+\tcase OBJ_TREE:\n+\t\tstrbuf_addstr(sb, \"tree object\");\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n+\t\tstrbuf_addstr(sb, \"blob object\");\n+\t\tbreak;\n+\tcase OBJ_TAG:\n+\t\tstrbuf_addstr(sb, \"other tag object\");\n+\t\tbreak;\n+\t}\n+}\n+\n struct msg_arg {\n \tint given;\n \tstruct strbuf buf;\n@@ -335,6 +378,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf ref = STRBUF_INIT;\n+\tstruct strbuf reflog_msg = STRBUF_INIT;\n \tunsigned char object[20], prev[20];\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n@@ -349,7 +393,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstruct ref_filter filter;\n \tstatic struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;\n \tconst char *format = NULL;\n-\tint icase = 0;\n+\tint icase = 0, i;\n \tstruct option options[] = {\n \t\tOPT_CMDMODE('l', \"list\", &cmdmode, N_(\"list tag names\"), 'l'),\n \t\t{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_(\"n\"),\n@@ -391,6 +435,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \n \tgit_config(git_tag_config, sorting_tail);\n \n+\t/* Record the command line for the reflog */\n+\tstrbuf_addstr(&default_rla, \"tag\");\n+\tfor (i = 1; i < argc; i++)\n+\t\tstrbuf_addf(&default_rla, \" %s\", argv[i]);\n+\n \tmemset(&opt, 0, sizeof(opt));\n \tmemset(&filter, 0, sizeof(filter));\n \tfilter.lines = -1;\n@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n+\tcreate_reflog_msg(object, &reflog_msg);\n+\n \tif (create_tag_object) {\n \t\tif (force_sign_annotate && !annotate)\n \t\t\topt.sign = 1;\n@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (!transaction ||\n \t    ref_transaction_update(transaction, ref.buf, object, prev,\n \t\t\t\t   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,\n-\t\t\t\t   NULL, &err) ||\n+\t\t\t\t   reflog_msg.buf, &err) ||\n \t    ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&err);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&ref);\n+\tstrbuf_release(&reflog_msg);\n \treturn 0;\n }\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 072e6c6..9c80bc9 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n \ttest_must_fail git reflog exists refs/tags/mytag\n '\n \n+git log -1 > expected \\\n+\t--format=\"format:tag --create-reflog tag_with_reflog: %s%n\"\n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag --create-reflog tag_with_reflog &&\n-\tgit reflog exists refs/tags/tag_with_reflog\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+git log -1 > expected \\\n+\t--format='format:tag -m annotated tag --create-reflog tag_with_reflog: %s%n'\n+test_expect_success 'annotated tag with --create-reflog has correct message' '\n+\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n+\tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success '--create-reflog does not create reflog on failure' '\n-- \n2.10.2\n\n"},{"id":"310891","messageId":"xmqqo9yg43uo.fsf@gitster.mtv.corp.google.com","threadId":"45054","inReplyTo":"20170205214254.24560-1-cornelius.weig@tngtech.com","subject":"Re: [PATCH] tag: generate useful reflog message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-05T23:25:51Z","receivedAt":"2017-02-05T23:25:58Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"cornelius.weig@tngtech.com writes:\n\n> Now, a reflog message is generated when creating a tag. The message\n> follows the pattern \"commit: <subject>\" where the subject is taken from\n> the commit the tag points to. For example:\n> \"6e3a7b3 refs/tags/tag_with_reflog@{0}: commit: Git 2.12-rc0\"\n\nBecause the reflog records the actions, shouldn't it be saying that\nyou \"tagged\"?  The reflog for HEAD says things like \"reset: moving\nto...\", \"am: $subject\", and the reflog for a branch says things like\n\"branch: created from master\", \"am: $subject\", \"rebase -i (finish)\".\n\nFor a tag, I would imagine something like \"tag: tagged 4e59582ff7\n(\"Seventh batch for 2.12\", 2017-01-23)\" would be more appropriate.\n\n> Notes:\n>     While playing around with tag reflogs I also found a bug that was present\n>     before this patch. It manifests itself when the sha1-ref in the reflog does not\n>     point to a commit object but something else.\n\nThe underlying machinery for \"log\" and \"rev-list\" is about showing a\nstream of commits, and most of the reflog entries point at commits.\n\nOn the other hand, the \"walking reflogs to and show the sequence of\nthe tip of refs\", and there is no reason to expect the tip of refs\nwill always be commits, but an ancient design mistake bolted the\nlatter on top of the former (perhaps because in practice the tip of\nrefs are almost always commits); \"reflog\" aka \"log -g\" and \"rev-list\n--walk-reflogs\" share the same issue coming from that misdesign,\nwhich needs to be corrected to solve this issue.\n\nThe exact same design mistake also makes \"git reflog\" to accept\noptions like \"--topo-order\", even though many of the options that\nmake sense for the \"commit DAG walking\" (which is what \"log\" and\n\"rev-list\" are about) do not make any sense when walking a reflog.\nAnd the command would give nonsense output when given such an\noption, because a reflog is a single strand of pearl of objects (not\nnecessarily commits) and the order in which these objects appear in\nthe reflog does not have anything to do with the underlying commit\nDAG topology.  Fixing the ancient misdesign would fix this issue,\ntoo.\n\nI think the fix would involve first ripping out the \"reflog walking\"\ncode that was bolted on and stop allowing it to inject the entries\ntaken from the reflog into the \"walk the commit DAG\" machinery.\nThen \"reflog walking\" code needs to be taught to have its own \"now\nwe got a single object to show, show it (using the helper functions\nto show a single object that is already used by 'git show')\" code,\ninstead of piggy-backing on the output codepath used by \"log\" and\n\"rev-list\".\n\n"},{"id":"310910","messageId":"20170206135834.19637-1-cornelius.weig@tngtech.com","threadId":"45054","inReplyTo":"xmqqo9yg43uo.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v3] tag: generate useful reflog message","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-06T13:58:34Z","receivedAt":"2017-02-06T13:59:28Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig <cornelius.weig@tngtech.com>\n\nWhen tags are created with `--create-reflog` or with the option\n`core.logAllRefUpdates` set to 'always', a reflog is created for them.\nSo far, the description of reflog entries for tags was empty, making the\nreflog hard to understand. For example:\n6e3a7b3 refs/tags/test@{0}:\n\nNow, a reflog message is generated when creating a tag, following the\npattern \"tag: tagging <short-sha1> (<description>)\". If\nGIT_REFLOG_ACTION is set, the message becomes \"$GIT_REFLOG_ACTION\n(<description>)\" instead. If the tag references a commit object, the\ndescription is set to the subject line of the commit, followed by its\ncommit date. For example:\n6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)\n\nIf the tag points to a tree/blob/tag objects, the following static\nstrings are taken as description:\n\n - \"tree object\"\n - \"blob object\"\n - \"other tag object\"\n\nSigned-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\n---\n builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t7004-tag.sh | 16 +++++++++++++++-\n 2 files changed, 68 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a9..638b68e 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t}\n }\n \n+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)\n+{\n+\tenum object_type type;\n+\tstruct commit *c;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint subject_len = 0;\n+\tconst char *subject_start;\n+\n+\tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n+\tif (rla) {\n+\t\tstrbuf_addstr(sb, rla);\n+\t} else {\n+\t\tstrbuf_addstr(sb, _(\"tag: tagging \"));\n+\t\tstrbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);\n+\t}\n+\n+\tstrbuf_addstr(sb, \" (\");\n+\ttype = sha1_object_info(sha1, NULL);\n+\tswitch (type) {\n+\tdefault:\n+\t\tstrbuf_addstr(sb, _(\"internal object\"));\n+\t\tbreak;\n+\tcase OBJ_COMMIT:\n+\t\tc = lookup_commit_reference(sha1);\n+\t\tbuf = read_sha1_file(sha1, &type, &size);\n+\t\tif (!c || !buf) {\n+\t\t\tdie(_(\"commit object %s could not be read\"),\n+\t\t\t\tsha1_to_hex(sha1));\n+\t\t}\n+\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n+\t\tstrbuf_addf(sb, \", %s\", show_date(c->date, 0, DATE_MODE(SHORT)));\n+\t\tfree(buf);\n+\t\tbreak;\n+\tcase OBJ_TREE:\n+\t\tstrbuf_addstr(sb, _(\"tree object\"));\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n+\t\tstrbuf_addstr(sb, _(\"blob object\"));\n+\t\tbreak;\n+\tcase OBJ_TAG:\n+\t\tstrbuf_addstr(sb, _(\"other tag object\"));\n+\t\tbreak;\n+\t}\n+\tstrbuf_addch(sb, ')');\n+}\n+\n struct msg_arg {\n \tint given;\n \tstruct strbuf buf;\n@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf ref = STRBUF_INIT;\n+\tstruct strbuf reflog_msg = STRBUF_INIT;\n \tunsigned char object[20], prev[20];\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n+\tcreate_reflog_msg(object, &reflog_msg);\n+\n \tif (create_tag_object) {\n \t\tif (force_sign_annotate && !annotate)\n \t\t\topt.sign = 1;\n@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (!transaction ||\n \t    ref_transaction_update(transaction, ref.buf, object, prev,\n \t\t\t\t   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,\n-\t\t\t\t   NULL, &err) ||\n+\t\t\t\t   reflog_msg.buf, &err) ||\n \t    ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&err);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&ref);\n+\tstrbuf_release(&reflog_msg);\n \treturn 0;\n }\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 072e6c6..3c4cb58 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n \ttest_must_fail git reflog exists refs/tags/mytag\n '\n \n+git log -1 > expected \\\n+\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag --create-reflog tag_with_reflog &&\n-\tgit reflog exists refs/tags/tag_with_reflog\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+git log -1 > expected \\\n+\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n+test_expect_success 'annotated tag with --create-reflog has correct message' '\n+\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n+\tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success '--create-reflog does not create reflog on failure' '\n-- \n2.10.2\n\n"},{"id":"310922","messageId":"dad6002e-a31c-3be6-3141-2e9e678742b1@tngtech.com","threadId":"45054","inReplyTo":"xmqqo9yg43uo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] tag: generate useful reflog message","fromName":"Cornelius Weig","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-06T16:54:55Z","receivedAt":"2017-02-06T16:55:04Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"\nOn 02/06/2017 12:25 AM, Junio C Hamano wrote:\n> cornelius.weig@tngtech.com writes\n> For a tag, I would imagine something like \"tag: tagged 4e59582ff7\n> (\"Seventh batch for 2.12\", 2017-01-23)\" would be more appropriate.\n\nYes, I agree that this is much clearer. The revised version v3\nimplements this behavior.\n\n>> Notes:\n>>     While playing around with tag reflogs I also found a bug that was present\n>>     before this patch. It manifests itself when the sha1-ref in the reflog does not\n>>     point to a commit object but something else.\n> \n> I think the fix would involve first ripping out the \"reflog walking\"\n> code that was bolted on and stop allowing it to inject the entries\n> taken from the reflog into the \"walk the commit DAG\" machinery.\n> Then \"reflog walking\" code needs to be taught to have its own \"now\n> we got a single object to show, show it (using the helper functions\n> to show a single object that is already used by 'git show')\" code,\n> instead of piggy-backing on the output codepath used by \"log\" and\n> \"rev-list\".\n\nI'll start investigating how that could be done. My first glance tells\nme that it won't be easy. Especially because I'm not yet familiar with\nthe git code.\n\nThanks for your advice!\n"},{"id":"310937","messageId":"xmqqpoiv15ew.fsf@gitster.mtv.corp.google.com","threadId":"45054","inReplyTo":"20170206135834.19637-1-cornelius.weig@tngtech.com","subject":"Re: [PATCH v3] tag: generate useful reflog message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-06T19:32:39Z","receivedAt":"2017-02-06T19:32:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"cornelius.weig@tngtech.com writes:\n\n> +\tstrbuf_addstr(sb, \" (\");\n> +\ttype = sha1_object_info(sha1, NULL);\n> +\tswitch (type) {\n> +\tdefault:\n> +\t\tstrbuf_addstr(sb, _(\"internal object\"));\n> +\t\tbreak;\n\nThe code does not even know if this is an \"internal\" object, does\nit?  What it got was simply an object of an unknown type that it is\nnot prepared to handle.  It's not like you are trying to die() in\nthis function (I see a die() upon failing to read the referent\ncommit), so I wonder if this should be a die(\"BUG\").\n\nOn the other hand, it's not like failing to describe the tagged\ncommit in the reflog is such a grave error.  If we can get away with\nbeing vague on a tag that points at an object of unknown type like\nthe above code does, we could loosen the \"oops, we thought we got a\ncommit, but it turns out that we cannot read it\" case below from\ndie() to just stuffing generic _(\"commit object\") in the reflog.\n\nBetween the two extremes above, I am leaning towards making it more\nlenient myself, but others may have different opinions.\n\n> +\tcase OBJ_COMMIT:\n> +\t\tc = lookup_commit_reference(sha1);\n> +\t\tbuf = read_sha1_file(sha1, &type, &size);\n> +\t\tif (!c || !buf) {\n> +\t\t\tdie(_(\"commit object %s could not be read\"),\n> +\t\t\t\tsha1_to_hex(sha1));\n> +\t\t}\n> +\t\tsubject_len = find_commit_subject(buf, &subject_start);\n> +\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n> +\t\tstrbuf_addf(sb, \", %s\", show_date(c->date, 0, DATE_MODE(SHORT)));\n> +\t\tfree(buf);\n> +\t\tbreak;\n> +\tcase OBJ_TREE:\n> +\t\tstrbuf_addstr(sb, _(\"tree object\"));\n> +\t\tbreak;\n> ...\n\n> +git log -1 > expected \\\n> +\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n>  test_expect_success 'creating a tag with --create-reflog should create reflog' '\n>  \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n>  \tgit tag --create-reflog tag_with_reflog &&\n> -\tgit reflog exists refs/tags/tag_with_reflog\n> +\tgit reflog exists refs/tags/tag_with_reflog &&\n> +\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n\nI'd spell that \"\\t\" with an actual HT to make it portable [*1*].  \n\nWe have one example that uses the form in git-filter-branch\ndocumentation and a script in the contrib/ area, but otherwise do\nnot have anything that relies on \\t to be turned into HT by sed.\n\n> +\ttest_cmp expected actual\n> +'\n> +\n> +git log -1 > expected \\\n> +\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n> +test_expect_success 'annotated tag with --create-reflog has correct message' '\n> +\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n> +\tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n> +\tgit reflog exists refs/tags/tag_with_reflog &&\n> +\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n\nLikewise.\n\n\n[Reference]\n\n*1* http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03\n\n    9.3.2 BRE Ordinary Characters\n\n    An ordinary character is a BRE that matches itself: any character in\n    the supported character set, except for the BRE special characters\n    listed in BRE Special Characters.\n\n    The interpretation of an ordinary character preceded by an unescaped\n    <backslash> ( '\\\\' ) is undefined, except for:\n\n    - The characters ')', '(', '{', and '}'\n\n    - The digits 1 to 9 inclusive (see BREs Matching Multiple Characters)\n\n    - A character inside a bracket expression\n"},{"id":"310949","messageId":"20170206222416.28720-1-cornelius.weig@tngtech.com","threadId":"45054","inReplyTo":"xmqqpoiv15ew.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v4] tag: generate useful reflog message","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-06T22:24:15Z","receivedAt":"2017-02-06T22:25:14Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig <cornelius.weig@tngtech.com>\n\nThanks for taking a look at my last version.\n\n> On the other hand, it's not like failing to describe the tagged\n> commit in the reflog is such a grave error.  If we can get away with\n> being vague on a tag that points at an object of unknown type like\n> the above code does, we could loosen the \"oops, we thought we got a\n> commit, but it turns out that we cannot read it\" case below from\n> die() to just stuffing generic _(\"commit object\") in the reflog.\n\nGood point. I agree that failing to create the message should be no reason to\ndie().\nAs you also pointed out, \"internal object\" is no reliable description\nfor unhandled object types. I changed that as well.\n\nChanges wrt v3 (interdiff below):\n - change default message for unhandled object types\n - do not die if commit is not readable, but use default description instead\n - test: use verbatim HT character instead of \\t\n\n\nCornelius Weig (1):\n  tag: generate useful reflog message\n\n builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t7004-tag.sh | 16 +++++++++++++++-\n 2 files changed, 68 insertions(+), 2 deletions(-)\n\nInterdiff v3..v4:\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex 638b68e..9b2eabd 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -323,19 +323,19 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)\n \ttype = sha1_object_info(sha1, NULL);\n \tswitch (type) {\n \tdefault:\n-\t\tstrbuf_addstr(sb, _(\"internal object\"));\n+\t\tstrbuf_addstr(sb, _(\"object of unknown type\"));\n \t\tbreak;\n \tcase OBJ_COMMIT:\n-\t\tc = lookup_commit_reference(sha1);\n-\t\tbuf = read_sha1_file(sha1, &type, &size);\n-\t\tif (!c || !buf) {\n-\t\t\tdie(_(\"commit object %s could not be read\"),\n-\t\t\t\tsha1_to_hex(sha1));\n+\t\tif ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {\n+\t\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n+\t\t} else {\n+\t\t\tstrbuf_addstr(sb, _(\"commit object\"));\n \t\t}\n-\t\tsubject_len = find_commit_subject(buf, &subject_start);\n-\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n-\t\tstrbuf_addf(sb, \", %s\", show_date(c->date, 0, DATE_MODE(SHORT)));\n \t\tfree(buf);\n+\n+\t\tif ((c = lookup_commit_reference(sha1)) != NULL)\n+\t\t\tstrbuf_addf(sb, \", %s\", show_date(c->date, 0, DATE_MODE(SHORT)));\n \t\tbreak;\n \tcase OBJ_TREE:\n \t\tstrbuf_addstr(sb, _(\"tree object\"));\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 3c4cb58..894959f 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -86,7 +86,7 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag --create-reflog tag_with_reflog &&\n \tgit reflog exists refs/tags/tag_with_reflog &&\n-\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n \ttest_cmp expected actual\n '\n\n@@ -96,7 +96,7 @@ test_expect_success 'annotated tag with --create-reflog has correct message' '\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n \tgit reflog exists refs/tags/tag_with_reflog &&\n-\tsed -e \"s/^.*\\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n \ttest_cmp expected actual\n '\n-- \n2.10.2\n\n"},{"id":"310950","messageId":"20170206222416.28720-2-cornelius.weig@tngtech.com","threadId":"45054","inReplyTo":"20170206222416.28720-1-cornelius.weig@tngtech.com","subject":"[PATCH v4] tag: generate useful reflog message","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-06T22:24:16Z","receivedAt":"2017-02-06T22:25:20Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig <cornelius.weig@tngtech.com>\n\nWhen tags are created with `--create-reflog` or with the option\n`core.logAllRefUpdates` set to 'always', a reflog is created for them.\nSo far, the description of reflog entries for tags was empty, making the\nreflog hard to understand. For example:\n6e3a7b3 refs/tags/test@{0}:\n\nNow, a reflog message is generated when creating a tag, following the\npattern \"tag: tagging <short-sha1> (<description>)\". If\nGIT_REFLOG_ACTION is set, the message becomes \"$GIT_REFLOG_ACTION\n(<description>)\" instead. If the tag references a commit object, the\ndescription is set to the subject line of the commit, followed by its\ncommit date. For example:\n6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)\n\nIf the tag points to a tree/blob/tag objects, the following static\nstrings are taken as description:\n\n - \"tree object\"\n - \"blob object\"\n - \"other tag object\"\n\nSigned-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\nReviewed-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t7004-tag.sh | 16 +++++++++++++++-\n 2 files changed, 68 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a9..bca890f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t}\n }\n \n+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)\n+{\n+\tenum object_type type;\n+\tstruct commit *c;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint subject_len = 0;\n+\tconst char *subject_start;\n+\n+\tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n+\tif (rla) {\n+\t\tstrbuf_addstr(sb, rla);\n+\t} else {\n+\t\tstrbuf_addstr(sb, _(\"tag: tagging \"));\n+\t\tstrbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);\n+\t}\n+\n+\tstrbuf_addstr(sb, \" (\");\n+\ttype = sha1_object_info(sha1, NULL);\n+\tswitch (type) {\n+\tdefault:\n+\t\tstrbuf_addstr(sb, _(\"object of unknown type\"));\n+\t\tbreak;\n+\tcase OBJ_COMMIT:\n+\t\tif ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {\n+\t\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n+\t\t} else {\n+\t\t\tstrbuf_addstr(sb, _(\"commit object\"));\n+\t\t}\n+\t\tfree(buf);\n+\n+\t\tif ((c = lookup_commit_reference(sha1)) != NULL)\n+\t\t\tstrbuf_addf(sb, \", %s\", show_date(c->date, 0, DATE_MODE(SHORT)));\n+\t\tbreak;\n+\tcase OBJ_TREE:\n+\t\tstrbuf_addstr(sb, _(\"tree object\"));\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n+\t\tstrbuf_addstr(sb, _(\"blob object\"));\n+\t\tbreak;\n+\tcase OBJ_TAG:\n+\t\tstrbuf_addstr(sb, _(\"other tag object\"));\n+\t\tbreak;\n+\t}\n+\tstrbuf_addch(sb, ')');\n+}\n+\n struct msg_arg {\n \tint given;\n \tstruct strbuf buf;\n@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf ref = STRBUF_INIT;\n+\tstruct strbuf reflog_msg = STRBUF_INIT;\n \tunsigned char object[20], prev[20];\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n+\tcreate_reflog_msg(object, &reflog_msg);\n+\n \tif (create_tag_object) {\n \t\tif (force_sign_annotate && !annotate)\n \t\t\topt.sign = 1;\n@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (!transaction ||\n \t    ref_transaction_update(transaction, ref.buf, object, prev,\n \t\t\t\t   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,\n-\t\t\t\t   NULL, &err) ||\n+\t\t\t\t   reflog_msg.buf, &err) ||\n \t    ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&err);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&ref);\n+\tstrbuf_release(&reflog_msg);\n \treturn 0;\n }\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 072e6c6..894959f 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n \ttest_must_fail git reflog exists refs/tags/mytag\n '\n \n+git log -1 > expected \\\n+\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag --create-reflog tag_with_reflog &&\n-\tgit reflog exists refs/tags/tag_with_reflog\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+git log -1 > expected \\\n+\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n+test_expect_success 'annotated tag with --create-reflog has correct message' '\n+\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n+\tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success '--create-reflog does not create reflog on failure' '\n-- \n2.10.2\n\n"},{"id":"311102","messageId":"xmqqshnov0c4.fsf@gitster.mtv.corp.google.com","threadId":"45054","inReplyTo":"20170206222416.28720-2-cornelius.weig@tngtech.com","subject":"Re: [PATCH v4] tag: generate useful reflog message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-08T21:28:43Z","receivedAt":"2017-02-08T21:30:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"cornelius.weig@tngtech.com writes:\n\n> From: Cornelius Weig <cornelius.weig@tngtech.com>\n>\n> When tags are created with `--create-reflog` or with the option\n> `core.logAllRefUpdates` set to 'always', a reflog is created for them.\n> So far, the description of reflog entries for tags was empty, making the\n> reflog hard to understand. For example:\n> 6e3a7b3 refs/tags/test@{0}:\n>\n> Now, a reflog message is generated when creating a tag, following the\n> pattern \"tag: tagging <short-sha1> (<description>)\". If\n> GIT_REFLOG_ACTION is set, the message becomes \"$GIT_REFLOG_ACTION\n> (<description>)\" instead. If the tag references a commit object, the\n> description is set to the subject line of the commit, followed by its\n> commit date. For example:\n> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)\n>\n> If the tag points to a tree/blob/tag objects, the following static\n> strings are taken as description:\n>\n>  - \"tree object\"\n>  - \"blob object\"\n>  - \"other tag object\"\n>\n> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\n> Reviewed-by: Junio C Hamano <gitster@pobox.com>\n\nThis last line is inappropriate, as I didn't review _THIS_ version,\nwhich is different from the previous one, and I haven't checked if\nthe way the comments on the previous review were addressed in this\nversion is agreeable.\n\n> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n> index 072e6c6..894959f 100755\n> --- a/t/t7004-tag.sh\n> +++ b/t/t7004-tag.sh\n> @@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n>  \ttest_must_fail git reflog exists refs/tags/mytag\n>  '\n>  \n> +git log -1 > expected \\\n> +\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n\nWe do not want to do this kind of thing outside the\ntest_expect_success immediately below, unless there is a good\nreason, and in this case I do not see any.\n\nAlso write redirection operator and redirection target pathname\nwithout SP in between.\n\n>  test_expect_success 'creating a tag with --create-reflog should create reflog' '\n>  \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n>  \tgit tag --create-reflog tag_with_reflog &&\n> -\tgit reflog exists refs/tags/tag_with_reflog\n> +\tgit reflog exists refs/tags/tag_with_reflog &&\n> +\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n> +\ttest_cmp expected actual\n> +'\n\nIn other words, something like:\n\ntest_expect_success 'creating a tag with --create-reflog should create reflog' '\n\tgit log -1 \\\n\t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n\t\t--date=format:%Y-%m-%d >expected &&\n\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n\tgit tag --create-reflog tag_with_reflog &&\n\tgit reflog exists refs/tags/tag_with_reflog &&\n\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog >actual &&\n\ttest_cmp expected actual\n'\t\n\nEven though %F may be shorter, spelling it out makes what we expect\nmore explicit, and what is what I did in the above example.\n\nThanks.\n"},{"id":"311111","messageId":"d0170d3a-3022-3bca-7c80-7ef0b1cf62a0@tngtech.com","threadId":"45054","inReplyTo":"xmqqshnov0c4.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH v4] tag: generate useful reflog message","fromName":"Cornelius Weig","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-08T22:28:37Z","receivedAt":"2017-02-08T22:28:46Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"\n\nOn 02/08/2017 10:28 PM, Junio C Hamano wrote:\n> cornelius.weig@tngtech.com writes:\n> \n>> From: Cornelius Weig <cornelius.weig@tngtech.com>\n>>\n>> When tags are created with `--create-reflog` or with the option\n>> `core.logAllRefUpdates` set to 'always', a reflog is created for them.\n>> So far, the description of reflog entries for tags was empty, making the\n>> reflog hard to understand. For example:\n>> 6e3a7b3 refs/tags/test@{0}:\n>>\n>> Now, a reflog message is generated when creating a tag, following the\n>> pattern \"tag: tagging <short-sha1> (<description>)\". If\n>> GIT_REFLOG_ACTION is set, the message becomes \"$GIT_REFLOG_ACTION\n>> (<description>)\" instead. If the tag references a commit object, the\n>> description is set to the subject line of the commit, followed by its\n>> commit date. For example:\n>> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)\n>>\n>> If the tag points to a tree/blob/tag objects, the following static\n>> strings are taken as description:\n>>\n>>  - \"tree object\"\n>>  - \"blob object\"\n>>  - \"other tag object\"\n>>\n>> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\n>> Reviewed-by: Junio C Hamano <gitster@pobox.com>\n> \n> This last line is inappropriate, as I didn't review _THIS_ version,\n> which is different from the previous one, and I haven't checked if\n> the way the comments on the previous review were addressed in this\n> version is agreeable.\n\nSorry for that confusion. I'm still not used to when adding what\nsign-off is appropriate. I thought that adding you as reviewer is also a\nquestion of courtesy.\n\nA version with revised tests will follow.\n"},{"id":"311121","messageId":"20170208224118.18425-1-cornelius.weig@tngtech.com","threadId":"45054","inReplyTo":"xmqqshnov0c4.fsf@gitster.mtv.corp.google.com","subject":"[PATCH v5] tag: generate useful reflog message","fromName":"","fromEmail":"cornelius.weig@tngtech.com","sentAt":"2017-02-08T22:41:18Z","receivedAt":"2017-02-08T23:36:48Z","isPatch":true,"sender":{"key":"cornelius.weig@tngtech.com","avatar":null},"body":"From: Cornelius Weig <cornelius.weig@tngtech.com>\n\nWhen tags are created with `--create-reflog` or with the option\n`core.logAllRefUpdates` set to 'always', a reflog is created for them.\nSo far, the description of reflog entries for tags was empty, making the\nreflog hard to understand. For example:\n6e3a7b3 refs/tags/test@{0}:\n\nNow, a reflog message is generated when creating a tag, following the\npattern \"tag: tagging <short-sha1> (<description>)\". If\nGIT_REFLOG_ACTION is set, the message becomes \"$GIT_REFLOG_ACTION\n(<description>)\" instead. If the tag references a commit object, the\ndescription is set to the subject line of the commit, followed by its\ncommit date. For example:\n6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)\n\nIf the tag points to a tree/blob/tag objects, the following static\nstrings are taken as description:\n\n - \"tree object\"\n - \"blob object\"\n - \"other tag object\"\n\nSigned-off-by: Cornelius Weig <cornelius.weig@tngtech.com>\n---\n\nNotes:\n    Interdiff v4..v5\n    diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\n    index 894959f..1a3230f 100755\n    --- a/t/t7004-tag.sh\n    +++ b/t/t7004-tag.sh\n    @@ -80,9 +80,10 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n     \ttest_must_fail git reflog exists refs/tags/mytag\n     '\n    \n    -git log -1 > expected \\\n    -\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n     test_expect_success 'creating a tag with --create-reflog should create reflog' '\n    +\tgit log -1 \\\n    +\t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n    +\t\t--date=format:%Y-%m-%d >expected &&\n     \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n     \tgit tag --create-reflog tag_with_reflog &&\n     \tgit reflog exists refs/tags/tag_with_reflog &&\n    @@ -90,9 +91,10 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '\n     \ttest_cmp expected actual\n     '\n    \n    -git log -1 > expected \\\n    -\t--format=\"format:tag: tagging %h (%s, %cd)%n\" --date=format:%F\n     test_expect_success 'annotated tag with --create-reflog has correct message' '\n    +\tgit log -1 \\\n    +\t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n    +\t\t--date=format:%Y-%m-%d >expected &&\n     \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n     \tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n     \tgit reflog exists refs/tags/tag_with_reflog &&\n\n builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-\n t/t7004-tag.sh | 18 +++++++++++++++++-\n 2 files changed, 70 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/tag.c b/builtin/tag.c\nindex e40c4a9..bca890f 100644\n--- a/builtin/tag.c\n+++ b/builtin/tag.c\n@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,\n \t}\n }\n \n+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)\n+{\n+\tenum object_type type;\n+\tstruct commit *c;\n+\tchar *buf;\n+\tunsigned long size;\n+\tint subject_len = 0;\n+\tconst char *subject_start;\n+\n+\tchar *rla = getenv(\"GIT_REFLOG_ACTION\");\n+\tif (rla) {\n+\t\tstrbuf_addstr(sb, rla);\n+\t} else {\n+\t\tstrbuf_addstr(sb, _(\"tag: tagging \"));\n+\t\tstrbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);\n+\t}\n+\n+\tstrbuf_addstr(sb, \" (\");\n+\ttype = sha1_object_info(sha1, NULL);\n+\tswitch (type) {\n+\tdefault:\n+\t\tstrbuf_addstr(sb, _(\"object of unknown type\"));\n+\t\tbreak;\n+\tcase OBJ_COMMIT:\n+\t\tif ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {\n+\t\t\tsubject_len = find_commit_subject(buf, &subject_start);\n+\t\t\tstrbuf_insert(sb, sb->len, subject_start, subject_len);\n+\t\t} else {\n+\t\t\tstrbuf_addstr(sb, _(\"commit object\"));\n+\t\t}\n+\t\tfree(buf);\n+\n+\t\tif ((c = lookup_commit_reference(sha1)) != NULL)\n+\t\t\tstrbuf_addf(sb, \", %s\", show_date(c->date, 0, DATE_MODE(SHORT)));\n+\t\tbreak;\n+\tcase OBJ_TREE:\n+\t\tstrbuf_addstr(sb, _(\"tree object\"));\n+\t\tbreak;\n+\tcase OBJ_BLOB:\n+\t\tstrbuf_addstr(sb, _(\"blob object\"));\n+\t\tbreak;\n+\tcase OBJ_TAG:\n+\t\tstrbuf_addstr(sb, _(\"other tag object\"));\n+\t\tbreak;\n+\t}\n+\tstrbuf_addch(sb, ')');\n+}\n+\n struct msg_arg {\n \tint given;\n \tstruct strbuf buf;\n@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct strbuf ref = STRBUF_INIT;\n+\tstruct strbuf reflog_msg = STRBUF_INIT;\n \tunsigned char object[20], prev[20];\n \tconst char *object_ref, *tag;\n \tstruct create_tag_options opt;\n@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \telse\n \t\tdie(_(\"Invalid cleanup mode %s\"), cleanup_arg);\n \n+\tcreate_reflog_msg(object, &reflog_msg);\n+\n \tif (create_tag_object) {\n \t\tif (force_sign_annotate && !annotate)\n \t\t\topt.sign = 1;\n@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tif (!transaction ||\n \t    ref_transaction_update(transaction, ref.buf, object, prev,\n \t\t\t\t   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,\n-\t\t\t\t   NULL, &err) ||\n+\t\t\t\t   reflog_msg.buf, &err) ||\n \t    ref_transaction_commit(transaction, &err))\n \t\tdie(\"%s\", err.buf);\n \tref_transaction_free(transaction);\n@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)\n \tstrbuf_release(&err);\n \tstrbuf_release(&buf);\n \tstrbuf_release(&ref);\n+\tstrbuf_release(&reflog_msg);\n \treturn 0;\n }\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 072e6c6..1a3230f 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -81,9 +81,25 @@ test_expect_success 'creating a tag using default HEAD should succeed' '\n '\n \n test_expect_success 'creating a tag with --create-reflog should create reflog' '\n+\tgit log -1 \\\n+\t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n+\t\t--date=format:%Y-%m-%d >expected &&\n \ttest_when_finished \"git tag -d tag_with_reflog\" &&\n \tgit tag --create-reflog tag_with_reflog &&\n-\tgit reflog exists refs/tags/tag_with_reflog\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'annotated tag with --create-reflog has correct message' '\n+\tgit log -1 \\\n+\t\t--format=\"format:tag: tagging %h (%s, %cd)%n\" \\\n+\t\t--date=format:%Y-%m-%d >expected &&\n+\ttest_when_finished \"git tag -d tag_with_reflog\" &&\n+\tgit tag -m \"annotated tag\" --create-reflog tag_with_reflog &&\n+\tgit reflog exists refs/tags/tag_with_reflog &&\n+\tsed -e \"s/^.*\t//\" .git/logs/refs/tags/tag_with_reflog > actual &&\n+\ttest_cmp expected actual\n '\n \n test_expect_success '--create-reflog does not create reflog on failure' '\n-- \n2.10.2\n\n"},{"id":"311128","messageId":"xmqqfujotfha.fsf@gitster.mtv.corp.google.com","threadId":"45054","inReplyTo":"d0170d3a-3022-3bca-7c80-7ef0b1cf62a0@tngtech.com","subject":"Re: [PATCH v4] tag: generate useful reflog message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-02-08T23:44:33Z","receivedAt":"2017-02-09T00:11:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Cornelius Weig <cornelius.weig@tngtech.com> writes:\n\n> A version with revised tests will follow.\n\nThanks; I think this is clean enough.  Let's queue this one and\nadvance it to 'next' soonish.\n\n"}]}