[PATCH] tag: generate useful reflog message

[PATCH] tag: generate useful reflog message

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
"6e3a7b3 refs/tags/tag_with_reflog@{0}:"

Now, a reflog message is generated when creating a tag. The message
follows the pattern "commit: <subject>" where the subject is taken from
the commit the tag points to. For example:
"6e3a7b3 refs/tags/tag_with_reflog@{0}: commit: Git 2.12-rc0"
If the tag points to a tree/blob/tag object, the following static
messages are used instead:

 - "tree object"
 - "blob object"
 - "other tag object"

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---

Notes:
    While playing around with tag reflogs I also found a bug that was present
    before this patch. It manifests itself when the sha1-ref in the reflog does not
    point to a commit object but something else.
    
    For example,
    
     - when the referenced sha1 is a tag object:
    	$ git tag --create-reflog -f -m'annotated tag' tag_with_reflog
     - when the referenced sha1 is a blob object:
    	$ git tag --create-reflog -f tag_with_reflog HEAD:<filename>
     - when the referenced sha1 is a tree object:
    	$ git tag --create-reflog -f tag_with_reflog HEAD^{tree}
    
    In each case, a proper reflog entry is generated, but
    	$ git reflog tag_with_reflog
    will sometimes segfault (if it does, it does so consistently), or only show the
    first few entries. The tree/blob cases are IMHO not so important, but the
    broken reflog for annotated tags I find quite severe.
    
    I guess it's because the reflog is funneled through the log.c code, where every
    reflog-entry is assumed to be a commit object? If this is the case, a fix would
    probably be quite involved.

 builtin/tag.c  | 43 ++++++++++++++++++++++++++++++++++++++++++-
 t/t7004-tag.sh | 14 +++++++++++++-
 2 files changed, 55 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..c0d9478 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,43 @@ static void create_tag(const unsigned char *object, const char *tag,
 	}
 }
 
+static void create_reflog_msg(const unsigned char *object, struct strbuf *sb)
+{
+	enum object_type type;
+	char *buf;
+	unsigned long size;
+	int subject_len = 0;
+	const char *subject_start;
+
+	type = sha1_object_info(object, NULL);
+	switch (type) {
+	default:
+		strbuf_addstr(sb, "internal object");
+		break;
+	case OBJ_COMMIT:
+		strbuf_addstr(sb, "commit: ");
+		buf = read_sha1_file(object, &type, &size);
+		if (buf) {
+			subject_len = find_commit_subject(buf, &subject_start);
+			strbuf_insert(sb, 8, subject_start, subject_len);
+			free(buf);
+		} else {
+			die("commit object %s could not be read",
+				sha1_to_hex(repl));
+		}
+		break;
+	case OBJ_TREE:
+		strbuf_addstr(sb, "tree object");
+		break;
+	case OBJ_BLOB:
+		strbuf_addstr(sb, "blob object");
+		break;
+	case OBJ_TAG:
+		strbuf_addstr(sb, "other tag object");
+		break;
+	}
+}
+
 struct msg_arg {
 	int given;
 	struct strbuf buf;
@@ -335,6 +372,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf ref = STRBUF_INIT;
+	struct strbuf reflog_msg = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
 	struct create_tag_options opt;
@@ -494,6 +532,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
+	create_reflog_msg(object, &reflog_msg);
+
 	if (create_tag_object) {
 		if (force_sign_annotate && !annotate)
 			opt.sign = 1;
@@ -504,7 +544,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (!transaction ||
 	    ref_transaction_update(transaction, ref.buf, object, prev,
 				   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
-				   NULL, &err) ||
+				   reflog_msg.buf, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
 	ref_transaction_free(transaction);
@@ -514,5 +554,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
+	strbuf_release(&reflog_msg);
 	return 0;
 }
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..0a92b2c 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -83,7 +83,19 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag --create-reflog tag_with_reflog &&
-	git reflog exists refs/tags/tag_with_reflog
+	git reflog exists refs/tags/tag_with_reflog &&
+	git log -1 --format="format:commit: %s%n" > expected &&
+	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+	test_when_finished "git tag -d tag_with_reflog" &&
+	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+	git reflog exists refs/tags/tag_with_reflog &&
+	git log -1 --format="format:commit: %s%n" > expected &&
+	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
 '
 
 test_expect_success '--create-reflog does not create reflog on failure' '
-- 
2.10.2

[PATCH v2] tag: generate useful reflog message

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
"6e3a7b3 refs/tags/test@{0}:"

Now, a reflog message is generated when creating a tag, following the
pattern "<action>: <description>". Here, action is the command line with
all arguments, or the value of GIT_REFLOG_ACTION if it is set. The
description is the commit subject, if the tag points to a commit. For
example:
"6e3a7b3 refs/tags/test@{0}: tag --create-reflog test: Git 2.12-rc0"
If the tag points to a tree/blob/tag object, the following static
strings are taken as description:

 - "tree object"
 - "blob object"
 - "other tag object"

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---

Notes:
    *This patch supersedes the version submitted a few hours earlier.*
    
    Sorry for the messup, but I realized that the pattern for the reflog message
    from my first draft did not comply to standard git behavior.
    
    Please also note my remarks from v1 (repeated here):
    
    While playing around with tag reflogs I also found a bug that was present
    before this patch. It manifests itself when the sha1-ref in the reflog does not
    point to a commit object but something else.
    
    For example,
    
     - when the referenced sha1 is a tag object:
    	$ git tag --create-reflog -f -m'annotated tag' tag_with_reflog
     - when the referenced sha1 is a blob object:
    	$ git tag --create-reflog -f tag_with_reflog HEAD:<filename>
     - when the referenced sha1 is a tree object:
    	$ git tag --create-reflog -f tag_with_reflog HEAD^{tree}
    
    In each case, a proper reflog entry is generated, but
    	$ git reflog tag_with_reflog
    will sometimes segfault (if it does, it does so consistently), or only show the
    first few entries. The tree/blob cases are IMHO not so important, but the
    broken reflog for annotated tags I find quite severe.
    
    I guess it's because the reflog is funneled through the log.c code, where every
    reflog-entry is assumed to be a commit object? If this is the case, a fix would
    probably be quite involved.

 builtin/tag.c  | 56 ++++++++++++++++++++++++++++++++++++++++++++++++++++++--
 t/t7004-tag.sh | 16 +++++++++++++++-
 2 files changed, 69 insertions(+), 3 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..3d9e105 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -30,6 +30,7 @@ static const char * const git_tag_usage[] = {
 
 static unsigned int colopts;
 static int force_sign_annotate;
+static struct strbuf default_rla = STRBUF_INIT;
 
 static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, const char *format)
 {
@@ -302,6 +303,48 @@ static void create_tag(const unsigned char *object, const char *tag,
 	}
 }
 
+static void create_reflog_msg(const unsigned char *object, struct strbuf *sb)
+{
+	enum object_type type;
+	char *buf;
+	unsigned long size;
+	int subject_len = 0;
+	const char *subject_start;
+
+	char *rla = getenv("GIT_REFLOG_ACTION");
+	if (!rla)
+		rla = default_rla.buf;
+
+	strbuf_addf(sb, "%s: ", rla ? rla : default_rla.buf);
+
+	type = sha1_object_info(object, NULL);
+	switch (type) {
+	default:
+		strbuf_addstr(sb, "internal object");
+		break;
+	case OBJ_COMMIT:
+		buf = read_sha1_file(object, &type, &size);
+		if (buf) {
+			subject_len = find_commit_subject(buf, &subject_start);
+			strbuf_insert(sb, sb->len, subject_start, subject_len);
+			free(buf);
+		} else {
+			die("commit object %s could not be read",
+				sha1_to_hex(object));
+		}
+		break;
+	case OBJ_TREE:
+		strbuf_addstr(sb, "tree object");
+		break;
+	case OBJ_BLOB:
+		strbuf_addstr(sb, "blob object");
+		break;
+	case OBJ_TAG:
+		strbuf_addstr(sb, "other tag object");
+		break;
+	}
+}
+
 struct msg_arg {
 	int given;
 	struct strbuf buf;
@@ -335,6 +378,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf ref = STRBUF_INIT;
+	struct strbuf reflog_msg = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
 	struct create_tag_options opt;
@@ -349,7 +393,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	struct ref_filter filter;
 	static struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	const char *format = NULL;
-	int icase = 0;
+	int icase = 0, i;
 	struct option options[] = {
 		OPT_CMDMODE('l', "list", &cmdmode, N_("list tag names"), 'l'),
 		{ OPTION_INTEGER, 'n', NULL, &filter.lines, N_("n"),
@@ -391,6 +435,11 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	git_config(git_tag_config, sorting_tail);
 
+	/* Record the command line for the reflog */
+	strbuf_addstr(&default_rla, "tag");
+	for (i = 1; i < argc; i++)
+		strbuf_addf(&default_rla, " %s", argv[i]);
+
 	memset(&opt, 0, sizeof(opt));
 	memset(&filter, 0, sizeof(filter));
 	filter.lines = -1;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
+	create_reflog_msg(object, &reflog_msg);
+
 	if (create_tag_object) {
 		if (force_sign_annotate && !annotate)
 			opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (!transaction ||
 	    ref_transaction_update(transaction, ref.buf, object, prev,
 				   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
-				   NULL, &err) ||
+				   reflog_msg.buf, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
 	ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
+	strbuf_release(&reflog_msg);
 	return 0;
 }
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..9c80bc9 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
 	test_must_fail git reflog exists refs/tags/mytag
 '
 
+git log -1 > expected \
+	--format="format:tag --create-reflog tag_with_reflog: %s%n"
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag --create-reflog tag_with_reflog &&
-	git reflog exists refs/tags/tag_with_reflog
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
+'
+
+git log -1 > expected \
+	--format='format:tag -m annotated tag --create-reflog tag_with_reflog: %s%n'
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+	test_when_finished "git tag -d tag_with_reflog" &&
+	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
 '
 
 test_expect_success '--create-reflog does not create reflog on failure' '
-- 
2.10.2

Re: [PATCH] tag: generate useful reflog message

[Junio C Hamano]

cornelius.weig@tngtech.com writes:

> Now, a reflog message is generated when creating a tag. The message
> follows the pattern "commit: <subject>" where the subject is taken from
> the commit the tag points to. For example:
> "6e3a7b3 refs/tags/tag_with_reflog@{0}: commit: Git 2.12-rc0"

Because the reflog records the actions, shouldn't it be saying that
you "tagged"?  The reflog for HEAD says things like "reset: moving
to...", "am: $subject", and the reflog for a branch says things like
"branch: created from master", "am: $subject", "rebase -i (finish)".

For a tag, I would imagine something like "tag: tagged 4e59582ff7
("Seventh batch for 2.12", 2017-01-23)" would be more appropriate.

> Notes:
>     While playing around with tag reflogs I also found a bug that was present
>     before this patch. It manifests itself when the sha1-ref in the reflog does not
>     point to a commit object but something else.

The underlying machinery for "log" and "rev-list" is about showing a
stream of commits, and most of the reflog entries point at commits.

On the other hand, the "walking reflogs to and show the sequence of
the tip of refs", and there is no reason to expect the tip of refs
will always be commits, but an ancient design mistake bolted the
latter on top of the former (perhaps because in practice the tip of
refs are almost always commits); "reflog" aka "log -g" and "rev-list
--walk-reflogs" share the same issue coming from that misdesign,
which needs to be corrected to solve this issue.

The exact same design mistake also makes "git reflog" to accept
options like "--topo-order", even though many of the options that
make sense for the "commit DAG walking" (which is what "log" and
"rev-list" are about) do not make any sense when walking a reflog.
And the command would give nonsense output when given such an
option, because a reflog is a single strand of pearl of objects (not
necessarily commits) and the order in which these objects appear in
the reflog does not have anything to do with the underlying commit
DAG topology.  Fixing the ancient misdesign would fix this issue,
too.

I think the fix would involve first ripping out the "reflog walking"
code that was bolted on and stop allowing it to inject the entries
taken from the reflog into the "walk the commit DAG" machinery.
Then "reflog walking" code needs to be taught to have its own "now
we got a single object to show, show it (using the helper functions
to show a single object that is already used by 'git show')" code,
instead of piggy-backing on the output codepath used by "log" and
"rev-list".

[PATCH v3] tag: generate useful reflog message

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:

Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging <short-sha1> (<description>)". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
(<description>)" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)

If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:

 - "tree object"
 - "blob object"
 - "other tag object"

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
 builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t7004-tag.sh | 16 +++++++++++++++-
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..638b68e 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,
 	}
 }
 
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+	enum object_type type;
+	struct commit *c;
+	char *buf;
+	unsigned long size;
+	int subject_len = 0;
+	const char *subject_start;
+
+	char *rla = getenv("GIT_REFLOG_ACTION");
+	if (rla) {
+		strbuf_addstr(sb, rla);
+	} else {
+		strbuf_addstr(sb, _("tag: tagging "));
+		strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+	}
+
+	strbuf_addstr(sb, " (");
+	type = sha1_object_info(sha1, NULL);
+	switch (type) {
+	default:
+		strbuf_addstr(sb, _("internal object"));
+		break;
+	case OBJ_COMMIT:
+		c = lookup_commit_reference(sha1);
+		buf = read_sha1_file(sha1, &type, &size);
+		if (!c || !buf) {
+			die(_("commit object %s could not be read"),
+				sha1_to_hex(sha1));
+		}
+		subject_len = find_commit_subject(buf, &subject_start);
+		strbuf_insert(sb, sb->len, subject_start, subject_len);
+		strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
+		free(buf);
+		break;
+	case OBJ_TREE:
+		strbuf_addstr(sb, _("tree object"));
+		break;
+	case OBJ_BLOB:
+		strbuf_addstr(sb, _("blob object"));
+		break;
+	case OBJ_TAG:
+		strbuf_addstr(sb, _("other tag object"));
+		break;
+	}
+	strbuf_addch(sb, ')');
+}
+
 struct msg_arg {
 	int given;
 	struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf ref = STRBUF_INIT;
+	struct strbuf reflog_msg = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
 	struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
+	create_reflog_msg(object, &reflog_msg);
+
 	if (create_tag_object) {
 		if (force_sign_annotate && !annotate)
 			opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (!transaction ||
 	    ref_transaction_update(transaction, ref.buf, object, prev,
 				   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
-				   NULL, &err) ||
+				   reflog_msg.buf, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
 	ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
+	strbuf_release(&reflog_msg);
 	return 0;
 }
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..3c4cb58 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
 	test_must_fail git reflog exists refs/tags/mytag
 '
 
+git log -1 > expected \
+	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag --create-reflog tag_with_reflog &&
-	git reflog exists refs/tags/tag_with_reflog
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
+'
+
+git log -1 > expected \
+	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+	test_when_finished "git tag -d tag_with_reflog" &&
+	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
 '
 
 test_expect_success '--create-reflog does not create reflog on failure' '
-- 
2.10.2

Re: [PATCH] tag: generate useful reflog message

[Cornelius Weig]

On 02/06/2017 12:25 AM, Junio C Hamano wrote:
> cornelius.weig@tngtech.com writes
> For a tag, I would imagine something like "tag: tagged 4e59582ff7
> ("Seventh batch for 2.12", 2017-01-23)" would be more appropriate.

Yes, I agree that this is much clearer. The revised version v3
implements this behavior.

>> Notes:
>>     While playing around with tag reflogs I also found a bug that was present
>>     before this patch. It manifests itself when the sha1-ref in the reflog does not
>>     point to a commit object but something else.
> 
> I think the fix would involve first ripping out the "reflog walking"
> code that was bolted on and stop allowing it to inject the entries
> taken from the reflog into the "walk the commit DAG" machinery.
> Then "reflog walking" code needs to be taught to have its own "now
> we got a single object to show, show it (using the helper functions
> to show a single object that is already used by 'git show')" code,
> instead of piggy-backing on the output codepath used by "log" and
> "rev-list".

I'll start investigating how that could be done. My first glance tells
me that it won't be easy. Especially because I'm not yet familiar with
the git code.

Thanks for your advice!

Re: [PATCH v3] tag: generate useful reflog message

[Junio C Hamano]

cornelius.weig@tngtech.com writes:

> +	strbuf_addstr(sb, " (");
> +	type = sha1_object_info(sha1, NULL);
> +	switch (type) {
> +	default:
> +		strbuf_addstr(sb, _("internal object"));
> +		break;

The code does not even know if this is an "internal" object, does
it?  What it got was simply an object of an unknown type that it is
not prepared to handle.  It's not like you are trying to die() in
this function (I see a die() upon failing to read the referent
commit), so I wonder if this should be a die("BUG").

On the other hand, it's not like failing to describe the tagged
commit in the reflog is such a grave error.  If we can get away with
being vague on a tag that points at an object of unknown type like
the above code does, we could loosen the "oops, we thought we got a
commit, but it turns out that we cannot read it" case below from
die() to just stuffing generic _("commit object") in the reflog.

Between the two extremes above, I am leaning towards making it more
lenient myself, but others may have different opinions.

> +	case OBJ_COMMIT:
> +		c = lookup_commit_reference(sha1);
> +		buf = read_sha1_file(sha1, &type, &size);
> +		if (!c || !buf) {
> +			die(_("commit object %s could not be read"),
> +				sha1_to_hex(sha1));
> +		}
> +		subject_len = find_commit_subject(buf, &subject_start);
> +		strbuf_insert(sb, sb->len, subject_start, subject_len);
> +		strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
> +		free(buf);
> +		break;
> +	case OBJ_TREE:
> +		strbuf_addstr(sb, _("tree object"));
> +		break;
> ...

> +git log -1 > expected \
> +	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
>  test_expect_success 'creating a tag with --create-reflog should create reflog' '
>  	test_when_finished "git tag -d tag_with_reflog" &&
>  	git tag --create-reflog tag_with_reflog &&
> -	git reflog exists refs/tags/tag_with_reflog
> +	git reflog exists refs/tags/tag_with_reflog &&
> +	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&

I'd spell that "\t" with an actual HT to make it portable [*1*].  

We have one example that uses the form in git-filter-branch
documentation and a script in the contrib/ area, but otherwise do
not have anything that relies on \t to be turned into HT by sed.

> +	test_cmp expected actual
> +'
> +
> +git log -1 > expected \
> +	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
> +test_expect_success 'annotated tag with --create-reflog has correct message' '
> +	test_when_finished "git tag -d tag_with_reflog" &&
> +	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
> +	git reflog exists refs/tags/tag_with_reflog &&
> +	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&

Likewise.


[Reference]

*1* http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03

    9.3.2 BRE Ordinary Characters

    An ordinary character is a BRE that matches itself: any character in
    the supported character set, except for the BRE special characters
    listed in BRE Special Characters.

    The interpretation of an ordinary character preceded by an unescaped
    <backslash> ( '\\' ) is undefined, except for:

    - The characters ')', '(', '{', and '}'

    - The digits 1 to 9 inclusive (see BREs Matching Multiple Characters)

    - A character inside a bracket expression

[PATCH v4] tag: generate useful reflog message

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

Thanks for taking a look at my last version.

> On the other hand, it's not like failing to describe the tagged
> commit in the reflog is such a grave error.  If we can get away with
> being vague on a tag that points at an object of unknown type like
> the above code does, we could loosen the "oops, we thought we got a
> commit, but it turns out that we cannot read it" case below from
> die() to just stuffing generic _("commit object") in the reflog.

Good point. I agree that failing to create the message should be no reason to
die().
As you also pointed out, "internal object" is no reliable description
for unhandled object types. I changed that as well.

Changes wrt v3 (interdiff below):
 - change default message for unhandled object types
 - do not die if commit is not readable, but use default description instead
 - test: use verbatim HT character instead of \t


Cornelius Weig (1):
  tag: generate useful reflog message

 builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t7004-tag.sh | 16 +++++++++++++++-
 2 files changed, 68 insertions(+), 2 deletions(-)

Interdiff v3..v4:
diff --git a/builtin/tag.c b/builtin/tag.c
index 638b68e..9b2eabd 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -323,19 +323,19 @@ static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
 	type = sha1_object_info(sha1, NULL);
 	switch (type) {
 	default:
-		strbuf_addstr(sb, _("internal object"));
+		strbuf_addstr(sb, _("object of unknown type"));
 		break;
 	case OBJ_COMMIT:
-		c = lookup_commit_reference(sha1);
-		buf = read_sha1_file(sha1, &type, &size);
-		if (!c || !buf) {
-			die(_("commit object %s could not be read"),
-				sha1_to_hex(sha1));
+		if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+			subject_len = find_commit_subject(buf, &subject_start);
+			strbuf_insert(sb, sb->len, subject_start, subject_len);
+		} else {
+			strbuf_addstr(sb, _("commit object"));
 		}
-		subject_len = find_commit_subject(buf, &subject_start);
-		strbuf_insert(sb, sb->len, subject_start, subject_len);
-		strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
 		free(buf);
+
+		if ((c = lookup_commit_reference(sha1)) != NULL)
+			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
 		break;
 	case OBJ_TREE:
 		strbuf_addstr(sb, _("tree object"));
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 3c4cb58..894959f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -86,7 +86,7 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag --create-reflog tag_with_reflog &&
 	git reflog exists refs/tags/tag_with_reflog &&
-	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
 	test_cmp expected actual
 '

@@ -96,7 +96,7 @@ test_expect_success 'annotated tag with --create-reflog has correct message' '
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
 	git reflog exists refs/tags/tag_with_reflog &&
-	sed -e "s/^.*\t//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
 	test_cmp expected actual
 '
-- 
2.10.2

[PATCH v4] tag: generate useful reflog message

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:

Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging <short-sha1> (<description>)". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
(<description>)" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)

If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:

 - "tree object"
 - "blob object"
 - "other tag object"

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Reviewed-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t7004-tag.sh | 16 +++++++++++++++-
 2 files changed, 68 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..bca890f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,
 	}
 }
 
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+	enum object_type type;
+	struct commit *c;
+	char *buf;
+	unsigned long size;
+	int subject_len = 0;
+	const char *subject_start;
+
+	char *rla = getenv("GIT_REFLOG_ACTION");
+	if (rla) {
+		strbuf_addstr(sb, rla);
+	} else {
+		strbuf_addstr(sb, _("tag: tagging "));
+		strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+	}
+
+	strbuf_addstr(sb, " (");
+	type = sha1_object_info(sha1, NULL);
+	switch (type) {
+	default:
+		strbuf_addstr(sb, _("object of unknown type"));
+		break;
+	case OBJ_COMMIT:
+		if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+			subject_len = find_commit_subject(buf, &subject_start);
+			strbuf_insert(sb, sb->len, subject_start, subject_len);
+		} else {
+			strbuf_addstr(sb, _("commit object"));
+		}
+		free(buf);
+
+		if ((c = lookup_commit_reference(sha1)) != NULL)
+			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
+		break;
+	case OBJ_TREE:
+		strbuf_addstr(sb, _("tree object"));
+		break;
+	case OBJ_BLOB:
+		strbuf_addstr(sb, _("blob object"));
+		break;
+	case OBJ_TAG:
+		strbuf_addstr(sb, _("other tag object"));
+		break;
+	}
+	strbuf_addch(sb, ')');
+}
+
 struct msg_arg {
 	int given;
 	struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf ref = STRBUF_INIT;
+	struct strbuf reflog_msg = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
 	struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
+	create_reflog_msg(object, &reflog_msg);
+
 	if (create_tag_object) {
 		if (force_sign_annotate && !annotate)
 			opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (!transaction ||
 	    ref_transaction_update(transaction, ref.buf, object, prev,
 				   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
-				   NULL, &err) ||
+				   reflog_msg.buf, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
 	ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
+	strbuf_release(&reflog_msg);
 	return 0;
 }
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..894959f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
 	test_must_fail git reflog exists refs/tags/mytag
 '
 
+git log -1 > expected \
+	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag --create-reflog tag_with_reflog &&
-	git reflog exists refs/tags/tag_with_reflog
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
+'
+
+git log -1 > expected \
+	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+	test_when_finished "git tag -d tag_with_reflog" &&
+	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
 '
 
 test_expect_success '--create-reflog does not create reflog on failure' '
-- 
2.10.2

Re: [PATCH v4] tag: generate useful reflog message

[Junio C Hamano]

cornelius.weig@tngtech.com writes:

> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> When tags are created with `--create-reflog` or with the option
> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
> So far, the description of reflog entries for tags was empty, making the
> reflog hard to understand. For example:
> 6e3a7b3 refs/tags/test@{0}:
>
> Now, a reflog message is generated when creating a tag, following the
> pattern "tag: tagging <short-sha1> (<description>)". If
> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
> (<description>)" instead. If the tag references a commit object, the
> description is set to the subject line of the commit, followed by its
> commit date. For example:
> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
>
> If the tag points to a tree/blob/tag objects, the following static
> strings are taken as description:
>
>  - "tree object"
>  - "blob object"
>  - "other tag object"
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> Reviewed-by: Junio C Hamano <gitster@pobox.com>

This last line is inappropriate, as I didn't review _THIS_ version,
which is different from the previous one, and I haven't checked if
the way the comments on the previous review were addressed in this
version is agreeable.

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 072e6c6..894959f 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -80,10 +80,24 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
>  	test_must_fail git reflog exists refs/tags/mytag
>  '
>  
> +git log -1 > expected \
> +	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F

We do not want to do this kind of thing outside the
test_expect_success immediately below, unless there is a good
reason, and in this case I do not see any.

Also write redirection operator and redirection target pathname
without SP in between.

>  test_expect_success 'creating a tag with --create-reflog should create reflog' '
>  	test_when_finished "git tag -d tag_with_reflog" &&
>  	git tag --create-reflog tag_with_reflog &&
> -	git reflog exists refs/tags/tag_with_reflog
> +	git reflog exists refs/tags/tag_with_reflog &&
> +	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
> +	test_cmp expected actual
> +'

In other words, something like:

test_expect_success 'creating a tag with --create-reflog should create reflog' '
	git log -1 \
		--format="format:tag: tagging %h (%s, %cd)%n" \
		--date=format:%Y-%m-%d >expected &&
	test_when_finished "git tag -d tag_with_reflog" &&
	git tag --create-reflog tag_with_reflog &&
	git reflog exists refs/tags/tag_with_reflog &&
	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog >actual &&
	test_cmp expected actual
'	

Even though %F may be shorter, spelling it out makes what we expect
more explicit, and what is what I did in the above example.

Thanks.

Re: [PATCH v4] tag: generate useful reflog message

[Cornelius Weig]

On 02/08/2017 10:28 PM, Junio C Hamano wrote:
> cornelius.weig@tngtech.com writes:
> 
>> From: Cornelius Weig <cornelius.weig@tngtech.com>
>>
>> When tags are created with `--create-reflog` or with the option
>> `core.logAllRefUpdates` set to 'always', a reflog is created for them.
>> So far, the description of reflog entries for tags was empty, making the
>> reflog hard to understand. For example:
>> 6e3a7b3 refs/tags/test@{0}:
>>
>> Now, a reflog message is generated when creating a tag, following the
>> pattern "tag: tagging <short-sha1> (<description>)". If
>> GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
>> (<description>)" instead. If the tag references a commit object, the
>> description is set to the subject line of the commit, followed by its
>> commit date. For example:
>> 6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)
>>
>> If the tag points to a tree/blob/tag objects, the following static
>> strings are taken as description:
>>
>>  - "tree object"
>>  - "blob object"
>>  - "other tag object"
>>
>> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
>> Reviewed-by: Junio C Hamano <gitster@pobox.com>
> 
> This last line is inappropriate, as I didn't review _THIS_ version,
> which is different from the previous one, and I haven't checked if
> the way the comments on the previous review were addressed in this
> version is agreeable.

Sorry for that confusion. I'm still not used to when adding what
sign-off is appropriate. I thought that adding you as reviewer is also a
question of courtesy.

A version with revised tests will follow.

[PATCH v5] tag: generate useful reflog message

[cornelius.weig]

From: Cornelius Weig <cornelius.weig@tngtech.com>

When tags are created with `--create-reflog` or with the option
`core.logAllRefUpdates` set to 'always', a reflog is created for them.
So far, the description of reflog entries for tags was empty, making the
reflog hard to understand. For example:
6e3a7b3 refs/tags/test@{0}:

Now, a reflog message is generated when creating a tag, following the
pattern "tag: tagging <short-sha1> (<description>)". If
GIT_REFLOG_ACTION is set, the message becomes "$GIT_REFLOG_ACTION
(<description>)" instead. If the tag references a commit object, the
description is set to the subject line of the commit, followed by its
commit date. For example:
6e3a7b3 refs/tags/test@{0}: tag: tagging 6e3a7b3398 (Git 2.12-rc0, 2017-02-03)

If the tag points to a tree/blob/tag objects, the following static
strings are taken as description:

 - "tree object"
 - "blob object"
 - "other tag object"

Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---

Notes:
    Interdiff v4..v5
    diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
    index 894959f..1a3230f 100755
    --- a/t/t7004-tag.sh
    +++ b/t/t7004-tag.sh
    @@ -80,9 +80,10 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
     	test_must_fail git reflog exists refs/tags/mytag
     '
    
    -git log -1 > expected \
    -	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
     test_expect_success 'creating a tag with --create-reflog should create reflog' '
    +	git log -1 \
    +		--format="format:tag: tagging %h (%s, %cd)%n" \
    +		--date=format:%Y-%m-%d >expected &&
     	test_when_finished "git tag -d tag_with_reflog" &&
     	git tag --create-reflog tag_with_reflog &&
     	git reflog exists refs/tags/tag_with_reflog &&
    @@ -90,9 +91,10 @@ test_expect_success 'creating a tag with --create-reflog should create reflog' '
     	test_cmp expected actual
     '
    
    -git log -1 > expected \
    -	--format="format:tag: tagging %h (%s, %cd)%n" --date=format:%F
     test_expect_success 'annotated tag with --create-reflog has correct message' '
    +	git log -1 \
    +		--format="format:tag: tagging %h (%s, %cd)%n" \
    +		--date=format:%Y-%m-%d >expected &&
     	test_when_finished "git tag -d tag_with_reflog" &&
     	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
     	git reflog exists refs/tags/tag_with_reflog &&

 builtin/tag.c  | 54 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 t/t7004-tag.sh | 18 +++++++++++++++++-
 2 files changed, 70 insertions(+), 2 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index e40c4a9..bca890f 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -302,6 +302,54 @@ static void create_tag(const unsigned char *object, const char *tag,
 	}
 }
 
+static void create_reflog_msg(const unsigned char *sha1, struct strbuf *sb)
+{
+	enum object_type type;
+	struct commit *c;
+	char *buf;
+	unsigned long size;
+	int subject_len = 0;
+	const char *subject_start;
+
+	char *rla = getenv("GIT_REFLOG_ACTION");
+	if (rla) {
+		strbuf_addstr(sb, rla);
+	} else {
+		strbuf_addstr(sb, _("tag: tagging "));
+		strbuf_add_unique_abbrev(sb, sha1, DEFAULT_ABBREV);
+	}
+
+	strbuf_addstr(sb, " (");
+	type = sha1_object_info(sha1, NULL);
+	switch (type) {
+	default:
+		strbuf_addstr(sb, _("object of unknown type"));
+		break;
+	case OBJ_COMMIT:
+		if ((buf = read_sha1_file(sha1, &type, &size)) != NULL) {
+			subject_len = find_commit_subject(buf, &subject_start);
+			strbuf_insert(sb, sb->len, subject_start, subject_len);
+		} else {
+			strbuf_addstr(sb, _("commit object"));
+		}
+		free(buf);
+
+		if ((c = lookup_commit_reference(sha1)) != NULL)
+			strbuf_addf(sb, ", %s", show_date(c->date, 0, DATE_MODE(SHORT)));
+		break;
+	case OBJ_TREE:
+		strbuf_addstr(sb, _("tree object"));
+		break;
+	case OBJ_BLOB:
+		strbuf_addstr(sb, _("blob object"));
+		break;
+	case OBJ_TAG:
+		strbuf_addstr(sb, _("other tag object"));
+		break;
+	}
+	strbuf_addch(sb, ')');
+}
+
 struct msg_arg {
 	int given;
 	struct strbuf buf;
@@ -335,6 +383,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 {
 	struct strbuf buf = STRBUF_INIT;
 	struct strbuf ref = STRBUF_INIT;
+	struct strbuf reflog_msg = STRBUF_INIT;
 	unsigned char object[20], prev[20];
 	const char *object_ref, *tag;
 	struct create_tag_options opt;
@@ -494,6 +543,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	else
 		die(_("Invalid cleanup mode %s"), cleanup_arg);
 
+	create_reflog_msg(object, &reflog_msg);
+
 	if (create_tag_object) {
 		if (force_sign_annotate && !annotate)
 			opt.sign = 1;
@@ -504,7 +555,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (!transaction ||
 	    ref_transaction_update(transaction, ref.buf, object, prev,
 				   create_reflog ? REF_FORCE_CREATE_REFLOG : 0,
-				   NULL, &err) ||
+				   reflog_msg.buf, &err) ||
 	    ref_transaction_commit(transaction, &err))
 		die("%s", err.buf);
 	ref_transaction_free(transaction);
@@ -514,5 +565,6 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	strbuf_release(&err);
 	strbuf_release(&buf);
 	strbuf_release(&ref);
+	strbuf_release(&reflog_msg);
 	return 0;
 }
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 072e6c6..1a3230f 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -81,9 +81,25 @@ test_expect_success 'creating a tag using default HEAD should succeed' '
 '
 
 test_expect_success 'creating a tag with --create-reflog should create reflog' '
+	git log -1 \
+		--format="format:tag: tagging %h (%s, %cd)%n" \
+		--date=format:%Y-%m-%d >expected &&
 	test_when_finished "git tag -d tag_with_reflog" &&
 	git tag --create-reflog tag_with_reflog &&
-	git reflog exists refs/tags/tag_with_reflog
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
+'
+
+test_expect_success 'annotated tag with --create-reflog has correct message' '
+	git log -1 \
+		--format="format:tag: tagging %h (%s, %cd)%n" \
+		--date=format:%Y-%m-%d >expected &&
+	test_when_finished "git tag -d tag_with_reflog" &&
+	git tag -m "annotated tag" --create-reflog tag_with_reflog &&
+	git reflog exists refs/tags/tag_with_reflog &&
+	sed -e "s/^.*	//" .git/logs/refs/tags/tag_with_reflog > actual &&
+	test_cmp expected actual
 '
 
 test_expect_success '--create-reflog does not create reflog on failure' '
-- 
2.10.2

Re: [PATCH v4] tag: generate useful reflog message

[Junio C Hamano]

Cornelius Weig <cornelius.weig@tngtech.com> writes:

> A version with revised tests will follow.

Thanks; I think this is clean enough.  Let's queue this one and
advance it to 'next' soonish.