merged branch lyrixx/process (PR #5592)

Commits ------- 27b2df9 [Process] Fixed bug introduced by 7bafc69. 7a955c0 [Process][Tests] Prove process fail (Add more test case) 598dcf3 [Process][Tests] Prove process fail Discussion ---------- [Process][Tests] Prove process fail with chained commands Bug fix: no Feature addition: no Backwards compatibility break: no Symfony2 tests pass: no Fixes the following tickets: - Todo: Fix that License of the code: MIT This PR is against 2.1 branch. Previous PR was #5575 This PR try to hiligh a regression in Process component. ``` php $process = new Process("echo -n 1 && echo -n 1"); // or $process = new Process("echo -n 1 ; echo -n 1"); $process->run(); var_dump('11' == $process->getOutput()); // false, var_dump($process->getOutput()); // '1', ``` This test failed because of PR #5543 ; see 7bafc69#L0R233 --------------------------------------------------------------------------- by romainneutron at 2012-09-25T13:05:45Z You've to revert the change that causes the fail (ie: remove https://github.com/symfony/symfony/blob/2.1/src/Symfony/Component/Process/Process.php#L233) --------------------------------------------------------------------------- by romainneutron at 2012-09-25T13:06:56Z BTW, removing this line re-open #5030 --------------------------------------------------------------------------- by stof at 2012-09-25T13:11:15Z @lyrixx please add a commit reverting the addition of ``exec`` in the case of sigchild not being used (only this addition, not the full commit you linked) as it should fix your test. --------------------------------------------------------------------------- by stof at 2012-09-25T13:12:21Z @fabpot btw, this regression is quite important. As I said in the previous PR, it impacts composer in a bunch of places. --------------------------------------------------------------------------- by romainneutron at 2012-09-25T13:30:07Z You reverted too much things, you just had to remove line 233 --------------------------------------------------------------------------- by stof at 2012-09-25T13:42:49Z @lyrixx I explicitly asked you to revert only the ``exec`` addition for the case without sigchild. --------------------------------------------------------------------------- by lyrixx at 2012-09-25T13:55:57Z @stof Sorry, i fixed that. --------------------------------------------------------------------------- by romainneutron at 2012-09-25T13:56:26Z @lyrixx just remove the two last commit, edit Process.php and remove line 233 --------------------------------------------------------------------------- by lyrixx at 2012-09-25T13:59:59Z @romainneutron I think it's ok now. --------------------------------------------------------------------------- by romainneutron at 2012-09-25T14:11:28Z yep it's good :)
symfony · Sep 28, 2012 · 5fff626 · 5fff626
2 parents f62fa8a + 27b2df9
commit 5fff626
Show file tree

Hide file tree

Showing 2 changed files with 23 additions and 2 deletions.
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -229,8 +229,6 @@ public function start($callback = null)
                 $descriptors = array_merge($descriptors, array(array('pipe', 'w')));
 
                 $this->commandline = '('.$this->commandline.') 3>/dev/null; code=$?; echo $code >&3; exit $code';
-            } else {
-                $this->commandline = 'exec ' . $this->commandline;
             }
         }
 

diff --git a/src/Symfony/Component/Process/Tests/AbstractProcessTest.php b/src/Symfony/Component/Process/Tests/AbstractProcessTest.php
@@ -76,6 +76,29 @@ public function testProcessPipes($expected, $code)
         $this->assertSame($expected, $p->getErrorOutput());
     }
 
+    public function chainedCommandsOutputProvider()
+    {
+        return array(
+            array('11', ';', '1'),
+            array('22', '&&', '2'),
+        );
+    }
+
+    /**
+     *
+     * @dataProvider chainedCommandsOutputProvider
+     */
+    public function testChainedCommandsOutput($expected, $operator, $input)
+    {
+        if (defined('PHP_WINDOWS_VERSION_BUILD')) {
+            $this->markTestSkipped('Does it work on windows ?');
+        }
+
+        $process = $this->getProcess(sprintf('echo -n %s %s echo -n %s', $input, $operator, $input));
+        $process->run();
+        $this->assertEquals($expected, $process->getOutput());
+    }
+
     public function testCallbackIsExecutedForOutput()
     {
         $p = $this->getProcess(sprintf('php -r %s', escapeshellarg('echo \'foo\';')));