Re: [PATCH] send-email: more meaningful Message-ID

From: Eric Wong <normalperson@yhbt.net>
Date: Apr 5, 2016, 21:36 UTC
Message-ID: <20160405213607.GA15023@dcvr.yhbt.net>
In-Reply-To: <xmqqinzv224x.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> wrote:

Show 24 quoted lines

> Eric Wong <normalperson@yhbt.net> writes:
> 
> > Using a YYYYmmddHHMMSS date representation is more meaningful to
> > humans, especially when used for lookups on NNTP servers or linking
> > to archive sites via Message-ID (e.g. mid.gmane.org or
> > mid.mail-archive.com).  This timestamp format more easily gives a
> > reader of the URL itself a rough date of a linked message compared
> > to having them calculate the seconds since the Unix epoch.
> >
> > Furthermore, having the MUA name in the Message-ID seems to be a
> > rare oddity I haven't noticed outside of git-send-email.  We
> > already have an optional X-Mailer header field to advertise for
> > us, so extending the Message-ID by 15 characters can make for
> > unpleasant Message-ID-based URLs to archive sites.
> >
> > Signed-off-by: Eric Wong <normalperson@yhbt.net>
> > ---
> 
> Sounds like a sensible goal.  Just a few comments.
> 
>  - Is it safe to assume that we always can use POSIX::strftime(), or
>    do we need some fallback?  I am guessing that this is safe, as
>    POSIX has been part of the core modules for a long time, and the
>    script does "use 5.008" upfront.

I'm hoping so :) And none of the format specifiers used here should be subject to locale-dependent weirdness, at least.

+Cc both Johannes for Windows knowledge.

Show 6 quoted lines

>  - It is my understanding that, as "use" is a compilation-time
>    thing, hiding it inside a block does not help reducing the
>    start-up overhead (people can use "require" if they want to do a
>    lazy loading and optionally a fallback).  Is my Perl5 outdated?
>    Otherwise, let's have it near the beginning of the script, close
>    to where we use Term::ReadLine and others.

You're correct, I'll move the "use" to the top in v2.

I could call "require" and call the sub as "POSIX::strftime", but this code path is likely enough that any startup time improvement for uncommon cases wouldn't be worth it.

Will wait a bit for strftime portability comments before v2.

Show 12 quoted lines

> > --- a/git-send-email.perl
> > +++ b/git-send-email.perl
> > @@ -949,7 +949,8 @@ my ($message_id_stamp, $message_id_serial);
> >  sub make_message_id {
> >  	my $uniq;
> >  	if (!defined $message_id_stamp) {
> > -		$message_id_stamp = sprintf("%s-%s", time, $$);
> > +		use POSIX qw/strftime/;
> > +		$message_id_stamp = strftime("%Y%m%d%H%M%S.$$", gmtime(time));
> >  		$message_id_serial = 0;
> >  	}
> >  	$message_id_serial++;