Problem/Motivation

Issue #2456025: PHP warnings in PHP 5.6 because of always_populate_raw_post_data ini setting broke InstallUninstallTest in PHP 7. See #2454439: [META] Support PHP 7 for more details.

Proposed resolution

We need an additional condition to not run this on PHP 7 since this ini setting no longer exists there.

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

bzrudi71’s picture

sasanikolic’s picture

Issue summary: View changes
sasanikolic’s picture

Status: Active » Needs review
FileSize
682 bytes

Added the condition for ignoring PHP7.

Note: it might be easier to read if we use the comparing operator as the third argument to version_compare().

dawehner’s picture

Note: it might be easier to read if we use the comparing operator as the third argument to version_compare().

Do you mind specifying how this would look like exactly? Its pretty good to read already.

Do you checked whether we have a test for that requirement? I guess no, and I'm also not sure how we would be able to check it.

Berdir’s picture

The note is already implemented, that's the change that the patch is doing.

I don't see how we can test this. #2456025: PHP warnings in PHP 5.6 because of always_populate_raw_post_data ini setting also got it without a test. To be sure that this is working, I'll re-run the InstallUninstallTest on php7 tonight.

Fabianx’s picture

Category: Task » Bug report
Priority: Normal » Major
Status: Needs review » Reviewed & tested by the community

- Major, because soft-blocking a critical
- Bug, because broken on PHP-7

stefan.r’s picture

Status: Reviewed & tested by the community » Needs review
+++ b/core/modules/rest/rest.install
@@ -0,0 +1,23 @@
+      'description' => t('The always_populate_raw_post_data PHP setting should be set to -1 in PHP versions 5.6 and up. Please check the <a href="https://php.net/manual/en/ini.core.php#ini.always-populate-raw-post-data">PHP manual</a> for information on how to correct this.'),

In newer versions we don't have to set it as it has disappeared, so: s/PHP versions 5.6 and up/in PHP version 5.6/

In theory we could test this if we wrapped ini_get and the PHP_VERSION constant, such as in #2455455: Fix outdated Unicode requirements check, but indeed a test should not be needed

stefan.r’s picture

Status: Needs review » Needs work
klausi’s picture

Title: Add condition to ignore PHP 7 on rest tests » Add condition to ignore PHP 7 on rest requirements check
sasanikolic’s picture

Status: Needs work » Needs review
FileSize
970 bytes
1.28 KB

Corrected the comment about PHP 5.6+.

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community

Back to RTBC, looks great!

stefan.r’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
892 bytes
1.28 KB

One last nitpick

Fabianx’s picture

Status: Needs review » Reviewed & tested by the community
webchick’s picture

Status: Reviewed & tested by the community » Fixed

Onward, PHP7! :)

Committed and pushed to 8.0.x. Thanks!

  • webchick committed f4de679 on 8.0.x
    Issue #2496943 by sasanikolic, stefan.r: Add condition to ignore PHP 7...

Status: Fixed » Closed (fixed)

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