Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.96k stars 556 forks source link

[BUG] can't exit 0 from CHECK{} #1537

Closed p5pRT closed 6 years ago

p5pRT commented 24 years ago

Migrated from rt.perl.org#2754 (status was 'open')

Searchable as RT2754$

p5pRT commented 24 years ago

From tchrist@chthon.perl.com

From a CHECK{}\, you cannot exit(0). You may exit !0\, but not 0. If you put this in /tmp/a and run it​:

  #BEGIN { warn "testing exit from BEGIN"; exit }   #BEGIN { warn "testing exit N from BEGIN"; exit 1 }

  #INIT { warn "testing exit from INIT"; exit }   #INIT { warn "testing exit N from INIT"; exit 2 }

  CHECK { warn "testing exit from CHECK"; exit }   #CHECK { warn "testing exit N from CHECK"; exit 3 }

  #END { warn "testing exit from END"; exit }   #END { warn "testing exit N from END"; exit 4 }

  print "i am now the main program\n";   warn "testing exit 5 from main";   exit 5;

  die "XXX";

You will get​:

  % perl /tmp/a   testing exit from CHECK at /tmp/a line 7.   i am now the main program   testing exit 5 from main at /tmp/a line 14.   Exit 5

If you switch the comment on the two CHECKs\, you get

  % perl /tmp/a   testing exit N from CHECK at /tmp/a line 8.   Exit 3

--tom

p5pRT commented 22 years ago

From @floatingatoll

[tchrist@​chthon.perl.com - Tue Mar 28 03​:39​:19 2000]​:

From a CHECK{}\, you cannot exit(0). You may exit !0\, but not 0. If you put this in /tmp/a and run it​:

\#BEGIN \{ warn "testing exit from BEGIN"; exit \}
\#BEGIN \{ warn "testing exit N from BEGIN"; exit 1 \}

\#INIT \{ warn "testing exit from INIT"; exit \}
\#INIT \{ warn "testing exit N from INIT"; exit 2 \}

CHECK \{ warn "testing exit from CHECK"; exit \}
\#CHECK \{ warn "testing exit N from CHECK"; exit 3 \}

\#END \{ warn "testing exit from END"; exit \}
\#END \{ warn "testing exit N from END"; exit 4 \}

print "i am now the main program\\n";
warn "testing exit 5 from main";
exit 5;

die "XXX";

You will get​:

% perl /tmp/a
testing exit from CHECK at /tmp/a line 7\.
i am now the main program
testing exit 5 from main at /tmp/a line 14\.
Exit 5

If you switch the comment on the two CHECKs\, you get

% perl /tmp/a
testing exit N from CHECK at /tmp/a line 8\.
Exit 3

Confirmed this bug is present as of @​17821.

p5pRT commented 14 years ago

From @chorny

Still present in 5.12.0 RC0.

On Tue Mar 28 03​:39​:19 2000\, tchrist@​chthon.perl.com wrote​:

From a CHECK{}\, you cannot exit(0). You may exit !0\, but not 0. If you put this in /tmp/a and run it​:

\#BEGIN \{ warn "testing exit from BEGIN"; exit \}
\#BEGIN \{ warn "testing exit N from BEGIN"; exit 1 \}

\#INIT \{ warn "testing exit from INIT"; exit \}
\#INIT \{ warn "testing exit N from INIT"; exit 2 \}

CHECK \{ warn "testing exit from CHECK"; exit \}
\#CHECK \{ warn "testing exit N from CHECK"; exit 3 \}

\#END \{ warn "testing exit from END"; exit \}
\#END \{ warn "testing exit N from END"; exit 4 \}

print "i am now the main program\\n";
warn "testing exit 5 from main";
exit 5;

die "XXX";

You will get​:

% perl /tmp/a
testing exit from CHECK at /tmp/a line 7\.
i am now the main program
testing exit 5 from main at /tmp/a line 14\.
Exit 5

If you switch the comment on the two CHECKs\, you get

% perl /tmp/a
testing exit N from CHECK at /tmp/a line 8\.
Exit 3

-- Alexandr Ciornii\, http​://chorny.net

p5pRT commented 14 years ago

From @abigail

On Mon\, Apr 12\, 2010 at 03​:50​:52AM -0700\, Alexandr Ciornii via RT wrote​:

On Tue Mar 28 03​:39​:19 2000\, tchrist@​chthon.perl.com wrote​:

From a CHECK{}\, you cannot exit(0). You may exit !0\, but not 0. If you put this in /tmp/a and run it​:

\#BEGIN \{ warn "testing exit from BEGIN"; exit \}
\#BEGIN \{ warn "testing exit N from BEGIN"; exit 1 \}

\#INIT \{ warn "testing exit from INIT"; exit \}
\#INIT \{ warn "testing exit N from INIT"; exit 2 \}

CHECK \{ warn "testing exit from CHECK"; exit \}
\#CHECK \{ warn "testing exit N from CHECK"; exit 3 \}

\#END \{ warn "testing exit from END"; exit \}
\#END \{ warn "testing exit N from END"; exit 4 \}

print "i am now the main program\\n";
warn "testing exit 5 from main";
exit 5;

die "XXX";

You will get​:

% perl /tmp/a
testing exit from CHECK at /tmp/a line 7\.
i am now the main program
testing exit 5 from main at /tmp/a line 14\.
Exit 5

If you switch the comment on the two CHECKs\, you get

% perl /tmp/a
testing exit N from CHECK at /tmp/a line 8\.
Exit 3

Still present in 5.12.0 RC0.

And in 5.12.0-RC5 as well. However\, it's not a regression for a recent Perl. It behaves the same in 5.10.1\, 5.8.[89] and 5.6.2.

It is a regression from 5.005_04 though\, which will print the expected​:

  testing exit from CHECK at bb line 14.

Abigail

p5pRT commented 14 years ago

From @gannett-ggreer

On Tue Mar 28 03​:39​:19 2000\, tchrist@​chthon.perl.com wrote​:

From a CHECK{}\, you cannot exit(0). You may exit !0\, but not 0. If you put this in /tmp/a and run it​:

\#BEGIN \{ warn "testing exit from BEGIN"; exit \}
\#BEGIN \{ warn "testing exit N from BEGIN"; exit 1 \}

\#INIT \{ warn "testing exit from INIT"; exit \}
\#INIT \{ warn "testing exit N from INIT"; exit 2 \}

CHECK \{ warn "testing exit from CHECK"; exit \}
\#CHECK \{ warn "testing exit N from CHECK"; exit 3 \}

\#END \{ warn "testing exit from END"; exit \}
\#END \{ warn "testing exit N from END"; exit 4 \}

print "i am now the main program\\n";
warn "testing exit 5 from main";
exit 5;

die "XXX";

You will get​:

% perl /tmp/a
testing exit from CHECK at /tmp/a line 7\.
i am now the main program
testing exit 5 from main at /tmp/a line 14\.
Exit 5

If you switch the comment on the two CHECKs\, you get

% perl /tmp/a
testing exit N from CHECK at /tmp/a line 8\.
Exit 3

The problem (as of today's Perl) is that perl_parse() returns the exit status as if it were going back to the OS. In main()\, perl does​: "exitstatus = perl_parse(...); if (!exitstatus) perl_run(...);' Which means main() can't tell the difference between normal execution (0 from JMPENV) and my_exit (2 from JMPENV) because the my_exit case sets the return value to the exit code (0).

perl_parse() is flagged as public API so my patch below is unacceptable although it does fix the immediate problem. I include it more as illustration than for inclusion (which is why it is a 'diff'\, not a 'commit'). It does\, however\, pass all tests which makes me think there should be tests that the public API is stable.

I appeal to greater powers for a "public API"-acceptable fix. :)

-- George Greer

p5pRT commented 14 years ago

From @gannett-ggreer

nak.patch ```diff diff --git a/perl.c b/perl.c index 0edad78..8815ed1 100644 --- a/perl.c +++ b/perl.c @@ -1606,11 +1606,8 @@ perl_parse(pTHXx_ XSINIT_t xsinit, int argc, char **argv, char **env) call_list(oldscope, PL_unitcheckav); if (PL_checkav) call_list(oldscope, PL_checkav); - ret = 0; break; case 1: - STATUS_ALL_FAILURE; - /* FALL THROUGH */ case 2: /* my_exit() was called */ while (PL_scopestack_ix > oldscope) @@ -1621,11 +1618,9 @@ perl_parse(pTHXx_ XSINIT_t xsinit, int argc, char **argv, char **env) call_list(oldscope, PL_unitcheckav); if (PL_checkav) call_list(oldscope, PL_checkav); - ret = STATUS_EXIT; break; case 3: PerlIO_printf(Perl_error_log, "panic: top_env\n"); - ret = 1; break; } JMPENV_POP; ```
p5pRT commented 14 years ago

From tchrist@perl.com

  +---------------------------------------------------------------------+   | This is automatic mail from Tom Christiansen's answering service. |   | UPDATED​: Saturday\, 2010 July 10 18​:01​:10 MDT 2010 |   +---------------------------------------------------------------------+

  I'm away from the office\, up in the Colorado wilderness\,   minimally until Wednesday\, July 14th. I do *not* expect   to have email or phone access while I'm away.

  I'll do my best to get back to you as soon as I return.

  --tom

p5pRT commented 12 years ago

From @Hugmeir

On Sun Jul 11 00​:23​:23 2010\, greerga wrote​:

On Tue Mar 28 03​:39​:19 2000\, tchrist@​chthon.perl.com wrote​:

From a CHECK{}\, you cannot exit(0). You may exit !0\, but not 0. If you put this in /tmp/a and run it​:

\#BEGIN \{ warn "testing exit from BEGIN"; exit \}
\#BEGIN \{ warn "testing exit N from BEGIN"; exit 1 \}

\#INIT \{ warn "testing exit from INIT"; exit \}
\#INIT \{ warn "testing exit N from INIT"; exit 2 \}

CHECK \{ warn "testing exit from CHECK"; exit \}
\#CHECK \{ warn "testing exit N from CHECK"; exit 3 \}

\#END \{ warn "testing exit from END"; exit \}
\#END \{ warn "testing exit N from END"; exit 4 \}

print "i am now the main program\\n";
warn "testing exit 5 from main";
exit 5;

die "XXX";

You will get​:

% perl /tmp/a
testing exit from CHECK at /tmp/a line 7\.
i am now the main program
testing exit 5 from main at /tmp/a line 14\.
Exit 5

If you switch the comment on the two CHECKs\, you get

% perl /tmp/a
testing exit N from CHECK at /tmp/a line 8\.
Exit 3

The problem (as of today's Perl) is that perl_parse() returns the exit status as if it were going back to the OS. In main()\, perl does​: "exitstatus = perl_parse(...); if (!exitstatus) perl_run(...);' Which means main() can't tell the difference between normal execution (0 from JMPENV) and my_exit (2 from JMPENV) because the my_exit case sets the return value to the exit code (0).

perl_parse() is flagged as public API so my patch below is unacceptable although it does fix the immediate problem. I include it more as illustration than for inclusion (which is why it is a 'diff'\, not a 'commit'). It does\, however\, pass all tests which makes me think there should be tests that the public API is stable.

I appeal to greater powers for a "public API"-acceptable fix. :)

I reappeal to greater powers for a "public API"-acceptable fix. Would be nice to get this bug closed once and for all!

p5pRT commented 11 years ago

From @wolfsage

On Fri May 04 12​:15​:22 2012\, Hugmeir wrote​:

I appeal to greater powers for a "public API"-acceptable fix. :)

I reappeal to greater powers for a "public API"-acceptable fix. Would be nice to get this bug closed once and for all!

Hmm\, does this actually break the contract of the public API?

The docs that I've found don't specify any specific return values\, they just show through example that​:

(From perlembed.pod)   exitstatus = perl_parse(my_perl\, NULL\, 2\, embedding\, NULL);   PL_exit_flags |= PERL_EXIT_DESTRUCT_END;   if(!exitstatus) {   exitstatus = perl_run(my_perl);

So as long as perl_parse returns a false value when it succeeds\, we should be good?

-- Matthew Horsfall (alh)

p5pRT commented 11 years ago

From @cpansprout

On Sun Nov 25 05​:31​:01 2012\, alh wrote​:

On Fri May 04 12​:15​:22 2012\, Hugmeir wrote​:

I appeal to greater powers for a "public API"-acceptable fix. :)

I reappeal to greater powers for a "public API"-acceptable fix. Would be nice to get this bug closed once and for all!

Hmm\, does this actually break the contract of the public API?

The docs that I've found don't specify any specific return values\, they just show through example that​:

(From perlembed.pod) exitstatus = perl_parse(my_perl\, NULL\, 2\, embedding\, NULL); PL_exit_flags |= PERL_EXIT_DESTRUCT_END; if(!exitstatus) { exitstatus = perl_run(my_perl);

So as long as perl_parse returns a false value when it succeeds\, we should be good?

I think we need to rethink (carefully!) how perl handles this internally. That BEGIN{exit 0} exits the program and CHECK{exit 0} does not seems like the implementation details leaking through.

--

Father Chrysostomos

p5pRT commented 6 years ago

From zefram@fysh.org

Oh boy\, this was a fun one. The basic problem is that the return value from perl_parse() is overloaded. Is it a truth value or an exit code? If an exit code\, is it a Unix-style exit code or a native exit code? The perlembed tutorial disagreed with perlmain.c and had a bunch of flaws that mark it as an unreliable source\, and the function wasn't actually documented other than by reference to perlembed.

We can preserve most of the overloaded use of perl_parse()'s return value by the very simple expedient of returning 0x100 for a zero exit code\, leaving an actual zero return value to indicate that we're not exiting. On Unix this is a total fix. On other OSes an equivalent trick might be possible\, but might not\, depending on local conventions for exit codes. (It's clear enough that perl_parse()'s return value has the significance of a native exit code\, despite the Unix-centric assumptions that crept in.) It should in any case return non-zero for a zero native exit code regardless of platform​: that means that an exit with zero native exit code will at least always be interpreted as exiting\, so the remaining misbehaviour will only consist of exiting with the wrong exit code.

For users to get the correct exit code they need to interpret perl_parse()'s return value the way perlmain.c does​: as a truth value\, with the actual exit code coming from perl_destruct(). perlembed needs to show this mode of usage.

I have done all this (and some more) in commit 0301e899536a22752f40481d8a1d141b7a7dda82. That resolves this ticket.

-zefram

p5pRT commented 6 years ago

@cpansprout - Status changed from 'open' to 'pending release'

p5pRT commented 6 years ago

From zefram@fysh.org

In order to give Module​::Install distros more time to roll out a fix for [perl #132577]\, this bug [perl #2754] has been reintroduced in commit 857320cbf85e762add18885ae8a197b5e0c21b69. This ticket should be reopened. The plan is to revert the bug reintroduction during the 5.29 development cycle. This issue should block 5.30.0.

-zefram

p5pRT commented 6 years ago

From @xsawyerx

Following this\, we should formulate a plan for dealing with these distributions and having them release new versions. We have time but our goal is to still have this bug fix in the next version and this could only be done if we are able to have enough distributions released with the fix.

Zefram\, is it possible to provide a succinct way to explain this change to module authors when we submit patches or request the changes made?

On 12/27/2017 11​:26 PM\, Zefram wrote​:

In order to give Module​::Install distros more time to roll out a fix for [perl #132577]\, this bug [perl #2754] has been reintroduced in commit 857320cbf85e762add18885ae8a197b5e0c21b69. This ticket should be reopened. The plan is to revert the bug reintroduction during the 5.29 development cycle. This issue should block 5.30.0.

-zefram

p5pRT commented 6 years ago

From zefram@fysh.org

Sawyer X wrote​:

Zefram\, is it possible to provide a succinct way to explain this change to module authors when we submit patches or request the changes made?

"Module​::Install​::DSL versions below 1.19 rely on a core bug\, and are going to stop working on Perl versions above 5.28. To keep your module installable on future Perls\, please rerelease it with the bundled Module​::Install​::DSL updated to version 1.19."

-zefram

p5pRT commented 6 years ago

From @jkeenan

Re-opening ticket per discussion. -- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 6 years ago

@jkeenan - Status changed from 'pending release' to 'open'

p5pRT commented 6 years ago

From @karenetheridge

Use of Module​::Install is discouraged by the Perl Toolchain Gang\, and it may soon be formally deprecated. I would prefer the recommendation not simply suggest the author re-release with a newer MI\, but rather convert their distribution off of MI entirely and therefore avoid all other issues arising from its use.

On Wed\, Dec 27\, 2017 at 1​:46 PM\, Zefram \zefram@​fysh\.org wrote​:

Sawyer X wrote​:

Zefram\, is it possible to provide a succinct way to explain this change to module authors when we submit patches or request the changes made?

"Module​::Install​::DSL versions below 1.19 rely on a core bug\, and are going to stop working on Perl versions above 5.28. To keep your module installable on future Perls\, please rerelease it with the bundled Module​::Install​::DSL updated to version 1.19."

-zefram

p5pRT commented 6 years ago

From @khwilliamson

On 12/28/2017 01​:19 PM\, Karen Etheridge wrote​:

Use of Module​::Install is discouraged by the Perl Toolchain Gang\, and it may soon be formally deprecated. I would prefer the recommendation not simply suggest the author re-release with a newer MI\, but rather convert their distribution off of MI entirely and therefore avoid all other issues arising from its use.

+1

On Wed\, Dec 27\, 2017 at 1​:46 PM\, Zefram \<zefram@​fysh.org \mailto&#8203;:zefram@&#8203;fysh\.org> wrote​:

Sawyer X wrote&#8203;:
>Zefram\, is it possible to provide a succinct way to explain this change
>to module authors when we submit patches or request the changes made?

"Module&#8203;::Install&#8203;::DSL versions below 1\.19 rely on a core bug\, and
are going to stop working on Perl versions above 5\.28\.  To keep your
module installable on future Perls\, please rerelease it with the bundled
Module&#8203;::Install&#8203;::DSL updated to version 1\.19\."

\-zefram
p5pRT commented 6 years ago

From @eserte

Karen Etheridge \perl@&#8203;froods\.org writes​:

Use of Module​::Install is discouraged by the Perl Toolchain Gang\, and it may soon be formally deprecated.

Module​::Install is used by several thousand CPAN distributions. A deprecation seems impractical.

I would prefer the recommendation not simply suggest the author re-release with a newer MI\, but rather convert their distribution off of MI entirely and therefore avoid all other issues arising from its use.

On Wed\, Dec 27\, 2017 at 1​:46 PM\, Zefram \zefram@&#8203;fysh\.org wrote​:

Sawyer X wrote​:

Zefram\, is it possible to provide a succinct way to explain this change to module authors when we submit patches or request the changes made?

"Module​::Install​::DSL versions below 1.19 rely on a core bug\, and are going to stop working on Perl versions above 5.28. To keep your module installable on future Perls\, please rerelease it with the bundled Module​::Install​::DSL updated to version 1.19."

-zefram

-- Slaven Rezic - slaven \ rezic \ de

  Berlin Perl Mongers - http​://berlin.pm.org

p5pRT commented 6 years ago

From @karenetheridge

A module being deprecated doesn't mean it will be removed from cpan (unless that has been explicitly stated) -- rather it signals that it is no longer being actively maintained save for critical bug fixes\, which is exactly the case here. Authors must be aware that it is no longer recommended\, and if they are making a new release of their distribution\, they are strongly encouraged to use something else than Module​::Install.

We already have a breakage with Module​::Install since 5.26 (unless PERL_USE_UNSAFE_INC is set)\, which will necessitate re-release of MI-using distributions before 5.30; the Module​::Install​::DSL failure with INIT blocks is just one more nail in its coffin. The sad and unavoidable fact is that MI's practice of bundling its code into the distribution itself (rather than falling back to the latest cpan release via configure-requires) will continue to cause issues for all distributions that use it.

On Thu\, Dec 28\, 2017 at 12​:43 PM\, Slaven Rezic \slaven@&#8203;rezic\.de wrote​:

Karen Etheridge \perl@&#8203;froods\.org writes​:

Use of Module​::Install is discouraged by the Perl Toolchain Gang\, and it may soon be formally deprecated.

Module​::Install is used by several thousand CPAN distributions. A deprecation seems impractical.

I would prefer the recommendation not simply suggest the author re-release with a newer MI\, but rather convert their distribution off of MI entirely and therefore avoid all other issues arising from its use.

On Wed\, Dec 27\, 2017 at 1​:46 PM\, Zefram \zefram@&#8203;fysh\.org wrote​:

Sawyer X wrote​:

Zefram\, is it possible to provide a succinct way to explain this change to module authors when we submit patches or request the changes made?

"Module​::Install​::DSL versions below 1.19 rely on a core bug\, and are going to stop working on Perl versions above 5.28. To keep your module installable on future Perls\, please rerelease it with the bundled Module​::Install​::DSL updated to version 1.19."

-zefram

-- Slaven Rezic - slaven \ rezic \ de

Berlin Perl Mongers \- http&#8203;://berlin\.pm\.org
p5pRT commented 6 years ago

From @cpansprout

On Thu\, 28 Dec 2017 13​:05​:10 -0800\, perl@​froods.org wrote​:

On Thu\, Dec 28\, 2017 at 12​:43 PM\, Slaven Rezic \slaven@&#8203;rezic\.de wrote​:

Karen Etheridge \perl@&#8203;froods\.org writes​:

Use of Module​::Install is discouraged by the Perl Toolchain Gang\, and it may soon be formally deprecated.

Module​::Install is used by several thousand CPAN distributions. A deprecation seems impractical.

A module being deprecated doesn't mean it will be removed from cpan (unless that has been explicitly stated) -- rather it signals that it is no longer being actively maintained save for critical bug fixes\, which is exactly the case here. Authors must be aware that it is no longer recommended\, and if they are making a new release of their distribution\, they are strongly encouraged to use something else than Module​::Install.

We already have a breakage with Module​::Install since 5.26 (unless PERL_USE_UNSAFE_INC is set)\, which will necessitate re-release of MI- using distributions before 5.30; the Module​::Install​::DSL failure with INIT blocks is just one more nail in its coffin. The sad and unavoidable fact is that MI's practice of bundling its code into the distribution itself (rather than falling back to the latest cpan release via configure-requires) will continue to cause issues for all distributions that use it.

But that is also the reason why deprecating the newest\, fixed version of Module​::Install is not an immediate problem for those several thousand distributions. They are not using it yet.

--

Father Chrysostomos

p5pRT commented 6 years ago

From @grinnz

On Thu\, Dec 28\, 2017 at 4​:43 PM\, Father Chrysostomos via RT \< perlbug-followup@​perl.org> wrote​:

But that is also the reason why deprecating the newest\, fixed version of Module​::Install is not an immediate problem for those several thousand distributions. They are not using it yet.

In this case\, the main benefit of deprecation is forceful documentation that people should really stop using it and switch away from it when possible. This is a benefit because its bundling nature means future breakage can only be truly resolved\, as in this case\, by rereleasing all affected modules\, which is largely impractical for any more widespread issue.

-Dan

p5pRT commented 6 years ago

From @jkeenan

On Thu\, 28 Dec 2017 21​:05​:10 GMT\, perl@​froods.org wrote​:

A module being deprecated doesn't mean it will be removed from cpan (unless that has been explicitly stated) -- rather it signals that it is no longer being actively maintained save for critical bug fixes\, which is exactly the case here. Authors must be aware that it is no longer recommended\, and if they are making a new release of their distribution\, they are strongly encouraged to use something else than Module​::Install.

We already have a breakage with Module​::Install since 5.26 (unless PERL_USE_UNSAFE_INC is set)\, which will necessitate re-release of MI- using distributions before 5.30; the Module​::Install​::DSL failure with INIT blocks is just one more nail in its coffin. The sad and unavoidable fact is that MI's practice of bundling its code into the distribution itself (rather than falling back to the latest cpan release via configure-requires) will continue to cause issues for all distributions that use it.

+1

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 6 years ago

From @jkeenan

On Wed\, 27 Dec 2017 22​:43​:00 GMT\, jkeenan wrote​:

Re-opening ticket per discussion.

We should try to move this ticket and the related https://rt-archive.perl.org/perl5/Ticket/Display.html?id=132577 to resolution. What follows are my understanding of the issues and suggestions for resolution.

1. The ticket was filed eighteen-and-a-half years ago by Tom Christiansen. Discussion proceeded sporadically over the years.

2. Zefram reported solving the bug on Dec 10 2017 (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=2754#txn-1514806) and pushed the solution to blead in commit 0301e899536a22752f40481d8a1d141b7a7dda82.

3. By Dec 12 2017 CPAN breakage had been spotted -- specifically\, in Module​::Install -- and Zefram filed RT 132577. He observed that the breakage point was in Module​::Install​::DSL and opened a bug ticket in M-I's tracker​: https://rt.cpan.org/Ticket/Display.html?id=123867. So that put us in the situation where a legitimate change to blead triggered a failure in dodgy -- but heavily used -- code on CPAN.

4. While Module​::Install could be fixed and was fixed by Karen Etheridge on Dec 19 2017\, many CPAN distributions which embed an unfixed version of Module​::Install​::DSL remain "unfixed" and would need a new CPAN release to embed a "fixed" version of Module​::Install​::DSL. Karen provides a list of those distributions at https://rt.cpan.org/Ticket/Display.html?id=123867#txn-1762016. Most of them are distributions originally written by Adam Kennedy.

5. Back in RT 132577 (the BBC ticket)\, discussion ensued which led to a decision to revert Zefram's fix for RT 2754 for the balance of the 5.27 development cycle and to try again during the (current) 5.29 cycle. Zefram wrote​:

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

In RT 2754 (this ticket)\, he wrote​:

"In order to give Module​::Install distros more time to roll out a fix for [perl #132577]\, this bug [perl #2754] has been reintroduced in commit 857320cbf85e762add18885ae8a197b5e0c21b69."

Pumpking Sawyer X concurred\, writing​:

"Following this\, we should formulate a plan for dealing with these distributions and having them release new versions. We have time but our goal is to still have this bug fix in the next version [5.30.0 -- JK] and this could only be done if we are able to have enough distributions released with the fix."

6. The complicating factor in giving CPAN distributions using Module​::Install​::DSL "more time to roll out a fix for" RT 132577 is that many experienced contributors (e.g.\, the Perl Toolchain Gang) believe that Module​::Install is fundamentally unmaintainable over the long run; that should be deprecated for new usage in CPAN distros; and that any advice to authors of those distros should not only advise using an upgraded Module​::Install​::DSL but should also advise moving away from Module​::Install entirely.

7. In an attempt to get some empirical data to inform this discussion\, I embarked on the following research​:

a. I created a branch\, "smoke-me/jkeenan/2754-re-revert"\, from blead at commit 9d6ed46\, i.e.\, shortly after the release of perl-5.27.3. In that branch I reverted Zefram's reversion commit and nothing more (see b64ae69c). Smoke-testing rigs picked that branch up (http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2F2754-re-revert) with satisfactory results\, i.e.\, the rigs which are getting FAILs are not doing so because of this commit and are also getting FAILs in blead.

b. I then wrote a program similar to the "test CPAN river against dev releases" program upon which I have been reporting to the P5P list since June. I configured and built perl from the 2754-re-revert branch on a FreeBSD-11 machine\, installed cpanm against that perl and then attempted to install a large list of CPAN modules against that perl. That list was composed as follows​:

i. Start with the same "CPAN river 3000" modules I've been using in monthly test-against-dev testing.

ii. Remove those modules which got a non-PASS grade in last week's post-5.29.3 run. Among others\, this removed Catalyst-Runtime and its revdeps which were experiencing non-BBC-related issues.

iii. Add the CPAN distributions listed as being revdeps of Module​::Install at https://metacpan.org/requires/distribution/Module-Install?p=1&size=200&sort=%5B%5B2%2C1%5D%5D.

iv. Remove the M-I revdeps whose names begin with 'Task' or 'Bundle'.

c. I ran the program yesterday in an environment similar but not identical to that which I use for the monthly test-against-dev runs. The difference is that in this environment I have not installed as many external packages as I have in the test-against-dev VM\, so there were more non-PASS grades due to unsatisfied prerequisites.

d. I examined the resulting data for the purpose of determining the cause of non-PASS grades when run against the "2754-re-revert" perl. I identified 12 distros which were graded 'NA' due to failures in the configuration stage. All 12 of those distros were found in the list of distros with unfixed embedded Module​::Install​::DSL provided by Karen last December. When I examined these distros individually and ran them with the "stock" perl on this machine -- 5.26.2 -- each failed with "Can't locate inc/Module/Install/DSL.pm in @​INC". When I ran them with the post-5.23.4\, 2754-re-revert perl\, "perl Makefile.PL" created no Makefile -- which explains the configuration-stage NA grade. I could not attribute any other non-PASS grades in this run to RT-2754 related issues. (I can make the data and program available upon request.)

8. I therefore believe that we can predict with reasonable certainty what would happen if\, in the current dev cycle\, we re-implemented Zefram's fix for this ticket. Those CPAN distributions which have not upgraded their embedded Module​::Install​::DSL packages would break because the distros would no longer be able to generate a Makefile. But beyond that there would be little impact.

9. I therefore believe that we should move forward on this. To do so we would need to take the following steps​:

a. Re-examine the chain of commits to determine whether the 2754-re-revert branch fully gets us to the state intended by Zefram for the resolution of the original problem.

b. Come to consensus among P5P and Toolchain Gang as to what to recommend to the maintainers of CPAN distributions with non-upgraded versions of Module​::Install​::DSL concerning this breakage and advise them that the distros will not work in perl-5.30 unless they take action.

c. Merge the 2754-re-revert branch -- or an improved version thereof -- into blead and monitor results via CPANtesters\, test-against-dev and any other appropriate means. That should enable resolution of both RT 2754 and RT 132577.

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 6 years ago

From @jkeenan

On Tue\, 25 Sep 2018 14​:52​:40 GMT\, jkeenan wrote​:

On Wed\, 27 Dec 2017 22​:43​:00 GMT\, jkeenan wrote​:

Re-opening ticket per discussion.

We should try to move this ticket and the related https://rt-archive.perl.org/perl5/Ticket/Display.html?id=132577 to resolution. What follows are my understanding of the issues and suggestions for resolution.

1. The ticket was filed eighteen-and-a-half years ago by Tom Christiansen. Discussion proceeded sporadically over the years.

2. Zefram reported solving the bug on Dec 10 2017 (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=2754#txn-1514806) and pushed the solution to blead in commit 0301e899536a22752f40481d8a1d141b7a7dda82.

3. By Dec 12 2017 CPAN breakage had been spotted -- specifically\, in Module​::Install -- and Zefram filed RT 132577. He observed that the breakage point was in Module​::Install​::DSL and opened a bug ticket in M-I's tracker​: https://rt.cpan.org/Ticket/Display.html?id=123867. So that put us in the situation where a legitimate change to blead triggered a failure in dodgy -- but heavily used -- code on CPAN.

4. While Module​::Install could be fixed and was fixed by Karen Etheridge on Dec 19 2017\, many CPAN distributions which embed an unfixed version of Module​::Install​::DSL remain "unfixed" and would need a new CPAN release to embed a "fixed" version of Module​::Install​::DSL. Karen provides a list of those distributions at https://rt.cpan.org/Ticket/Display.html?id=123867#txn-1762016. Most of them are distributions originally written by Adam Kennedy.

5. Back in RT 132577 (the BBC ticket)\, discussion ensued which led to a decision to revert Zefram's fix for RT 2754 for the balance of the 5.27 development cycle and to try again during the (current) 5.29 cycle. Zefram wrote​:

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

In RT 2754 (this ticket)\, he wrote​:

"In order to give Module​::Install distros more time to roll out a fix for [perl #132577]\, this bug [perl #2754] has been reintroduced in commit 857320cbf85e762add18885ae8a197b5e0c21b69."

Pumpking Sawyer X concurred\, writing​:

"Following this\, we should formulate a plan for dealing with these distributions and having them release new versions. We have time but our goal is to still have this bug fix in the next version [5.30.0 -- JK] and this could only be done if we are able to have enough distributions released with the fix."

6. The complicating factor in giving CPAN distributions using Module​::Install​::DSL "more time to roll out a fix for" RT 132577 is that many experienced contributors (e.g.\, the Perl Toolchain Gang) believe that Module​::Install is fundamentally unmaintainable over the long run; that should be deprecated for new usage in CPAN distros; and that any advice to authors of those distros should not only advise using an upgraded Module​::Install​::DSL but should also advise moving away from Module​::Install entirely.

7. In an attempt to get some empirical data to inform this discussion\, I embarked on the following research​:

a. I created a branch\, "smoke-me/jkeenan/2754-re-revert"\, from blead at commit 9d6ed46\, i.e.\, shortly after the release of perl-5.27.3. In that branch I reverted Zefram's reversion commit and nothing more (see b64ae69c). Smoke-testing rigs picked that branch up (http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2F2754-re-revert) with satisfactory results\, i.e.\, the rigs which are getting FAILs are not doing so because of this commit and are also getting FAILs in blead.

b. I then wrote a program similar to the "test CPAN river against dev releases" program upon which I have been reporting to the P5P list since June. I configured and built perl from the 2754-re-revert branch on a FreeBSD-11 machine\, installed cpanm against that perl and then attempted to install a large list of CPAN modules against that perl. That list was composed as follows​:

i. Start with the same "CPAN river 3000" modules I've been using in monthly test-against-dev testing.

ii. Remove those modules which got a non-PASS grade in last week's post-5.29.3 run. Among others\, this removed Catalyst-Runtime and its revdeps which were experiencing non-BBC-related issues.

iii. Add the CPAN distributions listed as being revdeps of Module​::Install at https://metacpan.org/requires/distribution/Module- Install?p=1&size=200&sort=%5B%5B2%2C1%5D%5D.

iv. Remove the M-I revdeps whose names begin with 'Task' or 'Bundle'.

c. I ran the program yesterday in an environment similar but not identical to that which I use for the monthly test-against-dev runs. The difference is that in this environment I have not installed as many external packages as I have in the test-against-dev VM\, so there were more non-PASS grades due to unsatisfied prerequisites.

d. I examined the resulting data for the purpose of determining the cause of non-PASS grades when run against the "2754-re-revert" perl. I identified 12 distros which were graded 'NA' due to failures in the configuration stage. All 12 of those distros were found in the list of distros with unfixed embedded Module​::Install​::DSL provided by Karen last December. When I examined these distros individually and ran them with the "stock" perl on this machine -- 5.26.2 -- each failed with "Can't locate inc/Module/Install/DSL.pm in @​INC". When I ran them with the post-5.23.4\, 2754-re-revert perl\, "perl Makefile.PL" created no Makefile -- which explains the configuration-stage NA grade. I could not attribute any other non-PASS grades in this run to RT-2754 related issues. (I can make the data and program available upon request.)

8. I therefore believe that we can predict with reasonable certainty what would happen if\, in the current dev cycle\, we re-implemented Zefram's fix for this ticket. Those CPAN distributions which have not upgraded their embedded Module​::Install​::DSL packages would break because the distros would no longer be able to generate a Makefile. But beyond that there would be little impact.

9. I therefore believe that we should move forward on this. To do so we would need to take the following steps​:

a. Re-examine the chain of commits to determine whether the 2754-re-revert branch fully gets us to the state intended by Zefram for the resolution of the original problem.

b. Come to consensus among P5P and Toolchain Gang as to what to recommend to the maintainers of CPAN distributions with non-upgraded versions of Module​::Install​::DSL concerning this breakage and advise them that the distros will not work in perl-5.30 unless they take action.

c. Merge the 2754-re-revert branch -- or an improved version thereof -- into blead and monitor results via CPANtesters\, test-against-dev and any other appropriate means. That should enable resolution of both RT 2754 and RT 132577.

Thank you very much.

TonyC\, DaveM\, list​: Any feedback on this plan?

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 6 years ago

From @jkeenan

On Sat\, 29 Sep 2018 23​:37​:56 GMT\, jkeenan wrote​:

On Tue\, 25 Sep 2018 14​:52​:40 GMT\, jkeenan wrote​:

On Wed\, 27 Dec 2017 22​:43​:00 GMT\, jkeenan wrote​:

Re-opening ticket per discussion.

We should try to move this ticket and the related https://rt-archive.perl.org/perl5/Ticket/Display.html?id=132577 to resolution. What follows are my understanding of the issues and suggestions for resolution.

1. The ticket was filed eighteen-and-a-half years ago by Tom Christiansen. Discussion proceeded sporadically over the years.

2. Zefram reported solving the bug on Dec 10 2017 (https://rt-archive.perl.org/perl5/Ticket/Display.html?id=2754#txn-1514806) and pushed the solution to blead in commit 0301e899536a22752f40481d8a1d141b7a7dda82.

3. By Dec 12 2017 CPAN breakage had been spotted -- specifically\, in Module​::Install -- and Zefram filed RT 132577. He observed that the breakage point was in Module​::Install​::DSL and opened a bug ticket in M-I's tracker​: https://rt.cpan.org/Ticket/Display.html?id=123867. So that put us in the situation where a legitimate change to blead triggered a failure in dodgy -- but heavily used -- code on CPAN.

4. While Module​::Install could be fixed and was fixed by Karen Etheridge on Dec 19 2017\, many CPAN distributions which embed an unfixed version of Module​::Install​::DSL remain "unfixed" and would need a new CPAN release to embed a "fixed" version of Module​::Install​::DSL. Karen provides a list of those distributions at https://rt.cpan.org/Ticket/Display.html?id=123867#txn-1762016. Most of them are distributions originally written by Adam Kennedy.

5. Back in RT 132577 (the BBC ticket)\, discussion ensued which led to a decision to revert Zefram's fix for RT 2754 for the balance of the 5.27 development cycle and to try again during the (current) 5.29 cycle. Zefram wrote​:

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

In RT 2754 (this ticket)\, he wrote​:

"In order to give Module​::Install distros more time to roll out a fix for [perl #132577]\, this bug [perl #2754] has been reintroduced in commit 857320cbf85e762add18885ae8a197b5e0c21b69."

Pumpking Sawyer X concurred\, writing​:

"Following this\, we should formulate a plan for dealing with these distributions and having them release new versions. We have time but our goal is to still have this bug fix in the next version [5.30.0 -- JK] and this could only be done if we are able to have enough distributions released with the fix."

6. The complicating factor in giving CPAN distributions using Module​::Install​::DSL "more time to roll out a fix for" RT 132577 is that many experienced contributors (e.g.\, the Perl Toolchain Gang) believe that Module​::Install is fundamentally unmaintainable over the long run; that should be deprecated for new usage in CPAN distros; and that any advice to authors of those distros should not only advise using an upgraded Module​::Install​::DSL but should also advise moving away from Module​::Install entirely.

7. In an attempt to get some empirical data to inform this discussion\, I embarked on the following research​:

a. I created a branch\, "smoke-me/jkeenan/2754-re-revert"\, from blead at commit 9d6ed46\, i.e.\, shortly after the release of perl-5.27.3. In that branch I reverted Zefram's reversion commit and nothing more (see b64ae69c). Smoke-testing rigs picked that branch up (http​://perl.develop-help.com/?b=smoke-me%2Fjkeenan%2F2754-re-revert) with satisfactory results\, i.e.\, the rigs which are getting FAILs are not doing so because of this commit and are also getting FAILs in blead.

b. I then wrote a program similar to the "test CPAN river against dev releases" program upon which I have been reporting to the P5P list since June. I configured and built perl from the 2754-re-revert branch on a FreeBSD-11 machine\, installed cpanm against that perl and then attempted to install a large list of CPAN modules against that perl. That list was composed as follows​:

i. Start with the same "CPAN river 3000" modules I've been using in monthly test-against-dev testing.

ii. Remove those modules which got a non-PASS grade in last week's post-5.29.3 run. Among others\, this removed Catalyst-Runtime and its revdeps which were experiencing non-BBC-related issues.

iii. Add the CPAN distributions listed as being revdeps of Module​::Install at https://metacpan.org/requires/distribution/Module- Install?p=1&size=200&sort=%5B%5B2%2C1%5D%5D.

iv. Remove the M-I revdeps whose names begin with 'Task' or 'Bundle'.

c. I ran the program yesterday in an environment similar but not identical to that which I use for the monthly test-against-dev runs. The difference is that in this environment I have not installed as many external packages as I have in the test-against-dev VM\, so there were more non-PASS grades due to unsatisfied prerequisites.

d. I examined the resulting data for the purpose of determining the cause of non-PASS grades when run against the "2754-re-revert" perl. I identified 12 distros which were graded 'NA' due to failures in the configuration stage. All 12 of those distros were found in the list of distros with unfixed embedded Module​::Install​::DSL provided by Karen last December. When I examined these distros individually and ran them with the "stock" perl on this machine -- 5.26.2 -- each failed with "Can't locate inc/Module/Install/DSL.pm in @​INC". When I ran them with the post-5.23.4\, 2754-re-revert perl\, "perl Makefile.PL" created no Makefile -- which explains the configuration-stage NA grade. I could not attribute any other non-PASS grades in this run to RT-2754 related issues. (I can make the data and program available upon request.)

8. I therefore believe that we can predict with reasonable certainty what would happen if\, in the current dev cycle\, we re-implemented Zefram's fix for this ticket. Those CPAN distributions which have not upgraded their embedded Module​::Install​::DSL packages would break because the distros would no longer be able to generate a Makefile. But beyond that there would be little impact.

9. I therefore believe that we should move forward on this. To do so we would need to take the following steps​:

a. Re-examine the chain of commits to determine whether the 2754-re-revert branch fully gets us to the state intended by Zefram for the resolution of the original problem.

b. Come to consensus among P5P and Toolchain Gang as to what to recommend to the maintainers of CPAN distributions with non-upgraded versions of Module​::Install​::DSL concerning this breakage and advise them that the distros will not work in perl-5.30 unless they take action.

c. Merge the 2754-re-revert branch -- or an improved version thereof -- into blead and monitor results via CPANtesters\, test-against-dev and any other appropriate means. That should enable resolution of both RT 2754 and RT 132577.

Thank you very much.

TonyC\, DaveM\, list​: Any feedback on this plan?

Thank you very much.

sawyer\, list​: Any feedback on this plan?

Thank you very much.

-- James E Keenan (jkeenan@​cpan.org)

p5pRT commented 6 years ago

From @iabyn

On Sat\, Sep 29\, 2018 at 04​:37​:56PM -0700\, James E Keenan via RT wrote​:

TonyC\, DaveM\, list​: Any feedback on this plan?

The world has managed to live with this bug for at least 18 years\, so I see no need to rush to get the fix in for 5.30.

-- The crew of the Enterprise encounter an alien life form which is surprisingly neither humanoid nor made from pure energy.   -- Things That Never Happen in "Star Trek" #22

p5pRT commented 5 years ago

From @iabyn

I've moved this ticket from 5.30.0 blocker to 5.32.0 blocker

p5pRT commented 5 years ago

From @tonycoz

On Tue\, 23 Apr 2019 07​:49​:17 -0700\, davem wrote​:

I've moved this ticket from 5.30.0 blocker to 5.32.0 blocker

Here's a revert of the revert patch.

I'll apply this next week\, unless there's an objection.

Tony

p5pRT commented 5 years ago

From @tonycoz

0001-Revert-postpone-perl_parse-exit-0-bugfix.patch ```diff From ce6cbbca8ada5c17a3c34121fd8b896917c1a250 Mon Sep 17 00:00:00 2001 From: Tony Cook Date: Thu, 4 Jul 2019 15:02:46 +1000 Subject: Revert "postpone perl_parse() exit(0) bugfix" This reverts commit 857320cbf85e762add18885ae8a197b5e0c21b69, re-instating the [perl #2754] fix, which was reverted in late 2017 to allow Module::Install based distributions to update or re-work per [perl #132577]. # Conflicts: # t/op/blocks.t --- perl.c | 27 ++++++++------------------- t/op/blocks.t | 12 ++++++------ 2 files changed, 14 insertions(+), 25 deletions(-) diff --git a/perl.c b/perl.c index 2e80cfe940..aeb259676e 100644 --- a/perl.c +++ b/perl.c @@ -1624,16 +1624,13 @@ For historical reasons, the non-zero return value also attempts to be a suitable value to pass to the C library function C (or to return from C
), to serve as an exit code indicating the nature of the way initialisation terminated. However, this isn't portable, -due to differing exit code conventions. A historical bug is preserved -for the time being: if the Perl built-in C 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 being called (and therefore C blocks and the -main program running) despite a call to C. 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. +due to differing exit code conventions. An attempt is made to return +an exit code of the type required by the host operating system, but +because it is constrained to be non-zero, it is not necessarily possible +to indicate every type of exit. It is only reliable on Unix, where a +zero exit code can be augmented with a set bit that will be ignored. +In any case, this function is not the correct place to acquire an exit +code: one should get that from L. =cut */ @@ -1842,15 +1839,7 @@ perl_parse(pTHXx_ XSINIT_t xsinit, int argc, char **argv, char **env) call_list(oldscope, PL_checkav); } ret = STATUS_EXIT; - 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]. - */ - } + if (ret == 0) ret = 0x100; break; case 3: PerlIO_printf(Perl_error_log, "panic: top_env\n"); diff --git a/t/op/blocks.t b/t/op/blocks.t index 1fb369a1a1..ea6ca4d144 100644 --- a/t/op/blocks.t +++ b/t/op/blocks.t @@ -167,23 +167,23 @@ SKIP: { skip "VMS doesn't have the perl #2754 bug", 3 if $^O eq 'VMS'; fresh_perl_is( "$testblocks BEGIN { exit 0; }", - "begin\nunitcheck\ncheck\ninit\nend", + "begin\nunitcheck\ncheck\nend", {}, - "BEGIN{exit 0} doesn't exit yet" + "BEGIN{exit 0} should exit" ); fresh_perl_is( "$testblocks UNITCHECK { exit 0; }", - "begin\nunitcheck\ncheck\ninit\nmain\nend", + "begin\nunitcheck\ncheck\nend", {}, - "UNITCHECK{exit 0} doesn't exit yet" + "UNITCHECK{exit 0} should exit" ); fresh_perl_is( "$testblocks CHECK { exit 0; }", - "begin\nunitcheck\ncheck\ninit\nmain\nend", + "begin\nunitcheck\ncheck\nend", {}, - "CHECK{exit 0} doesn't exit yet" + "CHECK{exit 0} should exit" ); } -- 2.11.0 ```
p5pRT commented 5 years ago

From @tonycoz

On Wed\, 03 Jul 2019 22​:05​:40 -0700\, tonyc wrote​:

On Tue\, 23 Apr 2019 07​:49​:17 -0700\, davem wrote​:

I've moved this ticket from 5.30.0 blocker to 5.32.0 blocker

Here's a revert of the revert patch.

I'll apply this next week\, unless there's an objection.

Applied as 2773b4f50f991900e38d33daace2b9c6a0902c6a.

I expect this will cause some distributions with outdated Module​::Install​::DSL to fail\, hopefully they can be updated before 5.32.0

Tony