Page MenuHomePhabricator

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

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.

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?

/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

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:

  1. Document the "composer install" step
  2. 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.

TJones closed this task as Declined.Jan 17 2019, 2:27 PM

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!

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

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