Re: [PATCH v2] git-send-email.perl: fix In-Reply-To for second and subsequent patches

On Tue, 19 Oct 2010 11:26:33 -0700
Junio C Hamano [off-list ref] wrote:

Thanks for commenting Junio.

Antonio Ospite [off-list ref] writes:

quoted

Make second and subsequent patches appear as replies to the first patch,
even when an initial In-Reply-To is supplied; this is the typical
behaviour we want when we send a series with cover letter in reply to
some discussion, and this is also what the man page says about
the --in-reply-to option.

I am not so sure if that is what the documentation says.

Sorry, I meant the man page part about --[no-]chain-reply-to, I mistyped
that and generated more confusion than was "necessary".

 1. When --in-reply-to gives $reply_to, the first one becomes a reply to
    that message, with or without --chain-reply-to.

No doubts on that.

 2. When --chain-reply-to is in effect, all the messages are strung
    together to form a single chain.  The first message may be in reply to
    the $reply_to given by --in-reply-to command line option (see
    previous), or the root of the discussion thread.  The second one is a
    response to the first one, and the third one is a response to the
    second one, etc.

This is pretty clear as well.

 3. When --chain-reply-to is not in effect:

    a. When --in-reply-to is used, too, the second and the subsequent ones
       become replies to $reply_to.  Together with the first rule, all
       messages become replies to $reply_to given by --in-reply-to.

    b. When --in-reply-to is not used, presumably the second and
       subsequent ones become replies to the first one, which would be the
       root.

The documentation is reasonably clear about the 1., 2. and 3a. above, I
think, even though I do not think 3b. is clearly specified.

In general, 3b. is specified in --[no-]chain-reply-to section, the
"problem" with 3a. is that --in-reply-to _overrides_ the behavior
specified by --no-chain-reply-to.

So I think that the whole issue really boils down to the question:
Should --in-reply-to apply _only_ to the first email?
The doc for the corresponding git-format-patch option gives _one_
answer, and you know that :)

By answering to this question with a YES also in git-send-email, we are
making --in-reply-to *independent* from --[no-]chain-reply-to, hence
the very simple test.

If you are changing 3a. above so that the first message becomes a response
to $reply_to, and the second one becomes a response to the first message
(and the third and subsequent ones too when --chain-reply-to is not in
effect), you would need to update the documentation as well.  Even if it
might be of good kind, it would be a change of the established behaviour.

Right, the documentation needs be updated as well, thanks for pointing
this out. I think I am going to copy from the git-format-patch man
page.
So, do you agree to this change of behavior as long as it is documented?

quoted

diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index a298eb0..410b85f 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh

@@ -295,6 +295,20 @@ test_expect_success $PREREQ 'Valid In-Reply-To when prompting' '
 	! grep "^In-Reply-To: < *>" msgtxt1
 '
 
+test_expect_success $PREREQ 'In-Reply-To in second patch with --thread' '
+	clean_fake_sendmail &&
+	git send-email \
+		--from="Example <nobody@example.com>" \
+		--to=nobody@example.com \
+		--thread \
+		--in-reply-to="<unique-message-id@example.com>" \
+		--smtp-server="$(pwd)/fake.sendmail" \
+		$patches $patches \
+		2>errors

You are breaking the && chain here.

Some other tests do that as well, the last line is a command by
itself not and-chained with the git-send-email invocation. I guess the
logic behind this is that the test succeeds if the _last_ command
succeeds. If this is wrong then some other tests are affected too.

quoted

+        # The second patch should be seen as reply to the first one
+        test $(sed -n -e "s/^In-Reply-To:\(.*\)/\1/p" msgtxt2) = $(sed -n -e "s/^Message-Id:\(.*\)/\1/p" msgtxt1)
+'

You would need to test the interaction with --chain-reply-to as well, so
there should be another test, and you would probably need three messages
fed to send-email not just two to see the effect of the interaction.

I saw the tests in the other mail, but under my interpretation
we should just ensure --in-reply-to is applying to the first
message only (so we check the second one), if so from the third one on
--[no-]chain-reply-to is totally unrelated to --in-reply-to.

I think I can make the test more explicit tho, like:
("In-Reply-To" of second message) != $initial_reply_to

Thanks,
   Antonio

-- 
Antonio Ospite
http://ao2.it

PGP public key ID: 0x4553B001

A: Because it messes up the order in which people normally read text.
   See http://en.wikipedia.org/wiki/Posting_style
Q: Why is top-posting such a bad thing?