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

Minimum supported PHP version is 7.0 #14437

Merged
merged 1 commit into from
Jun 5, 2019
Merged

Minimum supported PHP version is 7.0 #14437

merged 1 commit into from
Jun 5, 2019

Conversation

agh1
Copy link
Contributor

@agh1 agh1 commented Jun 5, 2019

Overview

This actually changes the minimum PHP version to 7.0 from 5.6.

Before

You can install CiviCRM on a server with 5.6 or higher. A site on PHP 5.6 will have a warning status message saying, "This meets the minimum requirements for CiviCRM to function but is not recommended."

After

You can only install CiviCRM on a server with PHP 7.0 or higher. A site on PHP 5.6 will have an error status message saying, "To ensure the continued operation of CiviCRM, upgrade your server now." However, it will still be possible to upgrade CiviCRM on PHP 5.6.

Technical Details

This only changes CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER, and does not change CRM_Upgrade_Form::MINIMUM_PHP_VERSION. The former triggers status messages and the pre-install check, while the latter is the gatekeeper for upgrades.

Comments

I was asked to flag this in the release notes, but I noticed there actually wasn't any code change dropping PHP 5.6.

@civibot
Copy link

civibot bot commented Jun 5, 2019

(Standard links)

@civibot civibot bot added the 5.14 label Jun 5, 2019
@agh1 agh1 mentioned this pull request Jun 5, 2019
@colemanw
Copy link
Member

colemanw commented Jun 5, 2019

retest this please

@@ -49,6 +49,9 @@ class CRM_Upgrade_Form extends CRM_Core_Form {

/**
* Minimum php version required to run (equal to or lower than the minimum install version)
*
* Even though 5.6 is no longer supported, this value is left here for a while
* so as not to block stragglers from upgrading.
Copy link
Member

@totten totten Jun 5, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 on this incrementalism.

Tangentially... there are actually several "installers" (i.e. the typical D7/WP web-installer; the Joomla and drush and wp-cli installers; the setup.sh and civicrm_install() bash scripts) which each enforce different constraints, and those can be viewed as "stragglers" of another sort. We should eventually fix those too, but not by playing whack-a-mole specifically on the PHP version check. Rather, we should push forward on consolidating the installer code so that it's easier to maintain the installer logic generally.

But that's all tangential.

This PR seems like a fair and simple way to phase-in the elevated requirement.

@totten
Copy link
Member

totten commented Jun 5, 2019

Notes:

  • The test failure is unrelated.
  • The PR tests still use PHP 5.6, but this passes anyway (because the patch only updates the GUI installer), so that change can be async.
  • The demo sites (*.demo.civicrm.org) are already on PHP 7.x.
  • I'll figure on how to update the hydra test sites.

@totten totten merged commit 0b46460 into civicrm:5.14 Jun 5, 2019
@seamuslee001
Copy link
Contributor

Test fail is unrelated given @totten's comments i'm going to merge this in

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.

4 participants