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

Already on GitHub? Sign in to your account

[Process] Add a version check to the code added in #7102 #7106

Closed
Seldaek opened this Issue Feb 18, 2013 · 9 comments

Comments

Projects
None yet
4 participants
Member

Seldaek commented Feb 18, 2013

When PHP 5.5 is released, and if the code still seems to work fine, a version check could /should be added to the block added in #7102 so that it's only executed on 5.3/5.4 since these are the buggy versions.

Member

Seldaek commented Feb 18, 2013

The fix is going to be released in: 5.4.12 / 5.3.22 / 5.5.0, lower versions of 5.4 and 5.3 are affected and need this workaround.

Contributor

lizjulien commented Mar 3, 2013

@Seldaek ok for you like that ?

Member

Seldaek commented Mar 3, 2013

Looks good yes.

Member

Seldaek commented Mar 10, 2013

@pierrejoye it seems that PHP builds with --enable-maintainer-zts are also affected by this on linux, and even on 5.4.12 it still is present. @loicfrering just helped debug this on travis (where they added this configure flag recently). Seems like https://bugs.php.net/bug.php?id=50524 is related.

Is there an easy way to detect ZTS builds? That way we could enable this fix for ZTS builds only. Or maybe we should revert to always doing:

if (null === $this->cwd) {
    $this->cwd = getcwd() ?: null;
}

yes, there is a constant for that, ZEND_THREAD_SAFE

On Sun, Mar 10, 2013 at 6:58 PM, Jordi Boggiano notifications@github.comwrote:

@pierrejoye https://github.com/PierreJoye it seems that PHP builds with
--enable-maintainer-zts are also affected by this on linux, and even on
5.4.12 it still is present. @loicfrering https://github.com/loicfreringjust helped debug this on travis (where they added this configure flag
recently). Seems like https://bugs.php.net/bug.php?id=50524 is related.

Is there an easy way to detect ZTS builds? That way we could enable this
fix for ZTS builds only. Or maybe we should revert to always doing:

if (null === $this->cwd) {
$this->cwd = getcwd() ?: null;
}


Reply to this email directly or view it on GitHubhttps://github.com/symfony/symfony/issues/7106#issuecomment-14685623
.

Pierre

@pierrejoye | http://blog.thepimp.net | http://www.libgd.org

Member

Seldaek commented Mar 11, 2013

@lizjulien could you update your PR to enable this workaround for any php that has ZEND_THREAD_SAFE for now? Because it seems it's still not fixed in 5.4.12 on linux at least (haven't checked windows).

Contributor

lizjulien commented Mar 11, 2013

@Seldaek Sure, i'll do that. But just to be sure, we remove all check for version and we replace it by a constant check?

Member

Seldaek commented Mar 11, 2013

I think it's the only option until we can test for sure that it is fixed in a given version.

Contributor

lizjulien commented Mar 11, 2013

ok

@lizjulien lizjulien added a commit to lizjulien/symfony that referenced this issue Mar 20, 2013

@lizjulien lizjulien #7106 - fix for ZTS builds 11d3855

@fabpot fabpot added a commit that referenced this issue Mar 23, 2013

@lizjulien @fabpot lizjulien + fabpot #7106 - fix for ZTS builds 11c0fb5

@fabpot fabpot added a commit that referenced this issue Mar 23, 2013

@fabpot fabpot 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

@fabpot fabpot closed this Mar 23, 2013

@fabpot fabpot added a commit that referenced this issue Mar 23, 2013

@fabpot fabpot Merge branch '2.1' into 2.2
* 2.1:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129

Conflicts:
	.travis.yml
	src/Symfony/Bundle/FrameworkBundle/Routing/Router.php
	src/Symfony/Component/Routing/RouteCollection.php
03fc97d

@fabpot fabpot added a commit that referenced this issue Mar 23, 2013

@fabpot fabpot Merge branch '2.2'
* 2.2:
  #7106 - fix for ZTS builds
  Added '@@' escaping strategy for YamlFileLoader and YamlDumper
  [Yaml] fixed bugs with folded scalar parsing
  [Form] made DefaultCsrfProvider using session_status() when available
  Added unit tests to Dumper
  Update .travis.yml (closes #7355)
  [HttpFoudantion] fixed Request::getPreferredLanguage()
  Revert "merged branch jfsimon/issue-6928 (PR #7378)"
  Routing issue with installation in a sub-directory ref: #7129
77ec799
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment