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

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 27, 2023, 15:44 UTC
Message-ID: <xmqqedhgoysw.fsf@gitster.g>
In-Reply-To: <ZTp7xvXDw1GF-NUB@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.

Show 6 quoted lines

> 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.

Show 5 quoted lines

> 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.

Show 7 quoted lines

> 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.