Git Mailing List Archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Emily Shaffer <nasamuffin@google.com>
Cc: Git List <git@vger.kernel.org>,
	michael.strawbridge@amd.com, dianders@chromium.org
Subject: Re: bug report: cover letter is inheriting last patch's message ID with send-email
Date: Wed, 17 May 2023 12:22:01 -0700	[thread overview]
Message-ID: <xmqqjzx6pxuu.fsf@gitster.g> (raw)
In-Reply-To: <xmqqo7mipyt0.fsf@gitster.g> (Junio C. Hamano's message of "Wed, 17 May 2023 12:01:31 -0700")

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

>> # With the attached patches, where all of the patches have a
>> # Message-Id but the cover letter doesn't.
>> git send-email *.patch

I suspect this is a recent regression with the addition of the
pre_process_file step.  56adddaa (send-email: refactor header
generation functions, 2023-04-19) makes all messages parsed
before the first message is sent out, by calling a sub
"pre_process_file" before invoking the validate hook.  The same sub
is called again for each message when it is sent out, as the
processing in that step is shared between the time the message gets
vetted and the time the message gets sent.

Unfortunately, $message_id variable is assigned to in that sub.  So
it is very much understandable why this happens.

I wonder if it is just doing something silly like this?

--- >8 ---
Subject: [PATCH] send-email: clear the $message_id after validation

Recently git-send-email started parsing the same message twice, once
to validate _all_ the message before sending even the first one, and
then after the validation hook is happy and each message gets sent,
to read the contents to find out where to send to etc.

Unfortunately, the effect of reading the messages for validation
lingered even after the validation is done.  Namely $message_id gets
assigned if exists in the input files but the variable is global,
and it is not cleared before pre_process_file runs.  This causes
reading a message without a message-id followed by reading a message
with a message-id to misbehave---the sub reports as if the message
had the same id as the previously written one.

Clear the variable before starting to read the headers in
pre_process_file

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I am not surprised at all if there are similar problems in this
   function around variables other than $message_id; this patch is
   merely reacting to the bug report and not systematically hunting
   and fixing the bugs coming from the same root cause.  If the
   original author of the pre_process_file change is still around,
   the second sets of eyes from them is very much appreciated.

 git-send-email.perl | 1 +
 1 file changed, 1 insertion(+)

diff --git c/git-send-email.perl w/git-send-email.perl
index 89d8237e89..889ef388c8 100755
--- c/git-send-email.perl
+++ w/git-send-email.perl
@@ -1771,6 +1771,7 @@ sub send_message {
 sub pre_process_file {
 	my ($t, $quiet) = @_;
 
+	undef $message_id;
 	open my $fh, "<", $t or die sprintf(__("can't open file %s"), $t);
 
 	my $author = undef;

  reply	other threads:[~2023-05-17 19:22 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-17 18:38 bug report: cover letter is inheriting last patch's message ID with send-email Emily Shaffer
2023-05-17 19:01 ` Junio C Hamano
2023-05-17 19:22   ` Junio C Hamano [this message]
2023-05-17 20:14     ` Doug Anderson
2023-05-17 20:21       ` Junio C Hamano
2023-05-18  0:51     ` Michael Strawbridge
2023-05-18  1:06       ` Junio C Hamano
2023-05-17 19:24   ` Doug Anderson
2023-05-17 20:04     ` Junio C Hamano
2023-05-17 20:20       ` Doug Anderson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqjzx6pxuu.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=dianders@chromium.org \
    --cc=git@vger.kernel.org \
    --cc=michael.strawbridge@amd.com \
    --cc=nasamuffin@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).