Page MenuHomePhabricator

Cargo: Restore full-text search for MW 1.31 and lower
Closed, ResolvedPublic

Description

This change in core MediaWiki, in version 1.31, broke full-text search in Special:Drilldown in Cargo:

https://phabricator.wikimedia.org/rMW0824579606fe2f5920d2aeb5ab7d1a4bc4838007

I got full-text search working again for the latest MediaWiki version about a month ago, with this change:

https://phabricator.wikimedia.org/rECRGbbed4f2d590fccb560f16f31b3345e2fa5b370bb

Unfortunately, it broke full-text search for MediaWiki 1.31 and lower, which I didn't discover until recently.

I don't know of a way to have one "CargoSearchMySQL" class that will work with all versions of MediaWiki. Therefore, I think the easiest fix for this is to have two different files, both defining a "CargoSearchMySQL" class, with one file holding the old contents of CargoSearchMySQL.php and one holding the new contents. (Perhaps the files should be called CargoSearchMySQL.php and CargoSearchMySQL-old.php.) Instead of being loaded in extension.json and Cargo.php, they should be loaded in registerExtension() within Cargo.hooks.php, so that there can be an "if" statement call to load the right one, depending on the MW version. (please ignore this)

It's broken now for three reasons: "public" methods being overriden as "private" (which PHP doesn't allow), and because of two other changes in core MediaWiki, both in version 1.31:

https://phabricator.wikimedia.org/rMW90d4f56fe46140f9e97e2fa72698f98b57447fe5
https://phabricator.wikimedia.org/rMW39f0f919c5708f4c672a8eb7e0891f50bf16883e#change-gmLPkFngBcbf

For the first of these, CargoSearchMySQL should ideally support both the old $wgContLang and the new MediaWikiServices::getInstance()->getContentLanguage(), depending on what is available in the MediaWiki version being used.

For the second, it would similarly be good if the class could support both variants of suppressWarnings() and restoreWarnings().

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 4 2019, 4:57 AM
Smanmand added a comment.EditedMar 13 2019, 2:40 PM

@Yaron_Koren : I have fixed this issue following the instructions you gave. I tested this in MW 1.31 and MW 1.32 (See Screenshots). Do you think I should test this with other versions even (before I push the fix)?

@Smanmand - that's great! But you should definitely test it with a version of MediaWiki below 1.31. The current Cargo code already works with MW 1.31 and 1.32.

@Yaron_Koren : Just tested with MW 1.30.1.. Works like a charm!

I had to make all the functions in CargoSearchMySQL.php public for this purpose. @Yaron_Koren

Great! Please submit the patch.

I believe I dont have the rights to submit the patch
$ git push origin T217529
remote: Unauthorized
fatal: Authentication failed for 'https://gerrit.wikimedia.org/r/p/mediawiki/extensions/Cargo/'

From gerrit patch uploader:

git commit -a --author=ankita.mandal221@yahoo.com -F -
Fix for T217529

Change-Id: Ib1376bf0edd21df8b3ab3688d19f4886d8e23981

fatal: No existing author found with 'ankita.mandal221@yahoo.com'

See https://www.mediawiki.org/wiki/Gerrit/Tutorial , and note the "Create a Wikimedia developer account" part.

$ git push -f origin master
Enter passphrase for key '/c/Users/Ankita Mandal/.ssh/id_rsa':
Counting objects: 8, done.
Delta compression using up to 4 threads.
Compressing objects: 100% (8/8), done.
Writing objects: 100% (8/8), 1.08 KiB | 122.00 KiB/s, done.
Total 8 (delta 5), reused 0 (delta 0)
remote: Resolving deltas: 100% (5/5)
remote: Branch refs/heads/master:
remote: You are not allowed to perform this operation.
remote: To push into this reference you need 'Push' rights.
remote: User: smanmand
remote: Please read the documentation and contact an administrator
remote: if you feel the configuration is incorrect
remote: Processing changes: refs: 1, done
To ssh://gerrit.wikimedia.org:29418/mediawiki/extensions/Cargo
! [remote rejected] master -> master (prohibited by Gerrit: ref update access denied)
error: failed to push some refs to 'ssh://smanmand@gerrit.wikimedia.org:29418/mediawiki/extensions/Cargo'

I do have a developer account but still facing this issue.

See https://www.mediawiki.org/wiki/Gerrit/Tutorial.
You are not allowed to push any changes directly to the repository. You need to submit it for review first.

Change 496517 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Fixes for T217529

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

Yaron_Koren renamed this task from Cargo: Restore full-text search for MW 1.30 and lower to Cargo: Restore full-text search for MW 1.31 and lower.Mar 15 2019, 1:39 PM
Yaron_Koren updated the task description. (Show Details)
Yaron_Koren updated the task description. (Show Details)Mar 15 2019, 2:11 PM
Yaron_Koren removed Smanmand as the assignee of this task.Mar 20 2019, 11:35 AM
Yaron_Koren updated the task description. (Show Details)Mar 20 2019, 5:31 PM

Change 500012 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions. Fix for T217529.

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

This comment was removed by Smanmand.

Change 496517 abandoned by Yaron Koren:
Fixes for T217529

Reason:
Replaced by 500012.

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

Change 500018 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Added appropriate spacing and comments. Fix for T217529

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

Change 500018 abandoned by Yaron Koren:
Added appropriate spacing and comments. Fix for T217529

Reason:
This won't work - the changes all have to be checked in as one patch.

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

Change 502605 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions (with appropriate spacing and comments. Fix for T217529.

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

Change 502608 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions (with appropriate spacing and comments. Fix for T217529.

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

Change 502778 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions (with appropriate spacing and comments.) Fix for T217529.

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

Change 502608 abandoned by Yaron Koren:
Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions (with appropriate spacing and comments. Fix for T217529.

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

Change 502778 abandoned by Yaron Koren:
Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions (with appropriate spacing and comments.) Fix for T217529.

Reason:
One patch is required, not two.

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

Change 502796 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions. Added appropriate spacing and comments. Fix for T217529.

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

Change 502798 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions. Added appropriate spacing and comments. Fix for T217529.

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

Change 500012 abandoned by Yaron Koren:
Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions. Fix for T217529.

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

Change 502605 abandoned by Yaron Koren:
Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions (with appropriate spacing and comments. Fix for T217529.

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

Change 502796 abandoned by Yaron Koren:
Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions. Added appropriate spacing and comments. Fix for T217529.

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

Change 504893 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLsearch.php to supprt all MW versions. Added appropriate spacing and comments. Fix for T217529

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

Change 504900 had a related patch set uploaded (by Ankita Mandal; owner: Ankita Mandal):
[mediawiki/extensions/Cargo@master] Changes in CargoMySQLsearch.php to support all MW versions. Added appropriate spacing and comments. Fix for T217529

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

Change 502798 abandoned by Aklapper:
Changes in CargoMySQLSearch.php to make it work in all Mediawiki versions. Added appropriate spacing and comments. Fix for T217529.

Reason:
Assuming this has been superseded by https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Cargo/ /504900/

For future reference, see https://www.mediawiki.org/wiki/Gerrit/Tutorial#Amending_a_change_.28your_own_or_someone_else.27s.29 - thanks!

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

Change 504893 abandoned by Aklapper:
Changes in CargoMySQLsearch.php to supprt all MW versions. Added appropriate spacing and comments. Fix for T217529

Reason:
Abandoning per last comment and as Ankita Mandal did not reply.

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

Yaron_Koren closed this task as Resolved.May 13 2019, 3:06 PM