Skip to content

[Process] Deprecate not inheriting env vars + compat related settings #21470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ script:
- if [[ ! $deps && ! $PHP = hhvm* ]]; then echo "$COMPONENTS" | parallel --gnu '$PHPUNIT --exclude-group tty,benchmark,intl-data {}'"$REPORT"; fi
- if [[ ! $deps && ! $PHP = hhvm* ]]; then echo -e "\\nRunning tests requiring tty"; $PHPUNIT --group tty; fi
- if [[ ! $deps && $PHP = hhvm* ]]; then $PHPUNIT --exclude-group benchmark,intl-data; fi
- if [[ ! $deps && $PHP = ${MIN_PHP%.*} ]]; then echo -e "1\\n0" | xargs -I{} sh -c 'echo "\\nPHP --enable-sigchild enhanced={}" && ENHANCE_SIGCHLD={} php-$MIN_PHP/sapi/cli/php .phpunit/phpunit-4.8/phpunit --colors=always src/Symfony/Component/Process/'; fi
- if [[ ! $deps && $PHP = ${MIN_PHP%.*} ]]; then echo -e "1\\n0" | xargs -I{} sh -c 'echo "\\nPHP --enable-sigchild enhanced={}" && SYMFONY_DEPRECATIONS_HELPER=weak ENHANCE_SIGCHLD={} php-$MIN_PHP/sapi/cli/php .phpunit/phpunit-4.8/phpunit --colors=always src/Symfony/Component/Process/'; fi
- if [[ $deps = high ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --ansi; $PHPUNIT --exclude-group tty,benchmark,intl-data'$LEGACY"$REPORT"; fi
- if [[ $deps = low ]]; then echo "$COMPONENTS" | parallel --gnu -j10% 'cd {}; composer update --no-progress --ansi --prefer-lowest --prefer-stable; $PHPUNIT --exclude-group tty,benchmark,intl-data'"$REPORT"; fi
# Test the PhpUnit bridge using the original phpunit script
Expand Down
9 changes: 9 additions & 0 deletions UPGRADE-3.3.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,15 @@ HttpKernel
* The `Psr6CacheClearer::addPool()` method has been deprecated. Pass an array of pools indexed
by name to the constructor instead.

Process
-------

* Not inheriting environment variables is deprecated.

* Configuring `proc_open()` options is deprecated.

* Configuring Windows and sigchild compatibility is deprecated - they will be always enabled in 4.0.

Security
--------

Expand Down
9 changes: 9 additions & 0 deletions UPGRADE-4.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,15 @@ HttpKernel
* The `Psr6CacheClearer::addPool()` method has been removed. Pass an array of pools indexed
by name to the constructor instead.

Process
-------

* Environment variables are always inherited in sub-processes.

* Configuring `proc_open()` options has been removed.

* Configuring Windows and sigchild compatibility is not possible anymore - they are always enabled.

Security
--------

Expand Down
8 changes: 8 additions & 0 deletions src/Symfony/Component/Process/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,14 @@
CHANGELOG
=========

3.3.0
-----

* deprecated not inheriting environment variables
* deprecated configuring `proc_open()` options
* deprecated configuring enhanced Windows compatibility
* deprecated configuring enhanced sigchild compatibility

2.5.0
-----

Expand Down
5 changes: 4 additions & 1 deletion src/Symfony/Component/Process/PhpProcess.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class PhpProcess extends Process
* @param int $timeout The timeout in seconds
* @param array $options An array of options for proc_open
*/
public function __construct($script, $cwd = null, array $env = null, $timeout = 60, array $options = array())
public function __construct($script, $cwd = null, array $env = null, $timeout = 60, array $options = null)
{
$executableFinder = new PhpExecutableFinder();
if (false === $php = $executableFinder->find()) {
Expand All @@ -52,6 +52,9 @@ public function __construct($script, $cwd = null, array $env = null, $timeout =
// command with exec
$php = 'exec '.$php;
}
if (null !== $options) {
@trigger_error(sprintf('The $options parameter of the %s constructor is deprecated since version 3.3 and will be removed in 4.0.', __CLASS__), E_USER_DEPRECATED);
}

parent::__construct($php, $cwd, $env, $script, $timeout, $options);
}
Expand Down
84 changes: 56 additions & 28 deletions src/Symfony/Component/Process/Process.php
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ class Process implements \IteratorAggregate
private $lastOutputTime;
private $timeout;
private $idleTimeout;
private $options;
private $options = array('suppress_errors' => true);
private $exitcode;
private $fallbackStatus = array();
private $processInformation;
Expand Down Expand Up @@ -145,7 +145,7 @@ class Process implements \IteratorAggregate
*
* @throws RuntimeException When proc_open is not installed
*/
public function __construct($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60, array $options = array())
public function __construct($commandline, $cwd = null, array $env = null, $input = null, $timeout = 60, array $options = null)
{
if (!function_exists('proc_open')) {
throw new RuntimeException('The Process class relies on proc_open, which is not available on your PHP installation.');
Expand All @@ -171,7 +171,10 @@ public function __construct($commandline, $cwd = null, array $env = null, $input
$this->pty = false;
$this->enhanceWindowsCompatibility = true;
$this->enhanceSigchildCompatibility = '\\' !== DIRECTORY_SEPARATOR && $this->isSigchildEnabled();
$this->options = array_replace(array('suppress_errors' => true, 'binary_pipes' => true), $options);
if (null !== $options) {
@trigger_error(sprintf('The $options parameter of the %s constructor is deprecated since version 3.3 and will be removed in 4.0.', __CLASS__), E_USER_DEPRECATED);
$this->options = array_replace($this->options, $options);
}
}

public function __destruct()
Expand Down Expand Up @@ -268,26 +271,23 @@ public function start(callable $callback = null)
$descriptors = $this->getDescriptors();

$commandline = $this->commandline;
$envline = '';

if (null !== $this->env && $this->inheritEnv) {
if ('\\' === DIRECTORY_SEPARATOR && !empty($this->options['bypass_shell']) && !$this->enhanceWindowsCompatibility) {
throw new LogicException('The "bypass_shell" option must be false to inherit environment variables while enhanced Windows compatibility is off');
}
$env = '\\' === DIRECTORY_SEPARATOR ? '(SET %s)&&' : 'export %s;';
foreach ($this->env as $k => $v) {
$envline .= sprintf($env, ProcessUtils::escapeArgument("$k=$v"));
$env = $this->env;
$envBackup = array();
if (null !== $env && $this->inheritEnv) {
foreach ($env as $k => $v) {
$envBackup[$k] = getenv($v);
putenv(false === $v || null === $v ? $k : "$k=$v");
}
$env = null;
} else {
$env = $this->env;
} elseif (null !== $env) {
@trigger_error(sprintf('Not inheriting environment variables is deprecated since Symfony 3.3 and will always happen in 4.0. Set "Process::inheritEnvironmentVariables()" to true instead.', __METHOD__), E_USER_DEPRECATED);
}
if ('\\' === DIRECTORY_SEPARATOR && $this->enhanceWindowsCompatibility) {
$commandline = 'cmd /V:ON /E:ON /D /C "('.$envline.$commandline.')';
$commandline = 'cmd /V:ON /E:ON /D /C ('.str_replace("\n", ' ', $commandline).')';
foreach ($this->processPipes->getFiles() as $offset => $filename) {
$commandline .= ' '.$offset.'>'.ProcessUtils::escapeArgument($filename);
$commandline .= ' '.$offset.'>"'.$filename.'"';
}
$commandline .= '"';

if (!isset($this->options['bypass_shell'])) {
$this->options['bypass_shell'] = true;
Expand All @@ -297,18 +297,20 @@ public function start(callable $callback = null)
$descriptors[3] = array('pipe', 'w');

// See https://unix.stackexchange.com/questions/71205/background-process-pipe-input
$commandline = $envline.'{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
$commandline = '{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
$commandline .= 'pid=$!; echo $pid >&3; wait $pid; code=$?; echo $code >&3; exit $code';

// Workaround for the bug, when PTS functionality is enabled.
// @see : https://bugs.php.net/69442
$ptsWorkaround = fopen(__FILE__, 'r');
} elseif ('' !== $envline) {
$commandline = $envline.$commandline;
}

$this->process = proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $env, $this->options);

foreach ($envBackup as $k => $v) {
putenv(false === $v ? $k : "$k=$v");
}

if (!is_resource($this->process)) {
throw new RuntimeException('Unable to launch a new process.');
}
Expand Down Expand Up @@ -1089,6 +1091,8 @@ public function getEnv()
*
* An environment variable value should be a string.
* If it is an array, the variable is ignored.
* If it is false or null, it will be removed when
* env vars are otherwise inherited.
*
* That happens in PHP when 'argv' is registered into
* the $_ENV array for instance.
Expand All @@ -1099,15 +1103,7 @@ public function getEnv()
*/
public function setEnv(array $env)
{
// Process can not handle env values that are arrays
$env = array_filter($env, function ($value) {
return !is_array($value);
});

$this->env = array();
foreach ($env as $key => $value) {
$this->env[$key] = (string) $value;
}
$this->env = $env;

return $this;
}
Expand Down Expand Up @@ -1148,9 +1144,13 @@ public function setInput($input)
* Gets the options for proc_open.
*
* @return array The current options
*
* @deprecated since version 3.3, to be removed in 4.0.
*/
public function getOptions()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

return $this->options;
}

Expand All @@ -1160,9 +1160,13 @@ public function getOptions()
* @param array $options The new options
*
* @return self The current Process instance
*
* @deprecated since version 3.3, to be removed in 4.0.
*/
public function setOptions(array $options)
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

$this->options = $options;

return $this;
Expand All @@ -1174,9 +1178,13 @@ public function setOptions(array $options)
* This is true by default.
*
* @return bool
*
* @deprecated since version 3.3, to be removed in 4.0. Enhanced Windows compatibility will always be enabled.
*/
public function getEnhanceWindowsCompatibility()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Enhanced Windows compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);

return $this->enhanceWindowsCompatibility;
}

Expand All @@ -1186,9 +1194,13 @@ public function getEnhanceWindowsCompatibility()
* @param bool $enhance
*
* @return self The current Process instance
*
* @deprecated since version 3.3, to be removed in 4.0. Enhanced Windows compatibility will always be enabled.
*/
public function setEnhanceWindowsCompatibility($enhance)
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Enhanced Windows compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);

$this->enhanceWindowsCompatibility = (bool) $enhance;

return $this;
Expand All @@ -1198,9 +1210,13 @@ public function setEnhanceWindowsCompatibility($enhance)
* Returns whether sigchild compatibility mode is activated or not.
*
* @return bool
*
* @deprecated since version 3.3, to be removed in 4.0. Sigchild compatibility will always be enabled.
*/
public function getEnhanceSigchildCompatibility()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Sigchild compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);

return $this->enhanceSigchildCompatibility;
}

Expand All @@ -1214,9 +1230,13 @@ public function getEnhanceSigchildCompatibility()
* @param bool $enhance
*
* @return self The current Process instance
*
* @deprecated since version 3.3, to be removed in 4.0.
*/
public function setEnhanceSigchildCompatibility($enhance)
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Sigchild compatibility will always be enabled.', __METHOD__), E_USER_DEPRECATED);

$this->enhanceSigchildCompatibility = (bool) $enhance;

return $this;
Expand All @@ -1231,6 +1251,10 @@ public function setEnhanceSigchildCompatibility($enhance)
*/
public function inheritEnvironmentVariables($inheritEnv = true)
{
if (!$inheritEnv) {
@trigger_error(sprintf('Not inheriting environment variables is deprecated since Symfony 3.3 and will always happen in 4.0. Set "Process::inheritEnvironmentVariables()" to true instead.', __METHOD__), E_USER_DEPRECATED);
}

$this->inheritEnv = (bool) $inheritEnv;

return $this;
Expand All @@ -1240,9 +1264,13 @@ public function inheritEnvironmentVariables($inheritEnv = true)
* Returns whether environment variables will be inherited or not.
*
* @return bool
*
* @deprecated since version 3.3, to be removed in 4.0. Environment variables will always be inherited.
*/
public function areEnvironmentVariablesInherited()
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Environment variables will always be inherited.', __METHOD__), E_USER_DEPRECATED);

return $this->inheritEnv;
}

Expand Down
14 changes: 10 additions & 4 deletions src/Symfony/Component/Process/ProcessBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class ProcessBuilder
private $env = array();
private $input;
private $timeout = 60;
private $options = array();
private $options;
private $inheritEnv = true;
private $prefix = array();
private $outputDisabled = false;
Expand Down Expand Up @@ -120,9 +120,13 @@ public function setWorkingDirectory($cwd)
* @param bool $inheritEnv
*
* @return $this
*
* @deprecated since version 3.3, to be removed in 4.0.
*/
public function inheritEnvironmentVariables($inheritEnv = true)
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

$this->inheritEnv = $inheritEnv;

return $this;
Expand Down Expand Up @@ -217,9 +221,13 @@ public function setTimeout($timeout)
* @param string $value The option value
*
* @return $this
*
* @deprecated since version 3.3, to be removed in 4.0.
*/
public function setOption($name, $value)
{
@trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0.', __METHOD__), E_USER_DEPRECATED);

$this->options[$name] = $value;

return $this;
Expand Down Expand Up @@ -262,12 +270,10 @@ public function getProcess()
throw new LogicException('You must add() command arguments before calling getProcess().');
}

$options = $this->options;

$arguments = array_merge($this->prefix, $this->arguments);
$script = implode(' ', array_map(array(__NAMESPACE__.'\\ProcessUtils', 'escapeArgument'), $arguments));

$process = new Process($script, $this->cwd, $this->env, $this->input, $this->timeout, $options);
$process = new Process($script, $this->cwd, $this->env, $this->input, $this->timeout, $this->options);

if ($this->inheritEnv) {
$process->inheritEnvironmentVariables();
Expand Down
12 changes: 10 additions & 2 deletions src/Symfony/Component/Process/Tests/ProcessBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,23 @@

class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
{
/**
* @group legacy
*/
public function testInheritEnvironmentVars()
{
$proc = ProcessBuilder::create()
->add('foo')
->getProcess();

$this->assertTrue($proc->areEnvironmentVariablesInherited());

$proc = ProcessBuilder::create()
->add('foo')
->inheritEnvironmentVariables(false)
->getProcess();

$this->assertFalse($proc->areEnvironmentVariablesInherited());
}

public function testAddEnvironmentVariables()
Expand All @@ -35,12 +45,10 @@ public function testAddEnvironmentVariables()
->add('command')
->setEnv('foo', 'bar2')
->addEnvironmentVariables($env)
->inheritEnvironmentVariables(false)
->getProcess()
;

$this->assertSame($env, $proc->getEnv());
$this->assertFalse($proc->areEnvironmentVariablesInherited());
}

/**
Expand Down
Loading