Skip to content

BBC: LNATION/Devel-Required-0.15.tar.gz fails with perl 5.31.10 #17681

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
eserte opened this issue Mar 29, 2020 · 23 comments
Open

BBC: LNATION/Devel-Required-0.15.tar.gz fails with perl 5.31.10 #17681

eserte opened this issue Mar 29, 2020 · 23 comments
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)

Comments

@eserte
Copy link
Contributor

eserte commented Mar 29, 2020

See http://fast-matrix.cpantesters.org/?dist=Devel-Required%200.15 --- there are no pass reports with released 5.31.10 (though there are some passes for pre-release versions of 5.31.10).
@eserte eserte added BBC Blead Breaks CPAN - changes in blead broke a cpan module(s) Needs Triage affects-5.31 labels Mar 29, 2020
@hvds
Copy link
Contributor

hvds commented Mar 29, 2020 •
edited
Loading

The code does this in a BEGIN block to find the WriteMakefile() sub to overload:

    my $subname= caller() . '::WriteMakefile';

.. and the caller() is now returning Devel::Required; changing that to caller(2) allows tests to pass (but I don't know offhand if that works on older perls).

@iabyn this appears to be further fallout from "avoid identical stack traces". I'm a bit concerned that problems like this may affect a bunch of non-CPAN code too, and could be hard for people to diagnose without conscious awareness of the change. Is there anything we can do to mitigate that?

[Edited "caller(2)" to "caller(1)" after deciding my diagnostics had introduced an extra stack frame, then back again when I found that was wrong.]

@iabyn
Copy link
Contributor

iabyn commented Mar 30, 2020 via email

@hvds
Copy link
Contributor

hvds commented Mar 30, 2020

On Sun, Mar 29, 2020 at 07:30:40AM -0700, Hugo van der Sanden wrote:

I'm a bit concerned that problems like this may affect a bunch of non-CPAN code too, and could be hard for people to diagnose without conscious awareness of the change. Is there anything we can do to mitigate that?

Not really, apart from pulling the fix altogether. Previously perl screwed up the stack backtrace within BEGINs; now caller() returns the correct values. Seems like a binary choice - keep the fix, or revert to the broken behaviour for backwards compatibility. Of course if we revert now, we screw up any distributions which have just been fixed to match blead behaviour.

Ok, I've opened https://rt.cpan.org/Ticket/Display.html?id=132269 against Devel::Required.

I think we'll need at least to highlight this prominently in 5.32 perldelta.

@khwilliamson khwilliamson added this to the 5.32.0 milestone Mar 30, 2020
@karenetheridge
Copy link
Member

Is the underlying cause of this the same as in #17663?

@hvds
Copy link
Contributor

hvds commented Apr 9, 2020

Is the underlying cause of this the same as in #17663?

#17663 appears to be 3 distinct failures. From the information in that ticket, I'd say that CGI-ProgressBar is almost certainly the same as the case here; App-Framework-Lite is probably the same; and for DBIx-Class I have no clue.

@Leont
Copy link
Contributor

Leont commented Apr 13, 2020

Is the underlying cause of this the same as in #17663?

Yes.

@karenetheridge
Copy link
Member

karenetheridge commented Apr 15, 2020 •
edited
Loading

@iabyn this appears to be further fallout from "avoid identical stack traces". I'm a bit concerned that problems like this may affect a bunch of non-CPAN code too, and could be hard for people to diagnose without conscious awareness of the change. Is there anything we can do to mitigate that?

Not really, apart from pulling the fix altogether. Previously perl screwed up the stack backtrace within BEGINs; now caller() returns the correct values. Seems like a binary choice - keep the fix, or revert to the broken behaviour for backwards compatibility. Of course if we revert now, we screw up any distributions which have just been fixed to match blead behaviour.

As per #17663 (comment), there is still an open question as to whether this is a fix at all, or an undesired behaviour. I don't know the answer, but this was a thoughtful post and no one has responded to the concerns raised yet.

@xsawyerx
Copy link
Member

This is, IMHO, a correct fix, as I expressed in #17663.

@jkeenan
Copy link
Contributor

jkeenan commented Apr 26, 2020 via email

@khwilliamson
Copy link
Contributor

Is this now not a blocker, since commits for https://github.com/Perl/perl5/issues/17663 have been reverted?

@iabyn
Copy link
Contributor

iabyn commented Apr 29, 2020 via email

@eserte
Copy link
Contributor Author

eserte commented Apr 30, 2020

Confirmed. The distribution's test suite passes on my smokers with perl 5.31.11.

@khwilliamson khwilliamson removed this from the 5.32.0 milestone Apr 30, 2020
@xenu xenu removed the affects-5.31 label Nov 19, 2021
@eserte
Copy link
Contributor Author

eserte commented Mar 13, 2022 •
edited
Loading

The test suite of Devel-Required-0.16 started to fail again. See http://www.cpantesters.org/cpan/report/5ea15b0a-9549-11ec-a601-45351f24ea8f (the reported state is "unknown" for some reason, but it should really be "fail").

@jkeenan
Copy link
Contributor

jkeenan commented Mar 13, 2022

The test suite of Devel-Required-0.16 started to fail again. See http://www.cpantesters.org/cpan/report/5ea15b0a-9549-11ec-a601-45351f24ea8f (the reported state is "unknown" for some reason, but it should really be "fail").

Manual inspection points to commit f6387cf as the cause of the failures.

commit f6387cff9cb31db4cf18c8641917ea4639ac2b65
Author:     Yves Orton <[email protected]>
AuthorDate: Mon Feb 14 07:34:54 2022 +0100
Commit:     Paul Evans <[email protected]>
CommitDate: Sat Feb 19 11:23:43 2022 +0000

    Reapply squashed "avoid identical stack traces" patches
    
    This reapplies two reverted patches as a single squashed commit.

At the immediately preceding commit, Devel-Required PASSes its tests, albeit with some peculiar exiting.

cp lib/Devel/Required.pm blib/lib/Devel/Required.pm
PERL_DL_NONLAZY=1 "/usr/home/jkeenan/testing/f6387cff9cb31db4cf18c8641917ea4639ac2b65^/bin/perl" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harness(0, 'blib/lib', 'blib/arch')" t/*.t
t/001basic.t ......... ok
t/010specific.t ...... ok
t/020installation.t .. ok
All tests successful.
Files=3, Tests=76,  2 wallclock secs ( 0.04 usr  0.01 sys +  0.70 cusr  0.12 csys =  0.86 CPU)
Result: PASS
make: don't know how to make install. Stop

make: stopped in /usr/home/jkeenan/.cpanm/work/1647195442.32111/Devel-Required-0.16
-> FAIL Installing Devel::Required failed. See /home/jkeenan/.cpanm/work/1647195442.32111/build.log for details. Retry with --force to force install it.

This module is, IMO, too clever for its own good. make test removes the Makefile, which probably explains the don't know how to make install message. You get a similar message for make clean. I could not get this to work properly in Porting/bisect.pl; I suspect the module's cleverness is behind that as well.

But at least at that commit the tests are PASSing. At the following commit -- which is a revert of a revert -- the tests FAIL in the same manner they were two years ago.

t/001basic.t       (Wstat: 1536 Tests: 36 Failed: 6)
  Failed tests:  16, 19, 23, 26, 30, 33
  Non-zero exit status: 6
t/010specific.t    (Wstat: 512 Tests: 20 Failed: 2)
  Failed tests:  15, 18
  Non-zero exit status: 2
t/020installation.t (Wstat: 512 Tests: 20 Failed: 2)
  Failed tests:  15, 18
  Non-zero exit status: 2
Files=3, Tests=76,  1 wallclock secs ( 0.02 usr  0.02 sys +  0.63 cusr  0.15 csys =  0.82 CPU)
Result: FAIL
Failed 3/3 test programs. 10/76 subtests failed

The CPAN maintainer has never replied to the ticket which @hvds created two years ago.

How we should proceed is unclear?

@demerphq
Copy link
Collaborator

demerphq commented Mar 14, 2022 via email

@demerphq
Copy link
Collaborator

demerphq commented Mar 14, 2022 via email

@demerphq
Copy link
Collaborator

demerphq commented Mar 14, 2022 via email

@Leont
Copy link
Contributor

Leont commented Mar 14, 2022

I think you sent those two tickets to the wrong queue

@demerphq
Copy link
Collaborator

demerphq commented Mar 14, 2022 via email

@demerphq
Copy link
Collaborator

demerphq commented Mar 14, 2022 via email

@haarg
Copy link
Contributor

haarg commented Mar 14, 2022

Unfortunately there are two buttons. The "Report a new bug" button is the correct one, which pre-selects the current queue. The "Create new ticket" button doesn't, so it just ends up with the first one, "0.05", and the wrong button is the one highlighted in the UI. I believe it's been reported to Best Practical, but it hasn't been fixed.

@demerphq
Copy link
Collaborator

demerphq commented Mar 14, 2022 via email

@rjbs
Copy link
Member

rjbs commented Apr 2, 2022

I have marked this not-a-blocker. A fix has been sent upstream, and the library does not appear high up the river.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
BBC Blead Breaks CPAN - changes in blead broke a cpan module(s)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests