* [PATCH] send-email: remove documented requirement for Net::SMTP::SSL @ 2019-05-26 17:22 Chris Mayo 2019-05-27 19:35 ` Eric Wong 0 siblings, 1 reply; 11+ messages in thread From: Chris Mayo @ 2019-05-26 17:22 UTC (permalink / raw) To: git git-send-email uses the TLS support in the Net::SMTP core module from recent versions of Perl. Documenting the minimum version is complex because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34) and libnet (3.01). Version numbers from commit: bfbfc9a953 ("send-email: Net::SMTP::starttls was introduced in v2.34", 2017-05-31) Users of older Perl versions without Net::SMTP::SSL installed will get a clear error message. Signed-off-by: Chris Mayo <aklhfex@gmail.com> --- Documentation/git-send-email.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index 1afe9fc858..a2acdaffa3 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -500,8 +500,8 @@ app-specific or your regular password as appropriate. If you have credential helper configured (see linkgit:git-credential[1]), the password will be saved in the credential store so you won't have to type it the next time. -Note: the following perl modules are required - Net::SMTP::SSL, MIME::Base64 and Authen::SASL +Note: the following Perl modules are required + MIME::Base64 and Authen::SASL SEE ALSO -------- -- 2.21.0 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-26 17:22 [PATCH] send-email: remove documented requirement for Net::SMTP::SSL Chris Mayo @ 2019-05-27 19:35 ` Eric Wong 2019-05-27 20:36 ` Ævar Arnfjörð Bjarmason 2019-05-28 0:05 ` Todd Zullinger 0 siblings, 2 replies; 11+ messages in thread From: Eric Wong @ 2019-05-27 19:35 UTC (permalink / raw) To: Chris Mayo; +Cc: git Chris Mayo <aklhfex@gmail.com> wrote: > git-send-email uses the TLS support in the Net::SMTP core module from > recent versions of Perl. Documenting the minimum version is complex > because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34) > and libnet (3.01). Version numbers from commit: > bfbfc9a953 ("send-email: Net::SMTP::starttls was introduced in v2.34", > 2017-05-31) No disagreement for removing the doc requirement for Net::SMTP::SSL. But core modules can be split out by OS packagers. For Fedora/RH-based systems, the trend tends to be increasing granularity and having more optional packages. So I think documenting Net::SMTP (and Net::Domain) as requirements would still be good, perhaps with a note stating they're typically installed with Perl. Fwiw, I recently ran into some issues where core modules such as Devel::Peek, Encode, and autodie were separate packages on CentOS 7. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-27 19:35 ` Eric Wong @ 2019-05-27 20:36 ` Ævar Arnfjörð Bjarmason 2019-05-27 23:43 ` brian m. carlson 2019-05-28 1:22 ` Eric Wong 2019-05-28 0:05 ` Todd Zullinger 1 sibling, 2 replies; 11+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-27 20:36 UTC (permalink / raw) To: Eric Wong; +Cc: Chris Mayo, git, git-packagers, Dennis Kaarsemaker On Mon, May 27 2019, Eric Wong wrote: > Chris Mayo <aklhfex@gmail.com> wrote: >> git-send-email uses the TLS support in the Net::SMTP core module from >> recent versions of Perl. Documenting the minimum version is complex >> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34) >> and libnet (3.01). Version numbers from commit: >> bfbfc9a953 ("send-email: Net::SMTP::starttls was introduced in v2.34", >> 2017-05-31) > > No disagreement for removing the doc requirement for Net::SMTP::SSL. > > But core modules can be split out by OS packagers. For > Fedora/RH-based systems, the trend tends to be increasing > granularity and having more optional packages. > > So I think documenting Net::SMTP (and Net::Domain) as > requirements would still be good, perhaps with a note stating > they're typically installed with Perl. > > Fwiw, I recently ran into some issues where core modules such as > Devel::Peek, Encode, and autodie were separate packages on CentOS 7. I've done enough git-send-email patching in anger for a year at least with what's sitting in "next" so I'm not working on this, but just my 0.02: I wonder if we shouldn't just be much more aggressive about version requirements for something like git-send-email. Do we really have git users who want a new git *and* have an old perl *and* aren't just getting it from an OS package where the module is dual-life, so the distributor can just package up the newer version if we were to require it? I.e. couldn't we just remove the fallback code added in 0ead000c3a ("send-email: Net::SMTP::SSL is obsolete, use only when necessary", 2017-03-24) and do away with this version detection (which b.t.w. should just do a $obj->can("starttls") check instead). For shipping a newer Net::SMTP we aren't talking about upgrading /usr/bin/perl, just that module, and anyone who's packaging git (e.g. Debian) who cares about minimal dependencies is likely splitting out git-send-email.perl anyway. We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so we'd error out by default unless these modules were there when git was built, packagers could then still set some "no I can't be bothered with send-email at all" or "no I can't be bothered with its SSL support", in the latter case git-send-email would work except for the SSL parts. That would take care of the communication about module dependencies via manpage problem since we'd error by default. When I package things I much prefer that error mode to "parts of package silently don't work because we check at runtime and I didn't religiously scour the docs/release notes". I wouldn't say the same thing about git-add--interactive.perl due to more common its use is. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-27 20:36 ` Ævar Arnfjörð Bjarmason @ 2019-05-27 23:43 ` brian m. carlson 2019-05-28 2:17 ` Ævar Arnfjörð Bjarmason 2019-05-28 1:22 ` Eric Wong 1 sibling, 1 reply; 11+ messages in thread From: brian m. carlson @ 2019-05-27 23:43 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Eric Wong, Chris Mayo, git, git-packagers, Dennis Kaarsemaker [-- Attachment #1: Type: text/plain, Size: 2713 bytes --] On 2019-05-27 at 20:36:36, Ævar Arnfjörð Bjarmason wrote: > I've done enough git-send-email patching in anger for a year at least > with what's sitting in "next" so I'm not working on this, but just my > 0.02: > > I wonder if we shouldn't just be much more aggressive about version > requirements for something like git-send-email. > > Do we really have git users who want a new git *and* have an old perl > *and* aren't just getting it from an OS package where the module is > dual-life, so the distributor can just package up the newer version if > we were to require it? In my experience, shipping newer versions of packages shipped with the OS is a no-no. That's a great way to break unrelated software on the system, and if you're the distributor, to get users angry at you about breaking stuff on their systems. We do indeed have users who want a newer Git on those systems and are using the system Perl. The Git shipped with CentOS 7 (not to mention CentOS 6) is positively ancient and doesn't support useful features like worktrees, so it makes sense to upgrade it. But if you're not a Perl shop, nobody cares about the version of Perl on the system and fussing with it doesn't make sense. > I.e. couldn't we just remove the fallback code added in 0ead000c3a > ("send-email: Net::SMTP::SSL is obsolete, use only when necessary", > 2017-03-24) and do away with this version detection (which b.t.w. should > just do a $obj->can("starttls") check instead). > > For shipping a newer Net::SMTP we aren't talking about upgrading > /usr/bin/perl, just that module, and anyone who's packaging git > (e.g. Debian) who cares about minimal dependencies is likely splitting > out git-send-email.perl anyway. > > We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so > we'd error out by default unless these modules were there when git was > built, packagers could then still set some "no I can't be bothered with > send-email at all" or "no I can't be bothered with its SSL support", in > the latter case git-send-email would work except for the SSL parts. We had a problem with Homebrew sometime back where they stopped shipping git-send-email because it required Perl modules and there was a big uproar and a request for us to allow dynamic Perl support. I would like to discourage distributors from simply refusing to ship core pieces of Git because it tends to cause problems for users and for us down the line. I understand and am fine with splitting components out into multiple packages or omitting parts interfacing with other systems (e.g. git-svn). -- brian m. carlson: Houston, Texas, US OpenPGP: https://keybase.io/bk2204 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 868 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-27 23:43 ` brian m. carlson @ 2019-05-28 2:17 ` Ævar Arnfjörð Bjarmason 0 siblings, 0 replies; 11+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-28 2:17 UTC (permalink / raw) To: brian m. carlson Cc: Eric Wong, Chris Mayo, git, git-packagers, Dennis Kaarsemaker On Tue, May 28 2019, brian m. carlson wrote: > On 2019-05-27 at 20:36:36, Ævar Arnfjörð Bjarmason wrote: >> I've done enough git-send-email patching in anger for a year at least >> with what's sitting in "next" so I'm not working on this, but just my >> 0.02: >> >> I wonder if we shouldn't just be much more aggressive about version >> requirements for something like git-send-email. >> >> Do we really have git users who want a new git *and* have an old perl >> *and* aren't just getting it from an OS package where the module is >> dual-life, so the distributor can just package up the newer version if >> we were to require it? > > In my experience, shipping newer versions of packages shipped with the > OS is a no-no. That's a great way to break unrelated software on the > system, and if you're the distributor, to get users angry at you about > breaking stuff on their systems. Just because a package X needs dependency Y at version Z doesn't mean you need to package it up in such a way that everything that needs any version of Y must use it at version Z, you put the two versions in different places on the FS. So in this case the packager would grumble a bit and package up a Net::SMTP installed at another path. I do that myself to get some more modern versions of dependencies on CentOS 6 without breaking the base system. Does that mean we should do it in this case? No, and given the feedback in this thread about this in particular I think we should just keep the old code. But I think is important to clarify in general, i.e. just because you're on CentOS 6 and don't want to update the main perl-Net-SMTP you can still package up a for-git-perl-Net-SMTP or whatever. We had a similar discussion about whether to depend on a more recent libcurl in the past: https://public-inbox.org/git/CACBZZX78oKU5HuBEqb9qLy7--wcwhC_mW6x7Q+tB4suxohSCsQ@mail.gmail.com/ > We do indeed have users who want a newer Git on those systems and are > using the system Perl. The Git shipped with CentOS 7 (not to mention > CentOS 6) is positively ancient and doesn't support useful features like > worktrees, so it makes sense to upgrade it. But if you're not a Perl > shop, nobody cares about the version of Perl on the system and fussing > with it doesn't make sense. What I was suggesting was whether it was worth it to ask distributors to drop in a few *.pm files, and perhaps a *.so or two compiled against openssl, they wouldn't need to change the base /usr/bin/perl. In the case of the *.pm stuff we could drop fallbacks into perl/Git/LoadCPAN if we wanted things to work out of the box. >> I.e. couldn't we just remove the fallback code added in 0ead000c3a >> ("send-email: Net::SMTP::SSL is obsolete, use only when necessary", >> 2017-03-24) and do away with this version detection (which b.t.w. should >> just do a $obj->can("starttls") check instead). >> >> For shipping a newer Net::SMTP we aren't talking about upgrading >> /usr/bin/perl, just that module, and anyone who's packaging git >> (e.g. Debian) who cares about minimal dependencies is likely splitting >> out git-send-email.perl anyway. >> >> We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so >> we'd error out by default unless these modules were there when git was >> built, packagers could then still set some "no I can't be bothered with >> send-email at all" or "no I can't be bothered with its SSL support", in >> the latter case git-send-email would work except for the SSL parts. > > We had a problem with Homebrew sometime back where they stopped shipping > git-send-email because it required Perl modules and there was a big > uproar and a request for us to allow dynamic Perl support. I would like > to discourage distributors from simply refusing to ship core pieces of > Git because it tends to cause problems for users and for us down the > line. I'm sympathetic to packagers that do that, particularly when "add -i" and friends in s/Perl/C/ land, at which point we'd be asking some distributors who otherwise wouldn't have perl at all to install it just for us. It seems packagers mainly had issues with the Perl-SSL stuff. We already support SSL for https via libcurl usually, perhaps we'd be better off having some wrapper using that (https://curl.haxx.se/libcurl/c/smtp-tls.html), or having git-send-email.perl support something simpler like say stunnel. > I understand and am fine with splitting components out into multiple > packages or omitting parts interfacing with other systems (e.g. > git-svn). I tend to think of SVN as a system more closely related to Git than Git is related to E-Mail :) It's also something that's by definition part of an "egress" workflow, so it tends to be much easier to work around it being missing than say "add -i" or something you truly need locally not being there. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-27 20:36 ` Ævar Arnfjörð Bjarmason 2019-05-27 23:43 ` brian m. carlson @ 2019-05-28 1:22 ` Eric Wong 1 sibling, 0 replies; 11+ messages in thread From: Eric Wong @ 2019-05-28 1:22 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Chris Mayo, git, git-packagers, Dennis Kaarsemaker Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Mon, May 27 2019, Eric Wong wrote: > > Chris Mayo <aklhfex@gmail.com> wrote: > >> git-send-email uses the TLS support in the Net::SMTP core module from > >> recent versions of Perl. Documenting the minimum version is complex > >> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34) > >> and libnet (3.01). Version numbers from commit: > >> bfbfc9a953 ("send-email: Net::SMTP::starttls was introduced in v2.34", > >> 2017-05-31) > > > > No disagreement for removing the doc requirement for Net::SMTP::SSL. > > > > But core modules can be split out by OS packagers. For > > Fedora/RH-based systems, the trend tends to be increasing > > granularity and having more optional packages. > > > > So I think documenting Net::SMTP (and Net::Domain) as > > requirements would still be good, perhaps with a note stating > > they're typically installed with Perl. > > > > Fwiw, I recently ran into some issues where core modules such as > > Devel::Peek, Encode, and autodie were separate packages on CentOS 7. > > I've done enough git-send-email patching in anger for a year at least > with what's sitting in "next" so I'm not working on this, but just my > 0.02: > > I wonder if we shouldn't just be much more aggressive about version > requirements for something like git-send-email. > > Do we really have git users who want a new git *and* have an old perl > *and* aren't just getting it from an OS package where the module is > dual-life, so the distributor can just package up the newer version if > we were to require it? I started writing this earlier, but dropped my connection. And brian said the same thing I was going to say. If OS packagers were to start making Net::SMTP/Net::Domain optional (which I sorta expect...), we should make them optional, too (and add descriptive error messages and manpage updates). I have no need for Net::SMTP with sendemail.smtpserver=/usr/bin/msmtp > I.e. couldn't we just remove the fallback code added in 0ead000c3a > ("send-email: Net::SMTP::SSL is obsolete, use only when necessary", > 2017-03-24) and do away with this version detection (which b.t.w. should > just do a $obj->can("starttls") check instead). Too soon for the fallback code removal; maybe when CentOS 6 is EOL.... However, the "->can" check is nicer, yes. > For shipping a newer Net::SMTP we aren't talking about upgrading > /usr/bin/perl, just that module, and anyone who's packaging git > (e.g. Debian) who cares about minimal dependencies is likely splitting > out git-send-email.perl anyway. > > We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so > we'd error out by default unless these modules were there when git was > built, packagers could then still set some "no I can't be bothered with > send-email at all" or "no I can't be bothered with its SSL support", in > the latter case git-send-email would work except for the SSL parts. > > That would take care of the communication about module dependencies via > manpage problem since we'd error by default. When I package things I > much prefer that error mode to "parts of package silently don't work > because we check at runtime and I didn't religiously scour the > docs/release notes". I prefer we check options passed to the program and decide which dependencies to require based on that. The manpage and --help could be listing the modules required for certain option groups, even. > I wouldn't say the same thing about git-add--interactive.perl due to > more common its use is. I wish to see git-send-email (and vendor-neutral messaging in general) gain more traction. IMHO, send-email should reach the commonality of other parts of git. (*) if/when I find the time, I'd make "git svn find-rev" work w/o SVN::Perl, too. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-27 19:35 ` Eric Wong 2019-05-27 20:36 ` Ævar Arnfjörð Bjarmason @ 2019-05-28 0:05 ` Todd Zullinger 2019-05-28 1:31 ` Eric Wong 1 sibling, 1 reply; 11+ messages in thread From: Todd Zullinger @ 2019-05-28 0:05 UTC (permalink / raw) To: Eric Wong; +Cc: Chris Mayo, Jonathan Nieder, Dennis Kaarsemaker, git Eric Wong wrote: > Chris Mayo <aklhfex@gmail.com> wrote: >> git-send-email uses the TLS support in the Net::SMTP core module from >> recent versions of Perl. Documenting the minimum version is complex >> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34) >> and libnet (3.01). Version numbers from commit: >> bfbfc9a953 ("send-email: Net::SMTP::starttls was introduced in v2.34", >> 2017-05-31) > > No disagreement for removing the doc requirement for Net::SMTP::SSL. > > But core modules can be split out by OS packagers. For > Fedora/RH-based systems, the trend tends to be increasing > granularity and having more optional packages. > > So I think documenting Net::SMTP (and Net::Domain) as > requirements would still be good, perhaps with a note stating > they're typically installed with Perl. I didn't know that git-send-email.perl could take advantage of Net::SMTP::starttls until I read this. [Adding Dennis and Jonathan as the authors of 0ead000c3a ("send-email: Net::SMTP::SSL is obsolete, use only when necessary", 2017-03-24) bfbfc9a953 ("send-email: Net::SMTP::starttls was introduced in v2.34", 2017-05-31), respectively.] The current Fedora and Red Hat package have a requirement on Net::SMTP::SSL from long, long ago¹. As I looked at whether I could remove that (or more accurately, replace it with IO::Socket::SSL which is needed for Net::SMTP to handle starttls), I noticed that on RHEL7 the Net::SMTP version was 2.31, but starttls support has been backported². I wonder if it's (separately from this change) worth adjusting the conditional which sets $use_net_smtp_ssl to use "Net::SMTP->can('starttls')" rather than a strict version check? (It might not be if using 'can' is too fragile or would only benefit the Red Hat 7 packages which likely won't officially be updated to a newer git with such a change.) Something like: diff --git i/git-send-email.perl w/git-send-email.perl index 24859a7bc3..84ac03994d 100755 --- i/git-send-email.perl +++ w/git-send-email.perl @@ -1465,7 +1465,7 @@ sub send_message { } require Net::SMTP; - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); + my $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1; $smtp_domain ||= maildomain(); if ($smtp_encryption eq 'ssl') { ¹ https://bugzilla.redhat.com/443615 ² https://bugzilla.redhat.com/1557574 https://git.centos.org/rpms/perl/c/13dfe3?branch=c7 -- Todd ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-28 0:05 ` Todd Zullinger @ 2019-05-28 1:31 ` Eric Wong 2019-05-28 1:43 ` Ævar Arnfjörð Bjarmason 0 siblings, 1 reply; 11+ messages in thread From: Eric Wong @ 2019-05-28 1:31 UTC (permalink / raw) To: Todd Zullinger; +Cc: Chris Mayo, Jonathan Nieder, Dennis Kaarsemaker, git Todd Zullinger <tmz@pobox.com> wrote: > I wonder if it's (separately from this change) worth > adjusting the conditional which sets $use_net_smtp_ssl to > use "Net::SMTP->can('starttls')" rather than a strict > version check? (It might not be if using 'can' is too > fragile or would only benefit the Red Hat 7 packages which > likely won't officially be updated to a newer git with such > a change.) > > Something like: > > diff --git i/git-send-email.perl w/git-send-email.perl > index 24859a7bc3..84ac03994d 100755 > --- i/git-send-email.perl > +++ w/git-send-email.perl > @@ -1465,7 +1465,7 @@ sub send_message { > } > > require Net::SMTP; > - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); > + my $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1; > $smtp_domain ||= maildomain(); > > if ($smtp_encryption eq 'ssl') { Looks much better to me. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-28 1:31 ` Eric Wong @ 2019-05-28 1:43 ` Ævar Arnfjörð Bjarmason 2019-05-28 1:56 ` Todd Zullinger 2019-05-28 1:57 ` Eric Wong 0 siblings, 2 replies; 11+ messages in thread From: Ævar Arnfjörð Bjarmason @ 2019-05-28 1:43 UTC (permalink / raw) To: Eric Wong Cc: Todd Zullinger, Chris Mayo, Jonathan Nieder, Dennis Kaarsemaker, git On Tue, May 28 2019, Eric Wong wrote: > Todd Zullinger <tmz@pobox.com> wrote: >> I wonder if it's (separately from this change) worth >> adjusting the conditional which sets $use_net_smtp_ssl to >> use "Net::SMTP->can('starttls')" rather than a strict >> version check? (It might not be if using 'can' is too >> fragile or would only benefit the Red Hat 7 packages which >> likely won't officially be updated to a newer git with such >> a change.) >> >> Something like: >> >> diff --git i/git-send-email.perl w/git-send-email.perl >> index 24859a7bc3..84ac03994d 100755 >> --- i/git-send-email.perl >> +++ w/git-send-email.perl >> @@ -1465,7 +1465,7 @@ sub send_message { >> } >> >> require Net::SMTP; >> - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); >> + my $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1; >> $smtp_domain ||= maildomain(); >> >> if ($smtp_encryption eq 'ssl') { > > Looks much better to me. Same, but to bikeshed a bit, at this point we can just do: diff --git a/git-send-email.perl b/git-send-email.perl index 24859a7bc3..4ad2091a49 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1468 +1467,0 @@ sub send_message { - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); @@ -1485 +1484 @@ sub send_message { - if ($use_net_smtp_ssl) { + if (Net::SMTP->can('starttls')) { @@ -1507 +1506 @@ sub send_message { - if ($use_net_smtp_ssl) { + if (Net::SMTP->can('starttls')) { ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-28 1:43 ` Ævar Arnfjörð Bjarmason @ 2019-05-28 1:56 ` Todd Zullinger 2019-05-28 1:57 ` Eric Wong 1 sibling, 0 replies; 11+ messages in thread From: Todd Zullinger @ 2019-05-28 1:56 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Eric Wong, Chris Mayo, Jonathan Nieder, Dennis Kaarsemaker, git Hi, Sorry I missed your earlier reply which also mentioned using $obj->can() Ævar. That's what I get for typing a reply and then walking away for a few hours before hitting send. ;) Ævar Arnfjörð Bjarmason wrote: > Same, but to bikeshed a bit, at this point we can just do: > > diff --git a/git-send-email.perl b/git-send-email.perl > index 24859a7bc3..4ad2091a49 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -1468 +1467,0 @@ sub send_message { > - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); > @@ -1485 +1484 @@ sub send_message { > - if ($use_net_smtp_ssl) { > + if (Net::SMTP->can('starttls')) { > @@ -1507 +1506 @@ sub send_message { > - if ($use_net_smtp_ssl) { > + if (Net::SMTP->can('starttls')) { > I think we'd need to use 'if ! ...' there, or more likely, switch the blocks which follow because the code following 'if ($use_net_smtp_ssl)' is for Net::SMTP::SSL with the 'else' block handling the case where Net::SMTP has ssl/tls support. Right? I know I read the $use_net_smtp_ssl bit backwards the first time or two as well. -- Todd ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL 2019-05-28 1:43 ` Ævar Arnfjörð Bjarmason 2019-05-28 1:56 ` Todd Zullinger @ 2019-05-28 1:57 ` Eric Wong 1 sibling, 0 replies; 11+ messages in thread From: Eric Wong @ 2019-05-28 1:57 UTC (permalink / raw) To: Ævar Arnfjörð Bjarmason Cc: Todd Zullinger, Chris Mayo, Jonathan Nieder, Dennis Kaarsemaker, git Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote: > On Tue, May 28 2019, Eric Wong wrote: > > Todd Zullinger <tmz@pobox.com> wrote: > >> Something like: > >> > >> diff --git i/git-send-email.perl w/git-send-email.perl > >> index 24859a7bc3..84ac03994d 100755 > >> --- i/git-send-email.perl > >> +++ w/git-send-email.perl > >> @@ -1465,7 +1465,7 @@ sub send_message { > >> } > >> > >> require Net::SMTP; > >> - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); > >> + my $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1; > >> $smtp_domain ||= maildomain(); > >> > >> if ($smtp_encryption eq 'ssl') { > > > > Looks much better to me. > > Same, but to bikeshed a bit, at this point we can just do: > > diff --git a/git-send-email.perl b/git-send-email.perl > index 24859a7bc3..4ad2091a49 100755 > --- a/git-send-email.perl > +++ b/git-send-email.perl > @@ -1468 +1467,0 @@ sub send_message { > - my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse("2.34"); > @@ -1485 +1484 @@ sub send_message { > - if ($use_net_smtp_ssl) { > + if (Net::SMTP->can('starttls')) { > @@ -1507 +1506 @@ sub send_message { > - if ($use_net_smtp_ssl) { > + if (Net::SMTP->can('starttls')) { Higher probability of a typo slipping through the cracks, though :) (I was just bit by a similar case with hashes that would've been caught by "use fields") ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2019-05-28 2:17 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-05-26 17:22 [PATCH] send-email: remove documented requirement for Net::SMTP::SSL Chris Mayo 2019-05-27 19:35 ` Eric Wong 2019-05-27 20:36 ` Ævar Arnfjörð Bjarmason 2019-05-27 23:43 ` brian m. carlson 2019-05-28 2:17 ` Ævar Arnfjörð Bjarmason 2019-05-28 1:22 ` Eric Wong 2019-05-28 0:05 ` Todd Zullinger 2019-05-28 1:31 ` Eric Wong 2019-05-28 1:43 ` Ævar Arnfjörð Bjarmason 2019-05-28 1:56 ` Todd Zullinger 2019-05-28 1:57 ` Eric Wong
Code repositories for project(s) associated with this public inbox https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).