Open p5pRT opened 6 years ago
p5pRT
commented
6 years ago Module::Install fails one of its tests since core commit 0301e899536a22752f40481d8a1d141b7a7dda82 "properly define perl_parse() return value". I'm surprised that anything noticed such small code changes.
p5pRT
commented
6 years ago I wrote:
Module::Install fails one of its tests since core commit 0301e899536a22752f40481d8a1d141b7a7dda82 "properly define perl_parse() return value".
Turns out that Module::Install::DSL has some rather convoluted code in which an "import" method that runs from a Makefile.PL establishes an INIT block that will do the real work of Makefile.PL and then performs exit(0). The behaviour difference that's relevant boils down to:
$ perl5.27.6 -lwe 'print "main program"; INIT { print "init block"; } BEGIN { print "begin block"; exit 0; }' begin block init block $ perl-blead -lwe 'print "main program"; INIT { print "init block"; } BEGIN { print "begin block"; exit 0; }' begin block $
That is\, after a BEGIN block performs an exit(0)\, perl used to run INIT blocks and then exit\, but now it exits immediately without running INIT blocks. It always used to exit without running INIT blocks upon an exit with non-zero exit code\, or upon an exception.
This situation is not precisely the exit(0) from a CHECK block that was called out in [perl #2754]\, and indeed on that ticket it was thought that exit(0) from BEGIN blocks was operating correctly. Nevertheless\, for it to execute BEGIN blocks is clearly another form of the same bug with which that ticket is concerned\, and the commit has fixed it.
Module::Install::DSL does have a genuine\, if not entirely respectable\, reason for its exit(0). The essence of this module is that a Makefile.PL invokes the module and then stops containing Perl code\, instead switching to a DSL to specify attributes of its distro. The module opens up the Makefile.PL as a file to read it in and parse this DSL\, and uses exit(0) to prevent Perl's parsing of the top-level Makefile.PL from proceeding into the DSL code that would constitute syntax errors.
However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works. With that alteration to the module\, the Module-Install distro passes its test suite.
-zefram
p5pRT
commented
6 years ago I have opened [rt.cpan.org #123867 (https://rt.cpan.org/Public/Bug/Display.html?id=123867) ] on the Module-Install side.
-zefram
p5pRT
commented
6 years ago On Tue\, Dec 12\, 2017 at 8:13 PM\, Zefram \zefram@​fysh\.org wrote:
However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works. With that alteration to the module\, the Module-Install distro passes its test suite.
Given Module::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module::Install.
Leon
p5pRT
commented
6 years ago The RT System itself - Status changed from 'new' to 'open'
p5pRT
commented
6 years ago On Tue\, Dec 12\, 2017 at 11:03 PM\, Leon Timmermans \fawaka@​gmail\.com wrote:
On Tue\, Dec 12\, 2017 at 8:13 PM\, Zefram \zefram@​fysh\.org wrote:
However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works. With that alteration to the module\, the Module-Install distro passes its test suite.
Given Module::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module::Install.
Well\, all 119 modules using Module::Install::DSL\, Module::Install in general has quite a few more users.
Leon
p5pRT
commented
6 years ago On 12/13/2017 12:04 AM\, Leon Timmermans wrote:
On Tue\, Dec 12\, 2017 at 11:03 PM\, Leon Timmermans \<fawaka@gmail.com \mailto​:fawaka@​gmail\.com> wrote:
On Tue\, Dec 12\, 2017 at 8​:13 PM\, Zefram \<zefram@​fysh\.org \<mailto​:zefram@​fysh\.org>> wrote​: However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block\. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works\. With that alteration to the module\, the Module\-Install distro passes its test suite\. Given Module​::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module​::Install\.Well\, all 119 modules using Module::Install::DSL\, Module::Install in general has quite a few more users.
That is indeed a pain.
What is the cost of reverting this commit instead?
p5pRT
commented
6 years ago On Sun\, 17 Dec 2017 03:06:01 -0800\, xsawyerx@gmail.com wrote:
On 12/13/2017 12:04 AM\, Leon Timmermans wrote:
On Tue\, Dec 12\, 2017 at 11:03 PM\, Leon Timmermans \<fawaka@gmail.com \mailto​:fawaka@​gmail\.com> wrote:
On Tue\, Dec 12\, 2017 at 8​:13 PM\, Zefram \<zefram@​fysh\.org \<mailto​:zefram@​fysh\.org>> wrote​: However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block\. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works\. With that alteration to the module\, the Module\-Install distro passes its test suite\. Given Module​::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module​::Install\.Well\, all 119 modules using Module::Install::DSL\, Module::Install in general has quite a few more users.
That is indeed a pain.
I seem to remember an old blog or list post by Michael Schwern predicting this very problem with Module::Install.
What is the cost of reverting this commit instead?
Buggy\, unpredictable behaviour\, unless you can memorise the list of complex rules for when exit does and does not prevent other blocks from running.
--
Father Chrysostomos
p5pRT
commented
6 years ago On 12/17/2017 09:17 PM\, Father Chrysostomos via RT wrote:
On Sun\, 17 Dec 2017 03:06:01 -0800\, xsawyerx@gmail.com wrote:
On 12/13/2017 12:04 AM\, Leon Timmermans wrote:
On Tue\, Dec 12\, 2017 at 11:03 PM\, Leon Timmermans \<fawaka@gmail.com \mailto​:fawaka@​gmail\.com> wrote:
On Tue\, Dec 12\, 2017 at 8​:13 PM\, Zefram \<zefram@​fysh\.org \<mailto​:zefram@​fysh\.org>> wrote​: However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block\. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works\. With that alteration to the module\, the Module\-Install distro passes its test suite\. Given Module​::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module​::Install\.Well\, all 119 modules using Module::Install::DSL\, Module::Install in general has quite a few more users. That is indeed a pain. I seem to remember an old blog or list post by Michael Schwern predicting this very problem with Module::Install.
What is the cost of reverting this commit instead? Buggy\, unpredictable behaviour\, unless you can memorise the list of complex rules for when exit does and does not prevent other blocks from running.
Are you referring to long-term or immediate effect?
I'm wondering about temporary revert.
p5pRT
commented
6 years ago On Mon\, 18 Dec 2017 07:39:32 -0800\, xsawyerx@gmail.com wrote:
On 12/17/2017 09:17 PM\, Father Chrysostomos via RT wrote:
On Sun\, 17 Dec 2017 03:06:01 -0800\, xsawyerx@gmail.com wrote:
On 12/13/2017 12:04 AM\, Leon Timmermans wrote:
On Tue\, Dec 12\, 2017 at 11:03 PM\, Leon Timmermans \<fawaka@gmail.com \mailto​:fawaka@​gmail\.com> wrote:
On Tue\, Dec 12\, 2017 at 8:13 PM\, Zefram \<zefram@fysh.org \mailto​:zefram@​fysh\.org> wrote:
However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works. With that alteration to the module\, the Module-Install distro passes its test suite.
Given Module::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module::Install.
Well\, all 119 modules using Module::Install::DSL\, Module::Install in general has quite a few more users. That is indeed a pain. I seem to remember an old blog or list post by Michael Schwern predicting this very problem with Module::Install.
What is the cost of reverting this commit instead? Buggy\, unpredictable behaviour\, unless you can memorise the list of complex rules for when exit does and does not prevent other blocks from running.
Are you referring to long-term or immediate effect?
I’m referring to the bugs that Zefram fixed\, which went even deeper than Zefram realized when he was fixing them. So at present nobody but he can predict which blocks will trigger at what time if ‘exit’ is used in them.
I'm wondering about temporary revert.
That might be a good approach: revert and then reinstate after 5.28. In the mean time\, a bunch of modules need to be released.
--
Father Chrysostomos
p5pRT
commented
6 years ago On 12/18/2017 07:39 PM\, Father Chrysostomos via RT wrote:
On Mon\, 18 Dec 2017 07:39:32 -0800\, xsawyerx@gmail.com wrote:
On 12/17/2017 09:17 PM\, Father Chrysostomos via RT wrote:
On Sun\, 17 Dec 2017 03:06:01 -0800\, xsawyerx@gmail.com wrote:
On 12/13/2017 12:04 AM\, Leon Timmermans wrote:
On Tue\, Dec 12\, 2017 at 11:03 PM\, Leon Timmermans \<fawaka@gmail.com \mailto​:fawaka@​gmail\.com> wrote:
On Tue\, Dec 12\, 2017 at 8:13 PM\, Zefram \<zefram@fysh.org \mailto​:zefram@​fysh\.org> wrote:
However\, as far as I can see there's no need at all for the module to delay execution of the code it generates by putting it in an INIT block. If that code is instead executed immediately\, by deleting the word "INIT" to turn it into a bare block\, everything works. With that alteration to the module\, the Module-Install distro passes its test suite.
Given Module::Install's rather unfortunate bundling nature\, that would require rereleasing all 119 distributions using it to be rereleased with such a new Module::Install.
Well\, all 119 modules using Module::Install::DSL\, Module::Install in general has quite a few more users. That is indeed a pain. I seem to remember an old blog or list post by Michael Schwern predicting this very problem with Module::Install.
What is the cost of reverting this commit instead? Buggy\, unpredictable behaviour\, unless you can memorise the list of complex rules for when exit does and does not prevent other blocks from running. Are you referring to long-term or immediate effect? I’m referring to the bugs that Zefram fixed\, which went even deeper than Zefram realized when he was fixing them. So at present nobody but he can predict which blocks will trigger at what time if ‘exit’ is used in them.
I'm wondering about temporary revert. That might be a good approach: revert and then reinstate after 5.28. In the mean time\, a bunch of modules need to be released.
Exactly. A better phrasing for my question is "How problematic would it be to postpone this fix to after 5.28 so we could fix the modules and release?"
Zefram\, can you please weigh in here?
p5pRT
commented
6 years ago Sawyer X wrote:
Exactly. A better phrasing for my question is "How problematic would it be to postpone this fix to after 5.28 so we could fix the modules and release?"
The impact of retaining the bug for another year should be low. It's a long-standing bug\, with fairly obscure triggering conditions. There's little internally that depends on the bug being fixed; only the documentation would have to change to match. If there's a significant win to be made in CPAN readiness for the fix then it would be a reasonable course of action.
-zefram
p5pRT
commented
6 years ago On Thu\, 21 Dec 2017 23:03:05 -0800\, zefram@fysh.org wrote:
If there's a significant win to be made in CPAN readiness for the fix...
Is kicking the can down the road a significant win?
p5pRT
commented
6 years ago Also affected (discovered by Slaven):
MANWAR/Test-Strict-0.40.tar.gz
-- andreas
p5pRT
commented
6 years ago On 12/22/2017 09:02 AM\, Zefram wrote:
Sawyer X wrote:
Exactly. A better phrasing for my question is "How problematic would it be to postpone this fix to after 5.28 so we could fix the modules and release?" [...] If there's a significant win to be made in CPAN readiness for the fix then it would be a reasonable course of action.
I'm not sure there is such readiness. We are fairly behind.
Considering this fix is for such a long-standing low-priority bug\, perhaps we could provide temporary warning for now?
p5pRT
commented
6 years ago Sawyer X wrote:
perhaps we could provide temporary warning for now?
You mean... a deprecation warning?
Technically\, if we revert perl_parse() to return zero for exit(0)\, it is feasible for this to detect that perl_run() would do something if called\, and thus to warn that this bogus zero return matters. Thus using an early exit(0) with the intent that INIT blocks or the main program subsequently run could be deprecated. It's not feasible to do the same for perl_run() bogusly returning zero\, or for perl_parse() bogusly returning zero when the subsequent use the caller makes of the interpreter would be something other than just calling perl_run(). But we haven't seen any breakage of those kinds (yet). For the purposes of pure Perl code such as Module::Install\, as opposed to arbitrary C level embeddings\, the feasible deprecation is sufficient.
-zefram
p5pRT
commented
6 years ago On 12/25/2017 10:29 AM\, Zefram wrote:
Sawyer X wrote:
perhaps we could provide temporary warning for now? You mean... a deprecation warning?
Yes.
Technically\, if we revert perl_parse() to return zero for exit(0)\, it is feasible for this to detect that perl_run() would do something if called\, and thus to warn that this bogus zero return matters. Thus using an early exit(0) with the intent that INIT blocks or the main program subsequently run could be deprecated. It's not feasible to do the same for perl_run() bogusly returning zero\, or for perl_parse() bogusly returning zero when the subsequent use the caller makes of the interpreter would be something other than just calling perl_run(). But we haven't seen any breakage of those kinds (yet). For the purposes of pure Perl code such as Module::Install\, as opposed to arbitrary C level embeddings\, the feasible deprecation is sufficient.
Wherever we can warn here instead of die\, it is better. Where we cannot\, we already have a large enough list of breakages to churn through. We could try again later when the list has been reduced.
p5pRT
commented
6 years ago Following discussion with Sawyer\, where he determined that there is a need for more time to fix Module::Install distros\, I have reintroduced the perl_parse() exit(0) bug in commit 857320cbf85e762add18885ae8a197b5e0c21b69.
I tried adding a deprecation warning\, but this turned out to cause noise. There's an INIT block buried deep inside Test::More\, so the warning would fire for any test script that loaded Test::More and then did a skip_all in a BEGIN block\, which is quite a common pattern. The logic around this INIT block doesn't really care whether it runs in this case\, so there's nothing to fix. Rather than get these false warnings\, Sawyer decided it was better to have no deprecation warning.
We expect to fix the bug again during the 5.29 cycle. This would consist of reverting most of commit 857320cbf85e762add18885ae8a197b5e0c21b69: the documentation and code changes in perl.c\, and the test changes in t/op/blocks.t. This issue should block 5.30.0.
-zefram
p5pRT
commented
6 years ago On Wed\, Dec 27\, 2017 at 09:23:52PM +0000\, Zefram wrote:
Following discussion with Sawyer\, where he determined that there is a need for more time to fix Module::Install distros\, I have reintroduced the perl_parse() exit(0) bug in commit 857320cbf85e762add18885ae8a197b5e0c21b69.
I tried adding a deprecation warning\, but this turned out to cause noise. There's an INIT block buried deep inside Test::More\, so the warning would fire for any test script that loaded Test::More and then did a skip_all in a BEGIN block\, which is quite a common pattern. The logic around this INIT block doesn't really care whether it runs in this case\, so there's nothing to fix. Rather than get these false warnings\, Sawyer decided it was better to have no deprecation warning.
We expect to fix the bug again during the 5.29 cycle. This would consist of reverting most of commit 857320cbf85e762add18885ae8a197b5e0c21b69: the documentation and code changes in perl.c\, and the test changes in t/op/blocks.t. This issue should block 5.30.0.
Since this change has been reverted for 5.28\, I presume I can remove this ticket from the 5.28 blockers list.
-- If life gives you lemons\, you'll probably develop a citric acid allergy.
p5pRT
commented
6 years ago On 04/20/2018 12:36 AM\, Dave Mitchell wrote:
On Wed\, Dec 27\, 2017 at 09:23:52PM +0000\, Zefram wrote:
Following discussion with Sawyer\, where he determined that there is a need for more time to fix Module::Install distros\, I have reintroduced the perl_parse() exit(0) bug in commit 857320cbf85e762add18885ae8a197b5e0c21b69.
I tried adding a deprecation warning\, but this turned out to cause noise. There's an INIT block buried deep inside Test::More\, so the warning would fire for any test script that loaded Test::More and then did a skip_all in a BEGIN block\, which is quite a common pattern. The logic around this INIT block doesn't really care whether it runs in this case\, so there's nothing to fix. Rather than get these false warnings\, Sawyer decided it was better to have no deprecation warning.
We expect to fix the bug again during the 5.29 cycle. This would consist of reverting most of commit 857320cbf85e762add18885ae8a197b5e0c21b69: the documentation and code changes in perl.c\, and the test changes in t/op/blocks.t. This issue should block 5.30.0. Since this change has been reverted for 5.28\, I presume I can remove this ticket from the 5.28 blockers list.
Yup.
p5pRT
commented
5 years ago On Fri\, 20 Apr 2018 03:54:26 -0700\, xsawyerx@gmail.com wrote:
On 04/20/2018 12:36 AM\, Dave Mitchell wrote:
On Wed\, Dec 27\, 2017 at 09:23:52PM +0000\, Zefram wrote:
Following discussion with Sawyer\, where he determined that there is a need for more time to fix Module::Install distros\, I have reintroduced the perl_parse() exit(0) bug in commit 857320cbf85e762add18885ae8a197b5e0c21b69.
I tried adding a deprecation warning\, but this turned out to cause noise. There's an INIT block buried deep inside Test::More\, so the warning would fire for any test script that loaded Test::More and then did a skip_all in a BEGIN block\, which is quite a common pattern. The logic around this INIT block doesn't really care whether it runs in this case\, so there's nothing to fix. Rather than get these false warnings\, Sawyer decided it was better to have no deprecation warning.
We expect to fix the bug again during the 5.29 cycle. This would consist of reverting most of commit 857320cbf85e762add18885ae8a197b5e0c21b69: the documentation and code changes in perl.c\, and the test changes in t/op/blocks.t. This issue should block 5.30.0. Since this change has been reverted for 5.28\, I presume I can remove this ticket from the 5.28 blockers list.
Yup.
Pinging this ticket
-- Karl Williamson
p5pRT
commented
5 years ago On Thu\, 28 Mar 2019 19:33:04 -0700\, khw wrote:
Pinging this ticket
I've been re-releasing all the modules I am aware of (that I have the ability to) that use Module::Install::DSL; I think the upper portions of the cpan river are now clean. Are we able to assess the remaining impact to the river\, and perhaps establish the final list of distributions that are still of concern?
p5pRT
commented
5 years ago On Sat\, 30 Mar 2019 03:54:10 GMT\, ether wrote:
On Thu\, 28 Mar 2019 19:33:04 -0700\, khw wrote:
Pinging this ticket
I've been re-releasing all the modules I am aware of (that I have the ability to) that use Module::Install::DSL; I think the upper portions of the cpan river are now clean. Are we able to assess the remaining impact to the river\, and perhaps establish the final list of distributions that are still of concern?
For what it's worth ...
I went to this (possibly inaccurate) list of CPAN distributions which are dependent upon Module::Install:
http://deps.cpantesters.org/depended-on-by.pl?dist=Module-Install
I extracted the 127 top-level distros\, then excluded all distros beginning with Acme\, Bundle or Task. That left 114 distros\, which I attempted to install against Perl 5 blead (commit 930ded654\, Apr 2 2019) on FreeBSD-11.2 (threaded) using cpanm as the installer. (I.e.\, the same approach I take toward monthly testing of the "CPAN-River-3000".)
Certain modules failed to install for reasons that were expected given that they\, or their prerequisites\, do not get a PASS grade during the monthly CPAN-River-3000 process. Other modules failed to install because of upstream breakage in blead. However\, I couldn't identify any distributions which failed due to Module::Install itself.
Thank you very much. -- James E Keenan (jkeenan@cpan.org)
p5pRT
commented
5 years ago On 4/3/19 2:53 PM\, James E Keenan via RT wrote:
On Sat\, 30 Mar 2019 03:54:10 GMT\, ether wrote:
On Thu\, 28 Mar 2019 19:33:04 -0700\, khw wrote:
Pinging this ticket
I've been re-releasing all the modules I am aware of (that I have the ability to) that use Module::Install::DSL; I think the upper portions of the cpan river are now clean. Are we able to assess the remaining impact to the river\, and perhaps establish the final list of distributions that are still of concern? For what it's worth ...
I went to this (possibly inaccurate) list of CPAN distributions which are dependent upon Module::Install:
http://deps.cpantesters.org/depended-on-by.pl?dist=Module-Install
I extracted the 127 top-level distros\, then excluded all distros beginning with Acme\, Bundle or Task. That left 114 distros\, which I attempted to install against Perl 5 blead (commit 930ded654\, Apr 2 2019) on FreeBSD-11.2 (threaded) using cpanm as the installer. (I.e.\, the same approach I take toward monthly testing of the "CPAN-River-3000".)
Can you share this list\, please?
p5pRT
commented
5 years ago On 4/7/19 1:43 AM\, Sawyer X wrote:
On 4/3/19 2:53 PM\, James E Keenan via RT wrote:
On Sat\, 30 Mar 2019 03:54:10 GMT\, ether wrote:
On Thu\, 28 Mar 2019 19:33:04 -0700\, khw wrote:
Pinging this ticket
I've been re-releasing all the modules I am aware of (that I have the ability to) that use Module::Install::DSL; I think the upper portions of the cpan river are now clean. Are we able to assess the remaining impact to the river\, and perhaps establish the final list of distributions that are still of concern? For what it's worth ...
I went to this (possibly inaccurate) list of CPAN distributions which are dependent upon Module::Install:
http://deps.cpantesters.org/depended-on-by.pl?dist=Module-Install
I extracted the 127 top-level distros\, then excluded all distros beginning with Acme\, Bundle or Task. That left 114 distros\, which I attempted to install against Perl 5 blead (commit 930ded654\, Apr 2 2019) on FreeBSD-11.2 (threaded) using cpanm as the installer. (I.e.\, the same approach I take toward monthly testing of the "CPAN-River-3000".)
Can you share this list\, please?
Attached.
p5pRT
commented
5 years ago Module::Install Algorithm::FloodControl Algorithm::LibLinear App::AutoCRUD App::minecraft Cache::Elasticache::Memcache Catalyst::Devel Catalyst::Plugin::Authorization::Abilities CatalystX::ASP CatalystX::Action::Negotiate Class::Tie::InsideOut Conductrics::Agent Const::Exporter CrawlerCommons::RobotRulesParser DBIx::Class::AuditAny DBIx::Class::Objects DBIx::Class::Report Daemon::Control::Plugin::HotStandby Data::Type::Digger Dist::Maker Dist::Zilla::Plugin::ModuleInstall Egg::Release Encode::HP Encode::VN Finance::IBrokers::MooseCallback Google::reCAPTCHA Graph::Writer::DSM Iodef::Pb::Simple JavaScript::Console Jifty KelpX::AppBuilder Lingua::Numending MAD::Loader Mail::Milter::Authentication Mail::Milter::Authentication::Extra Mail::Milter::Authentication::Handler::ARC Mail::Milter::Authentication::Handler::SMIME Message::MongoDB MikroTik::API Module::Install::AckXXX Module::Install::Any::Moose Module::Install::AssertOS Module::Install::AuthorRequires Module::Install::AuthorTests Module::Install::AutoConf Module::Install::AutoLicense Module::Install::AutomatedTester Module::Install::Bugtracker Module::Install::CPANfile Module::Install::CheckLib Module::Install::CheckOptional Module::Install::Contributors Module::Install::CustomInstallationPath Module::Install::DOAP Module::Install::DOAPChangeSets Module::Install::DiffCheck Module::Install::GetProgramLocations Module::Install::GithubMeta Module::Install::HTML5Manifest Module::Install::Homepage Module::Install::InlineModule Module::Install::InstallDirs Module::Install::ManifestSkip Module::Install::MicroTemplate Module::Install::NoAutomatedTesting Module::Install::ORLite2Pod Module::Install::PadrePlugin Module::Install::ParseRequires Module::Install::PerlTar Module::Install::PodFromEuclid Module::Install::ProvidesClass Module::Install::RDF Module::Install::RTx Module::Install::ReadmeFromPod Module::Install::ReadmeMarkdownFromPod Module::Install::ReadmePodFromPod Module::Install::RequiresList Module::Install::Rust Module::Install::ShareFile Module::Install::Template Module::Install::TestBase Module::Install::TestML Module::Install::TestTarget Module::Install::VersionCheck Module::Install::XSUtil Module::Modular Module::Package Module::Package::RDF Module::Provision Mojo::UserAgent::Cached Package Perl::Police Plack::App::AutoCRUD Plack::Middleware::Debug::Timed::Logger Plack::Middleware::Timed::Logger Quiz::Flashcards::0.04b RTDevSys RapidApp Readonly::Enum Role::Kerberos Role::Markup::XML Role::MimeInfo SQL::Abstract::More Samba::Smbstatus Store::Digest Template::Jade Test::HTML::Spelling Text::LTSV Timed::Logger Timed::Logger::Dancer::AdoptPlack Tk::PlotDataset Validator::Lazy WWW::Correios::PrecoPrazo WebService::PayPal::NVP Zonemaster::CLI
p5pRT
commented
5 years ago On Wed\, 03 Apr 2019 11:53:39 GMT\, jkeenan wrote:
On Sat\, 30 Mar 2019 03:54:10 GMT\, ether wrote:
On Thu\, 28 Mar 2019 19:33:04 -0700\, khw wrote:
Pinging this ticket
I've been re-releasing all the modules I am aware of (that I have the ability to) that use Module::Install::DSL; I think the upper portions of the cpan river are now clean. Are we able to assess the remaining impact to the river\, and perhaps establish the final list of distributions that are still of concern?
For what it's worth ...
I went to this (possibly inaccurate) list of CPAN distributions which are dependent upon Module::Install:
http://deps.cpantesters.org/depended-on-by.pl?dist=Module-Install
I extracted the 127 top-level distros\, then excluded all distros beginning with Acme\, Bundle or Task. That left 114 distros\, which I attempted to install against Perl 5 blead (commit 930ded654\, Apr 2 2019) on FreeBSD-11.2 (threaded) using cpanm as the installer. (I.e.\, the same approach I take toward monthly testing of the "CPAN-River- 3000".)
Certain modules failed to install for reasons that were expected given that they\, or their prerequisites\, do not get a PASS grade during the monthly CPAN-River-3000 process. Other modules failed to install because of upstream breakage in blead. However\, I couldn't identify any distributions which failed due to Module::Install itself.
Thank you very much.
CPANtesters currently all green for this distro.
http://matrix.cpantesters.org/?dist=Module-Install;perl=5.29.10;reports=1
Can this ticket be marked Resolved?
Thank you very much.
-- James E Keenan (jkeenan@cpan.org)
p5pRT
commented
5 years ago On Sat\, Apr 13\, 2019 at 07:02:10PM -0700\, James E Keenan via RT wrote:
On Wed\, 03 Apr 2019 11:53:39 GMT\, jkeenan wrote:
On Sat\, 30 Mar 2019 03:54:10 GMT\, ether wrote:
On Thu\, 28 Mar 2019 19:33:04 -0700\, khw wrote:
Pinging this ticket
I've been re-releasing all the modules I am aware of (that I have the ability to) that use Module::Install::DSL; I think the upper portions of the cpan river are now clean. Are we able to assess the remaining impact to the river\, and perhaps establish the final list of distributions that are still of concern?
For what it's worth ...
I went to this (possibly inaccurate) list of CPAN distributions which are dependent upon Module::Install:
http://deps.cpantesters.org/depended-on-by.pl?dist=Module-Install
I extracted the 127 top-level distros\, then excluded all distros beginning with Acme\, Bundle or Task. That left 114 distros\, which I attempted to install against Perl 5 blead (commit 930ded654\, Apr 2 2019) on FreeBSD-11.2 (threaded) using cpanm as the installer. (I.e.\, the same approach I take toward monthly testing of the "CPAN-River- 3000".)
Certain modules failed to install for reasons that were expected given that they\, or their prerequisites\, do not get a PASS grade during the monthly CPAN-River-3000 process. Other modules failed to install because of upstream breakage in blead. However\, I couldn't identify any distributions which failed due to Module::Install itself.
Thank you very much.
CPANtesters currently all green for this distro.
http://matrix.cpantesters.org/?dist=Module-Install;perl=5.29.10;reports=1
Can this ticket be marked Resolved?
Not really. The change in blead\, v5.27.6-180-g0301e89953\, that broke M::I ras reverted by v5.28.0-RC1-15-g9f9606382c\, due to the M::I breakage. But in theory we would like to reapply athe change at some point in the future when a fixed M::I has been included in all the distributions which include it and are affected by it.
I'm going to move this from 5.30.0 blocker to 5.32.0 blocker
-- Nothing ventured\, nothing lost.
p5pRT
commented
5 years ago On Tue\, 23 Apr 2019 08:03:46 -0700\, davem wrote:
CPANtesters currently all green for this distro.
http://matrix.cpantesters.org/?dist=Module-Install;perl=5.29.10;reports=1
Can this ticket be marked Resolved?
Not really. The change in blead\, v5.27.6-180-g0301e89953\, that broke M::I ras reverted by v5.28.0-RC1-15-g9f9606382c\, due to the M::I breakage. But in theory we would like to reapply athe change at some point in the future when a fixed M::I has been included in all the distributions which include it and are affected by it.
I'm going to move this from 5.30.0 blocker to 5.32.0 blocker
Since nothing on cpan seems to be affected anymore\, this should be safe to resolve in 5.31 (by reapplying the reverted portion of the commit). If we are still unsure we could smoke the change against cpan first.
p5pRT
commented
5 years ago On Fri\, 26 Apr 2019 04:05:42 GMT\, ether wrote:
On Tue\, 23 Apr 2019 08:03:46 -0700\, davem wrote:
CPANtesters currently all green for this distro.
http://matrix.cpantesters.org/?dist=Module- Install;perl=5.29.10;reports=1
Can this ticket be marked Resolved?
Not really. The change in blead\, v5.27.6-180-g0301e89953\, that broke M::I ras reverted by v5.28.0-RC1-15-g9f9606382c\, due to the M::I breakage. But in theory we would like to reapply athe change at some point in the future when a fixed M::I has been included in all the distributions which include it and are affected by it.
I'm going to move this from 5.30.0 blocker to 5.32.0 blocker
Since nothing on cpan seems to be affected anymore\, this should be safe to resolve in 5.31 (by reapplying the reverted portion of the commit). If we are still unsure we could smoke the change against cpan first.
I have created the following branch:
smoke-me/jkeenan/rt-132577-module-install
... in which the reverted portion of the original commit is re-applied. This can facilitate CPAN smoking (which we will have to arrange).
Thank you very much.
-- James E Keenan (jkeenan@cpan.org)
jkeenan
commented
4 years ago From @jkeenan
On Fri, 26 Apr 2019 04:05:42 GMT, ether wrote:
On Tue, 23 Apr 2019 08:03:46 -0700, davem wrote:
CPANtesters currently all green for this distro. http://matrix.cpantesters.org/?dist=Module- Install;perl=5.29.10;reports=1 Can this ticket be marked Resolved?
Not really. The change in blead, v5.27.6-180-g0301e89953, that broke M::I ras reverted by v5.28.0-RC1-15-g9f9606382c, due to the M::I breakage. But in theory we would like to reapply athe change at some point in the future when a fixed M::I has been included in all the distributions which include it and are affected by it. I'm going to move this from 5.30.0 blocker to 5.32.0 blocker
Since nothing on cpan seems to be affected anymore, this should be safe to resolve in 5.31 (by reapplying the reverted portion of the commit). If we are still unsure we could smoke the change against cpan first.
I have created the following branch:
smoke-me/jkeenan/rt-132577-module-install
... in which the reverted portion of the original commit is re-applied. This can facilitate CPAN smoking (which we will have to arrange).
The relevant branch is now called smoke-me/jkeenan/gh-16300-module-install. I have re-based it on blead. To refresh our collective memory, the commit in question was this:
commit 429da47739b54c684c7c3b6b37de386c3ae59261
Author: James E Keenan <jkeenan@cpan.org>
AuthorDate: Fri May 31 08:21:16 2019 -0400
Commit: James E Keenan <jkeenan@cpan.org>
CommitDate: Tue Dec 17 15:13:31 2019 -0500
Revert "revert perl_run() 0 -> 256 return mapping"
This reverts commit 9f9606382c45ba5e9600dddf96abfe956762af99.
This reversion is for the purpose of seeing what CPAN breakage results.
In RT 132577, Dave Mitchell wrote:
"The change in blead, v5.27.6-180-g0301e89953, that broke M::I [was]
reverted by v5.28.0-RC1-15-g9f9606382c, due to the M::I breakage. But in
theory we would like to reapply athe change at some point in the future
when a fixed M::I has been included in all the distributions which
include it and are affected by it. I'm going to move this from 5.30.0
blocker to 5.32.0 blocker."
Signed-off-by: James E Keenan <jkeenan@cpan.org>
diff --git a/perl.c b/perl.c
index 70424cdbab..3e39fe35ed 100644
--- a/perl.c
+++ b/perl.c
@@ -2686,7 +2686,7 @@ int
perl_run(pTHXx)
{
I32 oldscope;
- int ret = 0;
+ int ret = 0, exit_called = 0;
dJMPENV;
PERL_ARGS_ASSERT_PERL_RUN;
@@ -2707,8 +2707,10 @@ perl_run(pTHXx)
case 0: /* normal completion */
redo_body:
run_body(oldscope);
- /* FALLTHROUGH */
+ goto handle_exit;
case 2: /* my_exit() */
+ exit_called = 1;
+ handle_exit:
while (PL_scopestack_ix > oldscope)
LEAVE;
FREETMPS;
@@ -2722,7 +2724,12 @@ perl_run(pTHXx)
if (PerlEnv_getenv("PERL_DEBUG_MSTATS"))
dump_mstats("after execution: ");
#endif
- ret = STATUS_EXIT;
+ if (exit_called) {
+ ret = STATUS_EXIT;
+ if (ret == 0) ret = 0x100;
+ } else {
+ ret = 0;
+ }
break;
case 3:
if (PL_restartop) {On FreeBSD-11, I installed this branch, installed cpanm against its perl, then used cpanm to attempt to install the 114 CPAN distributions identified earlier as being first-level revdeps of Module-Install.
Because those distros have prerequisites in addition to Module-Install, more than 900 distros were actually installed. I examined the cpanm build.log for approximately 35 distros which were graded something other than PASS. I could not find any non-PASS which could be attributed to the patch in question.
Inference: Right now would be an appropriate time to re-apply the patch provided originally by Zefram in Dec 2017 (https://github.com/Perl/perl5/commit/0301e899536a22752f40481d8a1d141b7a7dda82). If we were to re-apply it before Friday's anticipated release of perl-5.31.7, we would get CPAN-River-3000 and CPANtesters results. If those results were unfavorable, we could roll back before perl-5.31.8 and code-freeze in January 2020.
@iabyn, @tonycoz, @xsawyerx, @zefram, @karenetheridge, et al. please comment.
Thank you very much. Jim Keenan
haarg
commented
4 years ago Dependency lists are not appropriate to figure out the impact of Module::Install bugs. Module::Install gets bundled with the release, and normally won't be listed as a prerequisite. To find the distributions impacted by this, you need to search for Makefile.PL files using inc::Module::Install::DSL.
There are currently 110 distributions that could be impacted by this, and I'm certain that most of them have not had new releases in the last two years.
jkeenan
commented
4 years ago Dependency lists are not appropriate to figure out the impact of Module::Install bugs. Module::Install gets bundled with the release, and normally won't be listed as a prerequisite. To find the distributions impacted by this, you need to search for Makefile.PL files using inc::Module::Install::DSL.
There are currently 110 distributions that could be impacted by this, and I'm certain that most of them have not had new releases in the last two years.
Thanks for that clarification. Is there any way you could provide me with a list of those distros in a form suitable for feeding to cpanm? I.e., something like:
App::Midgen
Devel::Eval
...I could then feed them into the already installed branch perl. Thank you very much. Jim Keenan
haarg
commented
4 years ago All I have is the grep.cpan.me link that I provided.
jkeenan
commented
4 years ago Dependency lists are not appropriate to figure out the impact of Module::Install bugs. Module::Install gets bundled with the release, and normally won't be listed as a prerequisite. To find the distributions impacted by this, you need to search for Makefile.PL files using inc::Module::Install::DSL. There are currently 110 distributions that could be impacted by this, and I'm certain that most of them have not had new releases in the last two years.
Thanks for that clarification. Is there any way you could provide me with a list of those distros in a form suitable for feeding to
cpanm? I.e., something like:App::Midgen Devel::Eval ...I could then feed them into the already installed branch perl. Thank you very much. Jim Keenan
Not yet in This::Module form suitable for passing to cpanm ... but here are the distros:
ADAMK/Acme-Mom-Yours-0.02
ADAMK/Acme-SuperCollider-Programming-0.02
ADAMK/ADAMK-Release-0.02
ADAMK/Algorithm-Dependency-MapReduce-0.03
ADAMK/Archive-Builder-1.16
ADAMK/Aspect-1.04
ADAMK/Aspect-Library-Memoize-1.00
ADAMK/Aspect-Library-NYTProf-1.00
ADAMK/Aspect-Library-Profiler-1.00
ADAMK/Aspect-Library-TestClass-1.00
ADAMK/Aspect-Library-Timer-1.10
ADAMK/Aspect-Library-Trace-1.00
ADAMK/Business-AU-ABN-1.09
ADAMK/CGI-Capture-1.14
ADAMK/Chart-Math-Axis-1.06
ADAMK/Class-Autouse-2.01
ADAMK/CPANDB-0.18
ADAMK/CPANDB-Generator-0.19
ADAMK/CPAN-Mini-Extract-1.23
ADAMK/CPAN-Mini-Visit-1.15
ADAMK/CPANTS-Weight-0.15
ADAMK/CPAN-WWW-Top100-Generator-0.10
ADAMK/Data-JavaScript-Anon-1.03
ADAMK/DBIx-Export-0.01
ADAMK/Devel-Dumpvar-1.06
ADAMK/Devel-Eval-1.01
ADAMK/Devel-Leak-Module-0.02
ADAMK/Devel-Leak-Object-1.01
ADAMK/FBP-0.41
ADAMK/FBP-Demo-0.03
ADAMK/FBP-Perl-0.78
ADAMK/File-BLOB-1.08
ADAMK/File-Find-Rule-PPI-1.06
ADAMK/File-Find-Rule-VCS-1.08
ADAMK/File-LocalizeNewlines-1.12
ADAMK/GitHub-Extract-0.02
ADAMK/Graph-XGMML-0.01
ADAMK/HTML-InfoVis-0.03
ADAMK/HTTP-Online-0.02
ADAMK/Imager-Search-1.01
ADAMK/JSAN-Client-0.29
ADAMK/JSAN-Mini-1.04
ADAMK/JSAN-Shell-2.04
ADAMK/LWP-Online-1.08
ADAMK/Module-Changes-ADAMK-0.11
ADAMK/MooseX-Atom-0.02
ADAMK/Object-Destroyer-2.01
ADAMK/OpenGL-List-0.01
ADAMK/OpenGL-RWX-0.02
ADAMK/ORDB-AU-Census2006-0.01
ADAMK/ORDB-CPANMeta-0.10
ADAMK/ORDB-CPANMeta-Generator-0.12
ADAMK/ORDB-CPANRelease-0.01
ADAMK/ORDB-CPANRT-0.04
ADAMK/ORDB-CPANTesters-0.09
ADAMK/ORDB-CPANTS-0.05
ADAMK/ORDB-CPANTSWeight-0.03
ADAMK/ORDB-CPANUploads-1.08
ADAMK/ORDB-JSAN-0.01
ADAMK/ORLite-1.98
ADAMK/ORLite-Migrate-1.10
ADAMK/ORLite-Mirror-1.24
ADAMK/ORLite-PDL-0.02
ADAMK/ORLite-Pod-0.11
ADAMK/ORLite-Profile-0.01
ADAMK/ORLite-Statistics-0.03
ADAMK/Oz-0.01
ADAMK/Padre-Plugin-FormBuilder-0.04
ADAMK/Perl-Shell-0.04
ADAMK/Perl-Squish-1.06
ADAMK/PITA-0.60
ADAMK/PITA-Image-0.60
ADAMK/PITA-Scheme-0.43
ADAMK/PITA-XML-0.52
ADAMK/pler-1.06
ADAMK/POE-Declare-0.59
ADAMK/POE-Declare-HTTP-Client-0.05
ADAMK/POE-Declare-HTTP-Online-0.02
ADAMK/POE-Declare-HTTP-Server-0.05
ADAMK/POE-Declare-Log-File-0.01
ADAMK/Politics-AU-Geo-0.01
ADAMK/PPI-PowerToys-0.14
ADAMK/PPI-Tester-0.15
ADAMK/Process-0.30
ADAMK/SDL-Tutorial-3DWorld-0.33
ADAMK/SMS-Send-1.06
ADAMK/Test-POE-Stopping-1.09
ADAMK/Win32-Env-Path-0.03
ADAMK/Xtract-0.16
ADAMK/YAML-Tiny-Stream-0.01
AUDREYT/Convert-EastAsianWidth-1.02
AUDREYT/Devel-Hints-0.21
AUDREYT/YiJing-0.10
AZAWAWI/Locale-Msgfmt-0.15
BOWTIE/App-Midgen-0.34
BOWTIE/Padre-Plugin-Autodia-0.04
BOWTIE/Padre-Plugin-Git-0.12
BOWTIE/Padre-Plugin-Nopaste-0.08
BOWTIE/Padre-Plugin-SpellCheck-1.33
BOWTIE/Padre-Plugin-YAML-0.10
BRAMBLE/Padre-Plugin-Swarm-0.2
CHORNY/Text-FindIndent-0.11
CSJEWELL/Module-Install-PerlTar-1.001
JDENNES/Net-CampaignMonitor-v2.2.1
JEFFERY/Geo-MapInfo-MIF-0.02
JEFFERY/Net-MySourceMatrix-0.04
KMX/Portable-1.22
PMAKHOLM/ORLite-Array-0.02
SDOWIDEIT/Module-Install-ORLite2Pod-1.000
STIGTSP/Template-Plugin-File-StaticURL-0.02Thank you very much. Jim Keenan
Grinnz
commented
4 years ago cpanm accepts this form of release to install if you include the .tar.gz extension.
toddr
commented
4 years ago I'm going to remove the tag of BBC on this case since the commit originally raised was reverted. At this point, the ticket sounds like us discussing if we should put the bug fix back in. Can someone recommend a new Subject for this ticket?
toddr
commented
4 years ago If I'm understanding the issue, the discussion seems to be if we're going to fix #1537 Which jim has provided a branch here at: https://github.com/Perl/perl5/tree/smoke-me/jkeenan/gh-16300-module-install.
Looking closer at #1537, though I think the change is already in blead as 625e8b0bffed3334db9b3897f8713d570fd5385c and staged for 5.32. @jkeenan @tonycoz Am I groking this correctly?
tonycoz
commented
4 years ago @toddr, no that's the (re-)reversion of the fix. That comment in perl.c still has:
A historical bug is preserved
for the time being: if the Perl built-in C<exit> is called during this
function's execution, with a type of exit entailing a zero exit code
under the host operating system's conventions, then this function
returns zero rather than a non-zero value. This bug, [perl #2754],
leads to C<perl_run> being called (and therefore C<INIT> blocks and the
main program running) despite a call to C<exit>. It has been preserved
because a popular module-installing module has come to rely on it and
needs time to be fixed. This issue is [perl #132577], and the original
bug is due to be fixed in Perl 5.30.
xsawyerx
commented
4 years ago @tonycoz does this mean the fix is not in blead anymore?
Considering this list, my main concerns are whether they are still being maintained and whether the authors are able to produce new versions with the fixes. Considering these modules haven't had a release in a while is less concerning to me, especially due to the fact that the authors are highly responsive (at least the biggest releaser on the list - @andk).
Any strong objections?
haarg
commented
4 years ago The majority of the dists are from ADAMK, who is not active at all, but he has given blanket permission for some people to take over his modules.
tonycoz
commented
4 years ago @tonycoz does this mean the fix is not in blead anymore?
Correct, the code in perl.c:
if (ret == 0) {
/*
* At this point we should do
* ret = 0x100;
* to avoid [perl #2754], but that bugfix has been postponed
* because of the Module::Install breakage it causes
* [perl #132577].
*/
}I have no objections to re-reinstating the fix.
karenetheridge
commented
4 years ago Considering this list, my main concerns are whether they are still being maintained and whether the authors are able to produce new versions with the fixes
I have comaint on most of them, and have been doing releases for distributions higher on the cpan river, or when someone squawks (e.g. filing an RT ticket). Many of these distributions have no reverse dependencies at all though so I have not been feeling it is urgent to make updates to them.
jkeenan
commented
4 years ago Yesterday I (re-)created branch smoke-me/jkeenan/gh-16300-module-install in which I (re-)applied this patch on top of blead.
commit 7bd6ca05f713e309596c0242ad4a23a9148b3897
Author: James E Keenan <jkeenan@cpan.org>
AuthorDate: Fri May 31 08:21:16 2019 -0400
Commit: James E Keenan <jkeenan@cpan.org>
CommitDate: Thu Apr 2 00:53:50 2020 +0000
Revert "revert perl_run() 0 -> 256 return mapping"
This reverts commit 9f9606382c45ba5e9600dddf96abfe956762af99.
This reversion is for the purpose of seeing what CPAN breakage results.
In RT 132577, Dave Mitchell wrote:
"The change in blead, v5.27.6-180-g0301e89953, that broke M::I [was]
reverted by v5.28.0-RC1-15-g9f9606382c, due to the M::I breakage. But in
theory we would like to reapply athe change at some point in the future
when a fixed M::I has been included in all the distributions which
include it and are affected by it. I'm going to move this from 5.30.0
blocker to 5.32.0 blocker."
Signed-off-by: James E Keenan <jkeenan@cpan.org>
diff --git a/perl.c b/perl.c
index 9d1faf44da..d25c547d35 100644
--- a/perl.c
+++ b/perl.c
@@ -2659,7 +2659,7 @@ int
perl_run(pTHXx)
{
I32 oldscope;
- int ret = 0;
+ int ret = 0, exit_called = 0;
dJMPENV;
PERL_ARGS_ASSERT_PERL_RUN;
@@ -2680,8 +2680,10 @@ perl_run(pTHXx)
case 0: /* normal completion */
redo_body:
run_body(oldscope);
- /* FALLTHROUGH */
+ goto handle_exit;
case 2: /* my_exit() */
+ exit_called = 1;
+ handle_exit:
while (PL_scopestack_ix > oldscope)
LEAVE;
FREETMPS;
@@ -2695,7 +2697,12 @@ perl_run(pTHXx)
if (PerlEnv_getenv("PERL_DEBUG_MSTATS"))
dump_mstats("after execution: ");
#endif
- ret = STATUS_EXIT;
+ if (exit_called) {
+ ret = STATUS_EXIT;
+ if (ret == 0) ret = 0x100;
+ } else {
+ ret = 0;
+ }
break;
case 3:
if (PL_restartop) {I then installed that branch on FreeBSD-11, installed cpanm against that perl, and used cpanm to attempt to install the list of modules posted in this ticket (https://github.com/Perl/perl5/issues/16300#issuecomment-567195078) on Dec 18 2019 (except the Acme:: modules). I then used CPAN::cpanminus::reporter::RetainReports to parse the cpanm build.log file into .json files for each distribution reached during cpanm. I then conducted various inspections (gory details available upon request).
Rough overall conclusions:
The distributions which were not reached during cpanm: This non-reach was generally due to build-time or unit test failures in their prerequisites.
Quite a few of the distributions were reached and received a grade.
{
"Algorithm-Dependency-MapReduce" => "PASS",
"Archive-Builder" => "PASS",
"Aspect" => "PASS",
"Aspect-Library-Memoize" => "PASS",
"Aspect-Library-NYTProf" => "PASS",
"Aspect-Library-Profiler" => "PASS",
"Aspect-Library-TestClass" => "PASS",
"Aspect-Library-Timer" => "PASS",
"Aspect-Library-Trace" => "PASS",
"Business-AU-ABN" => "PASS",
"Chart-Math-Axis" => "PASS",
"Class-Autouse" => "PASS",
"CPAN-Mini-Extract" => "PASS",
"CPAN-Mini-Visit" => "PASS",
"CPANDB-Generator" => "PASS",
"Data-JavaScript-Anon" => "PASS",
"DBIx-Export" => "PASS",
"Devel-Dumpvar" => "PASS",
"Devel-Eval" => "PASS",
"Devel-Leak-Module" => "PASS",
"Devel-Leak-Object" => "PASS",
"FBP" => "PASS",
"FBP-Perl" => "PASS",
"File-BLOB" => "PASS",
"File-Find-Rule-PPI" => "PASS",
"File-Find-Rule-VCS" => "PASS",
"File-LocalizeNewlines" => "PASS",
"Geo-MapInfo-MIF" => "PASS",
"Graph-XGMML" => "PASS",
"HTML-InfoVis" => "PASS",
"HTTP-Online" => "PASS",
"Imager-Search" => "PASS",
"Locale-Msgfmt" => "PASS",
"LWP-Online" => "PASS",
"Module-Changes-ADAMK" => "PASS",
"Module-Install-PerlTar" => "PASS",
"MooseX-Atom" => "PASS",
"Net-MySourceMatrix" => "PASS",
"Object-Destroyer" => "PASS",
"ORDB-CPANMeta-Generator" => "PASS",
"ORLite-Array" => "PASS",
"Perl-Shell" => "PASS",
"Perl-Squish" => "PASS",
"pler" => "PASS",
"PPI-PowerToys" => "PASS",
"Process" => "PASS",
"SMS-Send" => "PASS",
"Template-Plugin-File-StaticURL" => "PASS",
"Text-FindIndent" => "PASS",
"Xtract" => "PASS",
"YAML-Tiny-Stream" => "PASS",
"YiJing" => "PASS",
}{
"App-Midgen" => "FAIL", # fail unit test
"CGI-Capture" => "FAIL", # fail unit test
"Convert-EastAsianWidth" => "FAIL", # fail unit test
"Devel-Hints" => "FAIL", # build-time error https://rt.cpan.org/Ticket/Display.html?id=76398 (2012)
"GitHub-Extract" => "FAIL", # fail unit test
"JSAN-Client" => "FAIL", # fail unit test
"Net-CampaignMonitor" => "FAIL", # fail unit test
"OpenGL-List" => "FAIL", # fail unit test (OpenGL fails to install)
"ORLite" => "FAIL", # tests failing since March 2018: https://rt.cpan.org/Ticket/Display.html?id=124854
"ORLite-PDL" => "FAIL", # fail unit test (ORLite)
"Oz" => "NA", # external dependency missing
"PITA-XML" => "FAIL", # fail unit test
"Politics-AU-Geo" => "FAIL", # fail unit test (missing Math::Polygon)
"Portable" => "FAIL", # fail unit test
}Now, since I've been staring at this for 2 days and am fatigued, there's a good chance that my analysis may have missed the point. But it does appear that re-applying the patch gives good results and hence should be applied to blead (though perhaps not for 5.32). Someone else should try to confirm my findings.
Thank you very much. Jim Keenan
toddr
commented
4 years ago Considering this list, my main concerns are whether they are still being maintained and whether the authors are able to produce new versions with the fixes
I have comaint on most of them, and have been doing releases for distributions higher on the cpan river, or when someone squawks (e.g. filing an RT ticket). Many of these distributions have no reverse dependencies at all though so I have not been feeling it is urgent to make updates to them.
@karenetheridge, Am I correct that if an up to date Module::Install::DSL is installed, then the file inc/Module/Install/DSL.pm located with the CPAN distro is not used? If so that reduces the scope of this problem, right?
karenetheridge
commented
4 years ago @karenetheridge, Am I correct that if an up to date Module::Install::DSL is installed, then the file inc/Module/Install/DSL.pm located with the CPAN distro is not used? If so that reduces the scope of this problem, right?
Not really. In the end, the local version takes over: https://metacpan.org/release/Module-Install/source/lib/Module/Install.pm#L3-17
jkeenan
commented
4 years ago Yesterday I (re-)created branch
smoke-me/jkeenan/gh-16300-module-installin which I (re-)applied this patch on top ofblead.commit 7bd6ca05f713e309596c0242ad4a23a9148b3897 Author: James E Keenan <jkeenan@cpan.org> AuthorDate: Fri May 31 08:21:16 2019 -0400 Commit: James E Keenan <jkeenan@cpan.org> CommitDate: Thu Apr 2 00:53:50 2020 +0000 Revert "revert perl_run() 0 -> 256 return mapping" This reverts commit 9f9606382c45ba5e9600dddf96abfe956762af99. This reversion is for the purpose of seeing what CPAN breakage results. In RT 132577, Dave Mitchell wrote: "The change in blead, v5.27.6-180-g0301e89953, that broke M::I [was] reverted by v5.28.0-RC1-15-g9f9606382c, due to the M::I breakage. But in theory we would like to reapply athe change at some point in the future when a fixed M::I has been included in all the distributions which include it and are affected by it. I'm going to move this from 5.30.0 blocker to 5.32.0 blocker." Signed-off-by: James E Keenan <jkeenan@cpan.org> diff --git a/perl.c b/perl.c index 9d1faf44da..d25c547d35 100644 --- a/perl.c +++ b/perl.c @@ -2659,7 +2659,7 @@ int perl_run(pTHXx) { I32 oldscope; - int ret = 0; + int ret = 0, exit_called = 0; dJMPENV; PERL_ARGS_ASSERT_PERL_RUN; @@ -2680,8 +2680,10 @@ perl_run(pTHXx) case 0: /* normal completion */ redo_body: run_body(oldscope); - /* FALLTHROUGH */ + goto handle_exit; case 2: /* my_exit() */ + exit_called = 1; + handle_exit: while (PL_scopestack_ix > oldscope) LEAVE; FREETMPS; @@ -2695,7 +2697,12 @@ perl_run(pTHXx) if (PerlEnv_getenv("PERL_DEBUG_MSTATS")) dump_mstats("after execution: "); #endif - ret = STATUS_EXIT; + if (exit_called) { + ret = STATUS_EXIT; + if (ret == 0) ret = 0x100; + } else { + ret = 0; + } break; case 3: if (PL_restartop) {I then installed that branch on FreeBSD-11, installed
cpanmagainst thatperl, and usedcpanmto attempt to install the list of modules posted in this ticket (#16300 (comment)) on Dec 18 2019 (except theAcme::modules). I then used CPAN::cpanminus::reporter::RetainReports to parse thecpanmbuild.logfile into.jsonfiles for each distribution reached duringcpanm. I then conducted various inspections (gory details available upon request).Rough overall conclusions:
1. The distributions which were **not** reached during `cpanm`: This non-reach was generally due to build-time or unit test failures in their prerequisites. 2. Quite a few of the distributions were reached and received a grade.[# snip PASSes ]
{ "App-Midgen" => "FAIL", # fail unit test "CGI-Capture" => "FAIL", # fail unit test "Convert-EastAsianWidth" => "FAIL", # fail unit test "Devel-Hints" => "FAIL", # build-time error https://rt.cpan.org/Ticket/Display.html?id=76398 (2012) "GitHub-Extract" => "FAIL", # fail unit test "JSAN-Client" => "FAIL", # fail unit test "Net-CampaignMonitor" => "FAIL", # fail unit test "OpenGL-List" => "FAIL", # fail unit test (OpenGL fails to install) "ORLite" => "FAIL", # tests failing since March 2018: https://rt.cpan.org/Ticket/Display.html?id=124854 "ORLite-PDL" => "FAIL", # fail unit test (ORLite) "Oz" => "NA", # external dependency missing "PITA-XML" => "FAIL", # fail unit test "Politics-AU-Geo" => "FAIL", # fail unit test (missing Math::Polygon) "Portable" => "FAIL", # fail unit test }
I then repeated this cpanm process on Linux. The list of FAILs was almost exactly the same as those above. None of the FAILs could be attributed to Module::Install. With two exceptions, rt.cpan.org tickets or github issues already existed for these distributions. I created or updated tickets for the remainder.
Thank you very much. Jim Keenan
xsawyerx
commented
4 years ago I would like to reintroduce @tonycoz's patch.
Any objections?
haarg
commented
4 years ago I'm not certain that people have been looking at the correct commits.
0301e899536a22752f40481d8a1d141b7a7dda82 is the original commit that broke modules using Module::Install::DSL.
This was later adjusted in two different commits.
9f9606382c45ba5e9600dddf96abfe956762af99 is a partial revert by Dave Mitchell to fix issues with embedded perl. This is related to #16565, not this issue. This is the commit @jkeenan was testing reverting. Reverting this patch will have no impact on Module::Install::DSL.
857320cbf85e762add18885ae8a197b5e0c21b69 is a different partial revert by Zefram to fix the issues with Module::Install::DSL. Reverting this commit will break all users of old Module::Install::DSL, which is all users of Module::Install::DSL.
khwilliamson
commented
4 years ago What needs to be done here, if anything, to at least remove this from being a 5.32 blocker?
Migrated from rt.perl.org#132577 (status was 'open')
Searchable as RT132577$