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

Support PHP 7.3 #6991

Merged
merged 9 commits into from Jan 28, 2020
Merged

Support PHP 7.3 #6991

merged 9 commits into from Jan 28, 2020

Conversation

@heathdutton
Copy link
Member

heathdutton commented Dec 10, 2018

Q A
Bug fix? No
New feature? Yes
Automated tests included? Yes
Related user documentation PR URL
Related developer documentation PR URL
Issues addressed (#s or URLs)
BC breaks? No
Deprecations? No

Description:

The excellent work by @Woeler @escopecz @StudioMaX and others getting us up and running with PHP 7.2 should put us in very close parity with PHP 7.3. There are very few deprecations in PHP 7.3, but it should give us a performance boost of up to 4% and get us security support till 2021. This PR is here to get the ball rolling in Travis and see what issues await us...

Surprise! It's great!

Known issues.

  1. PHP-CS-Fixer is currently incompatible. They are working on it actively. For now we'll nope out on PHPCS except for the PHP 7.2 builds.
  2. doctrine/orm needs to be upgraded to 2.6.3 to prevent a warning, but unfortunately that version requires PHP 7.1 as a hard limit. That would prevent us from supporting PHP 7.3 without dropping support for PHP 5.6 and PHP 7.0. Instead this PR applies a tiny lint patch so that we can continue to support 5.6 and 7.0 for a while and sidestep the warning.
  3. Symfony 2.8 is in security support only. It'll hit EOL in Nov 2019, so that should be our next deprecation concern. Getting on the 4.x branch of Symfony is recommended. The LTS version hasn't been determined yet, but will likely be supported till end of 2022. I'm not touching this in the PR yet, as it's not essential in this moment... but it's worth mentioning.

IT WORKS

@heathdutton heathdutton added this to the 2.15.1 milestone Dec 11, 2018
@heathdutton heathdutton requested a review from escopecz Dec 11, 2018
@heathdutton heathdutton added this to Code Review (2 required) in Mautic 2 Dec 12, 2018
@djeraseit

This comment has been minimized.

Copy link

djeraseit commented Dec 19, 2018

PHP 7.3 working fine for me with the MAXIMUM php version hack. Getting the warnings in the log file about Use of undefined constant MCRYPT_MODE_CBC - assumed 'MCRYPT_MODE_CBC'.

@heathdutton

This comment has been minimized.

Copy link
Member Author

heathdutton commented Dec 19, 2018

PHP 7.3 working fine for me with the MAXIMUM php version hack. Getting the warnings in the log file about Use of undefined constant MCRYPT_MODE_CBC - assumed 'MCRYPT_MODE_CBC'.

That's odd, please make sure you are on 2.15.0-beta or later, which should check for the existence of mcrypt before trying to use that global.

What line of code do you get that from?

@heathdutton

This comment has been minimized.

Copy link
Member Author

heathdutton commented Dec 19, 2018

We forgot to update MAUTIC_MAXIMUM_PHP in the last PR, so I've just fixed that for this one for those upgrading via the UI.

@heathdutton heathdutton mentioned this pull request Jan 10, 2019
@alanhartless alanhartless added this to Needs Testing in 2.15.1 Jan 14, 2019
@npracht npracht added the Code Review label Jan 24, 2019
@StudioMaX

This comment has been minimized.

Copy link
Contributor

StudioMaX commented Jan 28, 2019

PHP CS Fixer v2.14 was released with the support of PHP 7.3.

@alanhartless

This comment has been minimized.

Copy link
Contributor

alanhartless commented Jan 30, 2019

@heathdutton can you take care of the conflicts?

@alanhartless alanhartless moved this from Needs Testing to Has Conflicts and/or Failing Tests in 2.15.1 Jan 30, 2019
# Conflicts:
#	composer.json
#	composer.lock
#	upgrade.php
@heathdutton

This comment has been minimized.

Copy link
Member Author

heathdutton commented Feb 1, 2019

PHP CS Fixer v2.14 was released with the support of PHP 7.3.

Unfortunately that would require an upgrade to symfony/console. Which I'm fine to try, but that may add some BC breakage. friendsofphp/php-cs-fixer v2.14.0 requires symfony/console ^3.4.17 || ^4.1.6

I upgraded to v2.2.20 to get closer to PHP 7.3 analysis while running in PHP 7.2 till we can upgrade symfony a bit.

Just doing this to keep the scope of the PHP 7.3 PR as low as possible
right now.
@alanhartless alanhartless moved this from Has Conflicts and/or Failing Tests to Needs Testing in 2.15.1 Mar 5, 2019
@alanhartless alanhartless removed this from the 2.15.1 milestone Mar 11, 2019
@alanhartless alanhartless removed this from Needs Testing in 2.15.1 Mar 11, 2019
@npracht npracht removed this from Code Review (2 required) in Mautic 2 Apr 4, 2019
@calevans

This comment has been minimized.

Copy link

calevans commented May 2, 2019

Do we have an ETA on this PR?

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented May 2, 2019

ETA depends on community involvement in testing. Not only of this PR but all in milestones 2.15.2 and 2.16.0. It will move faster with your help!

@calevans

This comment has been minimized.

Copy link

calevans commented May 2, 2019

I'll spin up a 7.2 VM this weekend and see if I can test it out. 😃

@calevans

This comment has been minimized.

Copy link

calevans commented May 6, 2019

I have installed mautic on my test VM with PHP 7.3.
I installed this PR
I submitted a new diff to @heathdutton because the one references in this PR is now out of date.

Other than that, everything seems to work fine.

I spent more time trying to get the mautic unit tests to run (still have not had a full run and even as far as I've gotten, there are many errors) but I've run a few commands and pulled up the web front end and everything seems to work correctly.

Again, it's not exhaustive, but it's good enough so that I'm rolling this patch (and my diff) into my prod so I can get everything up to PHP 7.3.

Cheers!
=C=

@afonseca08

This comment has been minimized.

Copy link

afonseca08 commented Oct 1, 2019

@heathdutton I just set up a local instance with PHP 7.3.5, MySQL 8.0.15, nginx 1.16.0 and everything went smoothly. Installed just fine, clicked around the front end UI, campaign builder, email editor, settings. It seems to be working well, with no errors in the logs (just warnings that are already tracked in another issue).

This is my first time setting up a local dev instance of Mautic from source so let me know if there's something else you want me to try to call it good.

@rickmills

This comment has been minimized.

Copy link

rickmills commented Oct 30, 2019

Echoing @afonseca08 here - runs smoothly on 7.3.11 here (7.3.11-1+ubuntu18.04.1+deb.sury.org+1 to be precise). Would be great if we could get this merged in pretty soon given PHP 7.2 active support ends in less than a month now and usage of PHP 7.2 for anything new is strongly discouraged.

@afonseca08

This comment has been minimized.

Copy link

afonseca08 commented Oct 30, 2019

Any chance this could be rolled out in a timely hot fix or does it need to wait until a larger release?

@rickmills

This comment has been minimized.

Copy link

rickmills commented Oct 31, 2019

I ended up having to manually patch my live copy*, given how few changes are needed (it took less than 5 minutes to patch) then I'd assume a hot fix would be fine, there's no backward compatibility breaks which should make things much easier.

*7.2 goes into security updates only in a month, would rather not be running an out of date copy

@afonseca08

This comment has been minimized.

Copy link

afonseca08 commented Oct 31, 2019

Great to hear, I’m also concerned with a timely update path as Mautic is the last software holding our server back from PHP 7.3. What’s the process for getting a hot fix release going? Otherwise I’d have to look into manually patching a live install as well but not my preference.

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented Oct 31, 2019

You probably missed there is an event tomorrow creating base for Mautic 3. I guess this PR would be great to discuss there.

@afonseca08

This comment has been minimized.

Copy link

afonseca08 commented Oct 31, 2019

Unfortunately, I won’t be able to attend. Will there be a video recording made available? In any case, hopefully someone can represent this issue there.

@rickmills

This comment has been minimized.

Copy link

rickmills commented Nov 11, 2019

Baring the one remaining code review waiting on @escopecz this should be good to go now right?

Copy link
Member

escopecz left a comment

I'm fine with the code changes. I did not test Mautic with PHP 7.3 yet, though. Just don't want to block this PR from merging.

@afonseca08

This comment has been minimized.

Copy link

afonseca08 commented Nov 20, 2019

So what’s left to do?

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented Nov 20, 2019

Prepare a new feature release. We are working on Mautic 3.0.0 and I'd like to get it in there. If someone steps up to release 2.16.0 before that with this and more PRs merged soon then it might get to production faster.

@tcurdt

This comment has been minimized.

Copy link

tcurdt commented Dec 5, 2019

With even Debian Buster shipping with 7.3 it would really nice if there was a version released that supports 7.3. Is there a rough ETA for the 3.0 release?

@afonseca08

This comment has been minimized.

Copy link

afonseca08 commented Dec 5, 2019

PHP 7.4 has now been released, so getting this out there in a minor release would be preferable to waiting on 3.0 unless it can happen this month (December).
https://www.php.net/releases/7_4_0.php

@escopecz

This comment has been minimized.

Copy link
Member

escopecz commented Dec 12, 2019

FYI, I took Heath's branch and prepared PR for Mautic 3 out of it here: #8224

@rjong

This comment has been minimized.

Copy link

rjong commented Jan 18, 2020

So what are we waiting for now?

@kuzmany

This comment has been minimized.

Copy link
Contributor

kuzmany commented Jan 19, 2020

@npracht npracht added this to the 2.16.0 milestone Jan 23, 2020
@npracht npracht added this to Ready to Commit (passed testing) in Mautic 2 Jan 24, 2020
@dennisameling dennisameling merged commit 4e67210 into mautic:staging Jan 28, 2020
2 checks passed
2 checks passed
Scrutinizer Analysis: 1 new issues, 4 updated code elements – Tests: passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Mautic 2 automation moved this from Ready to Commit (passed testing) to Merged Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Mautic 2
  
Merged
You can’t perform that action at this time.