On Mon, Sep 25, 2023 at 10:48:29AM -0400, Todd Zullinger wrote:
> From the peanut gallery... could the presence or lack of the
> Email::Valid perl module be a factor?
Ah, thanks! The thought of differing modules even occurred to me, since
I know we have a few optimistic dependencies, but when I looked I didn't
manage to find that one (somehow I thought Mail::Address was the
interesting one here; I think I might be getting senile).
With Email::Valid installed, I can reproduce with just (in git.git, but
I think it would work in any repo):
$ echo "exit 0" >.git/hooks/sendemail-validate
$ chmod +x .git/hooks/sendemail-validate
$ git send-email --dry-run -1 --to=foo@xxxxxxxxxxx,bar@xxxxxxxxxxx
error: unable to extract a valid address from: foo@xxxxxxxxxxx,bar@xxxxxxxxxxx
Disabling the hook with "chmod -x" makes the problem go away (and this
is with current "master", hence the more readable error message).
I think the issue is that a8022c5f7b ends up in extract_valid_address()
via this call stack:
$ = main::extract_valid_address('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1161
$ = main::extract_valid_address_or_die('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 2087
@ = main::unique_email_list('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1507
@ = main::gen_header() called from file '/home/peff/compile/git/git-send-email' line 2113
. = main::validate_patch('/tmp/WfoPQSKCUa/0001-The-twelfth-batch.patch', 'auto') called from file '/home/peff/compile/git/git-send-email' line 815
whereas prior to that commit, we hit it later:
$ = main::extract_valid_address('foo@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1166
@ = main::validate_address('foo@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1189
@ = main::validate_address_list('foo@xxxxxxxxxxx', 'bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1348
@ = main::process_address_list('foo@xxxxxxxxxxx,bar@xxxxxxxxxxx') called from file '/home/peff/compile/git/git-send-email' line 1091
So the issue is the call to gen_header() added in validate_patch(). We
won't yet have processed the address lists by that point. We can move
those calls up, but it requires moving a bit of extra code, too (like
the parts prompting for the "to" list if it isn't filled in).
Possibly the validation checks need to be moved down, if they want to
see a more complete view of the emails. But now we're doing more work
(like asking the user to write the cover letter!) before we do
validation, which is probably bad.
So I dunno. Maybe gen_header() should be lazily doing this
process_address_list() stuff? I'm not very familiar with the send-email
code, so I'm not sure what secondary effects that could have.
-Peff
