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

Increase minimum php version requirements #11416

Merged
merged 3 commits into from
Dec 19, 2017
Merged

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Dec 15, 2017

Overview

As promised in our blog post we are incrementing the minimum php requirements for CiviCRM.

Issue is here: https://lab.civicrm.org/infrastructure/ops/issues/818

Before/After

Before After
Min to run 5.3 5.4
Min to install 5.3 5.4*
Min recommended ver 5.5 5.6
Recommended ver 5.6 7.0

* We'll bump this to 5.5 in the following release as promised.

@colemanw
Copy link
Member Author

@totten this refactors a function in the install script so you'll want to merge this before tackling that rewrite.

@colemanw colemanw added this to the 4.7.30 milestone Dec 15, 2017
@totten
Copy link
Member

totten commented Dec 16, 2017

jenkins, test this please

2 similar comments
@totten
Copy link
Member

totten commented Dec 16, 2017

jenkins, test this please

@totten
Copy link
Member

totten commented Dec 16, 2017

jenkins, test this please

@colemanw
Copy link
Member Author

@civicrm-builder test this please

This makes it a little easier to hack and simulate different PHP versions.
@totten
Copy link
Member

totten commented Dec 17, 2017

@colemanw I pushed up a couple small tweaks.

I was r-running it, and the install screen worked well. For the upgrade screen, I did this hack to simulate an old version:

diff --git a/CRM/Upgrade/Incremental/General.php b/CRM/Upgrade/Incremental/General.php
index 56a80610d2..c97acf55ef 100644
--- a/CRM/Upgrade/Incremental/General.php
+++ b/CRM/Upgrade/Incremental/General.php
@@ -65,7 +65,7 @@ class CRM_Upgrade_Incremental_General {
    */
   public static function setPreUpgradeMessage(&$preUpgradeMessage, $currentVer, $latestVer) {
     $dateFormat = Civi::Settings()->get('dateformatshortdate');
-    if (version_compare(phpversion(), self::MIN_RECOMMENDED_PHP_VER) < 0) {
+    if (version_compare('5.3.20', self::MIN_RECOMMENDED_PHP_VER) < 0) {
       $preUpgradeMessage .= '<p>';
       $preUpgradeMessage .= ts('You may proceed with the upgrade and CiviCRM %1 will continue working normally, but future releases will require PHP %2 or above. We recommend PHP version %3.', array(
          1 => $latestVer,

and the resulting message was this, which seems to suggest that you could continue with an upgrade on PHP 5.3:

screen shot 2017-12-17 at 1 31 19 pm

@colemanw
Copy link
Member Author

colemanw commented Dec 18, 2017

Hmm, @totten a fatal error ought to be triggered before we get to that screen, oughtn't it? Perhaps because you only changed the variable in one place that didn't happen.
Looks like \CRM_Upgrade_Form::checkUpgradeableVersion() should stop us from getting to that screen.

@totten
Copy link
Member

totten commented Dec 19, 2017

Thanks, @colemanw. Makes sense now. 👍

@totten totten merged commit 5eea534 into civicrm:master Dec 19, 2017
@colemanw colemanw deleted the phpversion branch December 20, 2017 10:05
@colemanw
Copy link
Member Author

@totten based on discussion on dev-post-release I've cherry-picked this into the 4.7.29-rc branch.

@colemanw colemanw modified the milestones: 4.7.30, 4.7.29 Dec 20, 2017
sluc23 pushed a commit to ixiam/civicrm-core that referenced this pull request Jan 10, 2018
Increase minimum php version requirements
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants