|
rcrews has asked for the wisdom of the Perl Monks concerning the following question:
I am wrapping my head around a new technique today: Use IPC::Open3 instead of backticks. Here is the program I started with:
#!/usr/bin/env perl
use 5.016;
use autodie;
use warnings;
our $VERSION = 'v.0.0.1';
say qx[/bin/ls];
exit 0;
__END__
It it simple and works. Then I ran perlcritic:
$ perlcritic -1 syscall1.pl
Backtick operator used at line 8, column 5. Use IPC::Open3 instead.
+(Severity: 3)
I am eager to learn new techniques and become a better Perl programmer. My goal is to update the script to pass the perlcritic test and use other best practices while keeping the code Perlish, readable, maintainable, and reusable. Here's what I came up with:
#!/usr/bin/env perl
use 5.016;
use autodie;
use warnings;
use English '-no_match_vars';
use IPC::Open3;
use Symbol 'gensym';
use Readonly;
our $VERSION = 'v.0.0.2';
say system_ipc_open3('/bin/ls')->{'output'};
sub system_ipc_open3 {
my @command = map { split m{\s+}xms } @ARG;
Readonly my $SYSCALL_EXIT_STATUS_MASK => 8;
Readonly my $BUFFER_SIZE => 2_000_000;
my (
$child_in_fh, $child_out_fh, $child_err_fh,
$child_in, $child_out, $child_err,
$buffer, $child_pid, $child_exit_status
) = q[];
$child_err_fh = gensym;
$child_pid = open3( $child_in_fh, $child_out_fh, $child_err_fh,
@command );
$child_exit_status = $CHILD_ERROR >> $SYSCALL_EXIT_STATUS_MASK;
while ( my $read = sysread $child_out_fh, $buffer,
$BUFFER_SIZE ) {
$child_out .= $buffer;
}
while ( my $read = sysread $child_err_fh, $buffer,
$BUFFER_SIZE ) {
$child_err .= $buffer;
}
waitpid $child_pid, 0;
return {
exit_status => $child_exit_status,
error => $child_err || q[],
output => $child_out || q[],
};
}
exit 0;
__END__
Version 2 is verbose and "low level". My question to the monks: How is this an improvement? Is there a better way to Use IPC::Open3 instead of backticks? |
Re: Using IPC::Open3 instead of backtick operator (updated)
by haukex (Archbishop) on Jun 09, 2016 at 06:44 UTC
|
Hi rcrews,
I personally wouldn't use IPC::Open3, as there's a huge list of CPAN modules to make your life easier, see for example the "See Also" section of Capture::Tiny. A while back I did some research on all of them, and my personal favorite is IPC::Run3: Runs on *NIX and Windows, has excellent test scores, very few dependencies, it captures STDERR too, and it allows you to avoid the shell (which avoids whitespace and interpolation issues). Two things it can't do is timeouts and interactive communication with the subprocess, but I've rarely needed those. For those, IPC::Run seems like a good solution. And Capture::Tiny is also an excellent module, it's useful if you also want to capture the output of the Perl process itself. Update: IPC::System::Simple, which the AM also mentioned, is also nice in that it provides drop-in replacements for system and qx with much better error handling (it's what autodie uses when you say use autodie qw/system/;).
Hope this helps,
-- Hauke D
| [reply]
[d/l] |
Re: Using IPC::Open3 instead of backtick operator
by BillKSmith (Monsignor) on Jun 09, 2016 at 03:55 UTC
|
I highly recommend the book "Perl Best Practices" ISBN 0596001738. In his preface, the author makes the case for adopting and consistently using a set of guidelines. In the rest of the book, he points out areas where guidelines are needed. He offers his recommendations and his reasons for them. Adopt only what makes sense in your environment. Follow your own rules consistently.
| [reply] |
Re: Using IPC::Open3 instead of backtick operator
by Marshall (Canon) on Jun 09, 2016 at 02:53 UTC
|
Perl critic doesn't like the backquote shelling out to the OS.
Your solution is way to complex.
#!/usr/bin/perl
use warnings;
use strict;
my @files = glob "*";
print join ("\n", @files),"\n";
open (my $IN, '-|', 'dir') or die "$!";
while (<$IN>)
{
print;
}
Not everything that PerlCritic says is "gospel" - just suggestions.
tested on Windows XP - don't have ls. | [reply]
[d/l] |
|
|
| [reply]
[d/l]
[select] |
|
|
My open will capture both the STDERR and STDOUT messages. See below code...
Main:
#!/usr/bin/perl
use warnings;
use strict;
open (IN, '-|', 'perl PrintStdoutStdErr.pl') or die "$!";
while (<IN>)
{
print;
}
__END__
prints:
this went to STDERR
this went to STDOUT
PrintStdoutStdErr.pl
#!/usr/bin/perl
use warnings;
use strict;
$|=1;
print STDOUT "this went to STDOUT\n";
print STDERR "this went to STDERR\n";
I actually wouldn't worry about Perl critic on an "ls" command. | [reply]
[d/l]
[select] |
|
|
Re: Using IPC::Open3 instead of backtick operator ( Capture::Tiny )
by Anonymous Monk on Jun 09, 2016 at 02:52 UTC
|
use Capture::Tiny qw/ capture /;
my @cmd = ( "...ls", 'dir' );
my( $stdout, $stderr, $exit ) = capture {
system { $cmd[0] } @cmd;
};;
$exit and die "it failed with $exit and $stderr ";
Also of interest might be String::ShellQuote/Win32::ShellQuote
| [reply]
[d/l] |
Re: Using IPC::Open3 instead of backtick operator
by fishy (Friar) on Jun 09, 2016 at 18:58 UTC
|
Hi rcrews,
without any critics, my programs use IPC::Cmd:
use strict;
use warnings;
use IPC::Cmd qw( run );
my $tool = 'tar';
my $cmd = [ $tool, 'xvzf', $in_filename ];
my $buffer = '';
my @out = run( command => $cmd,
buffer => \$buffer,
verbose => 1 );
die "Error extracting $in_filename: $buffer" unless $out[0];
Have fun!
| [reply]
[d/l] |
|
|
| [reply]
[d/l]
[select] |
|
|
Depending on configuration, IPC::Cmd will use either IPC::Run or IPC:Open3, so will capture STDERR as well as STDOUT.
Called with 1 buffer, IPC::Cmd will put both STDOUT and STDERR in that buffer. Called in list context, it will return seperate buffers for STDOUT and STDERR.
| [reply] |
|
|
Re: Using IPC::Open3 instead of backtick operator
by Laurent_R (Canon) on Jun 09, 2016 at 06:31 UTC
|
How is this an improvement?
The real question is:
Is this an improvement?
Do you think it is an impmrovement? I don't think so.
Use your own judgment when using perlcritic, it is quite useful, but, with all due respect to Damian Conway, it is not the final word on everything.
| [reply]
[d/l] |
Re: Using IPC::Open3 instead of backtick operator (perlcritic is dumb)
by Anonymous Monk on Jun 09, 2016 at 02:40 UTC
|
My goal is to update the script to pass the perlcritic test and use other best practices while keeping the code Perlish, readable, maintainable, and reusable :) What does that mean? You should seriously rethink accepting perlcritic "defaults", see some perspective on changing your code for perlcritic
perlcritic is dumb, dont let it make decisions for you, chose your own rules
| [reply] |
Re: Using IPC::Open3 instead of backtick operator
by RonW (Parson) on Jun 09, 2016 at 16:26 UTC
|
The core module IO::Pipe is easy to use and works on both Linux and MS Windows. However, it doesn't handle STDERR.
The reader and writer methods expect a program to run and a list of arguments. The shell is not used.
#!perl
use strict;
use warnings;
use IO::Pipe;
my $pipe = IO::Pipe->new();
$pipe->reader(qw(ls -l));
while(<$pipe>) {
...
}
| [reply]
[d/l]
[select] |
|
|
| [reply]
[d/l]
[select] |
|
|
| [reply]
[d/l] |
Re: Using IPC::Open3 instead of backtick operator
by karlgoethebier (Abbot) on Jun 11, 2016 at 12:16 UTC
|
..."...become a better Perl programmer...update the script to pass the perlcritic test..."
I fear you won't become a better Perl programmer if you use perlcritic. The opposite might happen and this would be annoying. From the linked node by our beloved deputy of the leader BUK:
..."Fully 95% of Perl::Critic's (and by implication PBP) justifictions are as puerile as banning ball games from school playgrounds because participants might skin their knees. Making their own mistakes and learning from them is how kids learn. Banning every construct and idiom, that might under some obscure circumstances cause the occasional program to fail, is like trying to wrap your kids in cotton wool. Overindulgent, counter-productive and ultimately futile..."
Below some code that i wrote a while ago for learning purposes and demonstration - how to capture everything from a compound external command:
Some mentioned already that IPC::Run might be the better tool.
The code fails with perlcritic, despite it is IMHO well formatted, easy to read, easy to understand and therefore easy to maintain. And it does what it should do.
OK, the command is idiotic, but perlcritic doesn't complain about that it is idiotic.
No Perl shari'ah, thank's.
Regards, Karl
«The Crux of the Biscuit is the Apostrophe»
| [reply]
[d/l]
[select] |
Re: Using IPC::Open3 instead of backtick operator
by ikegami (Patriarch) on Jun 10, 2016 at 15:12 UTC
|
That program suffers from a race condition that can result in a deadlock. Should the child send a sufficient amount of data to its STDERR, it will block until the parent reads from its STDERR, but the parent is blocked reading the from the child's STDOUT.
open3 is too low-level for most tasks. perlcritic should not have recommended this. Please use IPC::Run3 or IPC::Run instead.
I have just raised this issue here.
| [reply]
[d/l]
[select] |
Re: Using IPC::Open3 instead of backtick operator
by BrowserUk (Patriarch) on Jun 09, 2016 at 08:55 UTC
|
There is a really simple tool for dealing with that problem: rd /q /s \perl\site\lib\perl\critic. Z'ere, zee 'problem' est solv-ed.
With the rise and rise of 'Social' network sites: 'Computers are making people easier to use everyday'
Examine what is said, not who speaks -- Silence betokens consent -- Love the truth but pardon error.
In the absence of evidence, opinion is indistinguishable from prejudice. Not understood.
| [reply]
[d/l] |
|
|
