Re: [PATCH] send-email: move validation code below process_address_list

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 24, 2023, 21:55 UTC
Message-ID: <xmqqmsw73cua.fsf@gitster.g>
In-Reply-To: <ee56c4df-e030-45f9-86a9-94fb3540db60@amd.com>

Michael Strawbridge <michael.strawbridge@amd.com> writes:

> Subject: [PATCH] send-email: move validation code below process_address_list
>
> Move validation logic below processing of email address lists so that
> email validation gets the proper email addresses.

Hmph, without this patch, the tip of 'seen' passes t9001 on my box, but with it, it claims that it failed 58, 87, and 89.

Here is how #58 fails (the last part of "cd t && sh t9001-*.sh -i -v").

expecting success of 9001.58 'In-Reply-To without --chain-reply-to':
        clean_fake_sendmail &&
        echo "<unique-message-id@example.com>" >expect &&
        git send-email \
                --from="Example <nobody@example.com>" \
                --to=nobody@example.com \
                --no-chain-reply-to \
                --in-reply-to="$(cat expect)" \
                --smtp-server="$(pwd)/fake.sendmail" \
                $patches $patches $patches \
                2>errors &&
        # The first message is a reply to --in-reply-to
        sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt1 >actual &&
        test_cmp expect actual &&
        # Second and subsequent messages are replies to the first one
        sed -n -e "s/^Message-ID: *\(.*\)/\1/p" msgtxt1 >expect &&
        sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt2 >actual &&
        test_cmp expect actual &&
        sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt3 >actual &&
        test_cmp expect actual

0001-Second.patch
0001-Second.patch
0001-Second.patch
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
OK. Log says:
Sendmail: /usr/local/google/home/jch/w/git.git/t/trash directory.t9001-send-email/fake.sendmail -i nobody@example.com author@example.com one@example.com two@example.com committer@example.com
From: Example <nobody@example.com>
To: nobody@example.com
Cc: A <author@example.com>,
        One <one@example.com>,
        two@example.com,
        C O Mitter <committer@example.com>
Subject: [PATCH 1/1] Second.
Date: Tue, 24 Oct 2023 21:52:27 +0000
Message-ID: <20231024215229.1787922-1-nobody@example.com>
X-Mailer: git-send-email 2.42.0-705-g1a1f985ecc
In-Reply-To: <unique-message-id@example.com>
References: <unique-message-id@example.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Result: OK
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
OK. Log says:
Sendmail: /usr/local/google/home/jch/w/git.git/t/trash directory.t9001-send-email/fake.sendmail -i nobody@example.com author@example.com one@example.com two@example.com committer@example.com
From: Example <nobody@example.com>
To: nobody@example.com
Cc: A <author@example.com>,
        One <one@example.com>,
        two@example.com,
        C O Mitter <committer@example.com>
Subject: [PATCH 1/1] Second.
Date: Tue, 24 Oct 2023 21:52:28 +0000
Message-ID: <20231024215229.1787922-2-nobody@example.com>
X-Mailer: git-send-email 2.42.0-705-g1a1f985ecc
In-Reply-To: <unique-message-id@example.com>
References: <unique-message-id@example.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Result: OK
(mbox) Adding cc: A <author@example.com> from line 'From: A <author@example.com>'
(mbox) Adding cc: One <one@example.com> from line 'Cc: One <one@example.com>, two@example.com'
(mbox) Adding cc: two@example.com from line 'Cc: One <one@example.com>, two@example.com'
(body) Adding cc: C O Mitter <committer@example.com> from line 'Signed-off-by: C O Mitter <committer@example.com>'
OK. Log says:
Sendmail: /usr/local/google/home/jch/w/git.git/t/trash directory.t9001-send-email/fake.sendmail -i nobody@example.com author@example.com one@example.com two@example.com committer@example.com
From: Example <nobody@example.com>
To: nobody@example.com
Cc: A <author@example.com>,
        One <one@example.com>,
        two@example.com,
        C O Mitter <committer@example.com>
Subject: [PATCH 1/1] Second.
Date: Tue, 24 Oct 2023 21:52:29 +0000
Message-ID: <20231024215229.1787922-3-nobody@example.com>
X-Mailer: git-send-email 2.42.0-705-g1a1f985ecc
In-Reply-To: <unique-message-id@example.com>
References: <unique-message-id@example.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

Result: OK
--- expect      2023-10-24 21:52:29.115044899 +0000
+++ actual      2023-10-24 21:52:29.119045306 +0000

@@ -1 +1 @@
-<20231024215229.1787922-1-nobody@example.com>
+<unique-message-id@example.com>
not ok 58 - In-Reply-To without --chain-reply-to
#
#               clean_fake_sendmail &&
#               echo "<unique-message-id@example.com>" >expect &&
#               git send-email \
#                       --from="Example <nobody@example.com>" \
#                       --to=nobody@example.com \
#                       --no-chain-reply-to \
#                       --in-reply-to="$(cat expect)" \
#                       --smtp-server="$(pwd)/fake.sendmail" \
#                       $patches $patches $patches \
#                       2>errors &&
#               # The first message is a reply to --in-reply-to
#               sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt1 >actual &&
#               test_cmp expect actual &&
#               # Second and subsequent messages are replies to the first one
#               sed -n -e "s/^Message-ID: *\(.*\)/\1/p" msgtxt1 >expect &&
#               sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt2 >actual &&
#               test_cmp expect actual &&
#               sed -n -e "s/^In-Reply-To: *\(.*\)/\1/p" msgtxt3 >actual &&
#               test_cmp expect actual
#
1..58