{"thread":{"id":"51178","subject":"[PATCH] send-email: remove documented requirement for Net::SMTP::SSL","startedAt":"2019-05-26T17:22:46Z","lastAt":"2019-05-28T02:17:47Z","messageCount":11,"participants":["Chris Mayo","Eric Wong","Ævar Arnfjörð Bjarmason","brian m. carlson","Todd Zullinger"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"376229","messageId":"20190526172242.13000-1-aklhfex@gmail.com","threadId":"51178","inReplyTo":null,"subject":"[PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Chris Mayo","fromEmail":"aklhfex@gmail.com","sentAt":"2019-05-26T17:22:42Z","receivedAt":"2019-05-26T17:22:46Z","isPatch":true,"sender":{"key":"aklhfex@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921089?v=4"},"body":"git-send-email uses the TLS support in the Net::SMTP core module from\nrecent versions of Perl. Documenting the minimum version is complex\nbecause of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34)\nand libnet (3.01). Version numbers from commit:\nbfbfc9a953 (\"send-email: Net::SMTP::starttls was introduced in v2.34\",\n2017-05-31)\n\nUsers of older Perl versions without Net::SMTP::SSL installed will get a\nclear error message.\n\nSigned-off-by: Chris Mayo <aklhfex@gmail.com>\n---\n Documentation/git-send-email.txt | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt\nindex 1afe9fc858..a2acdaffa3 100644\n--- a/Documentation/git-send-email.txt\n+++ b/Documentation/git-send-email.txt\n@@ -500,8 +500,8 @@ app-specific or your regular password as appropriate.  If you have credential\n helper configured (see linkgit:git-credential[1]), the password will be saved in\n the credential store so you won't have to type it the next time.\n \n-Note: the following perl modules are required\n-      Net::SMTP::SSL, MIME::Base64 and Authen::SASL\n+Note: the following Perl modules are required\n+      MIME::Base64 and Authen::SASL\n \n SEE ALSO\n --------\n-- \n2.21.0\n\n"},{"id":"376244","messageId":"20190527193517.GA22013@dcvr","threadId":"51178","inReplyTo":"20190526172242.13000-1-aklhfex@gmail.com","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2019-05-27T19:35:17Z","receivedAt":"2019-05-27T19:35:18Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Chris Mayo <aklhfex@gmail.com> wrote:\n> git-send-email uses the TLS support in the Net::SMTP core module from\n> recent versions of Perl. Documenting the minimum version is complex\n> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34)\n> and libnet (3.01). Version numbers from commit:\n> bfbfc9a953 (\"send-email: Net::SMTP::starttls was introduced in v2.34\",\n> 2017-05-31)\n\nNo disagreement for removing the doc requirement for Net::SMTP::SSL.\n\nBut core modules can be split out by OS packagers.  For\nFedora/RH-based systems, the trend tends to be increasing\ngranularity and having more optional packages.\n\nSo I think documenting Net::SMTP (and Net::Domain) as\nrequirements would still be good, perhaps with a note stating\nthey're typically installed with Perl.\n\nFwiw, I recently ran into some issues where core modules such as\nDevel::Peek, Encode, and autodie were separate packages on CentOS 7.\n"},{"id":"376252","messageId":"87imtvmy7f.fsf@evledraar.gmail.com","threadId":"51178","inReplyTo":"20190527193517.GA22013@dcvr","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-27T20:36:36Z","receivedAt":"2019-05-27T20:36:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, May 27 2019, Eric Wong wrote:\n\n> Chris Mayo <aklhfex@gmail.com> wrote:\n>> git-send-email uses the TLS support in the Net::SMTP core module from\n>> recent versions of Perl. Documenting the minimum version is complex\n>> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34)\n>> and libnet (3.01). Version numbers from commit:\n>> bfbfc9a953 (\"send-email: Net::SMTP::starttls was introduced in v2.34\",\n>> 2017-05-31)\n>\n> No disagreement for removing the doc requirement for Net::SMTP::SSL.\n>\n> But core modules can be split out by OS packagers.  For\n> Fedora/RH-based systems, the trend tends to be increasing\n> granularity and having more optional packages.\n>\n> So I think documenting Net::SMTP (and Net::Domain) as\n> requirements would still be good, perhaps with a note stating\n> they're typically installed with Perl.\n>\n> Fwiw, I recently ran into some issues where core modules such as\n> Devel::Peek, Encode, and autodie were separate packages on CentOS 7.\n\nI've done enough git-send-email patching in anger for a year at least\nwith what's sitting in \"next\" so I'm not working on this, but just my\n0.02:\n\nI wonder if we shouldn't just be much more aggressive about version\nrequirements for something like git-send-email.\n\nDo we really have git users who want a new git *and* have an old perl\n*and* aren't just getting it from an OS package where the module is\ndual-life, so the distributor can just package up the newer version if\nwe were to require it?\n\nI.e. couldn't we just remove the fallback code added in 0ead000c3a\n(\"send-email: Net::SMTP::SSL is obsolete, use only when necessary\",\n2017-03-24) and do away with this version detection (which b.t.w. should\njust do a $obj->can(\"starttls\") check instead).\n\nFor shipping a newer Net::SMTP we aren't talking about upgrading\n/usr/bin/perl, just that module, and anyone who's packaging git\n(e.g. Debian) who cares about minimal dependencies is likely splitting\nout git-send-email.perl anyway.\n\nWe could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so\nwe'd error out by default unless these modules were there when git was\nbuilt, packagers could then still set some \"no I can't be bothered with\nsend-email at all\" or \"no I can't be bothered with its SSL support\", in\nthe latter case git-send-email would work except for the SSL parts.\n\nThat would take care of the communication about module dependencies via\nmanpage problem since we'd error by default. When I package things I\nmuch prefer that error mode to \"parts of package silently don't work\nbecause we check at runtime and I didn't religiously scour the\ndocs/release notes\".\n\nI wouldn't say the same thing about git-add--interactive.perl due to\nmore common its use is.\n"},{"id":"376257","messageId":"20190527234347.GH8616@genre.crustytoothpaste.net","threadId":"51178","inReplyTo":"87imtvmy7f.fsf@evledraar.gmail.com","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"brian m. carlson","fromEmail":"sandals@crustytoothpaste.net","sentAt":"2019-05-27T23:43:47Z","receivedAt":"2019-05-27T23:43:59Z","isPatch":true,"sender":{"key":"sandals@crustytoothpaste.net","avatar":"https://avatars.githubusercontent.com/u/497054?v=4"},"body":"On 2019-05-27 at 20:36:36, Ævar Arnfjörð Bjarmason wrote:\n> I've done enough git-send-email patching in anger for a year at least\n> with what's sitting in \"next\" so I'm not working on this, but just my\n> 0.02:\n> \n> I wonder if we shouldn't just be much more aggressive about version\n> requirements for something like git-send-email.\n> \n> Do we really have git users who want a new git *and* have an old perl\n> *and* aren't just getting it from an OS package where the module is\n> dual-life, so the distributor can just package up the newer version if\n> we were to require it?\n\nIn my experience, shipping newer versions of packages shipped with the\nOS is a no-no. That's a great way to break unrelated software on the\nsystem, and if you're the distributor, to get users angry at you about\nbreaking stuff on their systems.\n\nWe do indeed have users who want a newer Git on those systems and are\nusing the system Perl. The Git shipped with CentOS 7 (not to mention\nCentOS 6) is positively ancient and doesn't support useful features like\nworktrees, so it makes sense to upgrade it. But if you're not a Perl\nshop, nobody cares about the version of Perl on the system and fussing\nwith it doesn't make sense.\n\n> I.e. couldn't we just remove the fallback code added in 0ead000c3a\n> (\"send-email: Net::SMTP::SSL is obsolete, use only when necessary\",\n> 2017-03-24) and do away with this version detection (which b.t.w. should\n> just do a $obj->can(\"starttls\") check instead).\n> \n> For shipping a newer Net::SMTP we aren't talking about upgrading\n> /usr/bin/perl, just that module, and anyone who's packaging git\n> (e.g. Debian) who cares about minimal dependencies is likely splitting\n> out git-send-email.perl anyway.\n> \n> We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so\n> we'd error out by default unless these modules were there when git was\n> built, packagers could then still set some \"no I can't be bothered with\n> send-email at all\" or \"no I can't be bothered with its SSL support\", in\n> the latter case git-send-email would work except for the SSL parts.\n\nWe had a problem with Homebrew sometime back where they stopped shipping\ngit-send-email because it required Perl modules and there was a big\nuproar and a request for us to allow dynamic Perl support. I would like\nto discourage distributors from simply refusing to ship core pieces of\nGit because it tends to cause problems for users and for us down the\nline.\n\nI understand and am fine with splitting components out into multiple\npackages or omitting parts interfacing with other systems (e.g.\ngit-svn).\n-- \nbrian m. carlson: Houston, Texas, US\nOpenPGP: https://keybase.io/bk2204\n"},{"id":"376258","messageId":"20190528000544.GX3654@pobox.com","threadId":"51178","inReplyTo":"20190527193517.GA22013@dcvr","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-05-28T00:05:44Z","receivedAt":"2019-05-28T00:07:11Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Eric Wong wrote:\n> Chris Mayo <aklhfex@gmail.com> wrote:\n>> git-send-email uses the TLS support in the Net::SMTP core module from\n>> recent versions of Perl. Documenting the minimum version is complex\n>> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34)\n>> and libnet (3.01). Version numbers from commit:\n>> bfbfc9a953 (\"send-email: Net::SMTP::starttls was introduced in v2.34\",\n>> 2017-05-31)\n> \n> No disagreement for removing the doc requirement for Net::SMTP::SSL.\n> \n> But core modules can be split out by OS packagers.  For\n> Fedora/RH-based systems, the trend tends to be increasing\n> granularity and having more optional packages.\n> \n> So I think documenting Net::SMTP (and Net::Domain) as\n> requirements would still be good, perhaps with a note stating\n> they're typically installed with Perl.\n\nI didn't know that git-send-email.perl could take advantage\nof Net::SMTP::starttls until I read this.\n\n[Adding Dennis and Jonathan as the authors of 0ead000c3a\n(\"send-email: Net::SMTP::SSL is obsolete, use only when\nnecessary\", 2017-03-24) bfbfc9a953 (\"send-email:\nNet::SMTP::starttls was introduced in v2.34\", 2017-05-31),\nrespectively.]\n\nThe current Fedora and Red Hat package have a requirement on\nNet::SMTP::SSL from long, long ago¹.  As I looked at whether\nI could remove that (or more accurately, replace it with\nIO::Socket::SSL which is needed for Net::SMTP to handle\nstarttls), I noticed that on RHEL7 the Net::SMTP version was\n2.31, but starttls support has been backported².\n\nI wonder if it's (separately from this change) worth\nadjusting the conditional which sets $use_net_smtp_ssl to\nuse \"Net::SMTP->can('starttls')\" rather than a strict\nversion check?  (It might not be if using 'can' is too\nfragile or would only benefit the Red Hat 7 packages which\nlikely won't officially be updated to a newer git with such\na change.)\n\nSomething like:\n\ndiff --git i/git-send-email.perl w/git-send-email.perl\nindex 24859a7bc3..84ac03994d 100755\n--- i/git-send-email.perl\n+++ w/git-send-email.perl\n@@ -1465,7 +1465,7 @@ sub send_message {\n \t\t}\n \n \t\trequire Net::SMTP;\n-\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n+\t\tmy $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1;\n \t\t$smtp_domain ||= maildomain();\n \n \t\tif ($smtp_encryption eq 'ssl') {\n\n¹ https://bugzilla.redhat.com/443615\n² https://bugzilla.redhat.com/1557574\n  https://git.centos.org/rpms/perl/c/13dfe3?branch=c7\n\n-- \nTodd\n"},{"id":"376260","messageId":"20190528012214.p7t4kfthxia4y464@dcvr","threadId":"51178","inReplyTo":"87imtvmy7f.fsf@evledraar.gmail.com","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2019-05-28T01:22:14Z","receivedAt":"2019-05-28T01:22:16Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> On Mon, May 27 2019, Eric Wong wrote:\n> > Chris Mayo <aklhfex@gmail.com> wrote:\n> >> git-send-email uses the TLS support in the Net::SMTP core module from\n> >> recent versions of Perl. Documenting the minimum version is complex\n> >> because of separate numbering for Perl (5.21.5~169), Net:SMTP (2.34)\n> >> and libnet (3.01). Version numbers from commit:\n> >> bfbfc9a953 (\"send-email: Net::SMTP::starttls was introduced in v2.34\",\n> >> 2017-05-31)\n> >\n> > No disagreement for removing the doc requirement for Net::SMTP::SSL.\n> >\n> > But core modules can be split out by OS packagers.  For\n> > Fedora/RH-based systems, the trend tends to be increasing\n> > granularity and having more optional packages.\n> >\n> > So I think documenting Net::SMTP (and Net::Domain) as\n> > requirements would still be good, perhaps with a note stating\n> > they're typically installed with Perl.\n> >\n> > Fwiw, I recently ran into some issues where core modules such as\n> > Devel::Peek, Encode, and autodie were separate packages on CentOS 7.\n> \n> I've done enough git-send-email patching in anger for a year at least\n> with what's sitting in \"next\" so I'm not working on this, but just my\n> 0.02:\n> \n> I wonder if we shouldn't just be much more aggressive about version\n> requirements for something like git-send-email.\n> \n> Do we really have git users who want a new git *and* have an old perl\n> *and* aren't just getting it from an OS package where the module is\n> dual-life, so the distributor can just package up the newer version if\n> we were to require it?\n\nI started writing this earlier, but dropped my connection.\nAnd brian said the same thing I was going to say.\n\nIf OS packagers were to start making Net::SMTP/Net::Domain\noptional (which I sorta expect...), we should make them\noptional, too (and add descriptive error messages and manpage\nupdates).\n\nI have no need for Net::SMTP with sendemail.smtpserver=/usr/bin/msmtp\n\n> I.e. couldn't we just remove the fallback code added in 0ead000c3a\n> (\"send-email: Net::SMTP::SSL is obsolete, use only when necessary\",\n> 2017-03-24) and do away with this version detection (which b.t.w. should\n> just do a $obj->can(\"starttls\") check instead).\n\nToo soon for the fallback code removal; maybe when CentOS 6 is\nEOL.... However, the \"->can\" check is nicer, yes.\n\n> For shipping a newer Net::SMTP we aren't talking about upgrading\n> /usr/bin/perl, just that module, and anyone who's packaging git\n> (e.g. Debian) who cares about minimal dependencies is likely splitting\n> out git-send-email.perl anyway.\n> \n> We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so\n> we'd error out by default unless these modules were there when git was\n> built, packagers could then still set some \"no I can't be bothered with\n> send-email at all\" or \"no I can't be bothered with its SSL support\", in\n> the latter case git-send-email would work except for the SSL parts.\n> \n> That would take care of the communication about module dependencies via\n> manpage problem since we'd error by default. When I package things I\n> much prefer that error mode to \"parts of package silently don't work\n> because we check at runtime and I didn't religiously scour the\n> docs/release notes\".\n\nI prefer we check options passed to the program and decide\nwhich dependencies to require based on that.  The manpage and\n--help could be listing the modules required for certain option\ngroups, even.\n\n> I wouldn't say the same thing about git-add--interactive.perl due to\n> more common its use is.\n\nI wish to see git-send-email (and vendor-neutral messaging in\ngeneral) gain more traction.  IMHO, send-email should reach the\ncommonality of other parts of git.\n\n\n\n(*) if/when I find the time, I'd make \"git svn find-rev\"\n    work w/o SVN::Perl, too.\n"},{"id":"376261","messageId":"20190528013128.ynvvci4ul2exiyef@dcvr","threadId":"51178","inReplyTo":"20190528000544.GX3654@pobox.com","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2019-05-28T01:31:28Z","receivedAt":"2019-05-28T01:31:29Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Todd Zullinger <tmz@pobox.com> wrote:\n> I wonder if it's (separately from this change) worth\n> adjusting the conditional which sets $use_net_smtp_ssl to\n> use \"Net::SMTP->can('starttls')\" rather than a strict\n> version check?  (It might not be if using 'can' is too\n> fragile or would only benefit the Red Hat 7 packages which\n> likely won't officially be updated to a newer git with such\n> a change.)\n> \n> Something like:\n> \n> diff --git i/git-send-email.perl w/git-send-email.perl\n> index 24859a7bc3..84ac03994d 100755\n> --- i/git-send-email.perl\n> +++ w/git-send-email.perl\n> @@ -1465,7 +1465,7 @@ sub send_message {\n>  \t\t}\n>  \n>  \t\trequire Net::SMTP;\n> -\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n> +\t\tmy $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1;\n>  \t\t$smtp_domain ||= maildomain();\n>  \n>  \t\tif ($smtp_encryption eq 'ssl') {\n\nLooks much better to me.\n"},{"id":"376262","messageId":"87h89fmjzc.fsf@evledraar.gmail.com","threadId":"51178","inReplyTo":"20190528013128.ynvvci4ul2exiyef@dcvr","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-28T01:43:51Z","receivedAt":"2019-05-28T01:43:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, May 28 2019, Eric Wong wrote:\n\n> Todd Zullinger <tmz@pobox.com> wrote:\n>> I wonder if it's (separately from this change) worth\n>> adjusting the conditional which sets $use_net_smtp_ssl to\n>> use \"Net::SMTP->can('starttls')\" rather than a strict\n>> version check?  (It might not be if using 'can' is too\n>> fragile or would only benefit the Red Hat 7 packages which\n>> likely won't officially be updated to a newer git with such\n>> a change.)\n>>\n>> Something like:\n>>\n>> diff --git i/git-send-email.perl w/git-send-email.perl\n>> index 24859a7bc3..84ac03994d 100755\n>> --- i/git-send-email.perl\n>> +++ w/git-send-email.perl\n>> @@ -1465,7 +1465,7 @@ sub send_message {\n>>  \t\t}\n>>\n>>  \t\trequire Net::SMTP;\n>> -\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n>> +\t\tmy $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1;\n>>  \t\t$smtp_domain ||= maildomain();\n>>\n>>  \t\tif ($smtp_encryption eq 'ssl') {\n>\n> Looks much better to me.\n\nSame, but to bikeshed a bit, at this point we can just do:\n    \n    diff --git a/git-send-email.perl b/git-send-email.perl\n    index 24859a7bc3..4ad2091a49 100755\n    --- a/git-send-email.perl\n    +++ b/git-send-email.perl\n    @@ -1468 +1467,0 @@ sub send_message {\n    -               my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n    @@ -1485 +1484 @@ sub send_message {\n    -                       if ($use_net_smtp_ssl) {\n    +                       if (Net::SMTP->can('starttls')) {\n    @@ -1507 +1506 @@ sub send_message {\n    -                               if ($use_net_smtp_ssl) {\n    +                               if (Net::SMTP->can('starttls')) {\n"},{"id":"376264","messageId":"20190528015621.GZ3654@pobox.com","threadId":"51178","inReplyTo":"87h89fmjzc.fsf@evledraar.gmail.com","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Todd Zullinger","fromEmail":"tmz@pobox.com","sentAt":"2019-05-28T01:56:22Z","receivedAt":"2019-05-28T01:56:26Z","isPatch":true,"sender":{"key":"tmz@pobox.com","avatar":"https://avatars.githubusercontent.com/u/806319?v=4"},"body":"Hi,\n\nSorry I missed your earlier reply which also mentioned using\n$obj->can() Ævar.  That's what I get for typing a reply and\nthen walking away for a few hours before hitting send. ;)\n\nÆvar Arnfjörð Bjarmason wrote:\n> Same, but to bikeshed a bit, at this point we can just do:\n>     \n>     diff --git a/git-send-email.perl b/git-send-email.perl\n>     index 24859a7bc3..4ad2091a49 100755\n>     --- a/git-send-email.perl\n>     +++ b/git-send-email.perl\n>     @@ -1468 +1467,0 @@ sub send_message {\n>     -               my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n>     @@ -1485 +1484 @@ sub send_message {\n>     -                       if ($use_net_smtp_ssl) {\n>     +                       if (Net::SMTP->can('starttls')) {\n>     @@ -1507 +1506 @@ sub send_message {\n>     -                               if ($use_net_smtp_ssl) {\n>     +                               if (Net::SMTP->can('starttls')) {\n> \n\nI think we'd need to use 'if ! ...' there, or more likely,\nswitch the blocks which follow because the code following\n'if ($use_net_smtp_ssl)' is for Net::SMTP::SSL with the\n'else' block handling the case where Net::SMTP has ssl/tls\nsupport.  Right?\n\nI know I read the $use_net_smtp_ssl bit backwards the first\ntime or two as well.\n\n-- \nTodd\n"},{"id":"376265","messageId":"20190528015759.GA13521@dcvr","threadId":"51178","inReplyTo":"87h89fmjzc.fsf@evledraar.gmail.com","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Eric Wong","fromEmail":"e@80x24.org","sentAt":"2019-05-28T01:57:59Z","receivedAt":"2019-05-28T01:58:01Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:\n> On Tue, May 28 2019, Eric Wong wrote:\n> > Todd Zullinger <tmz@pobox.com> wrote:\n> >> Something like:\n> >>\n> >> diff --git i/git-send-email.perl w/git-send-email.perl\n> >> index 24859a7bc3..84ac03994d 100755\n> >> --- i/git-send-email.perl\n> >> +++ w/git-send-email.perl\n> >> @@ -1465,7 +1465,7 @@ sub send_message {\n> >>  \t\t}\n> >>\n> >>  \t\trequire Net::SMTP;\n> >> -\t\tmy $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n> >> +\t\tmy $use_net_smtp_ssl = Net::SMTP->can('starttls') ? 0 : 1;\n> >>  \t\t$smtp_domain ||= maildomain();\n> >>\n> >>  \t\tif ($smtp_encryption eq 'ssl') {\n> >\n> > Looks much better to me.\n> \n> Same, but to bikeshed a bit, at this point we can just do:\n>     \n>     diff --git a/git-send-email.perl b/git-send-email.perl\n>     index 24859a7bc3..4ad2091a49 100755\n>     --- a/git-send-email.perl\n>     +++ b/git-send-email.perl\n>     @@ -1468 +1467,0 @@ sub send_message {\n>     -               my $use_net_smtp_ssl = version->parse($Net::SMTP::VERSION) < version->parse(\"2.34\");\n>     @@ -1485 +1484 @@ sub send_message {\n>     -                       if ($use_net_smtp_ssl) {\n>     +                       if (Net::SMTP->can('starttls')) {\n>     @@ -1507 +1506 @@ sub send_message {\n>     -                               if ($use_net_smtp_ssl) {\n>     +                               if (Net::SMTP->can('starttls')) {\n\nHigher probability of a typo slipping through the cracks, though :)\n\n(I was just bit by a similar case with hashes that would've been\n caught by \"use fields\")\n"},{"id":"376266","messageId":"87ftozmiex.fsf@evledraar.gmail.com","threadId":"51178","inReplyTo":"20190527234347.GH8616@genre.crustytoothpaste.net","subject":"Re: [PATCH] send-email: remove documented requirement for Net::SMTP::SSL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2019-05-28T02:17:42Z","receivedAt":"2019-05-28T02:17:47Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, May 28 2019, brian m. carlson wrote:\n\n> On 2019-05-27 at 20:36:36, Ævar Arnfjörð Bjarmason wrote:\n>> I've done enough git-send-email patching in anger for a year at least\n>> with what's sitting in \"next\" so I'm not working on this, but just my\n>> 0.02:\n>>\n>> I wonder if we shouldn't just be much more aggressive about version\n>> requirements for something like git-send-email.\n>>\n>> Do we really have git users who want a new git *and* have an old perl\n>> *and* aren't just getting it from an OS package where the module is\n>> dual-life, so the distributor can just package up the newer version if\n>> we were to require it?\n>\n> In my experience, shipping newer versions of packages shipped with the\n> OS is a no-no. That's a great way to break unrelated software on the\n> system, and if you're the distributor, to get users angry at you about\n> breaking stuff on their systems.\n\nJust because a package X needs dependency Y at version Z doesn't mean\nyou need to package it up in such a way that everything that needs any\nversion of Y must use it at version Z, you put the two versions in\ndifferent places on the FS.\n\nSo in this case the packager would grumble a bit and package up a\nNet::SMTP installed at another path. I do that myself to get some more\nmodern versions of dependencies on CentOS 6 without breaking the base\nsystem.\n\nDoes that mean we should do it in this case? No, and given the feedback\nin this thread about this in particular I think we should just keep the\nold code.\n\nBut I think is important to clarify in general, i.e. just because you're\non CentOS 6 and don't want to update the main perl-Net-SMTP you can\nstill package up a for-git-perl-Net-SMTP or whatever.\n\nWe had a similar discussion about whether to depend on a more recent\nlibcurl in the past:\n\nhttps://public-inbox.org/git/CACBZZX78oKU5HuBEqb9qLy7--wcwhC_mW6x7Q+tB4suxohSCsQ@mail.gmail.com/\n\n> We do indeed have users who want a newer Git on those systems and are\n> using the system Perl. The Git shipped with CentOS 7 (not to mention\n> CentOS 6) is positively ancient and doesn't support useful features like\n> worktrees, so it makes sense to upgrade it. But if you're not a Perl\n> shop, nobody cares about the version of Perl on the system and fussing\n> with it doesn't make sense.\n\nWhat I was suggesting was whether it was worth it to ask distributors to\ndrop in a few *.pm files, and perhaps a *.so or two compiled against\nopenssl, they wouldn't need to change the base /usr/bin/perl.\n\nIn the case of the *.pm stuff we could drop fallbacks into\nperl/Git/LoadCPAN if we wanted things to work out of the box.\n\n>> I.e. couldn't we just remove the fallback code added in 0ead000c3a\n>> (\"send-email: Net::SMTP::SSL is obsolete, use only when necessary\",\n>> 2017-03-24) and do away with this version detection (which b.t.w. should\n>> just do a $obj->can(\"starttls\") check instead).\n>>\n>> For shipping a newer Net::SMTP we aren't talking about upgrading\n>> /usr/bin/perl, just that module, and anyone who's packaging git\n>> (e.g. Debian) who cares about minimal dependencies is likely splitting\n>> out git-send-email.perl anyway.\n>>\n>> We could then just add some flag similar to NO_PERL_CPAN_FALLBACKS so\n>> we'd error out by default unless these modules were there when git was\n>> built, packagers could then still set some \"no I can't be bothered with\n>> send-email at all\" or \"no I can't be bothered with its SSL support\", in\n>> the latter case git-send-email would work except for the SSL parts.\n>\n> We had a problem with Homebrew sometime back where they stopped shipping\n> git-send-email because it required Perl modules and there was a big\n> uproar and a request for us to allow dynamic Perl support. I would like\n> to discourage distributors from simply refusing to ship core pieces of\n> Git because it tends to cause problems for users and for us down the\n> line.\n\nI'm sympathetic to packagers that do that, particularly when \"add -i\"\nand friends in s/Perl/C/ land, at which point we'd be asking some\ndistributors who otherwise wouldn't have perl at all to install it just\nfor us.\n\nIt seems packagers mainly had issues with the Perl-SSL stuff. We already\nsupport SSL for https via libcurl usually, perhaps we'd be better off\nhaving some wrapper using that\n(https://curl.haxx.se/libcurl/c/smtp-tls.html), or having\ngit-send-email.perl support something simpler like say stunnel.\n\n> I understand and am fine with splitting components out into multiple\n> packages or omitting parts interfacing with other systems (e.g.\n> git-svn).\n\nI tend to think of SVN as a system more closely related to Git than Git\nis related to E-Mail :)\n\nIt's also something that's by definition part of an \"egress\" workflow,\nso it tends to be much easier to work around it being missing than say\n\"add -i\" or something you truly need locally not being there.\n"}]}