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.

Sign up for GitHub

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

dev/core#1528 - Consolidate to single constant for minimum PHP version #16753

Merged
merged 2 commits into from
Apr 2, 2020
Merged

dev/core#1528 - Consolidate to single constant for minimum PHP version #16753

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
243b25d
Consolidate to single constant for minimum PHP version
agh1 Mar 11, 2020
f955c30
Full PHP versions as recommendations
agh1 Mar 12, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 2 additions & 11 deletions CRM/Upgrade/Form.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,6 @@ class CRM_Upgrade_Form extends CRM_Core_Form {
*/
const MINIMUM_UPGRADABLE_VERSION = '4.2.9';

/**
* Minimum php version required to run (equal to or lower than the minimum install version)
*
* As of Civi 5.16, using PHP 5.x will lead to a hard crash during bootstrap.
*
* Tip: Keep in sync with composer.json ("config => platform => php")
*/
const MINIMUM_PHP_VERSION = '7.1.0';

Copy link
Member

Choose a reason for hiding this comment

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

For removing MINIMUM_PHP_VERSION, we'd probably want to sequence the merges s.t. references are first removed from civicrm-{backdrop,wordpress,drupal}.git and lastly civicrm-core.git.

Or if you go the other way (replacing MIN_INSTALL_PHP_VER), then I think it only needs one patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree about merging the other repos first. I think it belongs in CRM_Upgrade_Incremental_General both because the other PHP versions are defined there and because that class is intended as a general utility class rather than a form class.

/**
* @var \CRM_Core_Config
*/
Expand Down Expand Up @@ -457,10 +448,10 @@ public function checkUpgradeableVersion($currentVer, $latestVer) {
);
}

if (version_compare(phpversion(), self::MINIMUM_PHP_VERSION) < 0) {
if (version_compare(phpversion(), CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER) < 0) {
$error = ts('CiviCRM %3 requires PHP version %1 (or newer), but the current system uses %2 ',
[
1 => self::MINIMUM_PHP_VERSION,
1 => CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER,
2 => phpversion(),
3 => $latestVer,
]);
Expand Down
15 changes: 10 additions & 5 deletions CRM/Upgrade/Incremental/General.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,25 @@ class CRM_Upgrade_Incremental_General {

/**
* The recommended PHP version.
*
* The point release will be dropped in recommendations unless it's .1 or
* higher.
*/
const RECOMMENDED_PHP_VER = '7.3';
const RECOMMENDED_PHP_VER = '7.3.0';

/**
* The previous recommended PHP version.
* The minimum recommended PHP version.
*
* A site running an earlier version will be told to upgrade.
*/
const MIN_RECOMMENDED_PHP_VER = '7.2';
const MIN_RECOMMENDED_PHP_VER = '7.2.0';

/**
* The minimum PHP version required to install Civi.
*
* @see install/index.php
*/
const MIN_INSTALL_PHP_VER = '7.1';
const MIN_INSTALL_PHP_VER = '7.1.0';

/**
* Compute any messages which should be displayed before upgrade.
Expand All @@ -54,7 +59,7 @@ public static function setPreUpgradeMessage(&$preUpgradeMessage, $currentVer, $l
$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.', [
1 => $latestVer,
2 => self::MIN_RECOMMENDED_PHP_VER,
3 => self::RECOMMENDED_PHP_VER,
3 => preg_replace(';^(\d+\.\d+(?:\.[1-9]\d*)?).*$;', '\1', self::RECOMMENDED_PHP_VER),
]);
$preUpgradeMessage .= '</p>';
}
Expand Down
8 changes: 4 additions & 4 deletions CRM/Utils/Check/Component/Env.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public function checkPhpVersion() {
ts('This system uses PHP version %1 which meets or exceeds the recommendation of %2.',
[
1 => $phpVersion,
2 => CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER,
2 => preg_replace(';^(\d+\.\d+(?:\.[1-9]\d*)?).*$;', '\1', CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER),
]),
ts('PHP Up-to-Date'),
\Psr\Log\LogLevel::INFO,
Expand All @@ -56,7 +56,7 @@ public function checkPhpVersion() {
[
1 => $phpVersion,
2 => CRM_Upgrade_Incremental_General::MIN_RECOMMENDED_PHP_VER,
3 => CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER,
3 => preg_replace(';^(\d+\.\d+(?:\.[1-9]\d*)?).*$;', '\1', CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER),
]),
ts('PHP Out-of-Date'),
\Psr\Log\LogLevel::WARNING,
Expand All @@ -66,11 +66,11 @@ public function checkPhpVersion() {
else {
$messages[] = new CRM_Utils_Check_Message(
__FUNCTION__,
ts('This system uses PHP version %1. To ensure the continued operation of CiviCRM, upgrade your server now. At least PHP version %2 is recommended; the preferrred version is %3.',
ts('This system uses PHP version %1. To ensure the continued operation of CiviCRM, upgrade your server now. At least PHP version %2 is recommended; the preferred version is %3.',
[
1 => $phpVersion,
2 => CRM_Upgrade_Incremental_General::MIN_RECOMMENDED_PHP_VER,
3 => CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER,
3 => preg_replace(';^(\d+\.\d+(?:\.[1-9]\d*)?).*$;', '\1', CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER),
]),
ts('PHP Out-of-Date'),
\Psr\Log\LogLevel::ERROR,
Expand Down
2 changes: 1 addition & 1 deletion install/index.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,7 +904,7 @@ public function requirePHPVersion($testDetails) {
$testDetails[2] = ts('This webserver is running an outdated version of PHP (%1). It is strongly recommended to upgrade to PHP %2 or later, as older versions can present a security risk. The preferred version is %3.', array(
1 => $phpVersion,
2 => CRM_Upgrade_Incremental_General::MIN_RECOMMENDED_PHP_VER,
3 => CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER,
3 => preg_replace(';^(\d+\.\d+(?:\.[1-9]\d*)?).*$;', '\1', CRM_Upgrade_Incremental_General::RECOMMENDED_PHP_VER),
));
$this->warning($testDetails);
}
Expand Down
5 changes: 3 additions & 2 deletions tests/phpunit/CRM/Upgrade/FormTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
class CRM_Upgrade_FormTest extends CiviUnitTestCase {

/**
* "php" requirement (composer.json) should match MINIMUM_PHP_VERSION (CRM/Upgrade/Form.php).
* "php" requirement (composer.json) should match
* CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER.
*/
public function testComposerRequirementMatch() {
global $civicrm_root;
Expand All @@ -16,7 +17,7 @@ public function testComposerRequirementMatch() {
$composerJson = json_decode(file_get_contents($composerJsonPath), 1);
$composerJsonRequirePhp = preg_replace(';[~^];', '', $composerJson['require']['php']);
$actualMajorMinor = preg_replace(';^[\^]*(\d+\.\d+)\..*$;', '\1', $composerJsonRequirePhp);
$expectMajorMinor = preg_replace(';^[\^]*(\d+\.\d+)\..*$;', '\1', \CRM_Upgrade_Form::MINIMUM_PHP_VERSION);
$expectMajorMinor = preg_replace(';^(\d+\.\d+)\..*$;', '\1', \CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER);
$this->assertEquals($expectMajorMinor, $actualMajorMinor, "The PHP version requirements in CRM_Upgrade_Form ($expectMajorMinor) and composer.json ($actualMajorMinor) should specify same major+minor versions.");
}

Expand Down