-
Notifications
You must be signed in to change notification settings - Fork 543
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
BBC: Blead Breaks File::Tabular #22490
Comments
Probably 471c834 |
Confirmed through bisection.
@mauke, can you take a look? Thanks. |
I've opened a bug ticket with two alternative suggestions for fixing the code: https://rt.cpan.org/Ticket/Display.html?id=154767 IMHO the test code in File::Tabular relies on a bug in perl (#22385). It calls |
This problem will impact users of autodie as well. I'm not sure I really like the change to attempt to fix #22385. Requiring a literal undef for this kind of behavior doesn't feel like how things in perl should work. Having open use a temporary file when given any undef seems more sane to me. A warning could be issued if a mode other than |
Could you provide an example of the problem |
It would be more consistent, but it would also fail in annoying (and silent) ways. For example, let's say you extract the name of a file to be processed from your configuration and try to open it: my $filename = $config->{input_file};
open my $fh, "<", $filename
or die "Can't open $filename: $!"; If The current behavior of released perls (before my changes in #22385) catches the case above reliably, but is otherwise hard to characterize. The code simply checks open my $fh, "<", $config->{input_file}
or die "Can't open $config->{input_file}: $!"; … the behavior now depends on whether the So after rejecting the status quo (inconsistent, hard to describe without resorting to implementation details) and a simple
And so now |
Opening a temp file with a Various core functions do have special parsing, but we generally consider those things warts to avoid in modern designs. And some of them are clearly unique by their syntax alone. There is also precedence for odd compile time formulations being removed. Prior to perl 5.18, splitting on whitespace using |
Until this change, you could write: use autodie;
open my $fh, '+>', undef; This would open a file handle to a temp file. Since autodie overrides the open function, the magic added in #22465 won't apply. autodie is implemented in perl using normal functions, and its wrapper calls |
Hi there, Regarding File::Tabular, it's really not an issue if the behavior changes in 5.40 : the tempfile feature of open() was just convenient for tests, it's not essential to the module. However, if I understand correctly, replacing line 5 in
by
would solve the issue, correct ? Maybe a similar approach could be done for autodie and Fatal.pm. Besides, this magic opening of tempfiles through an undef arg was probably not a very good idea of perl 5.8 : in client code it would be more readable to make explicit calls to File::Temp. So maybe a deprecation policy for this feature could be envisioned, instead of trying to preserve all idiosyncrasies. |
No, this will not work. The third parameter is an expression, not a literal undef, so it won't trigger the temp file behavior. And even if it did, it would change how the parameters are handled vs the core open, which isn't how autodie is meant to work.
I agree that this isn't a great interface for doing this. |
File-Tabular is now passing on CPANtesters, @damil having released a new CPAN version on August 11. That would make this ticket closable, but there was a lot of discussion about larger issues between @mauke and @haarg. How should we proceed? |
This is a bug report for perl from "Carlos Guevara" [email protected],
generated with the help of perlbug 1.43 running under perl 5.41.3.
BBC: Blead Breaks File::Tabular
Please see http://fast-matrix.cpantesters.org/?dist=File::Tabular
Flags
Perl configuration
The text was updated successfully, but these errors were encountered: