Page MenuHomePhabricator

Unable to update libraries in MediaWiki core and vendor due to version mismatch and circular dependencies
Closed, ResolvedPublic

Description

We are currently unable to update OOjs UI library in MediaWiki core without forcing V+2, because updating PHP and JS versions requires two commits to two repositories, and tests immediately fail upon version mismatch.

When one of the changes was force-merged, tests for the other one passed.

I resolved the immediate issue by force-merging, but this will be a problem again in a week or two when there's another OOUI update.

Event Timeline

matmarex raised the priority of this task from to Needs Triage.
matmarex updated the task description. (Show Details)

I guess composer is confused because there are two different versions being shipped?

Now that we have mediawiki/vendor shouldn't we remove OOjs and OOui from mediawiki/core ? People from Librarization might be interested.

Legoktm triaged this task as High priority.
Legoktm set Security to None.
gerritbot subscribed.

Change 187932 had a related patch set uploaded (by Legoktm):
update.php: Add option to not check if external dependencies are up to date

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

Patch-For-Review

I guess composer is confused because there are two different versions being shipped?

Now that we have mediawiki/vendor shouldn't we remove OOjs and OOui from mediawiki/core ? People from Librarization might be interested.

No, that's not how it works. mediawiki/vendor is a manually maintained copy of the dependencies that core and WMF-deployed extensions require upon. mediawiki/core's composer.json specifies which version of oojs-ui is needed by core, and then we update vendor since we don't run composer on the cluster, and instead deploy the vendor git repository.

Legoktm renamed this task from Unable to update OOjs UI library in MediaWiki core without forcing V+2, because updating PHP and JS versions requires two commits to two repositories, and tests immediately fail upon version mismatch to Unable to update libraries in MediaWiki core and vendor due to version mismatch and circular dependencies.Feb 3 2015, 9:02 PM

I propose we stop using mediawiki-vendor for the majority of MediaWiki core and extension repositories Jenkins jobs (master and release branches).

In the wmf-branches (of MediaWiki core and extensions) we'd use mediawiki-vendor instead of composer. (And the same for the master branch of mediawiki-vendor itself).

The commits that create or modify wmf branches will then run with mediawiki-vendor and thus ensure (before deployment!) that everything is in order. This means that (if someone forgets to make a commit to mediawiki-vendor) issues are caught a bit later. I think that's worth the reduced overall complexity and gained flexibility for non-wmf extensions. It also makes it a lot easier to write commits against mediawiki/core that update composer.json. It'll enable developers to test these commits in Jenkins – without having to orchestrate inter-dependent commits and merge them before testing.

This also in light of the recent issue with update.php verifying the composer dependencies. See T88211: Unable to update libraries in MediaWiki core and vendor due to version mismatch and circular dependencies.

I would like us to write down the test / dependency strategy we want to use. To me the idea is to enforce having mediawiki/vendor and core/extensions to be in sync. If we start ignoring dependencies that kind of defeat the dependencies check.

@hashar: I don't think coupling upstream with downstream is realistic. mediawiki/core would still be checked against vendor in commits to vendor itself and to wmf-branches of core and extensions. Which is where it matters. It's slightly later than when on master, but we shouldn't try to do everything at once. That leaves responsibility no where. In the rare occasion someone forgets to update vendor, it'll still be caught before deployment.

If we don't do this, we need at alternate solutions for the following:

  • Being able to modify composer.json in mediawiki/core and run tests with the updated software before it lands in mediawiki/vendor. Making these commits to mediawiki/vendor is wrong, and wouldn't allow for integration in MediaWiki or for dependent commits to use it.
  • Being able to specify devDependencies in composer.json and use that as entry point for (some) of our unit tests in Jenkins (e.g. phpunit, phpcs, ..).

Change 187932 merged by jenkins-bot:
update.php: Add option to not check if external dependencies are up to date

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

The --skip-external-dependencies won't fix the circular dependency. The circular dependency is a result from of integrating master of everything plus a build of everything at the same time and thus can not be solved as long as multiple repos will be pulled from independently. The patch also won't prevent breaking core when vendor is updated or the other way around. The only thing that will make the breakage minimally short is automatically building vendor when pre-merge testing core then pushing those changes directly after another. There is still a race there when pulling core and vendor independently. There will be no race for beta because you would only deploy after pushing to both core and vendor is done and nothing else tries to deploy concurrently. There is no race for things that use vendor as a submodule (wmf branches) because then you can update core and its vendor submodule at the same time. I suppose the best long term plan is to discourage use of vendor for anything except deployment with submodules or a build that contains everything as one and building it automatically. To safely be able to build vendor automatically we need to change when the code introduced is reviewed to when composer.json is changed in core instead of when the build is updated. Then either pull the code from gerrit.w.o or always specify commit hashes (and never use tar files, only git) to verify that one downloaded the expected code from the internet during build time. For using dev. dependencies one would share only the composer.lock from the vendor build and only use composer install as opposed to update to also pull in the rest. This would solve the points from T88211#1039072.

If we would specify compatible version ranges like >=1.2.0 <2.0.0 in core and were able to compare them during update.php then that would at least work when doing upgrades that don't break backwards compatibility. This would need something like T99086 as a base. This would solve everything if we always do a compatibility dance like this: When we want to upgrade Foo from 1.0.0 to 2.0.0 which breaks compatibility make core and all extensions compatible with both versions at the same time, then upgrade the vendor repository and after some time remove the compatibility with 1.0.0. We currently don't always do this for e.g. breaking changes in core. Do we want to restrict ourselves to only do these compatibility dances for incompatible changes in the future? Or do we accept breaking of master integration? This not only is a question for composer dependencies but also for dependencies from extensions on core or other extensions.

If we do not do this dance then updates with backwards incompatibility will always break during the time when one of core and vendor is updated but not yet the other. The CI is currently just correctly showing just that. Ignoring the current version string comparison in vendor (the inverse of what the patch proposes) one will be able to run the tests with the updated dependencies. Though the CI for vendor will omit to tell you that core will break because of the version check until it is also updated. I.e. a user pulling from core and vendor after only vendor was updated will have a broken system, also the CI for every other thing using vendor will correctly break. However as the CI for core will then fail until it is changed, I think this is the best thing we can have now.

The proposed patch will not work as is as changing core composer.json first will still test with the old vendor instead of the changed composer.json and updating vendor first will still break the vendor CI. Using --skip-external-dependencies in the vendor CI (the inverse of the proposed patch) however will allow you to test the change in vendor (which will break core and all extensions) and then require you to submit the change in core to unbreak it.

TL;DR:

I suggest for the short term to use --skip-external-dependencies in the vendor CI. This allows one to update vendor ahead of core and then to unbreak core by doing the change in core.

Change 187933 merged by jenkins-bot:
Run update.php with --skip-external-dependencies

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