mwscript can take:

• a filename with extension, for a file in the core maintenance directory
• a filename with extension and some relative path prefix, that will be interpreted relative to the MediaWiki installation root
• an absolute path to a file (with extension)

run.php can take:

• a filename without extension, for a file in the core maintenance directory
• an absolute path to a file (with extension)
• a fully qualified class name
• <extension>:<class> where the fully qualified classname is expected to be MediaWiki\Extension\<extension>\Maintenance\<class> (and <class> itself might contain a backslash)
• one of the previous two options, but with dots instead of backslashes

It would be nice to expand run.php to include mwscript syntax:

• allow things like maintenance/run version.php which is currently interpreted as a specifier for the version\php maintenance class)
• allow specifying files relative to the MediaWiki root.

MWScript could convert its argument to an absolute path and pass that to run.php but it would make things unnecessarily confusing for people who need to interact with both.

Basically, run.php would need these additional rules:

• something that ends in .php is always a file and not a classname in dot-notation
• file references which do not start with a / are interpreted relative to the root directory of the MediaWiki installation run.php is in, except if they don't contain a / at all (just a filename), in which case they will be interpreted relateive to the core maintenance directory

That seems safe and unequivocal, given that we force camelcase for classnames.

@Tgr this sounds mostly good to me, and should be easy to implement.

There is one snag:

In T326800#8527122, @Tgr wrote:

• file references which do not start with a / are interpreted relative to the root directory of the MediaWiki installation run.php is in, except if they don't contain a / at all (just a filename), in which case they will be interpreted relateive to the core maintenance directory

run.php interprets paths relative to the current working directory, not relative to the installation directory. That is intentional - the idea is that you replace php with maintenance/run when executing a script. I would like to keep it this way.

I see two things we could do:

• make run.php fall back to looking in the instllation directory (or the maintenance directory?) of the file is not found in the cwd. [I'd rather like to avoid this]
• if there is no / in the name, always look in the maintenance dir, even if the ".php" suffice was present (so ./ would be required for running scripts from the current directory). [Currently, if the .php ending is present, then the file is expected to be in the local directory.]
• add a --dir option to the runner, which makes it cd to a given directory before looking for the script file. --mwdir could be used for the mw installation directory. [But it may be simpler to have mwscript call chdir()]

Change 880462 had a related patch set uploaded (by Daniel Kinzler; author: Daniel Kinzler):

[mediawiki/core@master] Maintenance: find file by name in maintenance dir

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

@Ladsgroup do you think it would be ok to have MWScript chdir() to the mediawiki base directory, so any relative path will be executed relative to the install base?

In T326800#8527186, @daniel wrote:

if there is no / in the name, always look in the maintenance dir, even if the ".php" suffice was present (so ./ would be required for running scripts from the current directory).

I would do it like this:

• run foo.php -> $IP/maintenance/foo.php
• run bar/foo.php -> $IP/bar/foo.php
• run ./foo.php -> $(pwd)/foo.php
• run ./bar/foo.php -> $(pwd)/bar/foo.php

IMO that's less confusing than the current behavior, where between run cleanupWatchlist, run CleanupWatchlist and run cleanupWatchlist.php, the first two run $IP/maintenance/cleanupWatchlist.php and the third looks for cleanupWatchlist.php in the working directory (which tends to be either $IP or the user's home directory).

In T326800#8528735, @daniel wrote:

@Ladsgroup do you think it would be ok to have MWScript chdir() to the mediawiki base directory, so any relative path will be executed relative to the install base?

It might break many scripts or tools. Not sure about it to be honest.

MWScript can set an absolute path, I don't think that part is problematic. (Basically the new logic would be: if $argv[1] ends in .php, run it through MWMultiVersion::getMediaWikiCli() and replace it with that. Then, require_once MWMultiVersion::getMediaWikiCli( 'run.php' ).) It's more about making mwscript and run accept identical input so people don't get confused all the time.

In T326800#8529117, @Tgr wrote:

In T326800#8527186, @daniel wrote:

if there is no / in the name, always look in the maintenance dir, even if the ".php" suffice was present (so ./ would be required for running scripts from the current directory).

I would do it like this:

• run foo.php -> $IP/maintenance/foo.php
• run bar/foo.php -> $IP/bar/foo.php
• run ./foo.php -> $(pwd)/foo.php
• run ./bar/foo.php -> $(pwd)/bar/foo.php

I would prefer either run bar/foo.php -> $(pwd)/bar/foo.php
or run bar/foo.php -> $IP/maintenance/bar/foo.php

IMO that's less confusing than the current behavior, where between run cleanupWatchlist, run CleanupWatchlist and run cleanupWatchlist.php, the first two run $IP/maintenance/cleanupWatchlist.php and the third looks for cleanupWatchlist.php in the working directory (which tends to be either $IP or the user's home directory).

I agree that run cleanupWatchlist.php should behave the same as run cleanupWatchlist.

IMO there should be similar behavior for running core maintenance scripts and extension maintenance scripts, so if you can use core scripts relative to $IP you should be able to do the same with extensions. Otherwise it's both unintuitive and unnecessarily inconvenient if a system is set so that you run.php is in your path (not sure if that would be a common use case, but that's how mwscript is set up on production / beta / Vagrant, and I think it's a highly appreciated feature, even on Vagrant where there is no multiversion setup to deal with))

There is nothing useful in the MediaWiki root directory, so it makes sense to map filenames without a path component to the core maintenance dir where the most commonly used scripts are. The subdirectories there aren't really useful though (although I suppose that can change over time): there are four of them with maintenance scripts, benchmarks (not relevant to the average user), mediawiki.Title (used by ResourceLoader internally, again not relevant), language and storage (they do have some general-purpose scripts but they are all very involved so I doubt they have a wide audience).

On the other hand, prefixing maintenance/ for bare filenames but not otherwise is unintuitive, even if we are very used to it by now in Wikimedia production. Maybe there should be something separate for extensions, like ExtensionName:bar/foo.php being converted to extensions/ExtensionName/maintenance/bar/foo.php? That also aligns with the similar classname rule well. (Sucks if you have an identically named extension and skin, but maybe since the sunsetting of the Vector extension that's not possible anymore.)

I wonder, is there a long-term vision for the maintenance system to use some kind of modular setup (where you can say things like run storage compressOld or run CirrusSearch dumpIndex, with help, command listing, autocompletion etc. delegated hierarchically)? And if there is, would it be based on a directory hierarchy? That seems to me like the sensible end-game for a CLI component, so maybe worth considering what's the run.php syntax that will be easy to adapt to that.

In T326800#8533234, @Tgr wrote:

I wonder, is there a long-term vision for the maintenance system to use some kind of modular setup (where you can say things like run storage compressOld or run CirrusSearch dumpIndex, with help, command listing, autocompletion etc. delegated hierarchically)? And if there is, would it be based on a directory hierarchy? That seems to me like the sensible end-game for a CLI component, so maybe worth considering what's the run.php syntax that will be easy to adapt to that.

Hm, but then, how does run know where the args for the script start? Consider
run storage trackBlobs cluster1 cluster2

That's ambiguous. We'd have to do something like
run storage trackBlobs -- cluster1 cluster2

npm does it that way, but I can't say that I like it. I prefer run storage/trackBlobs.php cluster1 cluster2. Maybe, in the future, run storage/trackBlobs cluster1 cluster2. Closer to how things usually work on the shell.

On the other hand, prefixing maintenance/ for bare filenames but not otherwise is unintuitive, even if we are very used to it by now in Wikimedia production. Maybe there should be something separate for extensions, like ExtensionName:bar/foo.php being converted to extensions/ExtensionName/maintenance/bar/foo.php?

I think we can use the ./ prefix for paths relative tp cwd (and / for absolute), and otherwise make everything relative to maintenance. I like ExtensionName:bar/foo.php. We already have this for class names, so why not also apply the conventioned for files?

I covered a bunch more cases now, have a look at the test cases at https://gerrit.wikimedia.org/r/c/mediawiki/core/+/880462/4/tests/phpunit/unit/maintenance/MaintenanceRunnerTest.php

In T326800#8533792, @daniel wrote:

I think we can use the ./ prefix for paths relative tp cwd (and / for absolute), and otherwise make everything relative to maintenance.

Should ../ also be relative to the current working directory, similar to https://www.php.net/manual/en/function.include.php?

Also, do I understand correctly that MWScript would have to do something similar to what @Tgr said?

In T326800#8529837, @Tgr wrote:

MWScript can set an absolute path, I don't think that part is problematic. (Basically the new logic would be: if $argv[1] ends in .php, run it through MWMultiVersion::getMediaWikiCli() and replace it with that. Then, require_once MWMultiVersion::getMediaWikiCli( 'run.php' ).) It's more about making mwscript and run accept identical input so people don't get confused all the time.

Should ../ also be relative to the current working directory, similar to https://www.php.net/manual/en/function.include.php?

That is a good question - it seems intuitive that if ./ is relative to CWD, ../ should be as well.

Currently, ../ would effectively designated the installation dir. Which may be useful, but seems confusing. I'll fix it.

Change 880462 merged by jenkins-bot:

[mediawiki/core@master] Maintenance: find file by name in maintenance dir

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

Change 889259 had a related patch set uploaded (by Ladsgroup; author: Amir Sarabadani):

[operations/mediawiki-config@master] [WIP] mwscript: Switch to use run.php

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

The above patch works except when the script is using the legacy CommandLineInc

ladsgroup@mwdebug1002:~$ mwscript eval.php --wiki=aawiki
Notice: Constant MEDIAWIKI already defined in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/doMaintenance.php on line 54
Notice: Constant MW_DB already defined in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php on line 484
Notice: Constant MW_PREFIX already defined in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php on line 485
Notice: Constant MW_WIKI_NAME already defined in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php on line 486

*******************************************************************************
NOTE: Do not run maintenance scripts directly, use maintenance/run.php instead!
      Running scripts directly has been deprecated in MediaWiki 1.40.
      It may not work for some (or any) scripts in the future.
*******************************************************************************

Deprecated: Premature access to service container [Called from MediaWiki\Maintenance\MaintenanceRunner::run in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php at line 556] in /srv/mediawiki/php-1.40.0-wmf.22/includes/debug/MWDebug.php on line 381
Deprecated: Premature access to service 'HookContainer' [Called from MediaWiki\MediaWikiServices::getInstance in /srv/mediawiki/php-1.40.0-wmf.22/includes/MediaWikiServices.php at line 294] in /srv/mediawiki/php-1.40.0-wmf.22/includes/debug/MWDebug.php on line 381
Deprecated: Premature access to service 'ObjectFactory' [Called from Wikimedia\Services\ServiceContainer::{closure} in /srv/mediawiki/php-1.40.0-wmf.22/includes/ServiceWiring.php at line 738] in /srv/mediawiki/php-1.40.0-wmf.22/includes/debug/MWDebug.php on line 381
Deprecated: Premature access to service 'MainConfig' [Called from MediaWiki\Maintenance\MaintenanceRunner::run in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php at line 556] in /srv/mediawiki/php-1.40.0-wmf.22/includes/debug/MWDebug.php on line 381
Deprecated: Premature access to service 'ConfigFactory' [Called from Wikimedia\Services\ServiceContainer::{closure} in /srv/mediawiki/php-1.40.0-wmf.22/includes/ServiceWiring.php at line 985] in /srv/mediawiki/php-1.40.0-wmf.22/includes/debug/MWDebug.php on line 381
Deprecated: Premature access to service 'BootstrapConfig' [Called from Wikimedia\Services\ServiceContainer::{closure} in /srv/mediawiki/php-1.40.0-wmf.22/includes/ServiceWiring.php at line 430] in /srv/mediawiki/php-1.40.0-wmf.22/includes/debug/MWDebug.php on line 381
Warning: Called Profiler::instance before settings are loaded in /srv/mediawiki/php-1.40.0-wmf.22/includes/profiler/Profiler.php on line 109
Fatal error: Uncaught Error: Call to a member function setConfig() on null in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php:570
Stack trace:
#0 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/doMaintenance.php(99): MediaWiki\Maintenance\MaintenanceRunner->run()
#1 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/CommandLineInc.php(76): require_once('/srv/mediawiki/...')
#2 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/eval.php(40): require_once('/srv/mediawiki/...')
#3 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php(268): include('/srv/mediawiki/...')
#4 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php(349): MediaWiki\Maintenance\MaintenanceRunner->loadScriptFile('/srv/mediawiki/...')
#5 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php(371): MediaWiki\Maintenance\MaintenanceRunner->findScriptClass('eval.php')
#6 /srv/mediawiki/php-1.40.0-wmf.22/maintenance/run.php(42): MediaWiki\Maintenance\MaintenanceRunner->setup(Obje in /srv/mediawiki/php-1.40.0-wmf.22/maintenance/includes/MaintenanceRunner.php on line 570
ladsgroup@mwdebug1002:~$ mwscript version.php --wiki=aawiki
MediaWiki version: 1.40.0-wmf.22 (built: 23:15, 6 February 2023)
ladsgroup@mwdebug1002:~$ mwscript sql.php --wiki=aawiki
> ^Dladsgroup@mwdebug1002:~$

Not too many to fix: https://codesearch.wmcloud.org/search/?q=CommandLineInc&i=nope&files=&excludeFiles=&repos=

Change 889259 merged by jenkins-bot:

[operations/mediawiki-config@master] mwscript: Switch to use run.php

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

Mentioned in SAL (#wikimedia-operations) [2023-02-28T17:14:17Z] <ladsgroup@deploy2002> Started scap: Backport for [[gerrit:889259|mwscript: Switch to use run.php (T326800)]]

Mentioned in SAL (#wikimedia-operations) [2023-02-28T17:33:09Z] <ladsgroup@deploy2002> ladsgroup: Backport for [[gerrit:889259|mwscript: Switch to use run.php (T326800)]] synced to the testservers: mwdebug2001.codfw.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug1001.eqiad.wmnet

dancy opened https://gitlab.wikimedia.org/repos/releng/scap/-/merge_requests/93

Check for PHP notices during mwscript mergeMessageFileList.php

dancy merged https://gitlab.wikimedia.org/repos/releng/scap/-/merge_requests/93

Check for PHP notices during mwscript mergeMessageFileList.php

For the record, scaps runs the following command

sudo -u www-data -n PHP="php7.4" -- /usr/local/bin/mwscript mergeMessageFileList.php --wiki=aawiki --force-version "1.40.0-wmf.25" --list-file="/srv/mediawiki-staging/wmf-config/extension-list" --output=/tmp/output-file

and it choked with https://gerrit.wikimedia.org/r/c/operations/mediawiki-config/+/889259/ in place:

PHP Notice:  Trying to access array offset on value of type bool in /srv/mediawiki-staging/php-1.40.0-wmf.24/maintenance/mergeMessageFileList.php on line 180
PHP Warning:  Invalid argument supplied for foreach() in /srv/mediawiki-staging/php-1.40.0-wmf.24/maintenance/mergeMessageFileList.php on line 180

PHP Notice:  Undefined variable: wgExtensionMessagesFiles in /srv/mediawiki-staging/php-1.40.0-wmf.24/maintenance/mergeMessageFileList.php on line 212
PHP Notice:  Undefined variable: wgMessagesDirs in /srv/mediawiki-staging/php-1.40.0-wmf.24/maintenance/mergeMessageFileList.php on line 213
PHP Notice:  Undefined variable: IP in /srv/mediawiki-staging/php-1.40.0-wmf.24/maintenance/mergeMessageFileList.php on line 216
PHP Notice:  Undefined variable: IP in /srv/mediawiki-staging/php-1.40.0-wmf.24/maintenance/mergeMessageFileList.php on line 218
<?php
## This file is generated by mergeMessageFileList.php. Do not edit it directly.

if ( defined( "$IPMW_NO_EXTENSION_MESSAGES" ) ) return;

$wgExtensionMessagesFiles = NULL;

$wgMessagesDirs = NULL;

A new release of scap has been deployed which has a check for the situation above. It will error out before syncing if detected.

Change 893552 had a related patch set uploaded (by Ladsgroup; author: Ladsgroup):

[operations/mediawiki-config@master] Revert "Revert "mwscript: Switch to use run.php""

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

Change 893552 merged by jenkins-bot:

[operations/mediawiki-config@master] Revert "Revert "mwscript: Switch to use run.php""

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

Mentioned in SAL (#wikimedia-operations) [2023-03-30T11:02:29Z] <ladsgroup@deploy2002> Started scap: Backport for [[gerrit:893552|Revert "Revert "mwscript: Switch to use run.php"" (T326800)]]

Mentioned in SAL (#wikimedia-operations) [2023-03-30T11:03:58Z] <ladsgroup@deploy2002> ladsgroup: Backport for [[gerrit:893552|Revert "Revert "mwscript: Switch to use run.php"" (T326800)]] synced to the testservers: mwdebug1002.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1001.eqiad.wmnet, mwdebug2001.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-03-30T11:10:28Z] <ladsgroup@deploy2002> Finished scap: Backport for [[gerrit:893552|Revert "Revert "mwscript: Switch to use run.php"" (T326800)]] (duration: 07m 59s)

We have a bash script that runs:

/usr/bin/php7.4 /srv/mediawiki/multiversion/MWScript.php extensions/ContentTranslation/scripts/dump-corpora.php (plus some args)

and it failed with the message:

Script 'dump-corpora.php' not found (tried path '/srv/mediawiki/php-1.41.0-wmf.2/maintenance/dump-corpora.php' and class 'dump-corpora\php').

I am guessing this is related to your work, @Ladsgroup ?

There may be other examples, but I haven't seen errors reported yet from anything else.

hmm, yeah, I think it should try relative path from $IP before erroring out. That makes it fully backward compatible.

Wouldn't this be a thing for MWScript then to do? Should changes be made there?

In T326800#8745565, @ArielGlenn wrote:

Wouldn't this be a thing for MWScript then to do? Should changes be made there?

the abspath doesn't work either, something is broken there. In order to avoid leaving production in a semi-broken state over the weekend, I reverted it and will try to find a solution next week. Possibly either:

• Turn relpath to abspath if the value given ends with .php
• or make run.php accept relpath with $IP beside checking relpath with $IP/maintenance
• or turn the relpath in mwscript into a relpath from maint/ directory (by adding ../ before passing to run.php)

My personal preference is the second one. Will see.

Change 905609 had a related patch set uploaded (by Ladsgroup; author: Ladsgroup):

[operations/mediawiki-config@master] Revert "Revert "Revert "Revert "mwscript: Switch to use run.php""""

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

Change 905609 merged by jenkins-bot:

[operations/mediawiki-config@master] Revert "Revert "Revert "Revert "mwscript: Switch to use run.php""""

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

Mentioned in SAL (#wikimedia-operations) [2023-04-05T11:15:01Z] <ladsgroup@deploy2002> Started scap: Backport for [[gerrit:905609|Revert "Revert "Revert "Revert "mwscript: Switch to use run.php"""" (T326800)]]

Mentioned in SAL (#wikimedia-operations) [2023-04-05T11:16:27Z] <ladsgroup@deploy2002> ladsgroup: Backport for [[gerrit:905609|Revert "Revert "Revert "Revert "mwscript: Switch to use run.php"""" (T326800)]] synced to the testservers: mwdebug1001.eqiad.wmnet, mwdebug2002.codfw.wmnet, mwdebug1002.eqiad.wmnet, mwdebug2001.codfw.wmnet

Mentioned in SAL (#wikimedia-operations) [2023-04-05T11:23:47Z] <ladsgroup@deploy2002> Finished scap: Backport for [[gerrit:905609|Revert "Revert "Revert "Revert "mwscript: Switch to use run.php"""" (T326800)]] (duration: 08m 45s)

In the spirit of the third time is the charm (I'm actually not sure, I lost track) I think it's fixed now, worked in mwdebug when I pulled the patch and tested with as many scripts as I could imagine.

The biggest problem is the incompatibility between the path that mwscript accept and the one that run.php accepts. mwscript explicitly says it must be relative to $IP but for run.php it must be from $IP/maintenance so passing the argument directly from mwscript to run.php breaks proper path finding. You might say: "Amir that's easy, just prepend ../ to the argument", well if the path starts with ./ or ../ run.php treats it as path relative to pwd and everything breaks again. You might say "oh than go abspath, just append $IP", well the problem is that at the point we are changing things, the $IP is not even defined because we don't know what mw version it's going to be. I had to do something similar to monkey patching, i.e. change the argv pretty late in multiversion, to make it work.

Regardless, it works now. hopefully.

FWIW the way I did it for vagrant is

• pass as-is if it starts with /, ./ or ../, includes : or does not end in .php
• turn it into an absolute URL with MWMultiVersion::getMediaWikiCli( "maintenance/$script" ) if the script parameter does not contain a / (which requires some hacks as MWMultiVersion::getMediaWikiCli(), or at least vagrant's fork of it, makes use of $argv)
• turn it into an absolute URL with MWMultiVersion::getMediaWikiCli( $script ) otherwise .

That seemed to work well so far, although of course vagrant makes less use of maintenance scripts than production, and isn't truly heterogenous either.

It looks like mwscript only works with file names, but not with class names. Filed T336819: Maintenance script designed for run.php <class> syntax cannot be executed in Wikimedia production.

These were tested on the beta cluster:

$ mwscript /srv/mediawiki/php-master/extensions/OAuth/maintenance/testOAuthConsumer.php --wiki=enwiki --help
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Script 'testOAuthConsumer.php' not found (tried path '/srv/mediawiki/php-master/maintenance/testOAuthConsumer.php' and class 'testOAuthConsumer\php').
                                                                     ^^^^^^^^^^^^^^^^^^^^^^
$ mwscript OAuth:testOAuthConsumer.php --wiki=enwiki --help
...
Usage: php OAuth:testOAuthConsumer.php OAuth:testOAuthConsumer.php [OPTION]... --consumerKey <CONSUMERKEY>
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^