From: Todd Zullinger <tmz@pobox•com>
To: Michael Strawbridge <michael.strawbridge@amd•com>
Cc: "Junio C Hamano" <gitster@pobox•com>, "Jeff King" <peff@peff•net>,
"Uwe Kleine-König" <u.kleine-koenig@pengutronix•de>,
"Luben Tuikov" <luben.tuikov@amd•com>,
git@vger•kernel.org, entwicklung@pengutronix•de
Subject: Re: Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address"
Date: Thu, 26 Oct 2023 10:46:30 -0400 [thread overview]
Message-ID: <ZTp7xvXDw1GF-NUB@pobox.com> (raw)
In-Reply-To: <a71f2f1f-b5f0-4628-a4f3-6fd1319062a3@amd.com>
Michael Strawbridge wrote:
> On 10/26/23 08:41, Junio C Hamano wrote:
>> Jeff King <peff@peff•net> writes:
>>
>>> Note that the bug will only trigger if Email::Valid is installed.
>>
>> I recall we chased a different bug that depends on the use/non-use
>> of this package a few years ago. Is the difference significant
>> enough that we may want to install on one but not in another CI
>> environment, like we have a separate CI jobs with exotic settings, I
>> wonder.
>
> That would make sense to me. We have had 3 regressions threads
> recently for git send email where Email::Valid was important.
>
> - [REGRESSION] uninitialized value $address in git send-email when given multiple recipients separated by commas - (this thread)
> - [REGRESSION] uninitialized value $address in git send-email - https://public-inbox.org/git/20230918212004.GC2163162@coredump.intra.peff.net/T/#m9e0211a8ad387adbbadf31dcfcd7982d4046633d
> - Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address" - https://public-inbox.org/git/68d7e5c3-6b4a-4d0d-9885-f3d4e2199f26@amd.com/T/#m1411c155e11ad9c5d913d22d1d11180ed56eabc7
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.
As a (former) distribution packager, having these optional
dependencies which change the behavior is always a tough
position to be in.
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.
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.
Further validation will happen along the path of mail
transfer agents and failures should be reported to the
sender in the same way as any other invalid email address.
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.
¹ https://bugzilla.redhat.com/2046203
--
Todd
next prev parent reply other threads:[~2023-10-26 14:49 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-10-13 14:14 Regression: git send-email fails with "Use of uninitialized value $address" + "unable to extract a valid address" Uwe Kleine-König
2023-10-13 15:07 ` Kristoffer Haugsbakk
2023-10-20 10:04 ` Uwe Kleine-König
2023-10-20 21:06 ` Michael Strawbridge
2023-10-24 13:00 ` Uwe Kleine-König
2023-10-24 19:00 ` Michael Strawbridge
2023-10-24 20:43 ` Uwe Kleine-König
2023-10-25 7:21 ` Jeff King
2023-10-25 7:40 ` Uwe Kleine-König
2023-10-26 12:41 ` Junio C Hamano
2023-10-26 13:07 ` Michael Strawbridge
2023-10-26 14:46 ` Todd Zullinger [this message]
2023-10-27 15:44 ` Junio C Hamano
2023-10-30 9:29 ` Jeff King
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZTp7xvXDw1GF-NUB@pobox.com \
--to=tmz@pobox$(echo .)com \
--cc=entwicklung@pengutronix$(echo .)de \
--cc=git@vger$(echo .)kernel.org \
--cc=gitster@pobox$(echo .)com \
--cc=luben.tuikov@amd$(echo .)com \
--cc=michael.strawbridge@amd$(echo .)com \
--cc=peff@peff$(echo .)net \
--cc=u.kleine-koenig@pengutronix$(echo .)de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox