Problem/Motivation

Follow-up for #3225966: Consider loosening our constraint to allow sites to install Guzzle 6 or 7, or otherwise handle PHP 8.1 deprecations for Guzzle 6

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

  1. @Berdir have a drupal/core-recommends-81 package
  2. @xjm However, making Guzzle a top-level dependency of core-recommended sounds even simpler and totally legit from an architecture perspective.
  3. @xjm Is there a way core-recommended could be conditional based on the platform PHP version?

Remaining tasks

API changes

Release notes snippet

Comments

tedbow created an issue. See original summary.

tedbow’s picture

Issue summary: View changes
tedbow’s picture

Issue summary: View changes
xjm’s picture

Title: For core-recommended: Consider loosening our constraint to allow sites to install Guzzle 6 or 7 » Come up with a way to allow core-recommended and tarballs to install Guzzle 7 for PHP 8.1 compatibility
Version: 10.0.x-dev » 9.4.x-dev
Priority: Normal » Major

 

xjm’s picture

Title: Come up with a way to allow core-recommended and tarballs to install Guzzle 7 for PHP 8.1 compatibility » Come up with a way to allow core-recommended (and tarballs?) to install Guzzle 7 for PHP 8.1 compatibility

(Tarballs might be a lost cause.)

Mixologic’s picture

Another 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.

tedbow’s picture

re #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.

phenaproxima’s picture

I 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:

  • When you spin up a new project using legacy-project or recommended-project, may we could show a message if PHP 8.1 is detected, telling people that they should upgrade to Guzzle 7 to avoid deprecation errors (and maybe linking them to a page on drupal.org that explains how to do it, and the possible ramifications).
  • We could detect PHP 8.1 and Guzzle 6 in \Drupal\Core\Http\ClientFactory and trigger/log an official deprecation notice with a link to the aforementioned documentation. This would be help people who are using tarballs.
  • We could also detect that situation in system_requirements() and flag a warning. This has the advantage of being more visible, in a central location, than logging deprecations.
  • We could (and this is nuts, but it wouldn't be me if I didn't float something weird like this) try to shim the stuff that was removed from Guzzle 7, to the extent that we can.
lauriii’s picture

Discussed this with @alexpott, @tedbow, and @phenaproxima over Zoom.

Based on that, I understood that there are two problems that we are trying to address:

  1. Guzzle 6 triggers deprecation notices on PHP 8.1. This means that core-recommended would trigger deprecation warnings with PHP 8.1. This only impacts environments with deprecation warnings turned on, e.g. CI.
  2. #3225966: Consider loosening our constraint to allow sites to install Guzzle 6 or 7, or otherwise handle PHP 8.1 deprecations for Guzzle 6 makes it possible for contrib to start using Guzzle 7 features. This means that if a project is only compatible with Guzzle 7, it would not be compatible with core-recommended unless we make the Guzzle version constraint more permissive.

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.

catch’s picture

We could also detect that situation in system_requirements() and flag a warning. This has the advantage of being more visible, in a central location, than logging deprecations.

I think this is a good idea that we could easily spin-off into a separate issue.

lauriii’s picture

I'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?

effulgentsia’s picture

Given everything that's been said above, I think it makes sense to leave core-recommended as it currently is (pinned to Guzzle 6).

However, are there existing users of core-recommended for 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:

composer require drupal/core-recommended-unconstrained-guzzle
composer remove drupal/core-recommended
composer require guzzlehttp/guzzle:^7
mglaman’s picture

Not 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:

      - name: "Install dependencies"
        run: "composer update --no-progress --prefer-dist"
      - run: composer why-not guzzlehttp/guzzle ^7.0 -r -t
      - name: "Switch to drupal/core from drupal/core-recommended"
        run: |
          composer remove drupal/core-recommended --no-update
          composer require drupal/core --no-update
      - name: "Update to Guzzle 7"
        run: "composer require guzzlehttp/guzzle:^7.0 --no-update"
      - name: "Update dependencies"
        run: "composer update --no-progress --prefer-dist"

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.9 was released on December 7, 2022 and is the final full bugfix release for the Drupal 9.4.x series. Drupal 9.4.x will not receive any further development aside from security fixes. Drupal 9 bug reports should be targeted for the 9.5.x-dev branch from now on, and new development or disruptive changes should be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

somebodysysop’s picture

I 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!

newaytech’s picture

I 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...

effulgentsia’s picture

It would be great if we could get some guidance as to how to migrate safely toward Guzzle 7

https://www.drupal.org/node/3268032 links to https://www.drupal.org/docs/develop/using-composer/manage-dependencies#s....

effulgentsia’s picture

Title: Come up with a way to allow core-recommended (and tarballs?) to install Guzzle 7 for PHP 8.1 compatibility » Come up with a way to allow core-recommended (and tarballs?) to install Guzzle 7 for PHP 8.1 compatibility and Laminas-feed 2.19 for PHP 8.2 compatibility
Issue tags: +PHP 8.2

In 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.

effulgentsia’s picture

For 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?

xjm’s picture

Status: Active » Closed (won't fix)

The 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!