Skip to content
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.

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

Avoid the use and need of warnings FATAL in t/destinations.t #1

Open
wollmers opened this issue Jan 2, 2019 · 9 comments
Open

Avoid the use and need of warnings FATAL in t/destinations.t #1

wollmers opened this issue Jan 2, 2019 · 9 comments

Comments

@wollmers
Copy link

wollmers commented Jan 2, 2019

MacOS 10.14.1
perlbrew
cperl5.28.1
cpanm install

Relevant parts of build.log

Building and testing ExtUtils-InstallPaths-0.012
cp lib/ExtUtils/InstallPaths.pm blib/lib/ExtUtils/InstallPaths.pm
Manifying 1 pod document
PERL_DL_NONLAZY=1 "/Users/helmut/perl5/perlbrew/perls/cperl-5.28.1/bin/cperl5.28.1" "-MExtUtils::Command::MM" "-MTest::Harness" "-e" "undef *Test::Harness::Switches; test_harn
ess(0, 'blib/lib', 'blib/arch')" t/*.t
No autovivification of hash slice anymore at t/destinations.t line 33.
# Looks like your test exited with 255 before it could output anything.
t/destinations.t .. 
Dubious, test returned 255 (wstat 65280, 0xff00)
Failed 111/111 subtests 

Test Summary Report
-------------------
t/destinations.t (Wstat: 65280 Tests: 0 Failed: 0)
  Non-zero exit status: 255
  Parse errors: Bad plan.  You planned 111 tests but ran 0.
Files=1, Tests=0,  0 wallclock secs ( 0.01 usr  0.01 sys +  0.04 cusr  0.01 csys =  0.07 CPU)
Result: FAIL
Failed 1/1 test programs. 0/0 subtests failed.
@Leont
Copy link
Member

Leont commented Jan 2, 2019

That slice shouldn't autovivify in the first place, both elements should exist.

Can you tell me the output of:

perl -V:version
perl -V:archname

@wollmers
Copy link
Author

wollmers commented Jan 2, 2019

Is there a serious reason to use warnings FATAL?

$ perl -V:version
version='5.28.1';

$ perl -V:archname
archname='darwin-2level';

$ perl -v

This is cperl 5, version 28, subversion 1 (v5.28.1c) built for darwin-2level

Copyright 1987-2018, Larry Wall
cperl Copyright (c) 2012-2017 cPanel Inc, 2017-2018 Reini Urban

Perl may be copied only under the terms of either the Artistic License or the
GNU General Public License, which may be found in the Perl 5 source kit.

Complete documentation for Perl, including FAQ lists, should be found on
this system using "man perl" or "perldoc perl".  If you have access to the
Internet, point your browser at http://www.perl.org/, the Perl Home Page.

See also:
No autovivification of hash slice anymore
Fatal Warnings are a Ticking Time Bomb

@Leont
Copy link
Member

Leont commented Jan 2, 2019

Is there a serious reason to use warnings FATAL?

No there isn't, but unfatalizing the warning doesn't eliminate the problem it's pointing at.

Either something is wrong with my code, or something is wrong with cperl, and I'd like to know which.

@Leont
Copy link
Member

Leont commented Jan 2, 2019

On further inspection it seems the cperl warning is wrong (it warns for any slice-as-argument, not only when it actually autovivifies). This warning is decidedly non-helpful.

@wollmers
Copy link
Author

wollmers commented Jan 3, 2019

Sure, unfatalizing doesn't eliminate the problem. But I do not see the benefit of FATAL, especially in a test script. Maybe it makes sense in a developer release during a phase on CPAN-testers.

At the user (sysadmin, cpan author, end user) it crashes, the installation fails. This can happen with new versions of perl5 or other implementations inventing new warnings.

What will the user do?

  • blame EU-IP
  • blame the module which has EU-IP in the dependencies
  • blame the version or implementation of perl
  • install with notest or force, which can potentially hide serious problems in other modules
  • give up
  • a minority will submit an issue, because it takes time, especially the longish discussions with some authors

If it's unfatalized the test still throws a warning, but the installation works. A minority can still submit an issue.

The pragmatic sense of a warning is giving a hint about a potential problem. In this case it warns at compile time that that autovivification of slice-as-argument is not supported by cperl, while P5P-perl still supports this buggy (inconsistent behaviour, bug not solved in 19 years) feature. Using no warnings 'syntax';in a narrow lexical context will suppress the warning in all perls.

I agree, that this warning is a little bit unlucky. But it is AFAIK only in cperl-5.28.x and will be removed in cperl-5.30.

@Leont
Copy link
Member

Leont commented Jan 3, 2019

But I do not see the benefit of FATAL, especially in a test script. Maybe it makes sense in a developer release during a phase on CPAN-testers.

To the contrary, tests are the only place where it makes sense. In fact, it's probably better to use a module like Test::Warnings to make sure all warnings generate a test failure, instead of only ones in the test file.

Tests are exactly the right time to be strict.

What will the user do?

You're forgetting one option:

  • Not use cperl when it breaks their software.

@wollmers
Copy link
Author

wollmers commented Jan 3, 2019

I see, you are putting more energy in defending than improving.

I want my modules as portable and stable as possible, including cperl.

@Leont
Copy link
Member

Leont commented Jan 5, 2019

I see, you are putting more energy in defending than improving.

Quite frankly I find making this personal incredibly rude.

You come here not with a problem, but dive straight towards a solution. I rejected that solution, but not without explaining why. You're free to suggest other solutions to your problem, but this accusatory attitude is not appreciated.

@wollmers
Copy link
Author

wollmers commented Jan 6, 2019

That's your point of view, mine is opposite.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants