Bug #36365 for IPC-Cmd: IPC::Cmd error reporting

Mon Jun 02 09:37:05 2008 eda [...] waniasset.com - Ticket created

Subject:	IPC::Cmd error reporting
Date:	Mon, 2 Jun 2008 14:35:49 +0100
To:	<bug-ipc-cmd [...] rt.cpan.org>
From:	"Ed Avis" <eda [...] waniasset.com>

The documentation for IPC::Cmd::run() shows that it returns an 'ok' flag and an error code, which is the exit status of the command. But what happens if the command was killed by a signal? If I might put two related things in the bug report, it would be useful to have a 'checked' mode of run, which would throw an exception if anything went wrong in running the command (fork() or exec() failed, command killed with a signal, or exited with nonzero status), and the exception message would say what the problem was. -- Ed Avis <eda@waniasset.com> Show quoted text

______________________________________________________________________ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email

______________________________________________________________________

Sun Jun 29 17:36:47 2008 kane [...] cpan.org - Correspondence added

On Mon Jun 02 09:37:05 2008, eda@waniasset.com wrote: Show quoted text

> The documentation for IPC::Cmd::run() shows that it returns an 'ok' > flag and an error code, which is the exit status of the command. > But what happens if the command was killed by a signal?

Then the error code would be set to the signal number; the error code is simply $?. See perldoc perlvar for details. Basically, it Does what you Expect (tm). I've added a few lines to the documentation to make this extra clear. Show quoted text

> If I might put two related things in the bug report, it would be > useful to have a 'checked' mode of run, which would throw an > exception if anything went wrong in running the command (fork() or > exec() failed, command killed with a signal, or exited with nonzero > status), and the exception message would say what the problem was.

All that is, as far as i can see, covered by propagation $?. Currently, there's no exception being thrown, but obviously that's easy to add client side if that's the behaviour you'd prefer. Thanks for reporting,

Sun Jun 29 17:36:49 2008 The RT System itself - Status changed from 'new' to 'open'

Sun Jun 29 17:36:49 2008 kane [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Jun 30 06:48:24 2008 EDAVIS [...] cpan.org - Correspondence added

Show quoted text

>> But what happens if the command was killed by a signal?

> >Then the error code would be set to the signal number; the error >code is simply $?.

Show quoted text

>I've added a few lines to the documentation to make this extra clear.

Thanks. Show quoted text

>>it would be >>useful to have a 'checked' mode of run, which would throw an >>exception if anything went wrong in running the command

Show quoted text

>All that is, as far as i can see, covered by propagation $?.

Unfortunately to decode $? you need a lot of boilerplate, as illustrated in perlfunc(1): if ($? == -1) { print "failed to execute: $!\n"; } elsif ($? & 127) { printf "child died with signal %d, %s coredump\n", ($? & 127), ($? & 128) ? 'with' : 'without'; } else { printf "child exited with value %d\n", $? >> 8; } This is the sort of thing that IPC::Cmd might provide as a convenience. Further, just returning $? as the result loses some information - e.g. the difference between 'No such file and directory' and 'Permission denied'.

Mon Jun 30 06:48:26 2008 The RT System itself - Status changed from 'resolved' to 'open'

Mon Jun 30 08:40:50 2008 jos [...] dwim.org - Correspondence added

Subject:	Re: [rt.cpan.org #36365] IPC::Cmd error reporting
Date:	Mon, 30 Jun 2008 14:19:08 +0200
To:	bug-IPC-Cmd [...] rt.cpan.org
From:	Jos I.Boumans <jos [...] dwim.org>

On 30 Jun 2008, at 12:48, EDAVIS via RT wrote: Show quoted text

>> All that is, as far as i can see, covered by propagation $?.

> > Unfortunately to decode $? you need a lot of boilerplate, as > illustrated > in perlfunc(1): > > if ($? == -1) { > print "failed to execute: $!\n"; > } > elsif ($? & 127) { > printf "child died with signal %d, %s coredump\n", > ($? & 127), ($? & 128) ? 'with' : 'without'; > } > else { > printf "child exited with value %d\n", $? >> 8; > } > > This is the sort of thing that IPC::Cmd might provide as a > convenience. > > Further, just returning $? as the result loses some information - e.g. > the difference between 'No such file and directory' and 'Permission > denied'.

In that case, I'm not quite sure i understand what you're looking for. Could you give me an example how the API would look in your ideal case? -- Jos Boumans http://www.linkedin.com/in/josboumans How do I prove I'm not crazy to people who are?

Mon Jun 30 09:13:14 2008 EDAVIS [...] cpan.org - Correspondence added

This is roughly what I have in mind (just to show error handling, obviously this isn't a proper implementation): sub run { my $cmd = shift; my $r = system $cmd; if ($r == 0) { # All okay. return; } if ($? == -1) { # Include $! in the error message, so that the user can # see 'No such file or directory' versus 'Permission denied' # versus 'Cannot fork' or whatever the cause was. # die "failed to execute '$cmd': $!"; } if ($? & 127) { die sprintf("'$cmd' died with signal %d, %s coredump\n", ($? & 127), ($? & 128) ? 'with' : 'without'); } # Otherwise, the command run but gave error status. die "'$cmd' exited with value " . $? >> 8; } You might consider it overkill to die() for a nonzero exit status, and prefer to reserve die() for more serious things like unable to fork or the process being killed. In which case the return code of run() would be the exit status itself. But anyway the user wouldn't have to test two separate punctuation variables and do bit-shifting to get a readable error message. I would suggest the checked version be enabled with use IPC::Run qw(:checked); # Now we can just call 'run()' and it will throw exceptions on error If you like the idea, I can code it up and send you a patch.

Sun Jul 13 09:09:10 2008 kane [...] cpan.org - Correspondence added

On Mon Jun 30 09:13:14 2008, EDAVIS wrote: Show quoted text

> This is roughly what I have in mind (just to show error handling, > obviously this isn't a proper implementation):

I do like the suggestion, and I've actually implemented this in IPC::Cmd 0.41_03, which I just uploaded to CPAN. Why dont you take a look at it and tell me how you like it? Let me know if there's anything I overlooked. Cheers,

Sun Jul 13 09:09:13 2008 kane [...] cpan.org - Status changed from 'open' to 'stalled'

Mon Jul 14 06:17:16 2008 EDAVIS [...] cpan.org - Correspondence added

Show quoted text

>I do like the suggestion, and I've actually implemented this in >IPC::Cmd 0.41_03, which I just uploaded to CPAN.

I tried that but some tests fail, starting with not ok 452 - Ran /usr/bin/perl src/output.pl successfully # Failed test 'Ran /usr/bin/perl src/output.pl successfully' # at ./t/01_IPC-Cmd.t line 145. For this test the output of run(...) is: ( undef, '\'/usr/bin/perl src/output.pl\' exited with value 2', [ 'Can\'t open perl script "src/output.pl": No such file or directory ' ], [], [ 'Can\'t open perl script "src/output.pl": No such file or directory ' ] ) Indeed there is no file output.pl in the distribution.

Mon Jul 14 06:17:19 2008 The RT System itself - Status changed from 'stalled' to 'open'

Mon Jul 14 09:58:37 2008 jos [...] dwim.org - Correspondence added

Subject:	Re: [rt.cpan.org #36365] IPC::Cmd error reporting
Date:	Mon, 14 Jul 2008 15:58:16 +0200
To:	bug-IPC-Cmd [...] rt.cpan.org
From:	Jos I.Boumans <jos [...] dwim.org>

On 14 Jul 2008, at 12:17, EDAVIS via RT wrote: Show quoted text

> Queue: IPC-Cmd > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36365 > >

>> I do like the suggestion, and I've actually implemented this in >> IPC::Cmd 0.41_03, which I just uploaded to CPAN.

> > I tried that but some tests fail, starting with

[...] Show quoted text

> Indeed there is no file output.pl in the distribution.

That would be a packaging error -- i've uploaded 0.41_04 with the missing file. Thanks for catching this, -- Jos Boumans http://www.linkedin.com/in/josboumans How do I prove I'm not crazy to people who are?

Mon Jul 14 10:00:14 2008 EDAVIS [...] cpan.org - Correspondence added

Do you have a download link for 0.41_04?

Mon Jul 14 10:42:23 2008 jos [...] dwim.org - Correspondence added

Subject:	Re: [rt.cpan.org #36365] IPC::Cmd error reporting
Date:	Mon, 14 Jul 2008 16:10:35 +0200
To:	bug-IPC-Cmd [...] rt.cpan.org
From:	"Jos I. Boumans" <jos [...] dwim.org>

On 14 Jul 2008, at 16:00, EDAVIS via RT wrote: Show quoted text

> Queue: IPC-Cmd > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=36365 > > > Do you have a download link for 0.41_04?

Attached, -- Jos Boumans http://www.linkedin.com/in/josboumans How do I prove I'm not crazy to people who are?

Download IPC-Cmd-0.41_04.tar.gz
application/x-gzip 14.8k

Message body not shown because it is not plain text.

Mon Jul 14 11:15:46 2008 EDAVIS [...] cpan.org - Correspondence added

It's not quite right: take the following test program: #!/usr/bin/perl use warnings; use strict; use 5.010; use IPC::Cmd qw(run); my ($ok, $err) = run command => 'nonexistent'; die "$err\n" if not $ok; I expected this to print something like cannot run 'nonexistent': No such file or directory but instead it prints 'nonexistent' exited with value 255 which is not the case.

Mon Jul 14 11:21:22 2008 jos [...] dwim.org - Correspondence added

Subject:	Re: [rt.cpan.org #36365] IPC::Cmd error reporting
Date:	Mon, 14 Jul 2008 17:20:59 +0200
To:	bug-IPC-Cmd [...] rt.cpan.org
From:	"Jos I. Boumans" <jos [...] dwim.org>

On 14 Jul 2008, at 17:15, EDAVIS via RT wrote: Show quoted text

> I expected this to print something like > > cannot run 'nonexistent': No such file or directory > > but instead it prints > > 'nonexistent' exited with value 255 > > which is not the case.

In that case, i'll take you up on your offer for a patch; The code that creates this diagnostic is IPC::Cmd::_pp_child_error and is basically using the code that you provided (and comes straight from perldoc). If there's a logic error in there, I'd be grateful for a fix. Thanks for your help, -- Jos Boumans http://www.linkedin.com/in/josboumans How do I prove I'm not crazy to people who are?

Tue Jul 22 20:02:02 2008 kane [...] cpan.org - Status changed from 'open' to 'stalled'

Mon Jan 11 07:56:04 2010 EDAVIS [...] cpan.org - Correspondence added

This is essentially caused by IPC::Open3 not reporting that the executable couldn't be run, but instead returning a process id (which then exits with status 255). On the other hand, IPC::Open2 does the right thing IMHO. use IPC::Open2; my $pid = open2(\*CHLD_IN, \*CHLD_OUT, 'nonexistent'); say $pid; will die with 'open2: exec of nonexistent failed'. The exception would need to be handled by the caller, but at least it is reported separately. On the other hand, use IPC::Open3; my $pid = open3(\*CHLD_IN, \*CHLD_OUT, \*CHLD_ERR, 'nonexistent'); say $pid; prints a process id. I'll file a bug report against IPC::Open3 for it to report failed exec() better, then IPC::Cmd will be able to use that.

Mon Jan 11 07:56:12 2010 The RT System itself - Status changed from 'stalled' to 'open'

Mon Jan 11 07:59:56 2010 EDAVIS [...] cpan.org - Correspondence added

Actually, open2 does also return a process id, but then prints an error message asynchronously. So I don't know what exactly is the right way forward.

Mon Jan 11 10:12:50 2010 EDAVIS [...] cpan.org - Correspondence added

I've filed a bug for better error reporting in IPC::Open3: <http://rt.perl.org/rt3/Public/Bug/Display.html?id=72016>

Tue Feb 15 11:44:12 2011 EDAVIS [...] cpan.org - Correspondence added

Show quoted text

>I've filed a bug for better error reporting in IPC::Open3: ><http://rt.perl.org/rt3/Public/Bug/Display.html?id=72016>

This was fixed in IPC::Open3 version 1.06, but that has not been included in any stable Perl release so far.

Fri Sep 27 06:20:41 2013 EDAVIS [...] cpan.org - Correspondence added

Now that the newer IPC::Open3 is included in a Perl stable release, this bug is fixed. The earlier test program #!/usr/bin/perl use warnings; use strict; use 5.010; use IPC::Cmd qw(run); my ($ok, $err) = run command => 'nonexistent'; die "$err\n" if not $ok; correctly dies with an exec failure. So this bug can be closed - if you want to be strictly correct, I think IPC::Cmd could require IPC::Open3 version 1.06 or later.