threads / patch / 8664

patchgit-send-email: RFC2822 compliant Message-ID

Subject: [PATCH] git-send-email: RFC2822 compliant Message-ID

## tl;dr

4 messages between Jun 20, 2007 and Jun 21, 2007. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Michael Hendricks· Jun 20, 2007, 13:25 UTC · lore

RFC 2822 section 3.6.4 suggests that a "good method" for generating a
Message-ID is to put the domain name of the host on the right-side of
the "@" character.  Use Perl's Sys::Hostname to do the heavy lifting.
This module has been in the Perl core since version 5.
---
 git-send-email.perl |    7 ++++---
 1 files changed, 4 insertions(+), 3 deletions(-)

Show changes to git-send-email.perl +4 −3

diff --git a/git-send-email.perl b/git-send-email.perl
index 7c0c90b..2259f4b 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -21,6 +21,7 @@ use warnings;
 use Term::ReadLine;
 use Getopt::Long;
 use Data::Dumper;
+use Sys::Hostname;
 use Git;
 
 package FakeTerm;
@@ -411,9 +412,9 @@ sub extract_valid_address {
 # a random number to the end, in case we are called quicker than
 # 1 second since the last time we were called.
 
-# We'll setup a template for the message id, using the "from" address:
-my $message_id_from = extract_valid_address($from);
-my $message_id_template = "<%s-git-send-email-$message_id_from>";
+# We'll setup a template for the message id, using the hostname:
+my $hostname = hostname();
+my $message_id_template = "<%s-git-send-email\@$hostname>";
 
 sub make_message_id
 {

-- 
1.5.2.2.238.g7cbf2f2

Junio C Hamano· Jun 20, 2007, 20:18 UTC · re: Michael Hendricks · lore

Re: [PATCH] git-send-email: RFC2822 compliant Message-ID

Michael Hendricks <michael@ndrix.org> writes:

> RFC 2822 section 3.6.4 suggests that a "good method" for generating a
> Message-ID is to put the domain name of the host on the right-side of
> the "@" character.  Use Perl's Sys::Hostname to do the heavy lifting.
> This module has been in the Perl core since version 5.

Probably is a good idea for 50% of properly configured hosts. I think hosts can be configured so that hostname() already returns fqdn in which case your patch is fine but they can also be configured so that hostname() plus its domainname becomes fqdn, in which case it is probably not. In any case it is mere suggestion (not MUST nor even SHOULD), so we should judge its merits a bit carefully.

What happens if the machine you run send-email on does not have a valid hostname configured yet? People on home machines or laptops whose only contact outside are with their ISP mailservers should be able to send their patches without having to configure /etc/hostname, shouldn't they? Does Sys::Hostname die under some condition, such as "the host is not configured well enough"? If so I suspect the change to replace the existing one is not acceptable.

I think we should use something safe that gives reasonably unique identifier and the existing $message_id_from based method is one way to do so.

The message from vger mailmaster (DSM) suggests that somehow $message_id_from method returned an empty string. Maybe make your patch used as a fallback in such a case?

Finally could you resend this with your updated git-send-email? I suspect that this line in the mail header of your patch:

	Message-ID: <11823459011323-git-send-email-michael@ndrix.org>

should have read like so:

	Message-ID: <11823459011323-git-send-email@ndrix.org>

Junio C Hamano· Jun 20, 2007, 20:47 UTC · re: Michael Hendricks · lore

Re: [PATCH] git-send-email: RFC2822 compliant Message-ID

How about doing this instead?

 * Move call to make_message_id() to where it matters, namely,
   before the $message_id is needed to be placed in the
   generated e-mail header; this has an important side effect of
   making it clear that $from is already available.

 * Throw in Sys::Hostname::hostname() just for fun, although I
   suspect that the code would never trigger due to the modified
   call sequence that makes sure $from is always available.

---

Show changes to git-send-email.perl +12 −4

diff --git a/git-send-email.perl b/git-send-email.perl
index 7c0c90b..9f75551 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -412,13 +412,21 @@ sub extract_valid_address {
 # 1 second since the last time we were called.
 
 # We'll setup a template for the message id, using the "from" address:
-my $message_id_from = extract_valid_address($from);
-my $message_id_template = "<%s-git-send-email-$message_id_from>";
 
 sub make_message_id
 {
 	my $date = time;
 	my $pseudo_rand = int (rand(4200));
+	my $du_part;
+	for ($from, $committer, $author) {
+		$du_part = extract_valid_address($_);
+		last if ($du_part ne '');
+	}
+	if ($du_part eq '') {
+		use Sys::Hostname qw();
+		$du_part = 'user@' . Sys::Hostname::hostname();
+	}
+	my $message_id_template = "<%s-git-send-email-$du_part>";
 	$message_id = sprintf $message_id_template, "$date$pseudo_rand";
 	#print "new message id = $message_id\n"; # Was useful for debugging
 }
@@ -467,6 +475,8 @@ sub send_message
 		$ccline = "\nCc: $cc";
 	}
 	$from = sanitize_address_rfc822($from);
+	make_message_id();
+
 	my $header = "From: $from
 To: $to${ccline}
 Subject: $subject
@@ -533,7 +543,6 @@ X-Mailer: git-send-email $gitversion
 
 $reply_to = $initial_reply_to;
 $references = $initial_reply_to || '';
-make_message_id();
 $subject = $initial_subject;
 
 foreach my $t (@files) {
@@ -627,7 +636,6 @@ foreach my $t (@files) {
 			$references = "$message_id";
 		}
 	}
-	make_message_id();
 }
 
 if ($compose) {

Michael Hendricks· Jun 21, 2007, 04:09 UTC · re: Junio C Hamano · lore

Re: [PATCH] git-send-email: RFC2822 compliant Message-ID

On Wed, Jun 20, 2007 at 01:47:34PM -0700, Junio C Hamano wrote:

Show 67 quoted lines

> How about doing this instead?
> 
>  * Move call to make_message_id() to where it matters, namely,
>    before the $message_id is needed to be placed in the
>    generated e-mail header; this has an important side effect of
>    making it clear that $from is already available.
> 
>  * Throw in Sys::Hostname::hostname() just for fun, although I
>    suspect that the code would never trigger due to the modified
>    call sequence that makes sure $from is always available.
> 
> ---
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 7c0c90b..9f75551 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -412,13 +412,21 @@ sub extract_valid_address {
>  # 1 second since the last time we were called.
>  
>  # We'll setup a template for the message id, using the "from" address:
> -my $message_id_from = extract_valid_address($from);
> -my $message_id_template = "<%s-git-send-email-$message_id_from>";
>  
>  sub make_message_id
>  {
>  	my $date = time;
>  	my $pseudo_rand = int (rand(4200));
> +	my $du_part;
> +	for ($from, $committer, $author) {
> +		$du_part = extract_valid_address($_);
> +		last if ($du_part ne '');
> +	}
> +	if ($du_part eq '') {
> +		use Sys::Hostname qw();
> +		$du_part = 'user@' . Sys::Hostname::hostname();
> +	}
> +	my $message_id_template = "<%s-git-send-email-$du_part>";
>  	$message_id = sprintf $message_id_template, "$date$pseudo_rand";
>  	#print "new message id = $message_id\n"; # Was useful for debugging
>  }
> @@ -467,6 +475,8 @@ sub send_message
>  		$ccline = "\nCc: $cc";
>  	}
>  	$from = sanitize_address_rfc822($from);
> +	make_message_id();
> +
>  	my $header = "From: $from
>  To: $to${ccline}
>  Subject: $subject
> @@ -533,7 +543,6 @@ X-Mailer: git-send-email $gitversion
>  
>  $reply_to = $initial_reply_to;
>  $references = $initial_reply_to || '';
> -make_message_id();
>  $subject = $initial_subject;
>  
>  foreach my $t (@files) {
> @@ -627,7 +636,6 @@ foreach my $t (@files) {
>  			$references = "$message_id";
>  		}
>  	}
> -	make_message_id();
>  }
>  
>  if ($compose) {
>

I like it. It eliminates two globals and makes the context of make_message_id clearer.

-- 
Michael