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

[Michael Strawbridge <michael.strawbridge@amd.com>]

On 2023-01-18 11:35, Luben Tuikov wrote:
> On 2023-01-18 11:27, Junio C Hamano wrote:
>> Luben Tuikov <luben.tuikov@amd.com> writes:
>>
>>> On 2023-01-17 02:31, Junio C Hamano wrote:
>>>> Luben Tuikov <luben.tuikov@amd.com> writes:
>>>>
>>>>>> +test_expect_success $PREREQ "--validate hook supports header argument" '
>>>>>> +	write_script my-hooks/sendemail-validate <<-\EOF &&
>>>>>> +	if test -s "$2"
>>>>>> +	then
>>>>>> +		cat "$2" >actual
>>>>>> +		exit 1
>>>>>> +	fi
>>>>>> +	EOF
>>>> If "$2" is not given, or an empty "$2" is given, is that an error?
>>>> I am wondering if the lack of "else" clause (and the hook exits with
>>>> success when "$2" is an empty file) here is intentional.
>>> I think we'll always have a $2, since it is the SMTP envelope and headers.
>> We write our tests to verify _that_ assumption you have.  A future
>> developer mistakenly drops the code to append the file to the
>> command line that invokes the hook, and we want our test to catch
>> such a mistake.
>>
>> Do we really feed envelope?  E.g. if the --envelope-sender=<who> is
>> used, does $2 have the "From:" from the header and "MAIL TO" from
>> the envelope separately?
> I'm not sure--I thought we did, but yes, we should _test_ that we indeed
> 1) have/get $2, as a non-empty string,
> 2) it is a non-empty, readable file,
> 3) contains the test header we included in git-format-patch in the test.
>
> This is what I meant when I wrote "we'll always have $2 ...", not having it
> is failure of some kind and yes we should test for it.

I've tested using the envelope-sender=<who> and the hook only gets the headers.  I've applied the feedback above in patch set v8 including a test for the 2nd argument.  The new test will fail if either the supplied argument is not a file or the custom header is not found.