Skip to content
This repository

#7106 - check php version for getcwd() #7248

Closed
wants to merge 1 commit into from

6 participants

Julien Moulin Fabien Potencier Jordi Boggiano Joseph Bielawski Jakub Zalas Victor Berchet
Julien Moulin
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #7106
License MIT
src/Symfony/Component/Process/Process.php
... ...
@@ -131,8 +131,12 @@ public function __construct($commandline, $cwd = null, array $env = null, $stdin
131 131
 
132 132
         $this->commandline = $commandline;
133 133
         $this->cwd = $cwd;
  134
+
  135
+        $isOnRangeFor53 = version_compare(PHP_VERSION, '5.3.0', '>=') && version_compare(PHP_VERSION, '5.3.21', '<=');
2
Joseph Bielawski
stloyd added a note March 03, 2013

There is no reason for first check.

Victor Berchet
vicb added a note March 03, 2013

Could you link the PHP defect related to those checks in a comment ? thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Symfony/Component/Process/Process.php
((8 lines not shown))
134 138
         // on windows, if the cwd changed via chdir(), proc_open defaults to the dir where php was started
135  
-        if (null === $this->cwd && defined('PHP_WINDOWS_VERSION_BUILD')) {
  139
+        if (null === $this->cwd && defined('PHP_WINDOWS_VERSION_BUILD') && ($isOnRangeFor53 or $isOnRangeFor54)) {
1
Jakub Zalas Collaborator
jakzal added a note March 03, 2013

There's no point in using "or" here. I'd suggest || for consistency.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
src/Symfony/Component/Process/Process.php
((5 lines not shown))
134 135
         // on windows, if the cwd changed via chdir(), proc_open defaults to the dir where php was started
135  
-        if (null === $this->cwd && defined('PHP_WINDOWS_VERSION_BUILD')) {
  136
+        // on gnu/linux, PHP builds with --enable-maintainer-zts are also affected
  137
+        // @see : https://bugs.php.net/bug.php?id=51800
  138
+        // @see : https://bugs.php.net/bug.php?id=50524
  139
+
  140
+        if (defined('ZEND_THREAD_SAFE')) {
1
Fabien Potencier Owner
fabpot added a note March 20, 2013

Don't we need to keep the null === $this->cwd condition?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Julien Moulin

This seems indeed useless.
Related #7106, can you confirm @Seldaek ?

Fabien Potencier
Owner

I'm pretty sure we need to keep the null condition. If not, there is no way to override the cwd.

Jordi Boggiano
Collaborator

Sure the null check needs to say, it should be like this IMO: if (null === $this->cwd && (defined('ZEND_THREAD_SAFE') || defined('PHP_WINDOWS_VERSION_BUILD'))) { ... }

Julien Moulin

Ok i'll fix

Fabien Potencier fabpot referenced this pull request from a commit March 23, 2013
Fabien Potencier merged branch lizjulien/7106 (PR #7248)
This PR was submitted for the master branch but it was merged into the 2.1 branch instead (closes #7248).

Discussion
----------

#7106 - check php version for getcwd()

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7106
| License       | MIT

Commits
-------

11d3855  #7106 - fix for ZTS builds
78ebba5
Fabien Potencier fabpot closed this March 23, 2013
mmucklo mmucklo referenced this pull request from a commit March 23, 2013
Fabien Potencier merged branch lizjulien/7106 (PR #7248)
This PR was submitted for the master branch but it was merged into the 2.1 branch instead (closes #7248).

Discussion
----------

#7106 - check php version for getcwd()

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #7106
| License       | MIT

Commits
-------

11d3855  #7106 - fix for ZTS builds
4804562
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Mar 20, 2013
Julien Moulin #7106 - fix for ZTS builds 11d3855
This page is out of date. Refresh to see the latest.
7  src/Symfony/Component/Process/Process.php
@@ -131,8 +131,13 @@ public function __construct($commandline, $cwd = null, array $env = null, $stdin
131 131
 
132 132
         $this->commandline = $commandline;
133 133
         $this->cwd = $cwd;
  134
+
134 135
         // on windows, if the cwd changed via chdir(), proc_open defaults to the dir where php was started
135  
-        if (null === $this->cwd && defined('PHP_WINDOWS_VERSION_BUILD')) {
  136
+        // on gnu/linux, PHP builds with --enable-maintainer-zts are also affected
  137
+        // @see : https://bugs.php.net/bug.php?id=51800
  138
+        // @see : https://bugs.php.net/bug.php?id=50524
  139
+
  140
+        if (null === $this->cwd && (defined('ZEND_THREAD_SAFE') || defined('PHP_WINDOWS_VERSION_BUILD'))) {
136 141
             $this->cwd = getcwd();
137 142
         }
138 143
         if (null !== $env) {
Commit_comment_tip

Tip: You can add notes to lines in a file. Hover to the left of a line to make a note

Something went wrong with that request. Please try again.