Re: Regression: git send-email Message-Id: numbering doesn't start at 1 any more

[Junio C Hamano <gitster@pobox.com>]

Uwe Kleine-König <u.kleine-koenig@pengutronix.de> writes:

> Hello,
>
> Since commit 3ece9bf0f9e24909b090cf348d89e8920bd4f82f I experience that
> the generated Message-Ids don't start at ....-1-... any more. I have:
>
> $ git send-email w/*
> ...
> Subject: [PATCH 0/5] watchdog: Drop platform_driver_probe() and convert to platform remove callback returning void (part II)
> Date: Mon,  6 Nov 2023 16:10:04 +0100
> Message-ID: <20231106151003.3844134-7-u.kleine-koenig@pengutronix.de>
> ...
>
> So the cover letter is sent with Message-Id: ...-7-...

The above is consistent with the fact that a 5-patch series with a
cover letter consists of 6 messages.  Dry-run uses message numbers
1-6 and forgets to reset the counter, so the next message becomes 7.
As you identified, the fix in 3ece9bf0 (send-email: clear the
$message_id after validation, 2023-05-17) for the fallout from an
even earlier change to process each message twice still had left an
observable side effect subject to the Hyrum's law, it seems.

> +my ($message_id_stamp, $message_id_serial);
>  if ($validate) {
>  	# FIFOs can only be read once, exclude them from validation.
>  	my @real_files = ();
> @@ -821,6 +822,7 @@ sub is_format_patch_arg {
>  	}
>  	delete $ENV{GIT_SENDEMAIL_FILE_COUNTER};
>  	delete $ENV{GIT_SENDEMAIL_FILE_TOTAL};
> +	$message_id_serial = 0;
>  }

This fix looks quite logical to me, but even with this, the side
effects of the earlier "read message twice" persists in end-user
observable form, don't they?  IIRC, when sending out an N message
series, we start from the timestamp as of N seconds ago and give
each message the Date: header that increments by 1 second, which
would mean the validator will see Date: that is different from what
will actually be sent out, and more importantly, the messages sent
out for real will have timestamps from the future, negating the
point of starting from N seconds ago in the first place.  Your
script may not have been paying attention to it and only noticed the
difference in id_serial, but somebody else would complain the
difference coming from calling gen_header more than once for each
messages since a8022c5f (send-email: expose header information to
git-send-email's sendemail-validate hook, 2023-04-19).

So, I dunno.  Michael, what do you think?  It appears to me that a
more fundamental fix to the fallout from a8022c5f might be needed
(e.g., we still let gen_header run while validating, but once
validation is done, save the headers that validator saw and use them
without calling gen_header again when we send the messages out, or
something), if we truly want to be regression free.

By the way, out of curiosity, earlier you said your script looks at
the Message-IDs and counts the number of messages.  How does it do
that?  Does it read the output of send-email and pass the messages
to MTA for sending out for real?

Thanks.

>  @files = handle_backup_files(@files);
> @@ -1181,7 +1183,6 @@ sub validate_address_list {
>  
>  # We'll setup a template for the message id, using the "from" address:
>  
> -my ($message_id_stamp, $message_id_serial);
>  sub make_message_id {
>  	my $uniq;
>  	if (!defined $message_id_stamp) {
>
> But I guess this could be done prettier by someone who is fluent in
> Perl.
>
> Best regards
> Uwe