concretecms / concrete5-legacy

Legacy repository for concrete5
http://www.concrete5.org
559 stars 323 forks source link

PHP 8 compatibility #2009

Closed brandymedia closed 6 months ago

brandymedia commented 2 years ago

Are there any plans to support PHP 8 in the future?

Remo commented 2 years ago

Maintenance for 5.6 is very basic at this point. There's no active community for it anymore. I only keep an eye on pull requests to review and merge them.

There might be someone with the same need who is willing to work on it, but I'm not aware of anything. I guess either wait to be lucky or give it a try yourself.

brandymedia commented 2 years ago

Hey Remo, thanks for the reply.

I realise it was a long shot as concrete has progressed a lot since the 5.6 days. Thought it was worth asking though.

I’ll have a look in the forums to see if there is much appetite for this.

Out of interest, was there a lot of work involved to get 5.6.4.0 PHP 7 ready?

I’d love to take a shot at this but worry it’s too much for one dev alone.

On Tue, 28 Jun 2022 at 18:43, Remo Laubacher @.***> wrote:

Maintenance for 5.6 is very basic at this point. There's no active community for it anymore. I only keep an eye on pull requests to review and merge them.

There might be someone with the same need who is willing to work on it, but I'm not aware of anything. I guess either wait to be lucky or give it a try yourself.

— Reply to this email directly, view it on GitHub https://github.com/concretecms/concrete5-legacy/issues/2009#issuecomment-1169033007, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASW3G4II4NYDROJSF2GQHFLVRM2S5ANCNFSM52BGTASQ . You are receiving this because you authored the thread.Message ID: @.***>

Remo commented 2 years ago

Getting it working with PHP 7 was quite a big chunk of work. You can see most changes in this pull request https://github.com/concretecms/concrete5-legacy/pull/1955

I would assume that PHP 8 should be easier as we had a few quirks in the codebase before, but they were improved with that pull request, but that's nothing more than a gut feeling.

brandymedia commented 2 years ago

Thanks for the info, I’ll check out the PR and try and gauge what work was involved.

I don’t think there are loads of breaking changes between PHP 7 and PHP 8 but no doubt there will be a lot of work to do across such a big codebase.

Thanks again 👍

On Tue, 28 Jun 2022 at 19:04, Remo Laubacher @.***> wrote:

Getting it working with PHP 7 was quite a big chunk of work. You can see most changes in this pull request #1955 https://github.com/concretecms/concrete5-legacy/pull/1955

I would assume that PHP 8 should be easier as we had a few quirks in the codebase before, but they were improved with that pull request, but that's nothing more than a gut feeling.

— Reply to this email directly, view it on GitHub https://github.com/concretecms/concrete5-legacy/issues/2009#issuecomment-1169053760, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASW3G4PXNGI73YJBSFLWN33VRM5C7ANCNFSM52BGTASQ . You are receiving this because you authored the thread.Message ID: @.***>

ConcreteOwl commented 2 years ago

Many php fatal errors were suppressed in the 5.6.4.0 codebase using the '@' symbol. php 7 allowed the errors to be silenced when prepended by the @ symbol. php 8 does not allow the suppresion of these errors. Read this article for a full explanation, https://php.watch/versions/8.0/fatal-error-suppression

Remo commented 2 years ago

@ConcreteOwl yes, that's a good point. We did remove some of the @'s, but there are still plenty left. This could indeed by a main cause for headaches. I guess one would have to remove them and start praying or more likely, start fixing.

aembler commented 2 years ago

Given the crazy headaches supporting PHP 8 in Version 9, I can't imagine it's feasible in 5.6. Perhaps if you followed the instructions we've posted elsewhere for disabling the more strict error reporting that PHP 8 turns on by default, but otherwise I think it's going to be impossible.

brandymedia commented 2 years ago

That’s good to know, thanks David.

I’d have to check the codebase to see how common these are but from what you’ve said, I’d imagine there are quite a few.

This obviously could present a massive headache when looking to get this working with PHP 8 and beyond.

On Tue, 28 Jun 2022 at 21:49, David Gibson @.***> wrote:

Many php fatal errors were suppressed in the 5.6.4.0 codebase using the '@' symbol. php 7 allowed the errors to be silenced when prepended by the @ symbol. php 8 does not allow the suppresion of these errors. Read this article for a full explanation, https://php.watch/versions/8.0/fatal-error-suppression

— Reply to this email directly, view it on GitHub https://github.com/concretecms/concrete5-legacy/issues/2009#issuecomment-1169228744, or unsubscribe https://github.com/notifications/unsubscribe-auth/ASW3G4KN7SDEYVEMSCBQ6QLVRNQOZANCNFSM52BGTASQ . You are receiving this because you authored the thread.Message ID: @.***>

ConcreteOwl commented 2 years ago

@brandymedia (Andy) Yes a major headache for anyone wanting to update 5.6.4.0 to run on PHP 8. I have come to the conclusion your time is better spent finding a good hosting company that is going to continue supporting PHP 7. If you do try to update the codebase, please keep us informed of your progress. Best Regards David

JohntheFish commented 2 years ago

Its not just the core code. Its all the 3rd party code pulled in to the core. Then all the themes/addons/application code. You would also need to update these for php8 compatibility or to suppress the errors.

ktarasov commented 1 year ago

We tried to run code 5.6.4 on PHP 8, ran into problems in AdoDB. But we will slowly do this with an incomprehensible perspective.

ConcreteOwl commented 1 year ago

@ktarasov if you make any progress with this I would love to hear from you, I too have failed with the AdoDB errors and have not yet found a solution, although I have not spent a lot of time trying. Good Luck