Problem/Motivation
This issue did solve the problem for sites using the core-recommend template they will still use Guzzle 6 which is not compatible with PHP 8.1. Currently Guzzle 6 just give deprecation errors on PHP 8.1. Such deprecations are not user deprecations but they are language level deprecations. For example:
Return type of GuzzleHttp\Command\Command::count() should either be compatible with Countable::count(): int, or the #[\ReturnTypeWillChange] attribute should be used to temporarily suppress the notice
Proposed resolution
Possible solutions discussed in #3225966: Consider loosening our constraint to allow sites to install Guzzle 6 or 7, or otherwise handle PHP 8.1 deprecations for Guzzle 6
- @Berdir have a drupal/core-recommends-81 package
- @xjm However, making Guzzle a top-level dependency of core-recommended sounds even simpler and totally legit from an architecture perspective.
- @xjm Is there a way core-recommended could be conditional based on the platform PHP version?
Comments
Comment #2
tedbowComment #3
tedbowComment #4
tedbowComment #5
xjmComment #6
xjm(Tarballs might be a lost cause.)
Comment #7
MixologicAnother idea: instead of requiring guzzle directly in core-recommended, we could have core recommended require a metapackage.. say,
drupal/guzzle-director: *The metapackage would have 2 releases, 1.0 would have composer require php/php >=7.4, <8.1, and would also require our pinned version of guzzle 6. 2.0 would have php/php >=8.1, and whatever pinned version of guzzle 7 we have at the time.
If core upgrades guzzle, we upgrade the metapackage's pinned versions, and the php version of the project dictates whether guzzle 6 or guzzle 7 gets installed.
I think the primary challenge with this is that our current metapackage generation techniques all rely on the lockfile, so we'd need some way/somewhere to preserve both pinned guzzle versions.
Comment #8
tedbowre #7won't that cause problems if we force sites running on php 8.1 to use Guzzle 7. From my understanding Guzzle 6 would still work on PHP 8.1 but have deprecation notices where since Guzzle 7 would potentially some sites since it has BC breaks.
Comment #9
phenaproximaI discussed this in a Zoom with @TravisCarden and @tedbow. What we quickly agreed on is that this situation sucks.
As @tedbow alluded to in #8, the trickiness here lies in the fact that we don't know how contrib and custom code on any given site is using Guzzle. Switching to Guzzle 7 could break sites badly and we wouldn't be able to predict it. On the other hand, if you're using PHP 8.1, staying on Guzzle 6 could mean your logs potentially get flooded with deprecation notices and other such junk, which it wouldn't necessarily be clear how to get rid of.
The ideas we batted around generally leaned towards letting site owners/developers decide for themselves when they want to switch to Guzzle 7, and giving them a clear way to do that. For example:
Comment #10
lauriiiDiscussed this with @alexpott, @tedbow, and @phenaproxima over Zoom.
Based on that, I understood that there are two problems that we are trying to address:
However, if we changed the version constraint in core-recommended, it could accidentally update projects to Guzzle 7 even if their site is not compatible with Guzzle 7 since contrib modules are not expected to have to define which major version of a core dependency they support. Based on some research by @phenaproxima, the Guzzle 7 BC breaks doesn’t seem very disruptive, but it's almost certain that there are some projects that require changes to be Guzzle 7 compatible. This introduces some risk if we change the version in drupal/core-recommends.
Since it's unlikely that contrib modules depend on Guzzle 7 yet, it seems like this is something that would at the moment mostly interfere advanced use cases using CI / other environments where deprecation warnings are enabled (Guzzle 7 works with PHP 7, but not without deprecation notices). For these advanced users, there are multiple ways to try to tackle this; e.g. patch Guzzle or depend on core directly.
Based on that we could have core-recommended continue pin to Guzzle 6 since that's the recommended for majority of the sites. If that changes in future, we could start shipping multiple packages (e.g. drupal/core-recommends-guzzle) or multiple releases in core-recommended later if it's something that we need.
Comment #11
catchI think this is a good idea that we could easily spin-off into a separate issue.
Comment #12
lauriiiI'm not sure I understand what's the benefit of doing that since AFAIK there's no harm in running Guzzle 7 with PHP 7.4 unless you run into problems with CI / deprecation warnings?
Comment #13
effulgentsia commentedGiven everything that's been said above, I think it makes sense to leave
core-recommendedas it currently is (pinned to Guzzle 6).However, are there existing users of
core-recommendedfor whom Guzzle 6 will present a problem? E.g., if a contrib module decides to require Guzzle 7. Or, if PHP deprecation notices are a problem for some reason (interfering with CI? polluting logs?).If there's a non-trivial amount of those users, a way to help them out would be if we created an additional metapackage (e.g.,
core-recommended-unconstrained-guzzle) that was the same as core-recommended, but without Guzzle. That way, a site that needed to update to Guzzle 7 (but while still retaining the benefits of everything else that's in core-recommended), could do:Comment #14
mglamanNot being able to adopt Guzzle 7 can make Drupal 10 readiness difficult. Here's the workaround added to a GitHub Actions workflow to see why Guzzle 7 cannot be installed (contrib and their dependency dependency constraint.) Which of course is always blocked by drupal/core-recommended
Our script removes drupal/core-recommended for drupal/core, then runs an update for Guzzle 7:
Comment #16
somebodysysop commentedI am running 9.5.2. I would like to install the OpenAI-PHP/Client package https://packagist.org/packages/openai-php/client which requires Guzzle 7.5. My current Guzzle (in Drupal) version is 6.5.8.0.
Does anyone know if there are any issues with installing Guzzle 7.50 at this point while still running Drupal 9.5.2 sites?
Thanks!
Comment #17
newaytech commentedI am hoping to install the Shopify PHP library for a microservices project - but get an issue with Guzzle versions:
- Root composer.json requires shopify/shopify-api ^4.2 -> satisfiable by shopify/shopify-api[v4.2.0].
- shopify/shopify-api v4.2.0 requires guzzlehttp/guzzle ^7.0 -> found guzzlehttp/guzzle[dev-master, 7.0.0-beta.1, ..., 7.5.x-dev (alias of dev-master)] but the package is fixed to 6.5.8 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command.
It would be great if we could get some guidance as to how to migrate safely toward Guzzle 7...
Comment #18
effulgentsia commentedhttps://www.drupal.org/node/3268032 links to https://www.drupal.org/docs/develop/using-composer/manage-dependencies#s....
Comment #19
effulgentsia commentedIn addition to Guzzle, core-recommended also currently pins Laminas-feed to 2.17 whose composer.json does not allow PHP 8.2, but 2.19 does, so people wanting to install Drupal 9.5 on PHP 8.2 need to switch away from core-recommended in order to get Laminas-feed 2.19 or higher. We can't raise the laminas-feed requirement in core-recommended, because 2.19 also removed support for PHP 7.4.
Unlike the case with Guzzle 6/7, this prevents composer installation entirely with core-recommended 9.5 and PHP 8.2, which is a shame since in many cases the D9 site doesn't even need aggregator module enabled.
Comment #20
effulgentsia commentedFor the laminas-feed issue, I opened #3354670: Remove laminas-feed, laminas-escaper, and laminas-stdlib from drupal/core-recommended to allow Drupal 9.5 to be installed on PHP 8.2. Not sure if that's a good solution, but perhaps it can spark other ideas?
Comment #21
xjmThe remaining scope of this is no longer relevant since we're not going to be committing any non-security thing to D9 anymore (nor does D10 have Laminas dependencies). If we need core-recommended to do things that involve two-major constraints in the future, then we can open a new issue. Thanks!