Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Beta phase evaluation
Issue category | Task |
---|---|
Issue priority | Major because it offers great guidance to end users and will set D8 off in the right direction. |
Prioritized changes | Best practices |
Disruption | None |
Problem/Motivation
PHP 5.5 is reaching the end of active support next week, if Drupal doesn't depend on PHP 5.6 as a minimum we should at least tell users about the PHP versions.
Proposed resolution
- Add to documentation
- Add to README
- Add warning in installer
- Add warning in status page
Remaining tasks
all of above
User interface changes
additions to installer and status page
API changes
none
Data model changes
none
Comment | File | Size | Author |
---|---|---|---|
#59 | suggest_php_5_6_as_the-2524432-59.patch | 4.18 KB | timmillwood |
#45 | policy_suggest_php_5_6-2524432-44.patch | 3.93 KB | hussainweb |
#45 | interdiff-39-44.txt | 1.17 KB | hussainweb |
#39 | interdiff-32-39.txt | 2.58 KB | hussainweb |
#32 | interdiff-2524432-30-32.txt | 1.35 KB | timmillwood |
Comments
Comment #1
cilefen CreditAttribution: cilefen commentedComment #2
Crell CreditAttribution: Crell at Palantir.net commentedAdding a warning/notice when someone is running a less-than-supported PHP version seems like a fine idea to me. It doesn't stop anyone from using a previous version, but it makes it clear that they're doing so at their own risk.
We can also change the "suggested" version many times during the release as PHP evolves without changing the required version. I fully expect that in a year we'll want to tell people to use PHP 7, given its substantial performance benefits, but we can't force it.
+1 from me.
Comment #3
timmillwood@Crell - Thanks for the support.
Starting the ball rolling.
Comment #4
timmillwoodComment #5
timmillwoodComment #6
fgmSupporting this too : many times these last few years have I had customers refraining from upgrading their PHP for D6/D7 because there was no explicit mention of more up-to-date versions of PHP.
There seems to be an assumption that support notes like "PHP 5.2.5 (5.4 or higher recommended)" are understood, for some reason, as "PHP 5.2.5 and minor updates thereof, 5.4.x might work, and the devs expect 5.5 and higher should work but did not take the time to validate it", so users end up running the lowest supported version to have the best chance of core + contrib support.
This might come from habit with commercial off the shelf software, listing every supported Windows version instead of just saying something like "Windows Vista supported (Window/XP or higher recommended)".
Comment #7
timmillwoodHere's a patch that covers:
- Add warning in installer
- Add warning in status page
Comment #8
timmillwoodsending for review
Comment #9
almaudoh CreditAttribution: almaudoh commentedCould we call this "Recommended" version of PHP. That sounds stronger than "Suggested".
Or maybe even "recommended minimum".
Comment #10
timmillwood#22 I did consider the wording and don't really want to be strong because I expect over 80% of users won't be able to meet the suggestion. The idea is to have a polite guidance to best practices.
Comment #11
xjmWe support 5.5.; therefore, I think we should not make this a warning during the install phase as it will be one of the first things people encounter when they install and it willl appear that something is broken when it is not (resulting in a bad first impression of the product). I would make it REQUIREMENT_INFO, and discuss adding it in the runtime phase instead.
I'd suggest linking to the actual PHP docs about version support in the first sentence, and also clarifying that the current version is supported.
Comment #12
JeroenTCreated a new patch that addressed the feedback of xjm in #11.
Comment #13
cilefen CreditAttribution: cilefen commentedWe should use "minimum required" and "recommended", which would match the published documentation.
Comment #14
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedWhy the 'runtime' check? As long as this isn't a warning (which I agree it shouldn't be) then it's worth displaying every time the PHP version is displayed. This won't interrupt the installer, but if the installer is interrupted for some other reason this info should be printed there too for consistency.
Comment #15
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedThis also looks like it might conflict with the "Suggest to update to at least 5.5.21 or 5.6.5 for disabling multiple statements" part below it (since that sets the description too).
Comment #16
neclimdulSince we're expanding the documentation around this suggestion, would it be worth linking to https://www.drupal.org/requirements/php#drupalversions when we're making suggestions? or maybe all the time? This might help with #6.
Comment #17
timmillwood@David_Rothstein you may be right, the "disabling multiple statements" logic is
(version_compare($phpversion, '5.5.21', '>=') && version_compare($phpversion, '5.6.0', '<')) || version_compare($phpversion, '5.6.5', '>=')
. So the new 5.6 version check will still fire if the PHP version is less than 5.5.21 otherwise it will be overridden by the ""disabling multiple statements" logic.Maybe we should add to the "disabling multiple statements" wording?
Comment #18
Crell CreditAttribution: Crell as a volunteer commentedRe #16: Having minimal text in the UI (to avoid a wall of text problem) and linking to Drupal.org for more details sounds like a good approach. That also lets us revise the text on Drupal.org as needed without having to make a new release. (The version that triggers the link to appear would still require a release, but that's fine.)
Comment #19
timmillwoodWith a bit of a change from the last patched I have re-worked the "multiple statement disabling" section an increased the recommended version to 5.6.5 rather than 5.6.x. So to correct myself earlier the "multiple statement disabling" message only fires if:
- The user is in the install phase
- The user is using Mysql
- The user has less than 5.5.21 or 5.6 but less than 5.6.5
The new PHP info section we're adding here fires if
- The user is not in the install phase and using less than 5.6.5
- The user is in the install phase, not using mysql, and using less than 5.6.5
- The user is in the install phase, using mysql, and using less than 5.6.0 but greater than 5.5.21
I have linked the new message to https://www.drupal.org/requirements/php.
Comment #20
timmillwoodAdded some extra docs to support this: - https://www.drupal.org/requirements/php#8
Comment #21
cilefen CreditAttribution: cilefen commentedWhy major?
Comment #22
timmillwood@cilefen - I feel it's of relative importance and offers little to no disruption or effort.
Comment #23
cilefen CreditAttribution: cilefen commentedKeep in mind https://www.drupal.org/core/issue-priority. I believe the disruption is null. It may be major, considering that we have prioritized library upgrades. Please can you add the beta evaluation?
Comment #24
cilefen CreditAttribution: cilefen commentedYou could reduce the maintenance burden on this chunk of code by not repeating the version numbers here.
Comment #25
timmillwoodComment #26
daffie CreditAttribution: daffie commentedThe patch looks good to me.
The change that cilefen is asking for in #24 has nothing to do with this issue.
Comment #27
timmillwoodWorking on an updated patch now, but got lost in the world of
REQUIREMENT_INFO
,REQUIREMENT_OK
,REQUIREMENT_WARNING
,REQUIREMENT_ERROR
.Comment #28
timmillwoodMoved version number to public static variables.
Staying with
REQUIREMENT_INFO
as I intend to demote most things toREQUIREMENT_OK
to bubble upREQUIREMENT_INFO
. see #2528242: Add color-* classes to all rows on the status page and #2528270: Use REQUIREMENT_OK by default.Comment #30
timmillwoodStupid Mac. Stupid .DS_Store files.
Comment #31
daffie CreditAttribution: daffie commentedI know what these values are for, but the documentation is not very helpful. Can you please make them better. :-)
Comment #32
timmillwoodAdding a little more explanation
Comment #33
daffie CreditAttribution: daffie commentedLooks good to me. Back to RTBC.
Comment #34
tstoecklerI think #11 hasn't really been addressed yet, AFAICT. Maybe we need some input from the usability team here? I too think this is very valuable in nudging people towards a newer PHP version. I know many customers for whom it is just an integral part of deploying a Drupal site to get the status report all green and they would be nudged toward switching hosting providers / upgrade their infrastructure / etc.
However, I tend to agree with @xjm that annoying people with this immediately after they have downloaded Drupal for the very first time and do not yet have any impression of it is a bit too much. Maybe we can restrict the condition to not affect the installer?
Moving back to "needs review" for now.
Comment #35
cilefen CreditAttribution: cilefen commentedRe #26: huh?
Comment #36
timmillwood@tstoeckler - I think everything from #11 has been addressed and users (at install and in status report) are more likely to see the existing "PHP (multiple statement disabling)" information, the new patch just further adds to that.
The
REQUIREMENT_INFO
being used here will also not block installation unless there are further warnings or errors.@cilefen - Ignore it!
Comment #37
catchI think we should consider adding a countdown here for PHP versions on security support. So something like:
"Your PHP installation of %current_version is receiving official security support until %date. Upgrading to %version is recommended to take advantage of bugfixes etc."
Also PHP 5.6 as far as I know does not have significant performance gains compared to PHP 5.5, so let's not make promises we can't keep.
We try not say 'Drupal' in the UI generally especially as a subject, better to use passive 'It is recommended'.
Comment #38
David_Rothstein CreditAttribution: David_Rothstein as a volunteer commentedYeah, I think #11 was addressed by moving the REQUIREMENT_WARNING to a REQUIREMENT_INFO. (Haven't tested the patch myself, though.)
I think this would be problematic for the same reason I mentioned in #14: "...that will confuse people whose Linux distributions have a different PHP support schedule (which is pretty much everyone running Drupal on Linux). "
Comment #39
hussainwebI removed "Drupal" from the suggestion as per #37. I too think it is complex to have a counter as explained in #38. I am also fixing some other class variable names according to coding standard (but is that really in scope of this issue?)
Comment #41
hussainwebI have a feeling that this is a random failure.
Comment #44
tim.plunkettWhy not use a const? public static feels weird here.
Comment #45
hussainwebAh, found the problem. Hopefully this fixes all the errors.
Comment #46
hussainweb@tim.plunkett: I am thinking this is anyway out of scope. As to your question, I think constants would feel weird as this is a requirement for a very specific PHP feature (PDO multi statements). But I still think we could name the constants better. The whole name
SystemRequirements::$minLow
doesn't really explain what's the minimum for and what's low.Comment #47
timmillwoodI agree with @tim.plunkett that on reflection these should be constants.
Open to change the names of these, but not sure what to.
Comment #48
timmillwoodComment #49
anavarreWhat about
PHP_MIN|MAX_LOW
/PHP_MIN|MAX_HIGH
?Comment #50
timmillwoodUpdated the wording a little and took on @anavarre's suggestion.
Comment #52
tstoecklerRe #38: Thanks, yes, I had missed the change to
REQUIREMENT_INFO
. I liked aREQUIREMENT_WARNING
better (in the status report, not in the installer), but anyway thanks to the clarification! :-)Comment #53
timmillwoodComment #55
timmillwoodLooks like my code:
or
Works locally (php 5.6.6) but doesn't work for testbot (php 5.4).
Looking for a way to get a more human readable PHP version number than something like "5.6.6-1+deb.sury.org~utopic+1".
Comment #56
hussainwebAh, class constants! For some reason I thought we were talking about
define()
constants.Comment #57
hussainwebThis should be
self::PHP_MAJOR_VERSION
and so on...Secondly, assigning expressions to static members became possible in PHP 5.6. This means you have to do this in a constructor or some other way.
Lastly, I still don't like the variable names. The class is not intended to be specific to system requirements for PDO multi statement attribute. The constant names are generic and could mean min, max versions for anything. If you want to use constants (or properties), it should be named as PDO_MULTI_STATEMENT_ATTRIBUTE_MIN_PHP_VERSION or something like that. This is why I was not that keen on making it a constant.
Comment #58
timmillwoodAdding a public static function for the human readable PHP version rather than trying to do it in a variable or constant, because as @hussainweb mentioned what I want to do needs PHP 5.6 (Another reason to upgrade ;-) )
Comment #59
timmillwoodJust spotted an indent that shouldn't be there.
Comment #60
daffie CreditAttribution: daffie commented@cilefen: I had misread your comment on #24. Hopefully you are not offended. If so, my apologies.
Comment #61
cilefen CreditAttribution: cilefen commented@daffie Offended? No.
Comment #62
YesCT CreditAttribution: YesCT commentedComment #63
timmillwood#2508231: Raise minimum required version of PHP to 5.5.9 has now been committed so more of a reason to push forward with this.
Comment #64
effulgentsia CreditAttribution: effulgentsia at Acquia commentedI haven't read every comment in this issue, but I'm unclear what we gain from this. Why are we recommending that people go through the bother of installing a PHP version that is different from the one bundled with and security supported by their Linux distro? If that's been answered already, can that be added to the issue summary?
Comment #65
timmillwood@effulgentsia - My initial reason for opening this is that PHP 5.5 (which is now the minimum requirement) ends active support tomorrow. If feel this is enough to be why everyone should update to PHP 5.6. The 5.6.5 release was chosen specifically because of the PDO::MYSQL_ATTR_MULTI_STATEMENTS addition, which was already added in a different issue, but getting refined here.
Note: we are not stopping the installation or throwing a big warning, this is just an info message offering a bit of guidance towards best practices.
Comment #66
effulgentsia CreditAttribution: effulgentsia at Acquia commentedWhy is it best practice to deploy sites on a PHP version that has active (meaning bug fixes in addition to security fixes) support? Why is it not equally good practice to deploy on a version that is bundled with and security supported by the Linux distro on your server? Many sites don't care about non-security PHP bugs that get fixed after the site is deployed. Why should they? There might be good answers to this question, but for me, it's not obvious, so would like it answered in this issue explicitly.
Comment #67
webchickYeah, I'm a little lost both on why we're doing this and also why this is a major task. This seems like something a theoretical "Best Practices" module (sub-module of Devel?) in contrib could take care of, and could not only do so for just PHP, but all the other requirements (MySQL, Apache, etc.) as well, including some "if" branching for specific Linux distros to alleviate concerns of David Rothstein and others.
Comment #68
webchick...especially since the vast majority of Drupal's user base can do absolutely nothing about the version of PHP that their Drupal site is installed on. (shared hosts, managed hosts, their site was set up for them by some contractor, etc.)
It's a good message to provide for developers. Just don't see why we'd do it in core.
Comment #69
timmillwoodBecause it offers bug fixes.
Well they can ignore this then, it's just info, not an error or warning.
Why would anyone bother installing that?
That's fine, they too can ignore this info, or hassle their host for an upgrade, or change host (to something like Acquia which now offers 5.6 as an option).
I may be biased but agree with @crell in #2
Comment #70
webchickBasically every developer installs Devel module, and they are the only ones who can actually do something about the PHP version.
I agree it's a fine idea. My question was, "Why in core?"
Comment #71
timmillwood- So it get's flagged early in the development process
- So it get's flagged on dev / stage / prod boxes, where as a module like devel would often only be enabled locally.
- So it can be incremented along with the core development workflow
Comment #72
catchHmm so when reading this I was thinking about #2267551: [meta] Deal with PHP requirement vs. Debian PHP version numbering. While that issue was going on Debian started following actual PHP minor releases instead of sticking on the same version number and backporting individual patches i.e. https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=757342 and https://www.debian.org/security/2014/dsa-3064
This change (at least once Ubuntu Trusty either makes that change or is EOL - I think that's the only other place with a similar problem) means that within a major PHP version, it's meaningful to have specific requirements and recommend things like versions containing the PDO prepared statement fix - since it matches reality a lot more than it did previously.
However, this does not at all suggest that they'll move from PHP 5.4 to 5.5 when PHP 5.4 is EOL. Presumably they'll stick at the last release and start backporting patches after then. At that point, the distro release and PHP's security schedule diverge and this patch does not match reality at all. More information from Debian and other distros on exactly what their plans are for LTS releases vs. PHP's support schedule would be useful here. The PHP 5.4 EOL is really the first time this is kicking in properly, both for us and them.
I do think there is value in warning people if they're actually running an unsupported PHP version, and doing that in core. However that's not going to be the case until September 2016 for Drupal 8. So marking this postponed for now.
Something we can do now is try to revamp http://drupal.org/requirements to be clearer about what exactly we do and don't support for which major versions of core vs. PHP.
Comment #73
salvisI don't think the Devel maintainers would want to support the questions and endless discussions that such a recommendation in Devel could raise.
Another thought: It must be clear for contrib developers what PHP version they should support. If core sends mixed signals, this becomes a fuzzy target. Can a contrib developer allow himself to become negligent about SQL injection because core recommends a PHP version that has built-in guards against it (rhetorical question)?
As a Drupal user I happen to run a VPS from HostEurope.de with Plesk. They've made Ubuntu 14.04 LTS with PHP 5.5.9 available only a few months ago. Ubuntu 14.04 will probably stay on 5.5.9 forever (with backported security patches). Trying to update PHP is likely to break Plesk and maybe even the VPS. It'll be another 2 years before they offer an Ubuntu 16.04 LTS with a higher PHP version. (And no, I don't want to change to another provider.)
My real life situation is even worse, because I still have a D6 site with lots of images that I haven't been able to migrate to D7, so I'm still running on the older VPS version with Ubuntu 12.04 LTS (EOL April 2017) and PHP 5.3.10. Reinstalling the server in a new VPS is a major undertaking; I will eventually move to 14.04 when D8 is ready to replace my D6, but getting to PHP 5.6 is still a couple of years away, and any recommendations about "best practices" in this area are, sorry to say this, completely irrelevant for me.
Comment #74
XanoAs a contrib maintainer I don't have to do anything. If I deem it best to require a PHP version that is higher than what core requires, that's perfectly fine. Possible reasons are that contrib usually moves more quickly and can more easily require newer PHP versions, or that a module simply needs the features of a newer PHP version.
Comment #75
salvisRight. But it makes sense for core to set a clear standard and for contrib to follow it. Everything else causes friction on various levels, which does not help the project.
Comment #76
XanoLet me rephrase: there should not be such a standard. Different packages have different requirements and those are up to those packages' developers, based on what is needed. This is common practice, not unique to Drupal.
The only common factor here is: don't use unsupported PHP versions or you may find your website compromised.
Comment #77
mgifford@catch what is this postponed on?
Comment #78
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commentedWe wait till around near September 2016 - once PHP 5.5 becomes unsupported - to warn people of potentially unsupported PHP versions.
Comment #79
Fabianx CreditAttribution: Fabianx as a volunteer and at Tag1 Consulting commented.
Comment #80
catchYes. Either that or Debian changing policy and updating php major version during a stable release.
Comment #81
catchYes. Either that or Debian changing policy and updating php major version during a stable release.
Comment #82
dawehnerComment #85
xjmLet's close this as a duplicate of #2670966: Warn users of old PHP versions, since as @Crell points out, we should actually recommend PHP 7 now that it is stable.
Comment #86
gisleFor what it is worth: Red Hat's RHEL7 still ships with PHP 5.4.16. Red Hat maintains it with security patches.
Things are not bleeding edge in the enterprise world.