Remove laminas-feed, laminas-escaper, and laminas-stdlib from drupal/core-recommended to allow Drupal 9.5 to be installed on PHP 8.2

https://dispatcher.drupalci.org/job/drupal8_core_regression_tests/81342/... shows a lot of tests failing on PHP 8.2, including unit tests, so Drupal 9.5 might be quite a bit farther from PHP 8.2 compatibility than just Laminas-feed.

A lot of the failures in #2 might just be PHP deprecation notices. Here's a patch that suppresses them. Also, this changes config.platform.php to 8.2 and updates composer.lock with a composer update, just to see how that affects tests.

#3 updated too many dependencies causing a bunch of test failures. Instead of a full composer update, this patch only does a composer update laminas/*.

The last submitted patch, 4: d95-php82-3354670-4.patch, failed testing. View results →

Trying more ways to suppress PHP 8.2 deprecations.

Surprised the bot didn't pick this up.

But appeared to have test failures.

Very few of the PHP 8.2 compatibility issues got backported to 9.5 because several would have been disruptive, or a lot of effort to backport vs concentrating on compatibility in 10.0. I could see skipping deprecation messages to enable test runs though.

diff --git a/core/phpunit.xml.dist b/core/phpunit.xml.dist
index b8fe8ca4da..79a2d9f81f 100644
--- a/core/phpunit.xml.dist
+++ b/core/phpunit.xml.dist
@@ -14,8 +14,8 @@
          printerClass="\Drupal\Tests\Listeners\HtmlOutputPrinter"
          cacheResult="false">
   <php>
-    <!-- Set error reporting to E_ALL. -->
-    <ini name="error_reporting" value="32767"/>
+    <!-- Set error reporting to E_ALL & ~E_STRICT & ~E_DEPRECATED. -->
+    <ini name="error_reporting" value="22527"/>

I thought perhaps DrupalCI generated its phpunit.xml from this, but apparently not. So instead this patch adds a new phpunit.xml.

That didn't work. Back to modifying core/phpunit.xml.dist, but also uncommenting SYMFONY_DEPRECATIONS_HELPER instead of relying solely on drupalci.yml's suppress-deprecations.

+++ b/core/composer.json
@@ -17,7 +17,7 @@
-        "php": ">=7.3.0",
+        "php": ">=8.0.0",

+++ b/core/lib/Drupal.php
@@ -105,7 +105,7 @@ class Drupal {
-  const MINIMUM_SUPPORTED_PHP = '7.4.0';
+  const MINIMUM_SUPPORTED_PHP = '8.0.0';

@@ -123,7 +123,7 @@ class Drupal {
-  const MINIMUM_PHP = '7.3.0';
+  const MINIMUM_PHP = '8.0.0';

Maybe I misunderstand and this is still a proof-of-concept patch or something, but how can we possibly change this requirement in a patch release to 9.5.x? That would be highly disruptive... 😅

The issue title says "... to allow Drupal 9.5 to be installed on PHP 8.2" but doesn't say anything about "and prevent Drupal 9.5 from being installed with PHP 7".

I'm all for progress, but if this change requires PHP 8 to allow PHP 8.2, I'd vote for "won't fix".

Yeah, so far I'm just trying to see/prove if it's possible to run Drupal 9.5 on PHP 8.2. I think this patch will be green, proving that it is. Meaning that the only way in which 9.5 HEAD isn't currently compatible with PHP 8.2 is that:

• It uses PHP features that are deprecated in 8.2, but they still work, so it's fine for a site that doesn't need to report those deprecation notices.
• The root composer.json and composer.lock files pin to Laminas versions that are PHP 7.3 compatible but not PHP 8.2 compatible. But a site wanting to use PHP 8.2 doesn't need those pins in its own root composer.json and composer.lock files.
• Those old Laminas versions are also pinned in drupal/core-recommended, and dealing with that is this issue's original scope. The patches here go beyond that scope in order to determine/prove if the above statements are accurate.

Makes sense, thanks! Carry on, then... 😉

Actually, because Drupal's root composer.json sets config.platform.php, I wonder if we don't even need to update Laminas (or make any Composer changes) for tests to pass on PHP 8.2. Maybe we can just use the old Laminas versions and they'll in fact work on PHP 8.2 despite what their composer.json says? This patch is trying that out.

I opened 📌 Drupal 9 uses PHP syntax that's deprecated in PHP 8.2, so exclude that from error_reporting() and DeprecationListenerTrait Fixed to do part of what's in #19.

Everything in #19 is now in #3355675-12: Drupal 9 uses PHP syntax that's deprecated in PHP 8.2, so exclude that from error_reporting() and DeprecationListenerTrait → , but without needing to modify phpunit.xml.dist.

This patch now implements the actual scope of this issue :)

I'm not sure if we want to do it this way or if we'd prefer to fork the libraries like in 📌 Fork laminas/laminas-diactoros Fixed so that the same minor version can work for PHP 7.3 through 8.2.

Updated issue title and summary to consider forking as an alternate option, per #3356283-24: Fork laminas/laminas-diactoros, update guzzlehttp/psr7 → .

I think this is probably going to miss this week's patch release but it would be good to get it in for the next one. Not sure the answer to #21.

Rerolled now that 📌 Fork laminas/laminas-diactoros Fixed is in.

Here's my thought on how we can do the fork option if we want to...

Each of these 3 libraries currently has only a single tag that satisfies the drupal/core-recommended constraints. E.g. 2.17.0 for laminas-feed.

So, I think we can fork that tag, change its composer.json to allow PHP 8.2 and include a replace entry, and release that as 2.17.1. Let's assume we put this fork into the drupal namespace by making it a General Project on drupal.org (general projects on d.o. that contain a composer.json get automatically published to packagist), which I think would require us to change the hyphen to underscore, so it would be drupal/laminas_feed. I think we can then leave drupal/core as requiring laminas/laminas-feed, but change drupal/core-recommened to require drupal/laminas_feed: ~2.17.0 instead. That way, anyone without drupal/core-recommended can keep using the original laminas packages (and automatically get new minor versions of them when they're released) but anyone with drupal/core-recommended would switch to the drupal fork which would be the identical code as what they already have (and what's in composer.lock) other than relaxing the PHP constraint. This also means that this fork would not need any maintenance unless upstream releases new patches on these old minor versions (which they have no history of doing) or a security vulnerability is disclosed that we need to backport the fix for, which would require us to switch to the approach used in 📌 Fork laminas/laminas-diactoros Fixed .

Thoughts on this approach in general, and on this way of using the drupal namespace (which also means forking to d.o.'s GitLab rather than to GitHub)? To avoid cluttering d.o. with projects we don't use, I figured I'd hold off on creating the d.o. projects themselves until we decide to pursue this.

I think I'd be fine with either removing the constraint or forking here. There are benefits and drawbacks to both approaches, but in practice it's likely to be about the same, and we only have to live with it for 7 months-ish.

Given that our exposure to Laminas Feed, Escaper and Stdlib is only via Aggregator, which is deprecated and we assume many sites do not use, I think that option 1 is easier. The Laminas project tends to be fairly strict about semver and so I don't expect them to make a breaking change within the major version that affects us, and as @catch says this is only for 7 months or so anyway.

In the (presumably unlikely) event that they have a security release and refuse to backport it (as they did with Diactoros), we would have to go for option 2 at least for the package in question, but to me that is more work to do and not really worth the effort until it actually happens.

I think that means we can go ahead here?

Fixing title now we have made a decision.

Committed and pushed 5073245c963 to 9.5.x. Thanks!

Thanks! Not really related to this issue at all, but just commenting here to shamelessly advertise 📌 Remove psr/http-message from drupal/core-recommended Fixed :)

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