Permalink
Browse files

feature #21470 [Process] Deprecate not inheriting env vars + compat r…

…elated settings (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

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

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | no
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Turning compat on/off is not a feature in itself.

About env vars: if one has unwanted env vars, one will still be able to remove them explicitly for the command. From my experience, not having eg PATH or HTTP_PROXY, etc. is more problematic. I'd prefer people to care about setting/unsetting the environment vars **they know about**, rather than allowing them to start with no ENV and discover later that they missed setting some var.

Commits
-------

df14451 [Process] Deprecate not inheriting env vars + compat related settings
  • Loading branch information...
2 parents 1b28015 + df14451 commit 46daa359ea058d5a8ff4beb23d674c45b35e6774 @fabpot fabpot committed Feb 1, 2017
View
@@ -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
View
@@ -63,6 +63,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
--------
View
@@ -225,6 +225,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
--------
@@ -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
-----
@@ -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()) {
@@ -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);
}
@@ -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;
@@ -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.');
@@ -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()
@@ -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;
@@ -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.');
}
@@ -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.
@@ -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;
}
@@ -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;
}
@@ -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;
@@ -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;
}
@@ -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;
@@ -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;
}
@@ -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;
@@ -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;
@@ -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;
}
@@ -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;
@@ -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;
@@ -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;
@@ -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();
@@ -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()
@@ -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());
}
/**
Oops, something went wrong.

0 comments on commit 46daa35

Please sign in to comment.