Skip to content
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

PHP5.4 support #1041

Closed
brendo opened this issue Mar 2, 2012 · 32 comments
Closed

PHP5.4 support #1041

brendo opened this issue Mar 2, 2012 · 32 comments

Comments

@brendo
Copy link
Member

brendo commented Mar 2, 2012

PHP 5.4 is now officially released as a result, Symphony should ensure that is it compatible ASAP.

A migration guide is available from 5.3 to 5.4. From a quick skim we should be OK, but it's better to be safe than sorry (We know for a fact that the 2.2.x branch produces errors under 5.4).

Related issues:

@tonyarnold
Copy link
Contributor

I've got this setup locally, and it looks like there's a fair bit to fix. I know I've already spoken to @brendo about this, but there is some bad API design in here. Functions in subclasses that are overridden must match their parents, ie:

symphony/lib/toolkit/class.administrationpage.php

public function insertBreadcrumbs(array $values);

No, no, no. Please don't do this.
symphony/content/content.blueprintspages.php

public function insertBreadcrumbs($page_id, $preserve_last = true);

PHP 5.4 seems to enforce this now, so every instance where someone has decided that the original API didn't suit them and changed it in a subclass needs to be rewritten. saaad panda

@nickdunn
Copy link
Contributor

nickdunn commented Mar 2, 2012

Don't want to sound like I'm moaning, but does this need to be a 2.3.0 priority if PHP5.4 was only released yesterday?

Suggest it goes into 2.3.x.

(Unless I'm reading the way we're using milestones incorrectly, if so, apologies and I'll go back into my box.)

@brendo
Copy link
Member Author

brendo commented Mar 3, 2012

I had originally suggested 2.3 as the milestone as I wasn't aware of the number of warnings that PHP5.4 threw with Symphony (naive :(). Tony's work is a solid start, but I concur, not having this support won't block a 2.3 release.

I have updated the milestone to reflect this.

@brendo
Copy link
Member Author

brendo commented Mar 3, 2012

E_ALL now includes E_STRICT level errors, as per the error_reporting.

Ah, this is what is 'killing us'. @michael-e is an oracle ;)

@tonyarnold
Copy link
Contributor

We need to address this, not just recommend changing the setting. Less sloppy code is a good thing. I'll hopefully have more time this afternoon to keep going.

brendo added a commit that referenced this issue Mar 3, 2012
@michael-e
Copy link
Member

Ah, this is what is 'killing us'. @michael-e is an oracle ;)

Ah, yes, sorry that I have never told you. LOL.

@brendo
Copy link
Member Author

brendo commented Mar 7, 2012

@rowan-lewis got a copy of PHP5.4 today ran into similar warnings. After fixing a couple, we realised that it will be impossible (or very time consuming) to update the core so that it runs on PHP 5.4 (and 5.3) in E_STRICT mode whilst we maintain support for PHP5.2. The code in question that will cause warnings is to overcome the lack of late static bindings in PHP5.2. Thankfully 2.3.x is the last line of Symphony that will support 5.2, (see #746), so for 2.4 we should be able to ensure the codebase works without error with E_STRICT reporting enabling (or a least have a decent crack at it)

This work is still important and I don't want to discourage it, but for the 2.3 final it is most likely that we will modify these lines to additionally exclude E_STRICT.

brendo added a commit that referenced this issue Mar 8, 2012
Beginnings of changes for PHP 5.4 support. RE: #1041
@brendo
Copy link
Member Author

brendo commented Nov 9, 2012

@allen, is there a publicly available fork available with the work on removing notices/errors for PHP 5.4 versions? Do you expect this work will be available in a 2.3.x release or will it require 2.4 because of PHP 5.2 support?

@allen
Copy link
Contributor

allen commented Nov 9, 2012

Yep, @nils-werner is keeping a notices fork: https://github.com/nils-werner/symphony-2/tree/notices

I've fixed up all the warnings and notices on my repo and am waiting for him to pull in my changes.

The fixes can go into 2.3.x none of the work done will break compatibility away from PHP 5.2

@brendo
Copy link
Member Author

brendo commented Nov 18, 2012

Ok, great to know. Would like to have these fixes added to the next release (which is more of a hotfix release than anything). I'm hopeful we can have this release out before 2012 is done so we can start fresh in the new year.

@simoneeconomo
Copy link
Contributor

(Just found this thread, sorry about the insertBreadcrumbs mess. It was me expecting method overloading to be legal like in Java.)

@brendo
Copy link
Member Author

brendo commented Nov 29, 2012

@allen I noticed the PR has been merged by @nils-werner. Are we ok to merge into integration now?

@nils-werner
Copy link
Contributor

Yes, I have merged what I got from @allen. I don't know if everything has been done, all I know is that the coredevkit extension-submodule has to be removed before you can merge this.

@brendo
Copy link
Member Author

brendo commented Nov 29, 2012

Could you remove that submodule when you get the chance? I'm sure @allen will chime in here soon :)

@allen
Copy link
Contributor

allen commented Dec 3, 2012

There are still a few more dynamic actions (creating events and data sources) that still triggers notices. Aside from that we're pretty much there.

@designermonkey
Copy link
Member

Nice work guys.

brendo added a commit that referenced this issue Dec 18, 2012
Correcting notices and warnings for the greater good of man. RE: #1041
@brendo
Copy link
Member Author

brendo commented Jan 21, 2013

Everyone who has PHP5.4 installed and previously added the dirty little fix... I think we're now at the stage where we can undo this and start to fully test Symphony without it.

In fact, locally I'm running with error reporting set to E_ALL to try and filter out all the issues. So far so good but I could use some more eyes over the code, especially with the core extensions or edge case scenarios in the backend. Idiot. Was running the PHP5.3 install =\

@designermonkey
Copy link
Member

@brendo integration testing?

@designermonkey
Copy link
Member

Just a heads up about the installer when running on 5.4:


( ! ) Strict standards: Declaration of MD5::hash() should be compatible with Cryptography::hash($input, $algorithm = 'pbkdf2') in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/cryptography/class.md5.php on line 54
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67
3   0.0022  429560  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.administration.php' )    ../class.updater.php:3
4   0.0032  598944  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.symphony.php' )  ../class.administration.php:13
5   0.0088  1440648 require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/class.cryptography.php' )   ../class.symphony.php:22

( ! ) Strict standards: Declaration of MD5::compare() should be compatible with Cryptography::compare($input, $hash, $isHash = false) in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/cryptography/class.md5.php on line 54
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67
3   0.0022  429560  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.administration.php' )    ../class.updater.php:3
4   0.0032  598944  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.symphony.php' )  ../class.administration.php:13
5   0.0088  1440648 require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/class.cryptography.php' )   ../class.symphony.php:22

( ! ) Strict standards: Declaration of SHA1::hash() should be compatible with Cryptography::hash($input, $algorithm = 'pbkdf2') in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/cryptography/class.sha1.php on line 53
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67
3   0.0022  429560  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.administration.php' )    ../class.updater.php:3
4   0.0032  598944  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.symphony.php' )  ../class.administration.php:13
5   0.0088  1440648 require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/class.cryptography.php' )   ../class.symphony.php:22

( ! ) Strict standards: Declaration of SHA1::compare() should be compatible with Cryptography::compare($input, $hash, $isHash = false) in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/cryptography/class.sha1.php on line 53
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67
3   0.0022  429560  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.administration.php' )    ../class.updater.php:3
4   0.0032  598944  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.symphony.php' )  ../class.administration.php:13
5   0.0088  1440648 require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/class.cryptography.php' )   ../class.symphony.php:22

( ! ) Strict standards: Declaration of PBKDF2::compare() should be compatible with Cryptography::compare($input, $hash, $isHash = false) in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/cryptography/class.pbkdf2.php on line 170
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67
3   0.0022  429560  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.administration.php' )    ../class.updater.php:3
4   0.0032  598944  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/core/class.symphony.php' )  ../class.administration.php:13
5   0.0088  1440648 require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/symphony/lib/toolkit/class.cryptography.php' )   ../class.symphony.php:22

( ! ) Strict standards: Declaration of UpdaterPage::__build() should be compatible with HTMLPage::__build() in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updaterpage.php on line 131
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67

( ! ) Strict standards: Static function Migration::run() should not be abstract in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.migration.php on line 31
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67

( ! ) Strict standards: Static function Migration::getVersion() should not be abstract in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.migration.php on line 40
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67

( ! ) Strict standards: Static function Migration::getReleaseNotes() should not be abstract in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.migration.php on line 49
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67

( ! ) Strict standards: Declaration of Updater::initialiseLog() should be compatible with Symphony::initialiseLog($filename = NULL) in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php on line 9
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67

( ! ) Strict standards: Declaration of Updater::initialiseConfiguration() should be compatible with Symphony::initialiseConfiguration(array $data = Array) in /Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php on line 9
Call Stack
#   Time    Memory  Function    Location
1   0.0003  255856  {main}( )   ../index.php:0
2   0.0015  323208  require_once( '/Users/designermonkey/Sites/richard-designs.local.buginteractive.com/htdocs/install/lib/class.updater.php' ) ../index.php:67

@designermonkey
Copy link
Member

Sorry, forgot to ay, that's 2.3.2RC1

@brendo
Copy link
Member Author

brendo commented Mar 5, 2013

That can't be 2.3.2RC1, the Cryptography class doesn't have that code

@designermonkey
Copy link
Member

Dammit, was just about to delete it in shame. I must learn to remember to checkout the integration branch

@designermonkey
Copy link
Member

I have been running PHP 5.4 with Symphony now on two servers and have had no issues at all. Can we say this is done?

@jurajkapsz
Copy link
Contributor

I have been running PHP 5.4 with Symphony now on two servers and have had no issues at all. Can we say this is done?

+1

I've got PHP 5.4 running locally (for quite a time now) with some 8 SCMS sites mostly updated to 2.3.2 and everything runs well. I did not tried out the latest integration. Can I help this issue with some testing as an user?

@brendo
Copy link
Member Author

brendo commented Apr 17, 2013

Errors usually occur when the error reporting level is set to E_STRICT. While I'm pretty sure we have nailed a lot of the core, I'm unsure how extensions will fare.

We too have most of our Symphony sites running locally on PHP5.4, and a couple in production too.

@jurajkapsz
Copy link
Contributor

Thanks. So we should test all the extensions?

I am actually aiming for the 2.3.3 to be able to be released.

@brendo
Copy link
Member Author

brendo commented Apr 17, 2013

Yeah testing extensions is a great help, especially when using E_STRICT reporting

@designermonkey
Copy link
Member

Closing this as I don't get any errors now, and we're close to final release.

@jurajkapsz
Copy link
Contributor

I don't get any errors too.

@DavidOliver
Copy link
Member

Just to check, that includes JIT images working, right? I've set up Symphony with PHP 5.4 for the first time today and ran into this image problem.

@jurajkapsz
Copy link
Contributor

Thats nasty :/ I am fine with JIT.

@designermonkey
Copy link
Member

@DavidOliver, can you log this on JIT's repo please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

10 participants