Page MenuHomePhabricator

How to set up CI to have "PHP Parsoid" enabled?
Open, Needs TriagePublic

Description

In the BlueSpiceVisualEditorConnector extension, there is code that makes use of MWParsoid\ParsoidServices class. CI tests break [1][2] with Error: Class 'MWParsoid\ParsoidServices' not found.

Full stack:

07:14:00 1) RestStructureTest::testPathParameters with data set "Handler POST /bsvec/transform/{from}/to/{to}" (array('/bsvec/transform/{from}/to/{to}', 'BlueSpice\VisualEditorConnect...actory', 'POST'))
07:14:00 Error: Class 'MWParsoid\ParsoidServices' not found
07:14:00 
07:14:00 /workspace/src/extensions/BlueSpiceVisualEditorConnector/src/Rest/Handler/Transform.php:29
07:14:00 /workspace/src/vendor/wikimedia/object-factory/src/ObjectFactory.php:172
07:14:00 /workspace/src/vendor/wikimedia/object-factory/src/ObjectFactory.php:102
07:14:00 /workspace/src/includes/Rest/Router.php:336
07:14:00 /workspace/src/vendor/wikimedia/testing-access-wrapper/src/TestingAccessWrapper.php:71
07:14:00 /workspace/src/tests/phpunit/structure/RestStructureTest.php:47
07:14:00 /workspace/src/tests/phpunit/MediaWikiIntegrationTestCase.php:446
07:14:00 /workspace/src/maintenance/doMaintenance.php:107

Is there anything I can set in the CI config to avoid this error?

[1] https://gerrit.wikimedia.org/r/c/mediawiki/extensions/BlueSpiceVisualEditorConnector/+/639968
[2] https://integration.wikimedia.org/ci/job/quibble-composer-mysql-php73-noselenium-docker/3509/console

Event Timeline

I learnt about the existence of zuul/parameter_functions.py yesterday, and my understanding is that this is what you'd want to edit (I have https://gerrit.wikimedia.org/r/c/integration/config/+/730207 open to add VE support to Translate ;) )

Yup, but VE doesn't depend on parsoid, so it wouldn't help, I think. adding "parsoid" as a dependency might help.

Sooo my understanding may have been partial/incorrect (but I don't have more insight right now, except that this may be a bit much, because that would clone parsoid and run all of its tests there). To be continued... (sorry if I added to the confusion :( )

Change 730548 had a related patch set uploaded (by Jforrester; author: Robert Vogel):

[integration/config@master] Zuul: [mediawiki/extensions/BlueSpiceVisualEditorConnector] Add `parsoid` dep.

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

  1. You shouldn't directly reference anything in the MWParsoid namespace, those are all subject to change
  2. If you need a parsoid object, you should get it via MediaWikiServices; see how includes/Rest/Handler/ParsoidHTMLHelper does it in core. This may also be subject to change, but less so.
  3. You will probably still need to handle the case where parsoid is not installed, at least for another release or so

It may be that adding 'parsoid' as a dependency in zuul is necessary to get Parsoid to appear in MediaWikiServices, but I'd like to take a closer look at the code in BlueSpiceVisualEditorConnector which is requiring this, first. It seems like the parsoid-dependent code is actually in VisualEditor, not BlueSpiceVEConn, and thus the dependency perhaps should be from VE to parsoid. But the gerrit link in the task description doesn't seem to resolve to anything parsoid- or VE-related.

Thanks @cscott !
I will check if we can use the Parsoid-MediaWikiService in BSVEC.

Thanks @cscott for the explanation and the series of patch. I have deployed the patch to remove parsoid from Disambiguator dependencies https://gerrit.wikimedia.org/r/c/integration/config/+/730898 . I then rebased the Disambiguator patch that intentionally fails with parsoid/html https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Disambiguator/+/655101 but unfortunately it passed. That seems to indicate that parsoid is not properly loaded when it is not listed as a dependency?

Our test runner Quibble injects:

// Hack to support testing Parsoid as an extension, while overriding
// the composer library included with core. (T227352)
$parsoidDir = MW_INSTALL_PATH . '/services/parsoid';
if ( is_dir( $parsoidDir ) ) {
	AutoLoader::$psr4Namespaces += [
		// Keep this in sync with the "autoload" clause in
		// $PARSOID_INSTALL_DIR/composer.json
		'Wikimedia\\Parsoid\\' => "$parsoidDir/src",
	];
	wfLoadExtension( 'Parsoid', "$parsoidDir/extension.json" );
}
unset( $parsoidDir );

That Disambiguator build does have wikimedia/parsoid in composer, but does not clone mediawiki/services/parsoid hence it is not available as I understand it.

Quibble installs MediaWiki with maintenance/install.php --with-extensions which detects extensions and skins but does not detect $IP/services/parsoid hence why it is never loaded. Given parsoid is shipped as a composer dependency maybe the installer should load it from vendor/wikimedia/parsoid (unless the repository has been cloned). It seems we should run tests using the composer dependency rather than the parsoid repo, else we would test against the tip of the master branch rather than whatever version is pinned in core via composer.json.

Isabelle pointed me to https://www.mediawiki.org/wiki/Parsoid#Installation . One has to explicitly:

wfLoadExtension( 'Parsoid', 'vendor/wikimedia/parsoid/extension.json' );

Which maybe the installer should do for us nowadays?

If we add mediawiki/services/parsoid as a dependency, that triggers the LocalSettings.php code injected by Quibble which enables parsoid, albeit with the version from the git repository rather than the one in composer.

@hashar and I had a convo about what adding parsoid as a dependency does; I think he is going to put that on-wiki and then we can reference that here.

@Osnard My outstanding question is still: what exactly is BlueSpiceVisualEditorConnector doing with Parsoid? The gerrit patch you referenced (https://gerrit.wikimedia.org/r/c/mediawiki/extensions/BlueSpiceVisualEditorConnector/+/639968 ) doesn't seem to have anything to do with parsoid. I'd like to nail this down so that I understand whether we should be adding a parsoid dependency to BSVEC, to VisualEditor, or else whether we should be patching BSVEC to use a more-stable interface.

BlueSpiceVisualEditorConnector provides an API interface (REST since REL1_35, Action API before that), that allows to transform "html2wikitext" and "wikitext2html". In the older versions - before PHP Parsoid - we used this in private wikis environments (no RESTBase available) in order to allow switching between visual and source view mode in VE. We also use it with standalone versions of VE which are embedded into forms, in order to allow the form sending wikitext instead of VE html. Also some other extensions from the BlueSpice distribution may use it in differentt ways.

I am not sure if we really need all this with the Parsoid PHP. That's why I said I will need to check. Maybe there are new ways to archive this, which I am not aware of.

The referenced patch is not related to that functionality. Yet the CI tests break on this. That's why I filed this task.