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
Bleadperl v5.21.8-197-g5fe499a breaks 5 x CPAN #14474
Comments
From @andkbisect commit 5fe499a [perl #123218] "preserve" $/ if set to a bad value affected TLINDEN/Config-General-2.56.tar.gz sample reports http://www.cpantesters.org/cpan/report/b6273d06-ac60-11e4-a95c-f07e8971dd2f perl -V Summary of my perl5 (revision 5 version 21 subversion 9) configuration: Characteristics of this binary (from libperl): -- |
From @andkalso affected: ROBAU/Slurp-0.4.tar.gz HMBRAND/Spreadsheet-Read-0.56.tgz -- |
From @TuxOn Sun, 08 Feb 2015 08:11:49 +0100, Andreas Koenig
Text::CSV_XS still PASSes, but with a new worrying message, but only t/91_csv_cb.t ..... 1/20 Attempt to free unreferenced scalar: SV 0x1210148, Perl interpreter: 0xbe2010 during global destruction. Which it does not if run with prove (20 runs in a row) or on second or And yest, Text::CSV_XS does change $/ As Spreadsheet::Read doesn't do anything "special" with $/, it looks -- |
The RT System itself - Status changed from 'new' to 'open' |
From @jkeenanOn Sun Feb 08 03:31:42 2015, hmbrand wrote:
It would be unfortunate if Text::CSV_XS's behavior were adversely affected. I have one CPAN library based on Text::CSV and in its use I have never had to pay attention to the difference between the pure-perl and XS versions. And we have code at $job which uses Text::CSV, again without having to trouble itself as to which implementation is being invoked. Hence, this problem would be a blocker for us to go to perl-5.22. I encourage those who have the knowledge to deal with this problem to prioritize doing so. Thank you very much. -- |
From @TuxOn Sun, 8 Feb 2015 06:12:54 -0800, "James E Keenan via RT"
The patch resets $/ to PL_rs *ON EVERY GET* I opt to revert and find a better solution -- |
From @cpansproutOn Sun Feb 08 06:35:05 2015, hmbrand wrote:
So do I, *but* your code would be more robust if you did SvSETMAGIC on $/ after fiddling with it. (Am I right in assuming you skipped that?) -- Father Chrysostomos |
From @TuxOn Sun, 8 Feb 2015 07:14:11 -0800, "Father Chrysostomos via RT"
if (csv.eolx || csv.eol_is_cr) { It doesn't need setmagic, but removemagic, right? -- |
From @TuxOn Sun, 8 Feb 2015 17:39:42 +0100, "H.Merijn Brand"
FWIW, adding MAGIC still passes Text::CSV_XS' test suite, but still if (csv.eolx || csv.eol_is_cr) { -- |
From @TuxOn Sun, 8 Feb 2015 06:12:54 -0800, "James E Keenan via RT"
I have a fix for Spreadsheet::Read, and I think Text::CSV_XS is not I found a useless line in Spreadsheet::Read that does «local When I remove that, all tests pass. Can anyone come up with a test case I'll release Spreadsheet::Read ASAP -- |
From @cpansproutOn Sun Feb 08 08:46:38 2015, hmbrand wrote:
OK, the weird thing going on here is that If you are localising just PL_rs, then you will change the behaviour of <>, but anyone reading $/ will see the old value. In any case, this change happened too late in the developement series, so I am going to change it back (and fix the bug another way). So you don’t need to fix any of the modules. But you may want to consider whether the localised PL_rs is giving you the behaviour you want. In particular, I would appreciate feedback *from you* regarding what perl should be doing. -- Father Chrysostomos |
From @cpansproutOn Sun Feb 08 12:39:30 2015, sprout wrote:
I have done so in ddce084. I hope it fixes all the CPAN failures. -- Father Chrysostomos |
From @TuxOn Sun, 8 Feb 2015 12:39:30 -0800, "Father Chrysostomos via RT"
Ah, right. that is inconsistent and should be avoided
That is bad
I already fixed the module by removing the useless local
Reading your comments, we need a API call for XS to set $/ and/or What I think it should at least pass is the eol.t below, but that --8<--- eol.t use 5.20.1; my foreach my $eol (@eol) { $line = 1; $line = 1; EOL: { -- |
From @TuxOn Sun, 8 Feb 2015 12:39:30 -0800, "Father Chrysostomos via RT"
Here is an example of a test file that IMHO should PASS: --8<--- eol.t use 5.20.1; my foreach my $eol (@eol) { $line = 1; $test .= "line ".$line++.$_ for reverse @eol; $line = 1; EOL: done_testing; It currently fails alike under perl-5.20.1 x86_64-linux-thread-multi-ld $ perl eol.t $ perl5.21.9 eol.t -- |
From @tonycozOn Mon Feb 09 05:20:09 2015, hmbrand wrote:
There's a bug in your test code. The inner part of the string created has *four* lines ending with $deol - the three created by the initial loop, and the fourth by the iteration over the end-of-lines in reverse, but your test code only consumes 3 of them. If you change: is (scalar <$fh>, "line ".$line++.$eol, DDisplay ($eol)."\t<> after local \$/ = \$/"); The test passes. Closing this ticket. Tony |
@tonycoz - Status changed from 'open' to 'resolved' |
Migrated from rt.perl.org#123739 (status was 'resolved')
Searchable as RT123739$
The text was updated successfully, but these errors were encountered: