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

concrete5 version 8 with PHP version 7.3 #7138

Closed
mlocati opened this issue Sep 29, 2018 · 19 comments
Closed

concrete5 version 8 with PHP version 7.3 #7138

mlocati opened this issue Sep 29, 2018 · 19 comments
Assignees

Comments

@mlocati
Copy link
Contributor

mlocati commented Sep 29, 2018

concrete5 version 8 won't run on PHP 7.3.
The reason is that concrete5 uses Doctrine ORM version 2.5, which is not compatible with PHP 7.3. We'd need to update ORM to the upcoming 2.6.3, but it requires PHP 7.1+ whereas concrete5 version 8 still supports PHP 5.5.9 (see doctrine/orm#7325 ).
I don't think that Doctrine 2.5 will be fixed, so we have 2 options:

  1. don't let concrete5 version 8 run on PHP 7.3
  2. create a fork of Doctrine

I'd go with 2...

@mlocati
Copy link
Contributor Author

mlocati commented Sep 29, 2018

Thinking a bit more about this, I'd prefer this third option:
3. keep using the official doctrine repository, but patching it.

@aembler
Copy link
Member

aembler commented Sep 29, 2018

What is the nature of the bug and the change that we would have to support?

@mlocati
Copy link
Contributor Author

mlocati commented Sep 29, 2018

concrete5 stops running, showing this error message (I don't remember exactly when):

Warning: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"?

@joe-meyer
Copy link
Contributor

if the fix is really just switching continue to break then I see no reason why c5 shouldn't create a fork. I'm not sure I completely understand option 3, but if it means less divergence and an easier time getting back to standard doctrine later on I'm all for that. I'd agree that doctrine has seemed to move on in their support requirements so I highly doubt they'd be willing to patch their 2.5.x release.

Incidentally, I think if concrete5 is going to continue to use dependencies which force latest and greatest versions of PHP there should be some kind of standard plan for handling these sorts of things in place. Because concrete5 is closer to the end user than many libraries are and backward compatibility / longevity is important it would be good to have a standardized approach I think.

@aembler
Copy link
Member

aembler commented Oct 4, 2018

I agree. I think we ought to fork it and fix it and lock to that specific version. Then get back into their doctrine in version 9.

I think this ought to be our standard plan.

@mlocati
Copy link
Contributor Author

mlocati commented Oct 5, 2018

I agree. I think we ought to fork it and fix it and lock to that specific version. Then get back into their doctrine in version 9.

Ok. So, what about forking https://github.com/doctrine/doctrine2 to https://github.com/concrete5/doctrine2? If you do that and give me write access to it, I can apply the patch to that fork.

@mlocati
Copy link
Contributor Author

mlocati commented Oct 5, 2018

PS: for example, we can't even install concrete5: we have this error:

Unable to install database: "continue" targeting switch is equivalent to "break". Did you mean to use "continue 2"?.

Trace:
#0 /web/concrete/src/Package/StartingPointPackage.php(184): Concrete\Core\Package\StartingPointPackage->install_database()
#1 /web/concrete/controllers/install.php(397): Concrete\Core\Package\StartingPointPackage->executeInstallRoutine('install_databas...')
#2 [internal function]: Concrete\Controller\Install->run_routine('elemental_full', 'install_databas...')
#3 /web/concrete/src/Controller/AbstractController.php(294): call_user_func_array(Array, Array)
#4 /web/concrete/src/Routing/ControllerRouteAction.php(61): Concrete\Core\Controller\AbstractController->runAction('run_routine', Array)
#5 /web/concrete/src/Http/RouteDispatcher.php(37): Concrete\Core\Routing\ControllerRouteAction->execute(Object(Concrete\Core\Http\Request), Object(Concrete\Core\Routing\Route), Array)
#6 /web/concrete/src/Http/Middleware/DispatcherDelegate.php(39): Concrete\Core\Http\RouteDispatcher->dispatch(Object(Concrete\Core\Http\Request))
#7 /web/concrete/src/Http/Middleware\MiddlewareStack.php(86): Concrete\Core\Http\Middleware\DispatcherDelegate->next(Object(Concrete\Core\Http\Request))
#8 /web/concrete/src/Http/DefaultDispatcher.php(121): Concrete\Core\Http\Middleware\MiddlewareStack->process(Object(Concrete\Core\Http\Request))
#9 /web/concrete/src/Http/DefaultDispatcher.php(58): Concrete\Core\Http\DefaultDispatcher->handleDispatch(Object(Concrete\Core\Http\Request))
#10 /web/concrete/src/Http/Middleware/DispatcherDelegate.php(39): Concrete\Core\Http\DefaultDispatcher->dispatch(Object(Concrete\Core\Http\Request))
#11 /web/concrete/src/Http/Middleware/ThumbnailMiddleware.php(71): Concrete\Core\Http\Middleware\DispatcherDelegate->next(Object(Concrete\Core\Http\Request))
#12 /web/concrete/src/Http/Middleware/MiddlewareDelegate.php(50): Concrete\Core\Http\Middleware\ThumbnailMiddleware->process(Object(Concrete\Core\Http\Request), Object(Concrete\Core\Http\Middleware\DispatcherDelegate))
#13 /web/concrete/src/Http/Middleware/FrameOptionsMiddleware.php(39): Concrete\Core\Http\Middleware\MiddlewareDelegate->next(Object(Concrete\Core\Http\Request))
#14 /web/concrete/src/Http/Middleware/MiddlewareDelegate.php(50): Concrete\Core\Http\Middleware\FrameOptionsMiddleware->process(Object(Concrete\Core\Http\Request), Object(Concrete\Core\Http\Middleware\MiddlewareDelegate))
#15 /web/concrete/src/Http/Middleware/CookieMiddleware.php(35): Concrete\Core\Http\Middleware\MiddlewareDelegate->next(Object(Concrete\Core\Http\Request))
#16 /web/concrete/src/Http/Middleware/MiddlewareDelegate.php(50): Concrete\Core\Http\Middleware\CookieMiddleware->process(Object(Concrete\Core\Http\Request), Object(Concrete\Core\Http\Middleware\MiddlewareDelegate))
#17 /web/concrete/src/Http/Middleware/ApplicationMiddleware.php(29): Concrete\Core\Http\Middleware\MiddlewareDelegate->next(Object(Concrete\Core\Http\Request))
#18 /web/concrete/src/Http/Middleware/MiddlewareDelegate.php(50): Concrete\Core\Http\Middleware\ApplicationMiddleware->process(Object(Concrete\Core\Http\Request), Object(Concrete\Core\Http\Middleware\MiddlewareDelegate))
#19 /web/concrete/src/Http/Middleware\MiddlewareStack.php(86): Concrete\Core\Http\Middleware\MiddlewareDelegate->next(Object(Concrete\Core\Http\Request))
#20 /web/concrete/src/Http/DefaultServer.php(85): Concrete\Core\Http\Middleware\MiddlewareStack->process(Object(Concrete\Core\Http\Request))
#21 /web/concrete/src/Foundation/Runtime/Run/DefaultRunner.php(119): Concrete\Core\Http\DefaultServer->handleRequest(Object(Concrete\Core\Http\Request))
#22 /web/concrete/src/Foundation/Runtime/DefaultRuntime.php(102): Concrete\Core\Foundation\Runtime\Run\DefaultRunner->run()
#23 /web/concrete/dispatcher.php(36): Concrete\Core\Foundation\Runtime\DefaultRuntime->run()
#24 /web/index.php(3): require('/web/concrete/d...')

#25 {main}

@mlocati
Copy link
Contributor Author

mlocati commented Oct 5, 2018

It seems that PHP 7.3 has a breaking change when parsing regular expressions (bug report), and the sunra/php-simple-html-dom-parser package is affected by this.
I submitted a fix addressing this issue, but it seems that this package has been abandoned, so it's maybe another package to be forked...

@KorvinSzanto
Copy link
Member

It’s worth mentioning that we have the concrete5-forks organization, we should use that if we fork

Sent with GitHawk

@mlocati
Copy link
Contributor Author

mlocati commented Oct 8, 2018

It’s worth mentioning that we have the concrete5-forks organization, we should use that if we fork

If you fork https://github.com/doctrine/doctrine2 and https://github.com/sunra/php-simple-html-dom-parser to https://github.com/concrete5-forks and give me temporary write access to them, I can patch them.

@KorvinSzanto
Copy link
Member

Done 👍

@mlocati
Copy link
Contributor Author

mlocati commented Oct 9, 2018

We also need to fork zendframework/zend-stdlib...

@KorvinSzanto
Copy link
Member

Done. I added a comment on that issue, it seems reasonable that their 2.7 version is EOL and so they will not make changes to it. What are we actually using that dependency for? I'd bet illuminate/support gives us everything we need.

@mlocati
Copy link
Contributor Author

mlocati commented Jan 17, 2019

@mlocati mlocati closed this as completed Jan 17, 2019
@krebbi
Copy link
Contributor

krebbi commented Mar 1, 2019

Update from 8.4.2 to 8.4.5 fails with PHP 7.3.2

bildschirmfoto 2019-03-01 um 11 50 27

@mlocati
Copy link
Contributor Author

mlocati commented Mar 1, 2019

@krebbi concrete5 version 8.5 is the first version that's compatible with PHP 7.3.

@GregorVoelkl
Copy link

If you upgrade your php version for an existing composer installation of concrete5 please be aware, that you have to update your composer.json:

In particular for my case:

Additional dependency:
"concrete5/dependency-patches": "^1.4.0",

IN the "extra" section of your composer.json file:

"allow-subpatches": [
  "concrete5/dependency-patches"
],

The current composer.json can be found at:
https://github.com/concrete5/composer/blob/master/composer.json

@dvorpahl
Copy link

what a shame. current release (announched to work with php7) but most of its themes, addons will fail while using php7.4.

force the addon development to php7 .. cutoff the compatiblity to php < 7.x to ensure the use with current php. most "out-of-the-box" hoster does not support php5 anymore.

@aembler
Copy link
Member

aembler commented Nov 12, 2020

@dvorpahl ? We run version 8.5.x on PHP 7.4 without issues.

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

Successfully merging a pull request may close this issue.

7 participants