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

[agh1]

Overview

There have been two constants that handle the minimum PHP version: CRM_Upgrade_Form::MINIMUM_PHP_VERSION (for upgrades) and CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER (for new installs). They were allowed to deviate in #14437 with the intention of making a softer transition to PHP 7. However, dev/drupal#79 illustrated a situation where sites with PHP 5.6 would be allowed to upgrade and immediately run into a fatal error.

Since then, tests have been added to enforce constants in civicrm-drupal, civicrm-wordpress, civicrm-backdrop, and composer.json being equal to CRM_Upgrade_Form::MINIMUM_PHP_VERSION. In discussion on dev/core#1528 @totten indicated that there's little need or desire for there to be different minimums for upgrades and new installs.

In that case, there's no reason for separate constants: all they'll do is confuse everyone. This PR consolidates the two into CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER since it sits alongside the constants for the recommended PHP version and the minimum-recommended PHP version.

Before

Upgrades and tests in other repos look to one constant, while new installs and the system check look to another.

After

There is a single constant for the minimum allowable PHP version: CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER. The minimum PHP version for upgrades may never deviate from the minimum PHP version for new installations.

Comments

PRs forthcoming for civicrm-drupal, civicrm-wordpress, and civicrm-backdrop since they look to CRM_Upgrade_Form::MINIMUM_PHP_VERSION.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[totten]

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.

[agh1]

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.

[agh1]

The following pull requests should be merged ahead of this one:

• Consolidate to single constant for minimum PHP version civicrm-wordpress#187
• Consolidate to single constant for minimum PHP version civicrm-backdrop#114
• Use new consolidated constant for minimum PHP version civicrm-drupal#599

[agh1]

I updated the other repos because they were running into test failures along the lines that 7.1 doesn't equal 7.1.0 (including using version_compare(), interestingly enough). Anyway, I updated and standardized regex along the lines that CRM_Upgrade_Incremental_General::MIN_INSTALL_PHP_VER can be major.minor or major.minor.point without causing problems. If the point release is .0, it's treated as if it isn't there: 7.1 and 7.1.0 both resolve to 7.1, even as 7.1.9 will be considered as all three parts. This isn't necessary now, but theoretically CiviCRM could require a point release as a minimum.

[eileenmcnaughton]

@agh1 there is a style issue

[seamuslee001]

I agree with this I don't know which define is better but this at least migrates to just one

[agh1]

@eileenmcnaughton i updated the missing comma--thanks

[totten]

The related PRs for civicrm-wordpress@master, civicrm-backdrop@1.x-master, and civicrm-drupal@7.x-master have all been merged. :)

I also double-checked that civicrm-drupal@6.x-master, civicrm-drupal-8@master, and civicrm-joomla@master do not use MINIMUM_PHP_VERSION. That's maybe a hint that they merit some kind of update (to show better errors on incompatible deployments), but it's not a prerequisite for this PR.

[totten]

I've done some r-run on D7 with alternating PHP versions (7.0 and 7.1). Confirmed that the install screen, status-check, and upgrade screen all continue to report a sensible error on the unsupported PHP version - while also giving gentler warnings on 7.1.

Looks good to merge.