[Process] Allow merging env vars instead of replacing them

nicolas-grekas · nicolas-grekas · commit 524508cdcf49 · 2016-06-15T12:49:17.000+02:00
diff --git a/src/Symfony/Component/Process/Process.php b/src/Symfony/Component/Process/Process.php
@@ -73,6 +73,7 @@ class Process implements \IteratorAggregate
     private $incrementalErrorOutputOffset = 0;
     private $tty;
     private $pty;
+    private $inheritEnv = false;
 
     private $useFileHandles = false;
     /** @var PipesInterface */
@@ -267,9 +268,22 @@ 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 = null;
+        } else {
+            $env = $this->env;
+        }
         if ('\\' === DIRECTORY_SEPARATOR && $this->enhanceWindowsCompatibility) {
-            $commandline = 'cmd /V:ON /E:ON /D /C "('.$commandline.')';
+            $commandline = 'cmd /V:ON /E:ON /D /C "('.$envline.$commandline.')';
             foreach ($this->processPipes->getFiles() as $offset => $filename) {
                 $commandline .= ' '.$offset.'>'.ProcessUtils::escapeArgument($filename);
             }
@@ -283,15 +297,17 @@ public function start(callable $callback = null)
             $descriptors[3] = array('pipe', 'w');
 
             // See https://unix.stackexchange.com/questions/71205/background-process-pipe-input
-            $commandline = '{ ('.$this->commandline.') <&3 3<&- 3>/dev/null & } 3<&0;';
+            $commandline = $envline.'{ ('.$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, $this->env, $this->options);
+        $this->process = proc_open($commandline, $descriptors, $this->processPipes->pipes, $this->cwd, $env, $this->options);
 
         if (!is_resource($this->process)) {
             throw new RuntimeException('Unable to launch a new process.');
@@ -1197,6 +1213,30 @@ public function setEnhanceSigchildCompatibility($enhance)
         return $this;
     }
 
+    /**
+     * Sets whether environment variables will be inherited or not.
+     *
+     * @param bool $inheritEnv
+     *
+     * @return self The current Process instance
+     */
+    public function inheritEnvironmentVariables($inheritEnv = true)
+    {
+        $this->inheritEnv = (bool) $inheritEnv;
+
+        return $this;
+    }
+
+    /**
+     * Returns whether environment variables will be inherited or not.
+     *
+     * @return bool
+     */
+    public function areEnvironmentVariablesInherited()
+    {
+        return $this->inheritEnv;
+    }
+
     /**
      * Performs a check between the timeout definition and the time the process started.
      *
diff --git a/src/Symfony/Component/Process/ProcessBuilder.php b/src/Symfony/Component/Process/ProcessBuilder.php
@@ -267,14 +267,11 @@ public function getProcess()
         $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);
+
         if ($this->inheritEnv) {
-            $env = array_replace($_ENV, $_SERVER, $this->env);
-        } else {
-            $env = $this->env;
+            $process->inheritEnvironmentVariables();
         }
-
-        $process = new Process($script, $this->cwd, $env, $this->input, $this->timeout, $options);
-
         if ($this->outputDisabled) {
             $process->disableOutput();
         }
diff --git a/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php b/src/Symfony/Component/Process/Tests/ProcessBuilderTest.php
@@ -17,17 +17,11 @@ class ProcessBuilderTest extends \PHPUnit_Framework_TestCase
 {
     public function testInheritEnvironmentVars()
     {
-        $_ENV['MY_VAR_1'] = 'foo';
-
         $proc = ProcessBuilder::create()
             ->add('foo')
             ->getProcess();
 
-        unset($_ENV['MY_VAR_1']);
-
-        $env = $proc->getEnv();
-        $this->assertArrayHasKey('MY_VAR_1', $env);
-        $this->assertEquals('foo', $env['MY_VAR_1']);
+        $this->assertTrue($proc->areEnvironmentVariablesInherited());
     }
 
     public function testAddEnvironmentVariables()
@@ -46,22 +40,7 @@ public function testAddEnvironmentVariables()
         ;
 
         $this->assertSame($env, $proc->getEnv());
-    }
-
-    public function testProcessShouldInheritAndOverrideEnvironmentVars()
-    {
-        $_ENV['MY_VAR_1'] = 'foo';
-
-        $proc = ProcessBuilder::create()
-            ->setEnv('MY_VAR_1', 'bar')
-            ->add('foo')
-            ->getProcess();
-
-        unset($_ENV['MY_VAR_1']);
-
-        $env = $proc->getEnv();
-        $this->assertArrayHasKey('MY_VAR_1', $env);
-        $this->assertEquals('bar', $env['MY_VAR_1']);
+        $this->assertFalse($proc->areEnvironmentVariablesInherited());
     }
 
     /**
diff --git a/src/Symfony/Component/Process/Tests/ProcessTest.php b/src/Symfony/Component/Process/Tests/ProcessTest.php
@@ -1357,6 +1357,40 @@ public function testChainedProcesses()
         $this->assertSame('456', $p2->getOutput());
     }
 
+    public function testInheritEnvEnabled()
+    {
+        $process = $this->getProcess(self::$phpBin.' -r '.escapeshellarg('echo serialize($_SERVER);'), null, array('BAR' => 'BAZ'));
+
+        putenv('FOO=BAR');
+
+        $this->assertSame($process, $process->inheritEnvironmentVariables(1));
+        $this->assertTrue($process->areEnvironmentVariablesInherited());
+
+        $process->run();
+
+        $expected = array('BAR' => 'BAZ', 'FOO' => 'BAR');
+        $env = array_intersect_key(unserialize($process->getOutput()), $expected);
+
+        $this->assertSame($expected, $env);
+    }
+
+    public function testInheritEnvDisabled()
+    {
+        $process = $this->getProcess(self::$phpBin.' -r '.escapeshellarg('echo serialize($_SERVER);'), null, array('BAR' => 'BAZ'));
+
+        putenv('FOO=BAR');
+
+        $this->assertFalse($process->areEnvironmentVariablesInherited());
+
+        $process->run();
+
+        $expected = array('BAR' => 'BAZ', 'FOO' => 'BAR');
+        $env = array_intersect_key(unserialize($process->getOutput()), $expected);
+        unset($expected['FOO']);
+
+        $this->assertSame($expected, $env);
+    }
+
     /**
      * @param string      $commandline
      * @param null|string $cwd
