-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[Process] Inherit env vars by default in PhpProcess #16288
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,7 +20,7 @@ | |
"symfony/dom-crawler": "~2.0,>=2.0.5" | ||
}, | ||
"require-dev": { | ||
"symfony/process": "~2.0,>=2.0.5", | ||
"symfony/process": "~2.3.34|~2.7,>=2.7.6", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should really start to use the caret operator. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. someone should open a PR :) |
||
"symfony/css-selector": "~2.0,>=2.0.5" | ||
}, | ||
"suggest": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -283,7 +283,7 @@ private static function getClassHierarchy(\ReflectionClass $class) | |
|
||
$traits = array(); | ||
|
||
if (function_exists('get_declared_traits')) { | ||
if (method_exists('ReflectionClass', 'getTraits')) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. not related but still worth cleaning: |
||
foreach ($classes as $c) { | ||
foreach (self::resolveDependencies(self::computeTraitDeps($c), $c) as $trait) { | ||
if ($trait !== $c) { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,7 +121,7 @@ class Process | |
* | ||
* @param string $commandline The command line to run | ||
* @param string|null $cwd The working directory or null to use the working dir of the current PHP process | ||
* @param array|null $env The environment variables or null to inherit | ||
* @param array|null $env The environment variables or null to use the same environment as the current PHP process | ||
* @param string|null $stdin The STDIN content | ||
* @param int|float|null $timeout The timeout in seconds or null to disable | ||
* @param array $options An array of options for proc_open | ||
|
@@ -851,7 +851,7 @@ public function setEnv(array $env) | |
|
||
$this->env = array(); | ||
foreach ($env as $key => $value) { | ||
$this->env[(binary) $key] = (binary) $value; | ||
$this->env[$key] = (string) $value; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is strictly equivalent |
||
} | ||
|
||
return $this; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't we need to keep the TMPDIR/TEMPDIR? We need to see when and why it was added, but I suppose it was there for a reason.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fabpot I guess it is because the environment was not inherited by default, and so we forced inheriting part of it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this seemed to be the reason (see 1cec45c and #1785).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, this was just a workaround against a bug fixed a few lines below (PhpProcess defaulting to not inheriting the env)