This issue is about introducing the actual requirement. For the policy discussion about whether or not to make Drupal core require PHP 5.4, see #1498574: [policy, no patch] Require PHP 5.4

Bump Drupal core's PHP requirement to 5.4.2 as per #1498574: [policy, no patch] Require PHP 5.4.

CommentFileSizeAuthor
#18 drupal8.php54.18.patch10.88 KBdmouse
#11 drupal8.php54.11.patch10.55 KBsun
#1 drupal_2152073_1.patch10.55 KBxano

Comments

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented
Assigned: xano » Unassigned
Status: Active » Needs review
StatusFileSize
new drupal_2152073_1.patch10.55 KB

This patch was made by cosmicdreams and me back in #1498574: [policy, no patch] Require PHP 5.4.

klonos’s picture

klonos
he/him
Primary language English
Location 90% Melbourne, Australia - 10% Larissa, Greece
commented
Parent issue: » #1498574: [policy, no patch] Require PHP 5.4
webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Note that I'm pretty sure this will continue to fail until we get 5.4.x testbots, which is blocked on #2135199: Provide php 5.4 testing on testbots for D8 code without breaking everything else. But at least now we can track it separately from various opinions on whether or not PHP 5.4 was a good idea. :) Thanks!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Category: Feature request » Task

This isn't a feature; this is from a policy discussion that's already been decided.

Status: Needs review » Needs work

The last submitted patch, 1: drupal_2152073_1.patch, failed testing.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Needs work » Postponed
Related issues: +#2135199: Provide php 5.4 testing on testbots for D8 code without breaking everything else

Right. That. Ok, then...

jessebeach’s picture

jessebeach commented
Related issues: +#2156793: Unravel a PHP 5.4 workaround for Edit module once Drupal requires PHP 5.4
xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

There may be an issue with testing traits. See #2159845: Upgrade to PHPUnit 4.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Related issues: +#2159845: Upgrade to PHPUnit 4

Maybe 5.4.7?

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Related issues: +#2107287: PHP 5.4 calls a new stream_metadata() method on stream wrappers not implemented by Drupal

Adding a critical issue.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Assigned: Unassigned » sun
Status: Postponed » Needs review
StatusFileSize
new drupal8.php54.11.patch10.55 KB
xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

Is there any difference between that patch and the one from #1?

Status: Needs review » Needs work

The last submitted patch, 11: drupal8.php54.11.patch, failed testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Sorry, nope, that was just a safety re-roll to facilitate PHP 5.4 testbot testing.

We're re-testing HEAD right now, so as to double-check whether the failures in #11 are real (which would mean that the switch to PHP 5.4 uncovered some real test failures in HEAD).

sun’s picture

sun
Primary language German
Location Karlsruhe
commented

Created #2205367: [HEAD BROKEN] PHP 5.4

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs work » Needs review

11: drupal8.php54.11.patch queued for re-testing.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Assigned: sun » Unassigned
Status: Needs review » Reviewed & tested by the community
Issue tags: +PHP 5.4

Reviewed once more in depth, and all of the changes are awesomesauce.

To combat two potential questions/issues upfront:

  1. A debate of bumping to a later patch release than 5.4.2 should be moved to another issue. This patch gets us across the goal line and unblocks lots of happiness.
  2. There are probably many many more PHP 5.3 workarounds throughout the core code base in the meantime. Let's not hold up this patch on those, but tackle them in separate issue(s) instead.

Thanks!

dmouse’s picture

dmouse
Primary language Spanish
Location Mexico City
commented
StatusFileSize
new drupal8.php54.18.patch10.88 KB

We also can add the version in composer.json

dries’s picture

dries commented
Status: Reviewed & tested by the community » Fixed

I tried to commit this patch but it looks like core/lib/Drupal/Core/Mail/PhpMail.php was removed 8 hours ago. Looking at the patch, it seems like the PhpMail chunks can be removed safely, so I did exactly that. Committed to modified patch to 8.x. We may still have to delete death code from the new mailer class, if any.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented

Opened #2207021: Remove legacy PHP 5.3 code from PhpMail plugin as a follow-up.

xano’s picture

xano
he/they
Primary language English
Location Southampton
commented

See #2210257: Bump \Drupal\Component\Utility's PHP requirement to 5.4.2.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

yesct’s picture

yesct commented
Related issues: +#2246775: Suggest 5.4.11 as minimum PHP version for Windows and MacOS if XCache is enabled

related... #2246775: Suggest 5.4.11 as minimum PHP version for Windows and MacOS if XCache is enabled