Skip to content

Commit ab1794f

Browse files
nicolas-grekasnicolas-grekas
committed
[Process][Console] deprecated defining commands as strings
1 parent f27c3a8 commit ab1794f

File tree

11 files changed

+174
-48
lines changed

11 files changed

+174
-48
lines changed

‎UPGRADE-4.2.md

+32
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,22 @@ Config
1111

1212
* Deprecated constructing a `TreeBuilder` without passing root node information.
1313

14+
Console
15+
-------
16+
17+
* Deprecated passing a command as a string to `ProcessHelper::run()`,
18+
pass it the command as an array of arguments instead.
19+
20+
Before:
21+
```php
22+
$processHelper->run($output, 'ls -l');
23+
```
24+
25+
After:
26+
```php
27+
$processHelper->run($output, array('ls', '-l'));
28+
```
29+
1430
DoctrineBridge
1531
--------------
1632

@@ -37,6 +53,22 @@ Form
3753
{% endfor %}
3854
```
3955

56+
Process
57+
-------
58+
59+
* Deprecated the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods.
60+
* Deprecated passing commands as strings when creating a `Process` instance.
61+
62+
Before:
63+
```php
64+
$process = new Process('ls -l');
65+
```
66+
67+
After:
68+
```php
69+
$process = new Process(array('ls', '-l'));
70+
```
71+
4072
Security
4173
--------
4274

‎UPGRADE-5.0.md

+28
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,18 @@ Console
2121
* Removed the `getHorizontalBorderChar()` method in favor of the `getBorderChars()` method in `TableStyle`.
2222
* Removed the `setVerticalBorderChar()` method in favor of the `setVerticalBorderChars()` method in `TableStyle`.
2323
* Removed the `getVerticalBorderChar()` method in favor of the `getBorderChars()` method in `TableStyle`.
24+
* The `ProcessHelper::run()` method takes the command as an array of arguments.
25+
26+
Before:
27+
```php
28+
$processHelper->run($output, 'ls -l');
29+
```
30+
31+
After:
32+
```php
33+
$processHelper->run($output, array('ls', '-l'));
34+
```
35+
2436

2537
DependencyInjection
2638
-------------------
@@ -78,6 +90,22 @@ HttpFoundation
7890
* The `getClientSize()` method of the `UploadedFile` class has been removed.
7991
* The `getSession()` method of the `Request` class throws an exception when session is null.
8092

93+
Process
94+
-------
95+
96+
* Removed the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods.
97+
* Commands must be defined as arrays when creating a `Process` instance.
98+
99+
Before:
100+
```php
101+
$process = new Process('ls -l');
102+
```
103+
104+
After:
105+
```php
106+
$process = new Process(array('ls', '-l'));
107+
```
108+
81109
Security
82110
--------
83111

‎src/Symfony/Component/Console/CHANGELOG.md

+8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,14 @@
11
CHANGELOG
22
=========
33

4+
4.2.0
5+
-----
6+
7+
* allowed passing commands as `array($process, 'ENV_VAR' => 'value')` to `ProcessHelper::run()`
8+
* deprecated passing a command as a string to `ProcessHelper::run()`,
9+
pass it the command as an array of its arguments instead
10+
* made the `ProcessHelper` class final
11+
412
4.1.0
513
-----
614

‎src/Symfony/Component/Console/Helper/ProcessHelper.php

+24-9
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,20 @@
2020
* The ProcessHelper class provides helpers to run external processes.
2121
*
2222
* @author Fabien Potencier <fabien@symfony.com>
23+
*
24+
* @final since Symfony 4.2
2325
*/
2426
class ProcessHelper extends Helper
2527
{
2628
/**
2729
* Runs an external process.
2830
*
29-
* @param OutputInterface $output An OutputInterface instance
30-
* @param string|array|Process $cmd An instance of Process or an array of arguments to escape and run or a command to run
31-
* @param string|null $error An error message that must be displayed if something went wrong
32-
* @param callable|null $callback A PHP callback to run whenever there is some
33-
* output available on STDOUT or STDERR
34-
* @param int $verbosity The threshold for verbosity
31+
* @param OutputInterface $output An OutputInterface instance
32+
* @param array|Process $cmd An instance of Process or an array of the command and arguments
33+
* @param string|null $error An error message that must be displayed if something went wrong
34+
* @param callable|null $callback A PHP callback to run whenever there is some
35+
* output available on STDOUT or STDERR
36+
* @param int $verbosity The threshold for verbosity
3537
*
3638
* @return Process The process that ran
3739
*/
@@ -44,9 +46,22 @@ public function run(OutputInterface $output, $cmd, $error = null, callable $call
4446
$formatter = $this->getHelperSet()->get('debug_formatter');
4547

4648
if ($cmd instanceof Process) {
47-
$process = $cmd;
48-
} else {
49+
$cmd = array($cmd);
50+
}
51+
52+
if (!\is_array($cmd)) {
53+
@trigger_error(sprintf('Passing a command as a string to "%s()" is deprecated since Symfony 4.2, pass it the command as an array of arguments instead.', __METHOD__), E_USER_DEPRECATED);
54+
$cmd = array(\method_exists(Process::class, 'withShell') ? Process::withShell($cmd) : new Process($cmd));
55+
}
56+
57+
if (\is_string($cmd[0] ?? null)) {
4958
$process = new Process($cmd);
59+
$cmd = array();
60+
} elseif (($cmd[0] ?? null) instanceof Process) {
61+
$process = $cmd[0];
62+
unset($cmd[0]);
63+
} else {
64+
throw new \InvalidArgumentException(sprintf('Invalid command provided to "%s()": the command should an array whose first is element is either the path to the binary to run of a "Process" object.', __METHOD__));
5065
}
5166

5267
if ($verbosity <= $output->getVerbosity()) {
@@ -57,7 +72,7 @@ public function run(OutputInterface $output, $cmd, $error = null, callable $call
5772
$callback = $this->wrapCallback($output, $process, $callback);
5873
}
5974

60-
$process->run($callback);
75+
$process->run($callback, $cmd);
6176

6277
if ($verbosity <= $output->getVerbosity()) {
6378
$message = $process->isSuccessful() ? 'Command ran successfully' : sprintf('%s Command did not run successfully', $process->getExitCode());

‎src/Symfony/Component/Console/Tests/Helper/ProcessHelperTest.php

+17-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,10 @@ class ProcessHelperTest extends TestCase
2525
*/
2626
public function testVariousProcessRuns($expected, $cmd, $verbosity, $error)
2727
{
28+
if (\is_string($cmd)) {
29+
$cmd = \method_exists(Process::class, 'withShell') ? Process::withShell($cmd) : new Process($cmd);
30+
}
31+
2832
$helper = new ProcessHelper();
2933
$helper->setHelperSet(new HelperSet(array(new DebugFormatterHelper())));
3034
$output = $this->getOutputStream($verbosity);
@@ -41,7 +45,7 @@ public function testPassedCallbackIsExecuted()
4145
$executed = false;
4246
$callback = function () use (&$executed) { $executed = true; };
4347

44-
$helper->run($output, 'php -r "echo 42;"', null, $callback);
48+
$helper->run($output, array('php', '-r', 'echo 42;'), null, $callback);
4549
$this->assertTrue($executed);
4650
}
4751

@@ -81,12 +85,21 @@ public function provideCommandsAndOutput()
8185
OUT out message
8286
RES 252 Command did not run successfully
8387

88+
EOT;
89+
90+
$PHP = '\\' === \DIRECTORY_SEPARATOR ? '!PHP!' : '"$PHP"';
91+
$successOutputPhp = <<<EOT
92+
RUN php -r $PHP
93+
OUT 42
94+
RES Command ran successfully
95+
8496
EOT;
8597

8698
$errorMessage = 'An error occurred';
8799
$args = new Process(array('php', '-r', 'echo 42;'));
88100
$args = $args->getCommandLine();
89101
$successOutputProcessDebug = str_replace("'php' '-r' 'echo 42;'", $args, $successOutputProcessDebug);
102+
$withShell = \method_exists(Process::class, 'withShell') ? array(Process::class, 'withShell') : function ($cmd) { return new Process($cmd); };
90103

91104
return array(
92105
array('', 'php -r "echo 42;"', StreamOutput::VERBOSITY_VERBOSE, null),
@@ -100,7 +113,9 @@ public function provideCommandsAndOutput()
100113
array($syntaxErrorOutputVerbose.$errorMessage.PHP_EOL, 'php -r "fwrite(STDERR, \'error message\');usleep(50000);fwrite(STDOUT, \'out message\');exit(252);"', StreamOutput::VERBOSITY_VERY_VERBOSE, $errorMessage),
101114
array($syntaxErrorOutputDebug.$errorMessage.PHP_EOL, 'php -r "fwrite(STDERR, \'error message\');usleep(500000);fwrite(STDOUT, \'out message\');exit(252);"', StreamOutput::VERBOSITY_DEBUG, $errorMessage),
102115
array($successOutputProcessDebug, array('php', '-r', 'echo 42;'), StreamOutput::VERBOSITY_DEBUG, null),
103-
array($successOutputDebug, new Process('php -r "echo 42;"'), StreamOutput::VERBOSITY_DEBUG, null),
116+
array($successOutputDebug, $withShell('php -r "echo 42;"'), StreamOutput::VERBOSITY_DEBUG, null),
117+
array($successOutputProcessDebug, array(new Process(array('php', '-r', 'echo 42;'))), StreamOutput::VERBOSITY_DEBUG, null),
118+
array($successOutputPhp, array($withShell('php -r '.$PHP), 'PHP' => 'echo 42;'), StreamOutput::VERBOSITY_DEBUG, null),
104119
);
105120
}
106121

‎src/Symfony/Component/Dotenv/Dotenv.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -330,7 +330,7 @@ private function resolveCommands($value)
330330
throw new \LogicException('Resolving commands requires the Symfony Process component.');
331331
}
332332

333-
$process = new Process('echo '.$matches[0]);
333+
$process = \method_exists(Process::class, 'withShell') ? Process::withShell('echo '.$matches[0]) : new Process('echo '.$matches[0]);
334334
$process->inheritEnvironmentVariables(true);
335335
$process->setEnv($this->values);
336336
try {

‎src/Symfony/Component/Process/CHANGELOG.md

+7
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,13 @@
11
CHANGELOG
22
=========
33

4+
4.2.0
5+
-----
6+
7+
* added the `Process::withShell()` static constructor to define shell command-lines
8+
* deprecated passing a command as string when creating a `Process` instance
9+
* deprecated the `Process::setCommandline()` and the `PhpProcess::setPhpBinary()` methods
10+
411
4.1.0
512
-----
613

‎src/Symfony/Component/Process/PhpProcess.php

+7-2
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,12 @@ class PhpProcess extends Process
2929
* @param string|null $cwd The working directory or null to use the working dir of the current PHP process
3030
* @param array|null $env The environment variables or null to use the same environment as the current PHP process
3131
* @param int $timeout The timeout in seconds
32+
* @param array|null $php Path to the PHP binary to use with any additional arguments
3233
*/
33-
public function __construct(string $script, string $cwd = null, array $env = null, int $timeout = 60)
34+
public function __construct(string $script, string $cwd = null, array $env = null, int $timeout = 60, array $php = null)
3435
{
3536
$executableFinder = new PhpExecutableFinder();
36-
if (false === $php = $executableFinder->find(false)) {
37+
if (false === $php = $php ?? $executableFinder->find(false)) {
3738
$php = null;
3839
} else {
3940
$php = array_merge(array($php), $executableFinder->findArguments());
@@ -51,9 +52,13 @@ public function __construct(string $script, string $cwd = null, array $env = nul
5152

5253
/**
5354
* Sets the path to the PHP binary to use.
55+
*
56+
* @deprecated since Symfony 4.2, use the $php argument of the constructor instead.
5457
*/
5558
public function setPhpBinary($php)
5659
{
60+
@trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2, use the $php argument of the constructor instead.', __METHOD__), E_USER_DEPRECATED);
61+
5762
$this->setCommandLine($php);
5863
}
5964

‎src/Symfony/Component/Process/Process.php

+32-7
Original file line numberDiff line numberDiff line change
@@ -129,21 +129,25 @@ class Process implements \IteratorAggregate
129129
);
130130

131131
/**
132-
* @param string|array $commandline The command line to run
133-
* @param string|null $cwd The working directory or null to use the working dir of the current PHP process
134-
* @param array|null $env The environment variables or null to use the same environment as the current PHP process
135-
* @param mixed|null $input The input as stream resource, scalar or \Traversable, or null for no input
136-
* @param int|float|null $timeout The timeout in seconds or null to disable
132+
* @param array $command The command to run and its arguments listed as separate entries
133+
* @param string|null $cwd The working directory or null to use the working dir of the current PHP process
134+
* @param array|null $env The environment variables or null to use the same environment as the current PHP process
135+
* @param mixed|null $input The input as stream resource, scalar or \Traversable, or null for no input
136+
* @param int|float|null $timeout The timeout in seconds or null to disable
137137
*
138138
* @throws RuntimeException When proc_open is not installed
139139
*/
140-
public function __construct($commandline, string $cwd = null, array $env = null, $input = null, ?float $timeout = 60)
140+
public function __construct($command, string $cwd = null, array $env = null, $input = null, ?float $timeout = 60)
141141
{
142142
if (!function_exists('proc_open')) {
143143
throw new RuntimeException('The Process class relies on proc_open, which is not available on your PHP installation.');
144144
}
145145

146-
$this->commandline = $commandline;
146+
if (!\is_array($command)) {
147+
@trigger_error(sprintf('Passing a command as string when creating a "%s" instance is deprecated since Symfony 4.2, pass it as an array of its arguments instead, or use the "Process::withShell()" constructor if you need features provided by the shell.', __CLASS__), E_USER_DEPRECATED);
148+
}
149+
150+
$this->commandline = $command;
147151
$this->cwd = $cwd;
148152

149153
// on Windows, if the cwd changed via chdir(), proc_open defaults to the dir where PHP was started
@@ -163,6 +167,23 @@ public function __construct($commandline, string $cwd = null, array $env = null,
163167
$this->pty = false;
164168
}
165169

170+
/**
171+
* @param string $command The command line to pass to the shell of the OS
172+
* @param string|null $cwd The working directory or null to use the working dir of the current PHP process
173+
* @param array|null $env The environment variables or null to use the same environment as the current PHP process
174+
* @param mixed|null $input The input as stream resource, scalar or \Traversable, or null for no input
175+
* @param int|float|null $timeout The timeout in seconds or null to disable
176+
*
177+
* @throws RuntimeException When proc_open is not installed
178+
*/
179+
public static function withShell(string $command, string $cwd = null, array $env = null, $input = null, ?float $timeout = 60)
180+
{
181+
$process = new static(array(), $cwd, $env, $input, $timeout);
182+
$process->commandline = $command;
183+
184+
return $process;
185+
}
186+
166187
public function __destruct()
167188
{
168189
$this->stop(0);
@@ -892,9 +913,13 @@ public function getCommandLine()
892913
* @param string|array $commandline The command to execute
893914
*
894915
* @return self The current Process instance
916+
*
917+
* @deprecated since Symfony 4.2.
895918
*/
896919
public function setCommandLine($commandline)
897920
{
921+
@trigger_error(sprintf('The "%s()" method is deprecated since Symfony 4.2.', __METHOD__), E_USER_DEPRECATED);
922+
898923
$this->commandline = $commandline;
899924

900925
return $this;

‎src/Symfony/Component/Process/Tests/ProcessFailedExceptionTest.php

+7-7
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class ProcessFailedExceptionTest extends TestCase
2424
*/
2525
public function testProcessFailedExceptionThrowsException()
2626
{
27-
$process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful'))->setConstructorArgs(array('php'))->getMock();
27+
$process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful'))->setConstructorArgs(array(array('php')))->getMock();
2828
$process->expects($this->once())
2929
->method('isSuccessful')
3030
->will($this->returnValue(true));
@@ -52,7 +52,7 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(
5252
$errorOutput = 'FATAL: Unexpected error';
5353
$workingDirectory = getcwd();
5454

55-
$process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'getOutput', 'getErrorOutput', 'getExitCode', 'getExitCodeText', 'isOutputDisabled', 'getWorkingDirectory'))->setConstructorArgs(array($cmd))->getMock();
55+
$process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'getOutput', 'getErrorOutput', 'getExitCode', 'getExitCodeText', 'isOutputDisabled', 'getWorkingDirectory'))->setConstructorArgs(array(array($cmd)))->getMock();
5656
$process->expects($this->once())
5757
->method('isSuccessful')
5858
->will($this->returnValue(false));
@@ -84,8 +84,8 @@ public function testProcessFailedExceptionPopulatesInformationFromProcessOutput(
8484
$exception = new ProcessFailedException($process);
8585

8686
$this->assertEquals(
87-
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
88-
$exception->getMessage()
87+
"The command \"\"$cmd\"\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}\n\nOutput:\n================\n{$output}\n\nError Output:\n================\n{$errorOutput}",
88+
str_replace("'php'", '"php"', $exception->getMessage())
8989
);
9090
}
9191

@@ -100,7 +100,7 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput()
100100
$exitText = 'General error';
101101
$workingDirectory = getcwd();
102102

103-
$process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'isOutputDisabled', 'getExitCode', 'getExitCodeText', 'getOutput', 'getErrorOutput', 'getWorkingDirectory'))->setConstructorArgs(array($cmd))->getMock();
103+
$process = $this->getMockBuilder('Symfony\Component\Process\Process')->setMethods(array('isSuccessful', 'isOutputDisabled', 'getExitCode', 'getExitCodeText', 'getOutput', 'getErrorOutput', 'getWorkingDirectory'))->setConstructorArgs(array(array($cmd)))->getMock();
104104
$process->expects($this->once())
105105
->method('isSuccessful')
106106
->will($this->returnValue(false));
@@ -130,8 +130,8 @@ public function testDisabledOutputInFailedExceptionDoesNotPopulateOutput()
130130
$exception = new ProcessFailedException($process);
131131

132132
$this->assertEquals(
133-
"The command \"$cmd\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
134-
$exception->getMessage()
133+
"The command \"\"$cmd\"\" failed.\n\nExit Code: $exitCode($exitText)\n\nWorking directory: {$workingDirectory}",
134+
str_replace("'php'", '"php"', $exception->getMessage())
135135
);
136136
}
137137
}

0 commit comments

Comments
 (0)