Unneeded always_populate_raw_post_data requirements check while on CLI

#2456025: PHP warnings in PHP 5.6 because of always_populate_raw_post_data ini setting added that check.

This is because the PHP CLI has different settings, right? But then how can rest_requirements() actually require that this setting has a certain value? I think you're saying it can't?

Right: there is no POST in CLI, because there is no HTTP in CLI. I'm not questioning that.

But this is a php.ini configuration check that's necessary for the REST module to function correctly (apparently, I'd never seen it before). My concern is that making the change you proposed means that using drush en -y rest will now not do the necessary requirements checking.

Hm, apparently always_populate_raw_post_data is removed in PHP 7, and it never affected REST functionality, only tests. And the two affected tests no longer exist, they've been refactored away.

AFAICT this is a pure PHP bug work-around, so we should probably keep it until Drupal requires PHP >=7.

This issue is filed against Drupal 9. I think you want this to land in the next Drupal 8 minor though?

Sound good to me! I see no reason why this could not land in 8.4 already, since this is a simple bugfix with no behavioral impact on existing sites.

Hm, apparently always_populate_raw_post_data is removed in PHP 7, and it never affected REST functionality, only tests. And the two affected tests no longer exist, they've been refactored away.

If that's true, this requirements check could just be removed entirely. (But based on reading #2456025: PHP warnings in PHP 5.6 because of always_populate_raw_post_data ini setting it sounds like there might be a real effect somewhere; it is hard to tell.)

It would be nice to just remove the code - it's somewhat suspect anyway (since it enforces a particular solution which isn't even the most backwards-compatible one that the PHP developers recommend; they seem to recommend simply turning off display_startup_errors in production).

But just making it a runtime-only error (and non-CLI) does sound good as a first step.

Quick patch (untested) that just does what was suggested above (runtime-only and non-CLI).

RTBC++

Crediting @WimLeers as the discussion and reviews here shaped the patch

Moving to 8.5.x, as per https://www.drupal.org/core/release-cycle-overview#current-development-c... 8.4 is in 'critical fixes only', so this can't be backported to 8.4

Committed as 0428e20 and pushed to 8.5.x.

@larowlan: doesn't "criticals only for current minor" start when the beta of the next minor is released?

Hi Wim,

I checked with the release managers just to be sure, but definitely 8.4.5 will be criticals only.

Lee

Created a followup issue to discuss removing this check entirely.