Page MenuHomePhabricator

MediaWiki installer does not check composer dependencies are at the correct versions (like update.php does)
Open, LowPublic

Description

update.php will check that composer dependencies are at the correct versions (using CheckComposerLockUpToDate).

The Installer apparently doesn't, as we noticed during @Addshore's live hackathon demo.

update.php:

		// Check external dependencies are up to date
		if ( !$this->hasOption( 'skip-external-dependencies' ) ) {
			$composerLockUpToDate = $this->runChild( CheckComposerLockUpToDate::class );
			$composerLockUpToDate->execute();
		} else {
			$this->output(
				"Skipping checking whether external dependencies are up to date, proceed at your own risk\n"
			);
		}

Event Timeline

Reedy updated the task description. (Show Details)
Reedy updated the task description. (Show Details)
Reedy moved this task from Blocker to Not a blocker on the MW-1.36-release board.
Reedy moved this task from Blocker to Not a blocker on the MW-1.35-release board.

I think a lot of the code in checkComposerLockUpToDate.php wants refactoring out and putting in somewhere in includes/composer/

RazeSoldier subscribed.

I am very interested in this task.

Change 737198 had a related patch set uploaded (by 星耀晨曦; author: 星耀晨曦):

[mediawiki/core@master] Refactor checkComposerLockUpToDate.php logic about check composer dependencies

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

Going back to the task description.. Was it install.php or the Web Installer?

Going back to the task description.. Was it install.php or the Web Installer?

I can confirm that this is an issue with install.php (or it was on step 23rd)
I had to modify mwcli to check the composer dependencies by itself before running install.php
https://gitlab.wikimedia.org/releng/cli/-/commit/33e1d1dc0bf7fa39bfae2740b8ecc203fb75a957

Thanks.

The first patch is just prepatory refactoring, but we should definitely test/confirm this behaves nicely on install.php and the Web Installer

Change 737198 merged by jenkins-bot:

[mediawiki/core@master] Refactor checkComposerLockUpToDate.php logic out to reuseable class

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

Change 991782 had a related patch set uploaded (by 星耀晨曦; author: 星耀晨曦):

[mediawiki/core@master] Installer: Add check of the Composer lock file to env check

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

Change 993160 had a related patch set uploaded (by 星耀晨曦; author: 星耀晨曦):

[mediawiki/core@master] Installer: Add check of the Composer lock file to env check

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

Change 991782 abandoned by 星耀晨曦:

[mediawiki/core@master] Installer: Add check of the Composer lock file to env check

Reason:

https://gerrit.wikimedia.org/r/c/mediawiki/core/+/993160

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

Change #993160 merged by jenkins-bot:

[mediawiki/core@master] Installer: Add env check for composer.lock file

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

Change #1042315 had a related patch set uploaded (by Catrope; author: Catrope):

[mediawiki/core@master] Reapply "Installer: Add env check for composer.lock file"

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

Krinkle updated Other Assignee, added: Krinkle.EditedMon, Jun 17, 2:43 PM
Krinkle subscribed.

The installer has historically been deprioritised by WMF due to being associated with "for third-party support". In recent history we've reframed the Installer as (also) an essential part of Quickstart, dev environment, contributing (e.g. Hackathon events., and onboarding (incl for WMF staff). While this doesn't mean that the installer now has an owner, it does mean it's now more normalized for individual teams to improve/fix things as they see fit when it also benefits these other use cases.

Assigning to self for re-review, since I helped with this change before, and it got reverted due to an issue that I missed.

(Moving per Roan's comment on Gerrit. Awaiting updated patch before I can re-review/test.)