I have a complete implementation of GuzzleHttpRequest coded and tested locally, including a phpunit test. (Wanted to make sure it was going to work out well before I created the official task.)

Does https://www.mediawiki.org/wiki/Manual:External_libraries describe the steps I need to take before adding a new external library to mediawiki? In particular, do I need to request a security review before posting a Gerrit change?

In T202110#4508941, @BPirkle wrote:

I have a complete implementation of GuzzleHttpRequest coded and tested locally, including a phpunit test. (Wanted to make sure it was going to work out well before I created the official task.)

Does https://www.mediawiki.org/wiki/Manual:External_libraries describe the steps I need to take before adding a new external library to mediawiki? In particular, do I need to request a security review before posting a Gerrit change?

Feel free to upload whatever you want to Gerrit, we just can't merge anything that depends upon the new library until the security review is finished.

Change 453408 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/core@master] Create GuzzleHttpRequest class as new default for HttpRequestFactory

https://gerrit.wikimedia.org/r/453408

So doing some poking around here... We end up with a few extra other libraries, which is fine, just a bit more poking around needed.

Vendor patching incoming as I'm going through stuff

(we might also want to pin some more of these versions in mediawikis composer.json too)

reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/vendor$ composer update --no-dev
Loading composer repositories with package information
Updating dependencies
Package operations: 5 installs, 0 updates, 0 removals
  - Installing guzzlehttp/promises (v1.3.1): Downloading (100%)         
  - Installing ralouphie/getallheaders (2.0.5): Downloading (100%)         
  - Installing psr/http-message (1.0.1): Downloading (100%)         
  - Installing guzzlehttp/psr7 (1.5.0): Downloading (100%)         
  - Installing guzzlehttp/guzzle (6.3.3): Downloading (100%)         
Writing lock file
Generating optimized autoload files
reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/vendor$

Change 477538 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/vendor@wmf/1.33.0-wmf.6] Add guzzlehttp/guzzle 6.3.3 and dependancies

https://gerrit.wikimedia.org/r/477538

No longer blocked by T202143: Security review for Guzzle 6.3.3

My main question about bringing in Guzzle is how we're going to apply the deprecation policy to this. Typically we've said that extensions can depend upon any library that MediaWiki core depends upon, since we're going to keep back-compat or cautiously update external libraries. But the size of Guzzle is much much larger than any other library that core depends upon to date. If Guzzle makes a breaking change, I'm imagining a mess if we need to update every thing that makes HTTP requests.

My thinking right now is that we somehow/somewhere declare that Guzzle isn't part of the stable interface, and extensions need to use the MW abstraction layer instead (this seems like a good idea regardless given T202143#4802539). Does that seem reasonable?

I also wanted to say that I found the Guzzle codebase relatively well-written. My full comments/thoughts are:

• It would be nice if we didn't have any autoload files and everything was class loaded. E.g. for Guzzle there's no reason the default_user_agent() function couldn't be static on a class.
• Having CodeSniffer run upstream (https://github.com/guzzle/guzzle/pull/2194) would be nice too, would make it easy for some minor things I noticed like php_sapi_name() instead of PHP_SAPI.
• .gitattributes exclusions, but Reedy already sent PRs to everyone
• PHP only has the getallheaders function in the Apache and FPM (since 7.3) SAPIs so it needs a polyfill. Maybe we can ask PHP to provide the function on all SAPIs where it does nothing so it doesn't need a userland polyfill?

My thinking right now is that we somehow/somewhere declare that Guzzle isn't part of the stable interface, and extensions need to use the MW abstraction layer instead (this seems like a good idea regardless given T202143#4802539). Does that seem reasonable?

I think so. Or if they use it directly, MW isn't responsible for breaking changes and therefore won't accept bug reports

And for your bullet points, we can hopefully grab the little improvements in point releases in the near future

There's definitely some annoyances, but I don't see anything too major to not continue with doing this

@Legoktm on the deprecation policy and Guzzle support.. I point at PSR-17 and PSR-18 and I think that that is what we need to keep in mind in terms of any breaking changes and support.

So maybe have one new interface that is compatible with upcoming php interfaces and those psr's should be the only contract we promise to deliver to extensions. Either use that and be supported or use guzzle directly yourself and be responsible for any of your breakages.

Change 477538 merged by jenkins-bot:
[mediawiki/vendor@master] Add guzzlehttp/guzzle 6.3.3 and dependancies

https://gerrit.wikimedia.org/r/477538

Change 453408 merged by jenkins-bot:
[mediawiki/core@master] Create GuzzleHttpRequest class as new default for HttpRequestFactory

https://gerrit.wikimedia.org/r/453408