threads / rfc / 12803

patch[RFC] add Message-ID field to log on git-am operation

Subject: [PATCH] [RFC] add Message-ID field to log on git-am operation

## tl;dr

5 messages between Mar 21, 2008 and Mar 24, 2008. Diffs are folded; open one to read it.

replies: 4people: 2as markdown or json

Anton Gladkov· Mar 21, 2008, 22:00 UTC · lore
o For what?
 E.g. you have tuned your post-script
 to send a notification on patches committed to
 the main branch. It is usefull when such
 notification sended as a reply on original message
 to follow already committed patches.
o How to use?
 Just 'git-am' your message.
 To whatch a message ID in logs use '%M' in pretty format or
 'full' format.
---
 builtin-commit-tree.c |    2 ++
 builtin-mailinfo.c    |    5 ++++-
 git-am.sh             |    3 ++-
 pretty.c              |   12 ++++++++++++
 4 files changed, 20 insertions(+), 2 deletions(-)
Show changes to 4 files +20 −2

builtin-commit-tree.c, builtin-mailinfo.c, git-am.sh, pretty.c

diff --git a/builtin-commit-tree.c b/builtin-commit-tree.c
index 6610d18..b396ec4 100644
--- a/builtin-commit-tree.c
+++ b/builtin-commit-tree.c
@@ -100,6 +100,8 @@ int cmd_commit_tree(int argc, const char **argv, const char *prefix)
 	/* Person/date information */
 	strbuf_addf(&buffer, "author %s\n", git_author_info(IDENT_ERROR_ON_NO_NAME));
 	strbuf_addf(&buffer, "committer %s\n", git_committer_info(IDENT_ERROR_ON_NO_NAME));
+	if (getenv("GIT_MESSAGE_ID") != NULL)
+		strbuf_addf(&buffer, "message %s\n", getenv("GIT_MESSAGE_ID"));
 	if (!encoding_is_utf8)
 		strbuf_addf(&buffer, "encoding %s\n", git_commit_encoding);
 	strbuf_addch(&buffer, '\n');
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 11f154b..f800d86 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -289,7 +289,7 @@ static void cleanup_space(char *buf)
 
 static void decode_header(char *it, unsigned itsize);
 static const char *header[MAX_HDR_PARSED] = {
-	"From","Subject","Date",
+	"From","Subject","Date","Message-ID"
 };
 
 static int check_header(char *line, unsigned linesize, char **hdr_data, int overwrite)
@@ -905,6 +905,9 @@ static void handle_info(void)
 			handle_from(hdr);
 			fprintf(fout, "Author: %s\n", name);
 			fprintf(fout, "Email: %s\n", email);
+		} else if (!memcmp(header[i], "Message-ID", 10)) {
+			cleanup_space(hdr);
+			fprintf(fout, "%s: %s\n", header[i], hdr);
 		} else {
 			cleanup_space(hdr);
 			fprintf(fout, "%s: %s\n", header[i], hdr);
diff --git a/git-am.sh b/git-am.sh
index ac5c388..ba22bf8 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -338,6 +338,7 @@ do
 	GIT_AUTHOR_NAME="$(sed -n '/^Author/ s/Author: //p' "$dotest/info")"
 	GIT_AUTHOR_EMAIL="$(sed -n '/^Email/ s/Email: //p' "$dotest/info")"
 	GIT_AUTHOR_DATE="$(sed -n '/^Date/ s/Date: //p' "$dotest/info")"
+	GIT_MESSAGE_ID="$(sed -n '/^Message-ID/ s/Message-ID: //p' "$dotest/info")"
 
 	if test -z "$GIT_AUTHOR_EMAIL"
 	then
@@ -345,7 +346,7 @@ do
 		stop_here $this
 	fi
 
-	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE
+	export GIT_AUTHOR_NAME GIT_AUTHOR_EMAIL GIT_AUTHOR_DATE GIT_MESSAGE_ID
 
 	SUBJECT="$(sed -n '/^Subject/ s/Subject: //p' "$dotest/info")"
 	case "$keep_subject" in -k)  SUBJECT="[PATCH] $SUBJECT" ;; esac
diff --git a/pretty.c b/pretty.c
index 16bfb86..076718c 100644
--- a/pretty.c
+++ b/pretty.c
@@ -385,6 +385,7 @@ struct format_commit_context {
 	struct chunk subject;
 	struct chunk author;
 	struct chunk committer;
+	struct chunk message;
 	struct chunk encoding;
 	size_t body_off;
 
@@ -438,6 +439,9 @@ static void parse_commit_header(struct format_commit_context *context)
 		} else if (!prefixcmp(msg + i, "committer ")) {
 			context->committer.off = i + 10;
 			context->committer.len = eol - i - 10;
+		} else if (!prefixcmp(msg + i, "message ")) {
+			context->message.off = i + 8;
+			context->message.len = eol - i - 8;
 		} else if (!prefixcmp(msg + i, "encoding ")) {
 			context->encoding.off = i + 9;
 			context->encoding.len = eol - i - 9;
@@ -547,6 +551,9 @@ static size_t format_commit_item(struct strbuf *sb, const char *placeholder,
 	case 'c':	/* committer ... */
 		return format_person_part(sb, placeholder[1],
 		                   msg + c->committer.off, c->committer.len);
+	case 'M':	/* message */
+		strbuf_add(sb, msg + c->message.off, c->message.len);
+		return 1;
 	case 'e':	/* encoding */
 		strbuf_add(sb, msg + c->encoding.off, c->encoding.len);
 		return 1;
@@ -627,6 +634,11 @@ static void pp_header(enum cmit_fmt fmt,
 			strbuf_grow(sb, linelen + 80);
 			pp_user_info("Commit", fmt, sb, line + 10, dmode, encoding);
 		}
+		if (!memcmp(line, "message ", 8) &&
+		    (fmt == CMIT_FMT_FULL || fmt == CMIT_FMT_FULLER)) {
+			strbuf_grow(sb, linelen + 80);
+			pp_user_info("Message-ID", fmt, sb, line + 8, dmode, encoding);
+		}
 	}
 }
 
-- 
1.5.5.rc0.22.ga384d.dirty
Junio C Hamano· Mar 22, 2008, 19:51 UTC · re: Anton Gladkov · lore

Re: [PATCH] [RFC] add Message-ID field to log on git-am operation

This is a mixed bag.

Your changes to mailinfo is fine, and I think it may make even more sense to also parse out In-Reply-To: and References: to capture the message context better.

On the other hand, I'd NAK changes to pretty.c and commit-tree.c; it is wrong to place that information in new commit object header. The commit object header is a place to store information common to all commit objects (authorship and committer) and the structural information that is required to correctly handle the commit objects (pointers to trees and commits, and encoding that tells what the message part is in if it is not in UTF-8).

Just like workflows inspired by the kernel project use Signed-off-by: and
Acked-by: information in the commit message part to keep track of the flow
of patches, and some distro folks say "Closes #nnn" in their messages to
close their issue tracking system entries, your "message" is information
only useful to a particular workflow and convention, and belongs to the
commit log message body, not in the object header.

Wouldn't it work equally well to use applypatch-msg hook? Use your updated mailinfo to parse necessary information out of the incoming message, and add Message-ID: to the commit log messsage, perhaps at the end, in that hook?

Anton Gladkov· Mar 23, 2008, 17:16 UTC · re: Junio C Hamano · lore

Re: [PATCH] [RFC] add Message-ID field to log on git-am operation

Junio! Thank you for your response :)

On Sat, Mar 22, 2008 at 12:51:34PM -0700, Junio C Hamano wrote:
Show 5 quoted lines
> This is a mixed bag.
> 
> Your changes to mailinfo is fine, and I think it may make even more sense
> to also parse out In-Reply-To: and References: to capture the message
> context better.

I've found that all I need could be parsed by less changes in mailinfo. By adding header fields I need to 'header' array :)

Show 7 quoted lines
> 
> On the other hand, I'd NAK changes to pretty.c and commit-tree.c; it is
> wrong to place that information in new commit object header.  The commit
> object header is a place to store information common to all commit objects
> (authorship and committer) and the structural information that is required
> to correctly handle the commit objects (pointers to trees and commits, and
> encoding that tells what the message part is in if it is not in UTF-8).
I see...
Show 7 quoted lines
> 
> Just like workflows inspired by the kernel project use Signed-off-by: and
> Acked-by: information in the commit message part to keep track of the flow
> of patches, and some distro folks say "Closes #nnn" in their messages to
> close their issue tracking system entries, your "message" is information
> only useful to a particular workflow and convention, and belongs to the
> commit log message body, not in the object header.
Ok.
Show 5 quoted lines
> 
> Wouldn't it work equally well to use applypatch-msg hook?  Use your
> updated mailinfo to parse necessary information out of the incoming
> message, and add Message-ID: to the commit log messsage, perhaps at the
> end, in that hook?

applypatch-msg hook executed on message applying, after that there could be useful to test applied patch, so it is not the place for notification sending.

-- 
Best regards,
		anton
mailto:agladkov@sw.ru
Junio C Hamano· Mar 23, 2008, 18:32 UTC · re: Anton Gladkov · lore

Re: [PATCH] [RFC] add Message-ID field to log on git-am operation

Anton Gladkov <agladkov@parallels.com> writes:
Show 9 quoted lines
> On Sat, Mar 22, 2008 at 12:51:34PM -0700, Junio C Hamano wrote:
> ...
>> Wouldn't it work equally well to use applypatch-msg hook?  Use your
>> updated mailinfo to parse necessary information out of the incoming
>> message, and add Message-ID: to the commit log messsage, perhaps at the
>> end, in that hook?
>
> applypatch-msg hook executed on message applying, after that there could be
> useful to test applied patch, so it is not the place for notification sending.
And nobody suggested to send anything from the hook.

As I understood from your e-mail without proposed commit log message, your assumed workflow was:

	receive mail
        "git am" it
	test rewrite amend whatever you want to convince yourself that
         the received patch was Ok
        push the result out
        pushed-into repository has a hook to send notification,
         by picking up the Message-ID from the commit object
The only thing you need was that the message-id information is available
somewhere in the commit object.  I objected to your patch that puts it in
the commit object header and suggested it be placed in the commit log
message.  You can implement such custom commit munging to add an extra
Message-ID: line inside the hook.  You do not have to send out e-mail nor
anything from it.
Anton Gladkov· Mar 24, 2008, 06:43 UTC · re: Junio C Hamano · lore

Re: [PATCH] [RFC] add Message-ID field to log on git-am operation

On Sun, Mar 23, 2008 at 11:32:08AM -0700, Junio C Hamano wrote:
Show 17 quoted lines
> Anton Gladkov <agladkov@parallels.com> writes:
> > ...
> > applypatch-msg hook executed on message applying, after that there could be
> > useful to test applied patch, so it is not the place for notification sending.
> 
> And nobody suggested to send anything from the hook.
> 
> As I understood from your e-mail without proposed commit log message, your
> assumed workflow was:
> 
> 	receive mail
>         "git am" it
> 	test rewrite amend whatever you want to convince yourself that
>          the received patch was Ok
>         push the result out
>         pushed-into repository has a hook to send notification,
>          by picking up the Message-ID from the commit object
That is it.
Show 7 quoted lines
> 
> The only thing you need was that the message-id information is available
> somewhere in the commit object.  I objected to your patch that puts it in
> the commit object header and suggested it be placed in the commit log
> message.  You can implement such custom commit munging to add an extra
> Message-ID: line inside the hook.  You do not have to send out e-mail nor
> anything from it.
I've got your idea! Thank you :)
-- 
Best regards,
		anton
mailto:agladkov@sw.ru

← back to recent threads