threads / patch / 4832

patchgit-format-patch: Make the second and subsequent mails replies to the first

Subject: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first

## tl;dr

13 messages between Jul 10, 2006 and Jul 15, 2006. Diffs are folded; open one to read it.

replies: 12people: 7as markdown or json

Josh Triplett· Jul 10, 2006, 15:44 UTC · lore

Add message_id and ref_message_id fields to struct rev_info, used in show_log with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively. Use these in git-format-patch to make the second and subsequent patch mails replies to the first patch mail.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
git-send-email already does this, but this change helps people who use
things like git-imap-send or similar to send the patch emails by other
means.
 builtin-log.c |   23 +++++++++++++++++++++++
 log-tree.c    |    5 +++++
 revision.h    |    2 ++
 3 files changed, 30 insertions(+), 0 deletions(-)
Show changes to 3 files +30 −0

builtin-log.c, log-tree.c, revision.h

diff --git a/builtin-log.c b/builtin-log.c
index 864c6cd..9d0cae1 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -220,6 +220,17 @@ static void get_patch_ids(struct rev_inf
 	o2->flags = flags2;
 }
 
+static void gen_message_id(char *dest, unsigned int length, char *base)
+{
+	const char *committer = git_committer_info(1);
+	const char *email_start = strrchr(committer, '<');
+	const char *email_end = strrchr(committer, '>');
+	if(!email_start || !email_end || email_start > email_end - 1)
+		die("Could not extract email from committer identity.");
+	snprintf(dest, length, "%s.%u.git.%.*s", base, time(NULL),
+		 email_end - email_start - 1, email_start + 1);
+}
+
 int cmd_format_patch(int argc, const char **argv, char **envp)
 {
 	struct commit *commit;
@@ -233,6 +244,8 @@ int cmd_format_patch(int argc, const cha
 	int ignore_if_in_upstream = 0;
 	struct diff_options patch_id_opts;
 	char *add_signoff = NULL;
+	char message_id[1024];
+	char ref_message_id[1024];
 
 	init_revisions(&rev);
 	rev.commit_format = CMIT_FMT_EMAIL;
@@ -359,6 +372,16 @@ int cmd_format_patch(int argc, const cha
 		int shown;
 		commit = list[nr];
 		rev.nr = total - nr + (start_number - 1);
+		/* Make the second and subsequent mails replies to the first */
+		if (nr == (total - 2)) {
+			strncpy(ref_message_id, message_id,
+				sizeof(ref_message_id));
+			ref_message_id[sizeof(ref_message_id)-1] = '\0';
+			rev.ref_message_id = ref_message_id;
+		}
+		gen_message_id(message_id, sizeof(message_id),
+			       sha1_to_hex(commit->object.sha1));
+		rev.message_id = message_id;
 		if (!use_stdout)
 			reopen_stdout(commit, rev.nr, keep_subject);
 		shown = log_tree_commit(&rev, commit);
diff --git a/log-tree.c b/log-tree.c
index 9d8d46f..4971988 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -97,6 +97,11 @@ void show_log(struct rev_info *opt, cons
 			subject = "Subject: ";
 
 		printf("From %s Mon Sep 17 00:00:00 2001\n", sha1);
+		if (opt->message_id)
+			printf("Message-Id: <%s>\n", opt->message_id);
+		if (opt->ref_message_id)
+			printf("In-Reply-To: <%s>\nReferences: <%s>\n",
+			       opt->ref_message_id, opt->ref_message_id);
 		if (opt->mime_boundary) {
 			static char subject_buffer[1024];
 			static char buffer[1024];
diff --git a/revision.h b/revision.h
index c010a08..e23ec8f 100644
--- a/revision.h
+++ b/revision.h
@@ -61,6 +61,8 @@ struct rev_info {
 	struct log_info *loginfo;
 	int		nr, total;
 	const char	*mime_boundary;
+	const char	*message_id;
+	const char	*ref_message_id;
 	const char	*add_signoff;
 	const char	*extra_headers;
 
-- 
1.4.1.gbe4c7-dirty
Erik Mouw· Jul 10, 2006, 16:29 UTC · re: Johannes Schindelin · lore

Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first

On Mon, Jul 10, 2006 at 06:01:48PM +0200, Johannes Schindelin wrote:
> please make that behaviour optional.

Rather make it consistent with git-send-email. Principle of least surprise.

Erik
-- 
+-- Erik Mouw -- www.harddisk-recovery.com -- +31 70 370 12 90 --
| Lab address: Delftechpark 26, 2628 XH, Delft, The Netherlands
Josh Triplett· Jul 10, 2006, 16:43 UTC · re: Erik Mouw · lore

Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first

On Mon, 2006-07-10 at 18:29 +0200, Erik Mouw wrote:
Show 5 quoted lines
> On Mon, Jul 10, 2006 at 06:01:48PM +0200, Johannes Schindelin wrote:
> > please make that behaviour optional.
> 
> Rather make it consistent with git-send-email. Principle of least
> surprise.

Well, git-send-email does not include an option to disable the threading headers, so consistency with git-send-email would imply not including any such option. I can, however, implement a --no-thread option to omit the headers, as well as git-send-email's --in-reply-to option to set an initial In-Reply-To/References. New patch series shortly.

- Josh Triplett
Jakub Narebski· Jul 10, 2006, 20:25 UTC · re: Josh Triplett · lore

Re: [PATCH] git-format-patch: Make the second and subsequent mails replies to the first

Josh Triplett wrote:
Show 12 quoted lines
> On Mon, 2006-07-10 at 18:29 +0200, Erik Mouw wrote:
>> On Mon, Jul 10, 2006 at 06:01:48PM +0200, Johannes Schindelin wrote:
>> > please make that behaviour optional.
>> 
>> Rather make it consistent with git-send-email. Principle of least
>> surprise.
> 
> Well, git-send-email does not include an option to disable the threading
> headers, so consistency with git-send-email would imply not including
> any such option.  I can, however, implement a --no-thread option to omit
> the headers, as well as git-send-email's --in-reply-to option to set an
> initial In-Reply-To/References.  New patch series shortly.
git-send-email has three ways of sending files:
 1. Chain Reply-To:, where every patch refers to earlier in series.
    Ugly in threaded mail/news readers, harder to comment, but there is
    no way to loose the order (e.g. if patches are not numbered *blush*)
 2. No chain reply-to, with cover letter introducing patch series.
    IMHO nicest format... provided there are no errors nor mistakes.
 3. No chain reply-to, without cover letter. I presonally don't like 
    this format, YMMV.
-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Josh Triplett· Jul 10, 2006, 18:41 UTC · re: Erik Mouw · lore

[PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

Add message_id and ref_message_id fields to struct rev_info, used in show_log with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively. Use these in git-format-patch to make the second and subsequent patch mails replies to the first patch mail.

Signed-off-by: Josh Triplett <josh@freedesktop.org>
---
Resend of previous patch as part of new patch series.
 builtin-log.c |   23 +++++++++++++++++++++++
 log-tree.c    |    5 +++++
 revision.h    |    2 ++
 3 files changed, 30 insertions(+), 0 deletions(-)
Show changes to 3 files +30 −0

builtin-log.c, log-tree.c, revision.h

diff --git a/builtin-log.c b/builtin-log.c
index 864c6cd..9d0cae1 100644
--- a/builtin-log.c
+++ b/builtin-log.c
@@ -220,6 +220,17 @@ static void get_patch_ids(struct rev_inf
 	o2->flags = flags2;
 }
 
+static void gen_message_id(char *dest, unsigned int length, char *base)
+{
+	const char *committer = git_committer_info(1);
+	const char *email_start = strrchr(committer, '<');
+	const char *email_end = strrchr(committer, '>');
+	if(!email_start || !email_end || email_start > email_end - 1)
+		die("Could not extract email from committer identity.");
+	snprintf(dest, length, "%s.%u.git.%.*s", base, time(NULL),
+		 email_end - email_start - 1, email_start + 1);
+}
+
 int cmd_format_patch(int argc, const char **argv, char **envp)
 {
 	struct commit *commit;
@@ -233,6 +244,8 @@ int cmd_format_patch(int argc, const cha
 	int ignore_if_in_upstream = 0;
 	struct diff_options patch_id_opts;
 	char *add_signoff = NULL;
+	char message_id[1024];
+	char ref_message_id[1024];
 
 	init_revisions(&rev);
 	rev.commit_format = CMIT_FMT_EMAIL;
@@ -359,6 +372,16 @@ int cmd_format_patch(int argc, const cha
 		int shown;
 		commit = list[nr];
 		rev.nr = total - nr + (start_number - 1);
+		/* Make the second and subsequent mails replies to the first */
+		if (nr == (total - 2)) {
+			strncpy(ref_message_id, message_id,
+				sizeof(ref_message_id));
+			ref_message_id[sizeof(ref_message_id)-1] = '\0';
+			rev.ref_message_id = ref_message_id;
+		}
+		gen_message_id(message_id, sizeof(message_id),
+			       sha1_to_hex(commit->object.sha1));
+		rev.message_id = message_id;
 		if (!use_stdout)
 			reopen_stdout(commit, rev.nr, keep_subject);
 		shown = log_tree_commit(&rev, commit);
diff --git a/log-tree.c b/log-tree.c
index 9d8d46f..4971988 100644
--- a/log-tree.c
+++ b/log-tree.c
@@ -97,6 +97,11 @@ void show_log(struct rev_info *opt, cons
 			subject = "Subject: ";
 
 		printf("From %s Mon Sep 17 00:00:00 2001\n", sha1);
+		if (opt->message_id)
+			printf("Message-Id: <%s>\n", opt->message_id);
+		if (opt->ref_message_id)
+			printf("In-Reply-To: <%s>\nReferences: <%s>\n",
+			       opt->ref_message_id, opt->ref_message_id);
 		if (opt->mime_boundary) {
 			static char subject_buffer[1024];
 			static char buffer[1024];
diff --git a/revision.h b/revision.h
index c010a08..e23ec8f 100644
--- a/revision.h
+++ b/revision.h
@@ -61,6 +61,8 @@ struct rev_info {
 	struct log_info *loginfo;
 	int		nr, total;
 	const char	*mime_boundary;
+	const char	*message_id;
+	const char	*ref_message_id;
 	const char	*add_signoff;
 	const char	*extra_headers;
 
-- 
1.4.1.gf029
Junio C Hamano· Jul 10, 2006, 21:44 UTC · re: Josh Triplett · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

Josh Triplett <josht@us.ibm.com> writes:
Show 8 quoted lines
> Add message_id and ref_message_id fields to struct rev_info, used in show_log
> with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively.
> Use these in git-format-patch to make the second and subsequent patch mails
> replies to the first patch mail.
>
> Signed-off-by: Josh Triplett <josh@freedesktop.org>
> ---
> Resend of previous patch as part of new patch series.

While I understand what you said about imap-send, I really would feel better if this was optional. Do not change the default output format, please.

Josh Triplett· Jul 14, 2006, 17:16 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

On Mon, 2006-07-10 at 14:44 -0700, Junio C Hamano wrote:
Show 14 quoted lines
> Josh Triplett <josht@us.ibm.com> writes:
> 
> > Add message_id and ref_message_id fields to struct rev_info, used in show_log
> > with CMIT_FMT_EMAIL to set Message-Id and In-Reply-To/References respectively.
> > Use these in git-format-patch to make the second and subsequent patch mails
> > replies to the first patch mail.
> >
> > Signed-off-by: Josh Triplett <josh@freedesktop.org>
> > ---
> > Resend of previous patch as part of new patch series.
> 
> While I understand what you said about imap-send, I really would
> feel better if this was optional.  Do not change the default
> output format, please.

So rather than the --no-thread option provided in the second patch of this series, you'd prefer a --thread option to enable setting the In-Reply-To/References headers?

Note that I based the direction of the --no-thread switch on git-send-email's inclusion of these headers by default with no way to turn them off, figuring that having an option to do so gave it an advantage over git-send-email while remaining consistent with it. I also figured that most people would not mess with the defaults, and thus the default should make a patch series more readable on mailing lists.

- Josh Triplett
Junio C Hamano· Jul 14, 2006, 18:23 UTC · re: Josh Triplett · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

Josh Triplett <josht@us.ibm.com> writes:
Show 7 quoted lines
>> While I understand what you said about imap-send, I really would
>> feel better if this was optional.  Do not change the default
>> output format, please.
>
> So rather than the --no-thread option provided in the second patch of
> this series, you'd prefer a --thread option to enable setting the
> In-Reply-To/References headers?
Eh, that's not what I meant.

I do not mind the code you added to log-tree.c and revision.h, and honestly I do not care which of threading or non-threading mode is the default, although I think your explanation that it parallels what send-email does makes a lot of sense.

But I do mind that the code added by the first patch to cmd_format_patch runs by default, and worse yet, there is no option turn it off. Setting message_id and ref_message_id in rev_info struct should be something the end user should ask for explicitly by invoking the command with an option, perhaps --with-message-id, which you probably would also want to turn on when any of --no-thread, --thread or --initial-reply-to options are given. -

Josh Triplett· Jul 14, 2006, 19:20 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

On Fri, 2006-07-14 at 11:23 -0700, Junio C Hamano wrote:
Show 25 quoted lines
> Josh Triplett <josht@us.ibm.com> writes:
> 
> >> While I understand what you said about imap-send, I really would
> >> feel better if this was optional.  Do not change the default
> >> output format, please.
> >
> > So rather than the --no-thread option provided in the second patch of
> > this series, you'd prefer a --thread option to enable setting the
> > In-Reply-To/References headers?
> 
> Eh, that's not what I meant.
> 
> I do not mind the code you added to log-tree.c and revision.h,
> and honestly I do not care which of threading or non-threading
> mode is the default, although I think your explanation that it
> parallels what send-email does makes a lot of sense.
> 
> But I do mind that the code added by the first patch to
> cmd_format_patch runs by default, and worse yet, there is no
> option turn it off.  Setting message_id and ref_message_id in
> rev_info struct should be something the end user should ask for
> explicitly by invoking the command with an option, perhaps
> --with-message-id, which you probably would also want to turn on
> when any of --no-thread, --thread or --initial-reply-to options
> are given.

How would that work though? Threading requires a Message-Id on at least the first message, so to avoid Message-IDs by default would require turning off threading by default; I can do that if you like, but you suggested that you didn't mind having threading as the default. I could, however, avoid generating Message-Id on the subsequent messages, and avoid generating that Message-Id if you give --no-thread. Would that work?

- Josh Triplett
Junio C Hamano· Jul 14, 2006, 19:32 UTC · re: Josh Triplett · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

Josh Triplett <josht@us.ibm.com> writes:
> ..., but you
> suggested that you didn't mind having threading as the default.
Did I? ... then that was either a mistake or miscommunication.

I do mind changing the default output. I do not mind threading as the default ONLY IF user asks for output with these extra headers.

Petr Baudis· Jul 15, 2006, 07:45 UTC · re: Junio C Hamano · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

Dear diary, on Fri, Jul 14, 2006 at 09:32:27PM CEST, I got a letter where Junio C Hamano <junkio@cox.net> said that...

Show 10 quoted lines
> Josh Triplett <josht@us.ibm.com> writes:
> 
> > ..., but you
> > suggested that you didn't mind having threading as the default.
> 
> Did I? ... then that was either a mistake or miscommunication.
> 
> I do mind changing the default output.  I do not mind threading
> as the default ONLY IF user asks for output with these extra
> headers.

What's the big deal? It's not like we didn't change those things in the past if it doesn't horribly break everything and the new behaviour is clearly more sensible.

It would be good to know what the general policy on this is nowadays.
-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
Snow falling on Perl. White noise covering line noise.
Hides all the bugs too. -- J. Putnam
Junio C Hamano· Jul 15, 2006, 08:10 UTC · re: Petr Baudis · lore

Re: [PATCH 1/3] git-format-patch: Make the second and subsequent mails replies to the first

Petr Baudis <pasky@suse.cz> writes:
Show 16 quoted lines
> Dear diary, on Fri, Jul 14, 2006 at 09:32:27PM CEST, I got a letter
> where Junio C Hamano <junkio@cox.net> said that...
>> Josh Triplett <josht@us.ibm.com> writes:
>> 
>> > ..., but you
>> > suggested that you didn't mind having threading as the default.
>> 
>> Did I? ... then that was either a mistake or miscommunication.
>> 
>> I do mind changing the default output.  I do not mind threading
>> as the default ONLY IF user asks for output with these extra
>> headers.
>
> What's the big deal? It's not like we didn't change those things in the
> past if it doesn't horribly break everything and the new behaviour is
> clearly more sensible.

While I agree to the whole three lines, I do not think adding the Message-Id and In-Reply-To header lines by default is more sensible at all.

Adding phoney Message-Id to format-patch output makes some sense only when you are sending messages, and if I recall original "motive" message correctly only with git-imap-send. We do not need this for git-send-email, since it can do its own threading.

Although I've already accepted the series to "next", now after you brought up the issue, I started to suspect that it might even make sense not to do this in format-patch but make it a responsibility for MUA-looking commands instead.

More importantly, format-patch is used to extract patches into separate files (I do that myself often, and I think Andrew Morten uses it to extract stuff from git-maintained trees). In such a case having phoney Message-Id is simply a waste. Running "head -n X 0*.txt" now needs one or two larger X to view the same information, and fewer patches fit on the screen than before. So the new behaviour, if it were not optional, is clearly less useful for such purpose.

It could even be confusing and inviting mistakes. When quoting a change from somebody that was sent in an e-mail to the list, giving its Message-Id is often helpful to others who want to go to the source themselves, but if a file that was generated by format-patch by default carries a phoney Message-Id, it can be mistakenly used in such a quote.

← back to recent threads