Description: ------------ Hi, I noticed that there has been a new release of 1.0.5 and I also note that new STDERR option, however as far as I can tell, under certain circumstances this will actually not have any effect at all. For instance, as taken from Line 420, System_Command.php: =========8<=========== if ($returnVal !== 0) { // command returned nonzero; that's always an error $return = PEAR::raiseError(null, SYSTEM_COMMAND_NONZERO_EXIT, null, E_USER_WARNING, null, 'System_Command_Error', true); } else if (!$this->options['STDERR']) { // caller does not care about stderr; return success $return = implode("\n", $result); } else { // our caller cares about stderr; check stderr output clearstatcache(); if (filesize($tmpFile) > 0) { // the command actually wrote to stderr $stderr_output = file_get_contents($tmpFile); $return = PEAR::raiseError(null, SYSTEM_COMMAND_STDERR, null, E_USER_WARNING, $stderr_output, 'System_Command_Error', true); } else { // total success; return stdout gathered by exec() $return = implode("\n", $result); } } =========>8=========== With this condition we will never actually get to 'else', as in most cases, if a program is going to write to stderr, they will usually exit with a non zero. In anycase, there is no way for the caller to retrieve the exit code from the program, which means there is no way for the user to really do much with the response other than just say that it died for some reason. What I propose is that the debugInfo actually be populated with an array, I have not checked if this breaks PEAR schemas/standards, however it does work. This could be done as follows: Line 420, System_Command.php: =========8<=========== if ($returnVal !== 0) { if($this->options['STDERR']) { // our caller cares about stderr; check stderr output clearstatcache(); if (filesize($tmpFile) > 0) { // the command actually wrote to stderr $stderr_output = file_get_contents($tmpFile); /** * Just incase the program doesn't behave properly and sends error messages * to standard output and not standard error, lets get the standard output */ if(empty($stderr_output) && !empty($result)) { $stderr_output = implode(" ", $result); } $return = PEAR::raiseError( null, SYSTEM_COMMAND_STDERR, null, E_USER_WARNING, array( 'code' => $returnVal, 'stderr' => trim($stderr_output), ), 'System_Command_Error', true ); } else { // total success; return stdout gathered by exec() $return = implode("\n", $result); } } else { // command returned nonzero; that's always an error $return = PEAR::raiseError(null, SYSTEM_COMMAND_NONZERO_EXIT, null, E_USER_WARNING, null, 'System_Command_Error', true); } } else { // caller does not care about stderr; return success $return = implode("\n", $result); } =========>8=========== This both allows the user to retrieve the exit code and also stderr. Thanks Alex Test script: --------------- <?php require_once 'System/Command.php'; $cmd = new System_Command; $cmd->setOption('STDERR', true); $cmd->pushCommand('gunzip'); $result = $cmd->execute(); echo $result->getMessage(); echo "\n"; var_dump($result->getDebugInfo()); ?> Expected result: ---------------- System_Command Error: command wrote to stderr array(2) { ["code"]=> int(1) ["stderr"]=> string(37) "gunzip: stdin: unexpected end of file" } Note that I am not saying this is the only way to do it, its just that it would be nice to capture both the exit code and stderr. Presently to my knowledge you can't do this. This is just one solution to the problem, there is perhaps a much better way of doing this that will not break backwards compatibility, however since this does not appear to work currently then probably no one is relying on it. Actual result: -------------- System_Command Error: non-zero exit value from command NULL

Thank you for your input. As you surmised in your message, it's the BC-break that is the hurdle. At a glance... Changing the debugInfo to be an array would be a BC break since prev versions only put the exit val in there as a scalar. Also I believe this is a feature/change-req not a bug report (correct me if you disagree) -- and I've changed this bug accordingly.

Yes, capturing the exit code is a feature request as you state, however as I pointed out (probably not very clearly however, sorry), there is a problem with the if/else in that if a program exits with non 0, you can never get stderr. Since in most cases a user will want to capture stderr when a program has exited with non 0, do you think that this actually will break BC? I guess the other option is that you can add an option, say for example, 'STDERROR_EXIT', which would then cause the debugInfo to be populated with an array, which would definately not break BC.

correct. version 1.0.5 introduced the ability to get stderr's output when the command exits *zero*. Before v1.0.5, you could never get stderr's output. That was the widest improvement possible [wrt getting stderr] w/o a BC break.

...and replying to myself: your last idea -- add an option that gives access to the stderr output (prob in an array in debugInfo) -- is the way to go. Simply, low low priority -- unless you're experiencing an actual problem.

Well, that is why I created the bug/feature report ;) To give you a real world example, I am using the unzip program on the command line, as PHP implementation does not support password's. So, if I use System_Command with the Unzip program, which exits non zero upon error, I presently with version 1.0.5, can't capture the reason for failure in stderr, or the actual exit code, thus I have no way of actually knowing why the unzip failed. I will have a look at the source and submit some changes to you, as I have it working in my local copy, i just need to add the non BC fix as previsously stated. Perhaps you can then look at adding these changes to a future release. Thanks Alex