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

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

[Michael Hendricks]

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(-)

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

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

[Junio C Hamano]

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>

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

[Junio C Hamano]

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) {

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

[Michael Hendricks]

On Wed, Jun 20, 2007 at 01:47:34PM -0700, Junio C Hamano wrote:
> 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