BTW, this test fails on PHP 5.3.15

This is actually normal because echo -n 1 ; echo -n 1 is two process.
By design, Process return the output of the first command you pass.

There's a workaround for your problem :

$process = new Process("bash -c 'echo -n 1 ; echo -n 1'");
$process->run();
var_dump('11' == $process->getOutput()); // true, 

another solution is to run it as a single process: echo -n 1 && echo -n 1

I added more tests case , and it always fail.
I think it's a bug.

I found this bug in Sismo repo https://github.com/fabpot/Sismo/blob/master/tests/Sismo/Tests/GithubProjectTest.php#L51 : When I lunch the test suite, Sismo tests fails because git remote add ... is not executed.
So I tried to reproduce and to isolate this "bug".

Note: 86d9dea is tested on debian 6 and php 5.3.15

`1) Symfony\Component\Process\Tests\SigchildDisabledProcessTest::testChainedCommandsOutput with data set #0 ('11', ';', '1')
Failed asserting that '1' matches expected '11'.

/var/www/dev/labs/symfony/src/Symfony/Component/Process/Tests/AbstractProcessTest.php:99

2) Symfony\Component\Process\Tests\SigchildDisabledProcessTest::testChainedCommandsOutput with data set #1 ('22', '&&', '2')
Failed asserting that '2' matches expected '22'.

/var/www/dev/labs/symfony/src/Symfony/Component/Process/Tests/AbstractProcessTest.php:99

3) Symfony\Component\Process\Tests\SimpleProcessTest::testChainedCommandsOutput with data set #0 ('11', ';', '1')
Failed asserting that '1' matches expected '11'.

/var/www/dev/labs/symfony/src/Symfony/Component/Process/Tests/AbstractProcessTest.php:99

4) Symfony\Component\Process\Tests\SimpleProcessTest::testChainedCommandsOutput with data set #1 ('22', '&&', '2')
Failed asserting that '2' matches expected '22'.

/var/www/dev/labs/symfony/src/Symfony/Component/Process/Tests/AbstractProcessTest.php:99

This s still actually normal, @stof example is wrong as it's the same of your example, there is still two commands

You have to wrap it inside a bash script to get your expected result $process = new Process("bash -c 'echo -n 1 ; echo -n 1'");

@romainneutron But, how can sismo test pass ? It is impossible that @fabpot push failed tests ! ;)

@romainneutron is it really 2 processes when using && ?

BTW, my sample is not related to echo (echo is a shell builtin).

$process = new Process("git status && git status");

have the same problem.

This PR is related to #5398
Before this PR, tests passed

EDIT : I was wrong, the changeless happend in 7bafc69 ; Related PR : #5543

It's pretty weird, this PR is related to exceptions.

Are you sure you're pointing the right one ?

@stof If you run sleep 3 && sleep 3 and monitor, you'll see that two process with two differents pids are run

@lyrixx BTW I'm sure that if you encapsulate your commands with bash -c "...", you won't have these problems

@lyrixx I don't understand how it is possible. The PR is not changing the logic at all. It is simply using a subclass for exception it throws

Oh, sorry, i pointed a wrong PR, the changeless happend in 7bafc69 ; Related PR : #5543

@romainneutron I agree with you about bash -c "..."

Ok, the change appeared because we now preprend commands with exec.
I'm sorry, I did'nt notice it happened and introduce a BC break

This change is quite important and necessary because it allows us to get the exact Pid of the process, signal it and have more control over it.

If we remove it , the process will be wrapped by a sh -c as mentionned by in #5030 and we will not have access to its Pid ; we will not be able to signal it.

I do think we have to mention it in the doc and docblocks, I'll work on it

@romainneutron but without the wrapping in sh -c, it looks like it does not support all previous cases anymore, meaning this particular change should be reverted at least in the 2.1 branch as it is not BC.

@stof which cases ? such one : new Process('git status && git branch -a') ?

@romainneutron yes. And I can tell you that such cases are used in many places in Composer for instance.

So, this is an issue

@fabpot should we revert the prepending of exec then ?

Right now, I don't have any quick fix except revert. But revert will drop many features

we should probably revert for 2.1 and work on it on 2.2

And, if we wrap exec with parenthesis ?

$this->commandline = 'exec (' . $this->commandline . ')';

EDIT : does not work

@romainneutron drop many features ? It will not drop features comparing to the state in 2.0 but reintroduce the BC.
The prepending of exec is dropping some supported cases currently, and so breaking some use cases. Sending signals is only a feature request for 2.2, not a supported feature in 2.1.

Auto-answer : It does not work.

okay, you are correct. The only features I'm wondering that might have a substancial change is ones that use proc_terminate and could have some glitchy results

As proc_terminate will send signal on the sh wrapper, then this wrapper could lose its child which will run without parent process

@stof What do you think about this PR, Should it be merged ? If yes, into master, 2.1, 2.0 ?

@lyrixx in 2.1 as this is where the exec has been introduced and needs to be reverted.

The tests could also be added in 2.0 but it is not worth it IMO as merging tests between 2.0 and 2.1 is a real pain for @fabpot as we moved them.

OK, So i sould close this PR right now, and open it again against 2.1 branch ?

yup, and reference this PR in your new one