Page MenuHomePhabricator

MathHooks: table creation yields warnings on quibble
Open, MediumPublic

Description

...
Creating tables for enabled extensions
Warning: array_map(): Argument #2 should be an array or collection
Warning: in_array() expects parameter 2 to be an array or collection in /workspace/src/extensions/Math/src/MathHooks.php on line 322
Warning: array_map(): Argument #2 should be an array or collection
Warning: in_array() expects parameter 2 to be an array or collection in /workspace/src/extensions/Math/src/MathHooks.php on line 334

e.g. in https://integration.wikimedia.org/ci/job/quibble-vendor-mysql-hhvm-docker/11446/consoleText

MathRenderer::getValidModes() uses global $wgMathValidModes - maybe not correctly configured for quibble runs? (either the default configuration is not processed by the install script

INFO:mw.maintenance.install:php maintenance/install.php --scriptpath= --dbtype=mysql --dbname=wikidb --dbuser=wikiuser --dbpass=secret --dbserver=/tmp/quibble-mysql-pv9drd5r/socket --with-extensions --pass=testpass TestWiki WikiAdmin

or it is overwritten anyhow.

Discovered during investigating T202223

TODOs:

  • Test if math extension can be installed with the maintenance script. (Maybe @Paladox has experience with that?)

Details

Related Gerrit Patches:
mediawiki/extensions/Math : masterphpunit: relish MediaWikiTestCase db setup

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 20 2018, 10:17 AM
Pablo-WMDE updated the task description. (Show Details)Aug 20 2018, 1:57 PM
Pablo-WMDE updated the task description. (Show Details)Aug 20 2018, 5:28 PM

Change 454064 had a related patch set uploaded (by Pablo Grass (WMDE); owner: Pablo Grass (WMDE)):
[mediawiki/extensions/Math@master] phpunit: relish MediaWikiTestCase db setup

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

I missed the creation of this ticket, my comment from the other once applies:

installdbs in the mediawiki-docker-dev setup does install.php and then update.php
quibble does install.php with the --with-extensions parameter.
I remember there being some issues with some extensions and --with-extensions for install.php before, so I imagine this is something similar.

Addshore added a subscriber: Legoktm.EditedAug 21 2018, 8:14 AM

From another ticket:

The installer explicitly does not load the full extension during the install process. This means the extension has been broken with the web installer for quite a while now :( (also this is not ideal, but it's the rather fragile status quo, that hopefully we can move away from soon)

Other extensions that got hit by the same thing are:

Quoting @hashar on IRC:

the workaround is to have the extension to set sane defaults internally

In Math the problem code is the optional creation of DB tables based on this configuration:

		if ( in_array( 'latexml', MathRenderer::getValidModes() ) ) {
			if ( in_array( $type, [ 'mysql', 'sqlite', 'postgres' ] ) ) {
				$sql = __DIR__ . '/../db/mathlatexml.' . $type . '.sql';
				$updater->addExtensionTable( 'mathlatexml', $sql );
				if ( $type == 'mysql' ) {
					$sql = __DIR__ . '/../db/patches/mathlatexml.mathml-length-adjustment.mysql.sql';
					$updater->modifyExtensionField( 'mathlatexml', 'math_mathml', $sql );
				}
			} else {
				throw new Exception( "Math extension does not currently support $type database for LaTeXML." );
			}
		}
		if ( in_array( 'mathml', MathRenderer::getValidModes() ) ) {
			if ( in_array( $type, [ 'mysql', 'sqlite', 'postgres' ] ) ) {
				$sql = __DIR__ . '/../db/mathoid.' . $type . '.sql';
				$updater->addExtensionTable( 'mathoid', $sql );
				if ( $type == 'mysql' ) {
					$sql = __DIR__ . '/../db/patches/mathoid.add_png.mysql.sql';
					$updater->addExtensionField( 'mathoid', 'math_png', $sql );
				}
			} else {
				throw new Exception( "Math extension does not currently support $type database for Mathoid." );
			}
		}

Perhaps the extension should instead just always create all tables.

Addshore moved this task from Unsorted 💣 to Watching 👀 on the User-Addshore board.

Hi @Physikerwelt,
I'd be curious to see if T202223 is resolved if this is overcome. There has to be a hook combination to make sure the code setting the valid modes (in the global if need be) is invoked before trying the db setup.

@Pablo-WMDE no it's not. I'm a bit frustrated that I blocks me from doing anything. I don't understand how a change in Wikibase can affect the Math extension configuration. I suspected that the problem is the use of globals, but it did not help as well. Is there a way one can debug or reproduce the new unit testing environment?

@Pablo-WMDE no it's not. I'm a bit frustrated that I blocks me from doing anything. I don't understand how a change in Wikibase can affect the Math extension configuration. I suspected that the problem is the use of globals, but it did not help as well. Is there a way one can debug or reproduce the new unit testing environment?

I don't think this really has anything to do with Wikibase.
I believe this is because the CI used to not run quibble and now it does.

June 27 https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Math/+/441508/ ran the mwext-testextension-hhvm-jessie job and passed
7th July it was migrated to quibble https://github.com/wikimedia/integration-config/commit/3275fdb5b29f90bbf9d8dbeea5acdfd2d2bedb12#diff-48f8c0f4652ffe6e8964cb497ef6cc88
I'll tag T183512 and @hashar

Maybe this one slipped through broken?

Hi @Physikerwelt,

how a change in Wikibase can affect the Math extension configuration

As I understand it this is not the case in this situation. We both build extensions that are integrated by the CI system using their patch/latest version. The Math extension configuration seems to be partially incompatible with the CI system (now that it changed). Unfortunately it is not breaking right where this happens (this could improve) but keeps on going a bit further. The place where this finally is surfaced is in a Wikibase(Lexeme) test, but the root cause is in the configuration. Let's see if the tests are green when the warnings from the setup phase are gone.

Physikerwelt triaged this task as High priority.Sep 3 2018, 5:14 PM
Physikerwelt lowered the priority of this task from High to Medium.Sep 6 2018, 7:11 PM
Physikerwelt updated the task description. (Show Details)
Physikerwelt added subscribers: Paladox, tstarling.

Change 454064 abandoned by Pablo Grass (WMDE):
phpunit: relish MediaWikiTestCase db setup

Reason:
Mirrored (and then discontinued) in I2fb89ebeded45f97fa91aaf773ac55ee77314e86

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

Pkra moved this task from Incoming to MediaWiki interaction on the Math board.Dec 10 2018, 8:57 PM