Revert changes to TextCat that add dependency on autoload.php
Closed, DeclinedPublic
Actions

Description

Changes made last year under the title "Move TextCat class into src/" also changed the require statements in felis.php, catus.php, and lm2php.php from requiring TextCat,php to requiring /vendor/autoload.php, adding a dependency (on composer?) to TextCat that prevents it from working out of the box when downloaded from GitHub.

felis.php, catus.php, and lm2php.php are intended to be stand-alone command line tools to build new models, do language identification, and convert original Perl text_cat models to the new PHP format, respectively. CirrusSearch only uses TextCat.php and we shouldn't modify the others.

Another change, "Remove manual require'ing of the TextCat class," makes it so that phpunit will not run when TextCat is cloned as a stand-alone repo.

Revert these changes so that TextCat can still function as a stand-alone command-line utility.

Details

Related Gerrit Patches:

wikimedia/textcat : master

Remove Dependency on Composer from TextCat

Related Objects
Search...

Status	Assigned	Task
Open	None	T118278 EPIC: Improve Language Identification for use in Cirrus Search
Open	TJones	T138958 Detect "wrong keyboard" queries for Russian/American keyboards on EN/RU Wikipedias
Resolved	TJones	T213931 Update TextCat with wrong-keyboard models
Declined	TJones	T213935 Revert changes to TextCat that add dependency on autoload.php

Event Timeline

TJones triaged this task as High priority.Jan 16 2019, 3:36 PM

TJones created this task.

/vendor/autoload.php is provided by composer, not MediaWiki, isn't it?

TJones mentioned this in T213936: Deploy new version of TextCat.Jan 16 2019, 3:39 PM

TJones moved this task from In Progress to not in use - please delete on the Discovery-Search (Current work) board.

In T213935#4884739, @Jdforrester-WMF wrote:

/vendor/autoload.php is provided by composer, not MediaWiki, isn't it?

Perhaps I've misinterpreted the intent of the changes made (see catus.php on Gerrit). TextCat has not been updated in CirrusSearch since the changes have been made, so we are still running the previous version in production. I may have hallucinated a "../" in the require path. Ugh. Composer and php are not my strong suites by any stretch of the imagination. As it stands, I can't run the latest version of TextCat on the command line on my laptop (which doesn't have composer installed).

The change was made without any explanation in the patch or changes to documentation, and now when you download or clone from GitHub it doesn't work out of the box if you follow the documentation.

I'll update the task description. Thanks.

TJones renamed this task from Revert changes to TextCat that make MediaWiki a dependency to Revert changes to TextCat that add dependency on autoload.php.Jan 16 2019, 3:56 PM

TJones updated the task description. (Show Details)

Change 484727 had a related patch set uploaded (by Tjones; owner: Tjones):
[wikimedia/textcat@master] Remove Dependency on Composer from TextCat

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

gerritbot added a project: Patch-For-Review.Jan 16 2019, 6:28 PM

TJones moved this task from not in use - please delete to Needs review on the Discovery-Search (Current work) board.Jan 16 2019, 7:43 PM

Right now, from what I understand, textcat is a composer package, which means once checked out, it needs "composer install" to deliver all the dependencies (which are actually none in non-dev mode, but that's beside the point as all composer packages need that). This is pretty standard things for composer-based modules AFAIK and nothing unusual. However, for people that don't want to use composer ecosystem it may be non-obvious that this is what is happening. So we can:

Document the "composer install" step
Revert the composer requirement

I have slight preference for the first alternative, as composer is rather common in modern PHP world, but if it makes trouble, the second alternative is available. In any case, there's no reason to need any of the mediawiki, only composer, so if it doesn't work with composer alone it's a problem and needs to be fixed.

Thanks for the info and explanation, @Smalyshev. I should have waited for your reply.

I think it is still too high a barrier for someone who is not a PHP dev who wants to use TextCat as a standalone tool to have to install composer just to be able to run it, even though it has no dependencies. OTOH, there are other ports out there, and no one has complained yet, so I'll close this ticket.

Thanks!

TJones removed projects: Patch-For-Review, Discovery-Search (Current work).Jan 17 2019, 2:28 PM

Change 484727 abandoned by Tjones:
Remove Dependency on Composer from TextCat