Re: [PATCH v5 2/2] send-email: expose header information to git-send-email's sendemail-validate hook

From: Luben Tuikov <luben.tuikov@amd.com>
Date: Jan 17, 2023, 04:09 UTC
Message-ID: <f31f1480-d611-f4b4-0e7b-589574943eef@amd.com>
In-Reply-To: <xmqqfsccii86.fsf@gitster.g>

On 2023-01-14 22:34, Junio C Hamano wrote:

Show 22 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
> 
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> "Strawbridge, Michael" <Michael.Strawbridge@amd.com> writes:
>>>
>>>> +test_expect_success $PREREQ "--validate hook supports header argument" '
>>>> +	test_when_finished "rm my-hooks.ran" &&
>>>> +	write_script my-hooks/sendemail-validate <<-\EOF &&
>>>> +	filesize=$(stat -c%s "$2")
>>>
>>> That "stat -c" is a GNU-ism, I think.  macOS CI jobs at GitHub do
>>> not seem to like it.
>>>
>>>> +	if [ "$filesize" != "0" ]; then
>>>
>>> Also, please see Documentation/CodingGuidelines to learn the subset
>>> of shell script syntax and style we adopted for this project.
> 
> I'll tentatively queue this as a fix-up on top of the topic, but is
> this testing the right thing?  Should we inspect "$2" and verify
> that it gives us what we expect, not just it being non-empty?

Hi Junio,

Thanks for reviewing this patch set.

We're generally not interested in "what else" is in the SMTP envelope and headers.

The extension this patch set provides is that if a hook-writer is interested in some SMTP header, or the contents of that header, then there is a way to provide the SMTP envelope and thus check the headers.

Currently, $1, is identical to git-format-patch's output, (for which there are other hooks to check that output.) This was a bit disappointing, as it is a git-send-email hook after all, and we're interested in the "email" part of this Git command and hook.

The idea is that hook writers would merely be grepping for a particular header they're interested in--it could even be a custom header, "X-something" for instance, and if present, they'll check the contents of that header and validate the patch, or perform some other action.

So, checking that the SMTP envelope and headers, $2, is not empty suffices for what this patch set implements. We leave it up to the hook writers to inspect the SMTP envelope and headers for their particular hook purpose.

-- 
Regards,
Luben