Re: [PATCH 0/2] git-am: add --message-id/--no-message-id options

On 25/11/2014 22:21, Christian Couder wrote:

On Tue, Nov 25, 2014 at 6:01 PM, Paolo Bonzini [off-list ref] wrote:

quoted

On 25/11/2014 17:27, Christian Couder wrote:

quoted

quoted

quoted

From: Paolo Bonzini <pbonzini@redhat.com>

This series adds a --message-id option to git-mailinfo and git-am.
git-am also gets an am.messageid configuration key to set the default,
and a --no-message-id option to override the configuration key.
(I'm not sure of the usefulness of a mailinfo.messageid option, so
I left it out; this follows the example of -k instead of --scissors).

This option can be useful in order to associate commit messages with
mailing list discussions.

If both --message-id and -s are specified, the Signed-off-by goes
last.  This is coming out more or less naturally out of the git-am
implementation, but is also tested in t4150-am.sh.

Did you have a look at git interpret-trailers currently in master?

Hmm, now I have.

As far as I understand, all the git-am hooks are called on the commit
rather than the incoming email: all headers are lost by the time
git-mailinfo exits, including the Message-Id.  And you cannot call any
hook before git-mailinfo because git-mailinfo is where the
Content-Transfer-Encoding is processed.

How would you integrate git-interpret-trailers in git-mailinfo?

I don't know exactly, but people may want to add trailers when they
run git-am, see:

http://thread.gmane.org/gmane.comp.version-control.git/251412/

and we decided that it was better to let something like git
interpret-trailers decide how they should be handled.

Maybe if git-interpret-trailers could be called from git-mailinfo with
some arguments coming from git-am, it could be configured with
something like:

git config trailer.Message-Id.command 'perl -ne '\''print $1 if
m/^Message-Id: (.*)$/'\'' $ARG'

So "git am --trailer 'Message-Id: msg-file' msg-file" would call "git
mailinfo ..." that would call "git interpret-trailers --trailer
'Message-Id: msg-file'" that would call "perl -ne 'print $1 if
m/^Message-Id: (.*)$/' msg-file" and the output of this command, let's
call it $id, would be put into a "Message-Id: $id" trailer in the
commit message.

I think overloading trailer.Message-Id.command is not a good idea,
because it would prevent using "git interpret-trailers" to add a message
id manually ("git interpret-trailers --trailer message-id='<foo@bar>'").

Another possibility could be to add a third output file to git-mailinfo,
including all the headers.  Then a hook could be called with the headers
and commit message.

The question is: what would it be used for?  There aren't that many mail
headers, and most of them (From, Subject, Date) are recorded in the
commit anyway.  One idea could be to record who was a recipient of the
original message, even if no Cc line was added explicitly.  In most
projects, Cc is often added randomly, but I guess that's a valid
usecase.  I can certainly code the above hook instead of this approach
if Junio thinks it's better.

In the meanwhile, I have thought of a couple additions to "git
interpret-trailers" and I can submit patches for them.

Paolo

This way there is nothing specific to Message-Id in the code and
people can decide using other trailer.Message-Id.* config variables
exactly where the Message-Id trailer would be in the commit message.

Best,
Christian.