Change 628472 had a related patch set uploaded (by Ladsgroup; owner: Ladsgroup):
[mediawiki/extensions/Wikibase@master] Update guzzle to ^7.0.0

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

Change 628472 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update guzzle to ^7.0.0

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

Is there a pahb ticket for updating the core dep?
There should probably be a process to align and updater this in all places CI depends on at once?

In T263381#6478588, @Addshore wrote:

Is there a pahb ticket for updating the core dep?

No, just our normal upgrade work.

There should probably be a process to align and updater this in all places CI depends on at once?

My normal process is to use CodeSearch to find clashes, but I overlooked the Wikibase ones when I did it.

I think these all should be removed, they are duplicating the values from core.

We are seeing this for REL1_35 after a run there.

In T263381#6486444, @Pablo-WMDE wrote:

We are seeing this for REL1_35 after a run there.

this should be fixed

Hi,

@Ladsgroup wrote

I think these all should be removed,

This sounds like you thought about this already and have a specific group of deps in mind. Is that "all non-dev requires which are also in core"?

I interpret this dependency (maybe wrongfully; I did not check) to say we are using these deps directly as well. Or why else would those deps ever have been added?
If that's the case then I think things are breaking in the best spot we can get (not the best envisionable) - upon CI when mediawiki has made a move (and mediawiki-vendor is following suite) and wikibase needs to adapt. Simply not tracking our deps because "core already does that" sounds brittle as we'd only find out in practice that we expect a different version.

Thanks

Little bit of digging with the guzzle example:

We seem to almost only be using it in tests e.g. ApiPropertyDataTypeLookupTest where we probably would be better off using Psr\Http\Message\ResponseInterface instead of GuzzleHttp\Psr7\Response directly anyways.
There is one prod use (GuzzleHttp\Psr7\stream_for) which we may (or may not) be able to solve differently - but: at the moment we depend on guzzle directly.

Change 630201 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] HttpResponseMockerTrait: add trait to create mock HTTP ResponseInterface

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

In T263381#6494227, @Pablo-WMDE wrote:

Hi,

@Ladsgroup wrote

I think these all should be removed,

This sounds like you thought about this already and have a specific group of deps in mind. Is that "all non-dev requires which are also in core"?

I interpret this dependency (maybe wrongfully; I did not check) to say we are using these deps directly as well. Or why else would those deps ever have be added?
If that's the case then I think things are breaking in the best spot we can get (not the best envisionable) - upon CI when mediawiki has made a move (and mediawiki-vendor is following suite) and wikibase needs to adapt. Simply not tracking our deps because "core already does that" sounds brittle as we'd only find out in practice that we expect a different version.

This is correct but slightly different in practice, let me give you the example of Guzzle, in paper, we are declaring the dependency in composer because one test uses an interface, so assuming semver, if core requires higher than a certain major version (for security for example), we break because we pinned it to a smaller major version and versions are incompatible but in reality this interface is way more stable than all of guzzle and it won't change with one major version bump (or twenty if you ask me) thus I don't except us breaking because of major version upgrade (in which in this case, I just bumped the version and didn't require anything else).

So in short I agree it's brittle but as long as we don't do it for code we heavily use (and the usage is properly tested), declaring it through core should be fine. I don't have strong preference about it though.

as long as [...] the usage is properly tested[...], declaring it through core should be fine

I created https://gerrit.wikimedia.org/r/630201/ because I believe we should only be removing the dependency on a lib from package.json once we don't actually use it anymore. Leszek may want to have a word about taking fakes too far but as Psr\Http\Message\ResponseInterface governs both us and guzzle I don't think this is taking things too far.

There are two more bindings (e.g. stream_for()) which I did not look into yet but I think maybe we can lose the package.json entry (or trading it for one to psr/http-message, rather) without compromising.

In T263381#6494095, @WMDE-leszek wrote:

In T263381#6486444, @Pablo-WMDE wrote:

We are seeing this for REL1_35 after a run there.

this should be fixed

@Addshore was kind enough to point out that this is in fact the result of our (redundant) travis CI integrating the wikibase REL_ branches with MediaWiki master instead of with the respective MediaWiki branch (related).

I'll look into this as a short term solution.

In parallel, a requirement gathering will be started to remind us of the reasons which led to the introduction of the redundant (to jenkins) and retroactive CI and possible ways to satisfy those without having to maintain configurations in two places.

Change 630201 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] HttpResponseMockerTrait: add trait to create mock HTTP Response

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

In T263381#6497762, @Pablo-WMDE wrote:

as long as [...] the usage is properly tested[...], declaring it through core should be fine

I created https://gerrit.wikimedia.org/r/630201/ because I believe we should only be removing the dependency on a lib from package.json once we don't actually use it anymore. Leszek may want to have a word about taking fakes too far but as Psr\Http\Message\ResponseInterface governs both us and guzzle I don't think this is taking things too far.

I think it is pretty alright, and the change is the right way around of doing it

With T263994: Run wikibase travis CI against respective MediaWiki branch done REL1_35 is green again (and master prepared for future branch cuts).

During the cause of this ticket we also

• created T263989: Upstream MwHttpRequestToResponseInterfaceAdapter into core to remove the direct binding to guzzle and address the discussion here ff.
• created T264168: Drop PHP 7.2 support from Wikibase master branch, once Wikimedia production is on 7.4 as the reminder for dangling master patches which can not be merged yet due to production limitations as documented in T261872: Drop PHP 7.2 & 7.3 support from MediaWiki master branch, once Wikimedia production is on 7.4

I think this ticket here is done.

Change 633506 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Wikibase@master] remove last guzzle binding

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

Change 633506 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] remove last guzzle binding

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