Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"

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

Todd Zullinger <tmz@pobox.com> writes:

> Alternately, perhaps having Email::Valid as an optional
> dependency is worth reconsidering. If it's truly important
> to validation, make it a requirement.  If it's not, then
> drop it to simplify the code and avoid these sort of issues.

Reducing the possible "valid" configurations we support is a very
tempting proposition.

> If I make the git package require it to ensure consistent
> behavior then some folks will -quite rightly- complain that
> it should not be a requirement.  If I keep it an optional
> dependency, then debugging becomes more difficult for the
> reasons we've seen in these recent (and not-so-recent)
> threads.

Very true.

> I'd lean toward dropping the dependency entirely and leave
> the more basic validation of git-send-email in place.  That
> may not catch every type of address error, but I would argue
> that what we do without Email::Valid is perfectly reasonable
> for checking basic email address syntax sanity.

Yes, it is very tempting, and given that we have to keep our
fallback codepath working for those without Email::Valid ANYWAY,
as long as the dependency is merely optional, I very much agree
with your argument here.

> On a related note, one issue¹ we had reported in Fedora
> after making Email::Valid a requirement was that it rejected
> messages where the local part was too long, per the relevant
> RFC's.  But these were generated addresses from GitLab.  The
> addresses worked in practice.  While Email::Valid was
> technically correct in rejecting such addresses, it didn't
> improve the experience of git send-email users.

I am of two minds here.  I can sympathize with both positions.

 - Trying to be strict to what we send out to the world by using
   Email::Valid that tries to be more RFC kosher matches "be strict
   in what you send out, be lenient in what you receive" mantra

 - Rejecting what works in practice and in real world tend to help
   users.

If we require Email::Valid, then sriking the balance between the
above two will entirely become the responsibility of them; any
end-user who complains "the validation is overly strict" will get
"talk to authors of Email::Valid".

If we ditch Email::Valid, it will become _our_ responsibility, which
means a bit of extra maintenance burden to this project.  But perhaps
it is worth it?  I dunno.

Having Email::Valid as an optional dependency does not place us in a
position better than either of these two, so from that point of view,
too, I like your "we should either make it required or unused, not
an optional dependency" very much.

Thanks.