diff --git a/src/Symfony/Component/Process/Pipes/PipesInterface.php b/src/Symfony/Component/Process/Pipes/PipesInterface.php index 09d3f61d6e..12ec06e6c6 100644 --- a/src/Symfony/Component/Process/Pipes/PipesInterface.php +++ b/src/Symfony/Component/Process/Pipes/PipesInterface.php @@ -53,6 +53,13 @@ interface PipesInterface */ public function areOpen(); + /** + * Returns if pipes are able to read output. + * + * @return bool + */ + public function haveReadSupport(); + /** * Closes file handles and pipes. */ diff --git a/src/Symfony/Component/Process/Pipes/UnixPipes.php b/src/Symfony/Component/Process/Pipes/UnixPipes.php index f8a0d1997d..ffe1241508 100644 --- a/src/Symfony/Component/Process/Pipes/UnixPipes.php +++ b/src/Symfony/Component/Process/Pipes/UnixPipes.php @@ -27,13 +27,13 @@ class UnixPipes extends AbstractPipes /** @var bool */ private $ptyMode; /** @var bool */ - private $disableOutput; + private $haveReadSupport; - public function __construct($ttyMode, $ptyMode, $input, $disableOutput) + public function __construct($ttyMode, $ptyMode, $input, $haveReadSupport) { $this->ttyMode = (bool) $ttyMode; $this->ptyMode = (bool) $ptyMode; - $this->disableOutput = (bool) $disableOutput; + $this->haveReadSupport = (bool) $haveReadSupport; if (is_resource($input)) { $this->input = $input; @@ -52,7 +52,7 @@ class UnixPipes extends AbstractPipes */ public function getDescriptors() { - if ($this->disableOutput) { + if (!$this->haveReadSupport) { $nullstream = fopen('/dev/null', 'c'); return array( @@ -191,6 +191,14 @@ class UnixPipes extends AbstractPipes return $read; } + /** + * {@inheritdoc} + */ + public function haveReadSupport() + { + return $this->haveReadSupport; + } + /** * {@inheritdoc} */ @@ -209,6 +217,6 @@ class UnixPipes extends AbstractPipes */ public static function create(Process $process, $input) { - return new static($process->isTty(), $process->isPty(), $input, $process->isOutputDisabled()); + return new static($process->isTty(), $process->isPty(), $input, !$process->isOutputDisabled() || $process->hasCallback()); } } diff --git a/src/Symfony/Component/Process/Pipes/WindowsPipes.php b/src/Symfony/Component/Process/Pipes/WindowsPipes.php index 1472f8c6c4..c2d410d313 100644 --- a/src/Symfony/Component/Process/Pipes/WindowsPipes.php +++ b/src/Symfony/Component/Process/Pipes/WindowsPipes.php @@ -36,13 +36,13 @@ class WindowsPipes extends AbstractPipes Process::STDERR => 0, ); /** @var bool */ - private $disableOutput; + private $haveReadSupport; - public function __construct($disableOutput, $input) + public function __construct($haveReadSupport, $input) { - $this->disableOutput = (bool) $disableOutput; + $this->haveReadSupport = (bool) $haveReadSupport; - if (!$this->disableOutput) { + if ($this->haveReadSupport) { // Fix for PHP bug #51800: reading from STDOUT pipe hangs forever on Windows if the output is too big. // Workaround for this problem is to use temporary files instead of pipes on Windows platform. // @@ -76,7 +76,7 @@ class WindowsPipes extends AbstractPipes */ public function getDescriptors() { - if ($this->disableOutput) { + if (!$this->haveReadSupport) { $nullstream = fopen('NUL', 'c'); return array( @@ -138,6 +138,14 @@ class WindowsPipes extends AbstractPipes return $read; } + /** + * {@inheritdoc} + */ + public function haveReadSupport() + { + return $this->haveReadSupport; + } + /** * {@inheritdoc} */ @@ -168,7 +176,7 @@ class WindowsPipes extends AbstractPipes */ public static function create(Process $process, $input) { - return new static($process->isOutputDisabled(), $input); + return new static(!$process->isOutputDisabled() || $process->hasCallback(), $input); } /** diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php index 2ebda2675d..dc3255e587 100644 --- a/src/Symfony/Component/Process/Process.php +++ b/src/Symfony/Component/Process/Process.php @@ -256,9 +256,6 @@ class Process if ($this->isRunning()) { throw new RuntimeException('Process is already running'); } - if ($this->outputDisabled && null !== $callback) { - throw new LogicException('Output has been disabled, enable it to allow the use of a callback.'); - } $this->resetProcessData(); $this->starttime = $this->lastOutputTime = microtime(true); @@ -356,7 +353,12 @@ class Process $this->requireProcessIsStarted(__FUNCTION__); $this->updateStatus(false); + if (null !== $callback) { + if (!$this->processPipes->haveReadSupport()) { + $this->stop(0); + throw new \LogicException('Pass the callback to the Process:start method or enableOutput to use a callback with Process::wait'); + } $this->callback = $this->buildCallback($callback); } @@ -1181,6 +1183,18 @@ class Process return $result = (bool) @proc_open('echo 1', array(array('pty'), array('pty'), array('pty')), $pipes); } + /** + * Returns whether a callback is used on underlying process output. + * + * @internal + * + * @return bool + */ + public function hasCallback() + { + return (bool) $this->callback; + } + /** * Creates the descriptors needed by the proc_open. * @@ -1194,7 +1208,7 @@ class Process $this->processPipes = UnixPipes::create($this, $this->input); } - return $this->processPipes->getDescriptors($this->outputDisabled); + return $this->processPipes->getDescriptors(); } /** @@ -1207,10 +1221,19 @@ class Process * * @return \Closure A PHP closure */ - protected function buildCallback($callback) + protected function buildCallback(callable $callback = null) { + if ($this->outputDisabled) { + return function ($type, $data) use ($callback) { + if (null !== $callback) { + call_user_func($callback, $type, $data); + } + }; + } + $out = self::OUT; - $callback = function ($type, $data) use ($callback, $out) { + + return function ($type, $data) use ($callback, $out) { if ($out == $type) { $this->addOutput($data); } else { diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php index d71485ff44..eef81cd5b0 100644 --- a/src/Symfony/Component/Process/Tests/ProcessTest.php +++ b/src/Symfony/Component/Process/Tests/ProcessTest.php @@ -305,6 +305,19 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $this->assertTrue($called, 'The callback should be executed with the output'); } + public function testCallbackIsExecutedForOutputWheneverOutputIsDisabled() + { + $p = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('echo \'foo\';'))); + $p->disableOutput(); + + $called = false; + $p->run(function ($type, $buffer) use (&$called) { + $called = $buffer === 'foo'; + }); + + $this->assertTrue($called, 'The callback should be executed with the output'); + } + public function testGetErrorOutput() { $p = $this->getProcess(sprintf('%s -r %s', self::$phpBin, escapeshellarg('$n = 0; while ($n < 3) { file_put_contents(\'php://stderr\', \'ERROR\'); $n++; }'))); @@ -1108,29 +1121,6 @@ class ProcessTest extends \PHPUnit_Framework_TestCase $this->assertSame($process, $process->setIdleTimeout(null)); } - /** - * @dataProvider provideStartMethods - */ - public function testStartWithACallbackAndDisabledOutput($startMethod, $exception, $exceptionMessage) - { - $p = $this->getProcess('foo'); - $p->disableOutput(); - $this->setExpectedException($exception, $exceptionMessage); - if ('mustRun' === $startMethod) { - $this->skipIfNotEnhancedSigchild(); - } - $p->{$startMethod}(function () {}); - } - - public function provideStartMethods() - { - return array( - array('start', 'Symfony\Component\Process\Exception\LogicException', 'Output has been disabled, enable it to allow the use of a callback.'), - array('run', 'Symfony\Component\Process\Exception\LogicException', 'Output has been disabled, enable it to allow the use of a callback.'), - array('mustRun', 'Symfony\Component\Process\Exception\LogicException', 'Output has been disabled, enable it to allow the use of a callback.'), - ); - } - /** * @dataProvider provideOutputFetchingMethods * @expectedException \Symfony\Component\Process\Exception\LogicException