Page MenuHomePhabricator

Obvious bug in maintenance/Maintenance.php since 1.31
Closed, ResolvedPublic

Description

Forwarding a bug report from mediawiki-l:

[MediaWiki-l] Obvious bug in maintenance/Maintenance.php since 1.31

REL 1_31 introduced 'mwdebug' option in Maintenance.php (in the code it
mentions that --debug was taken by some script). The code incorrectly adds
the option via addOption() by setting the 4th argument to true (takes an
argument). This should be false. At least, the rest of the code is clear,
and so is the intent: there is no option to be taken.

https://phabricator.wikimedia.org/source/mediawiki/browse/REL1_31/maintenance/Maintenance.php$525

Event Timeline

matmarex created this task.Sep 18 2019, 7:55 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 18 2019, 7:55 PM

The 4th parameter signals whether the option requires a value (as opposed to a boolean flag). The impact is that it only works if you use --mwdebug=whatever.

I thought for a moment, from reading the code and the report, that it meant it was wrongly enabled by default. But that's (fortunately) not the case.

@Krinkle does the fourth value still/actually need to be changed from true to false? If so, I can do a commit with that if that helps any.

@TheSandDoctor Yes, I believe the problem has not yet been resolved.

Perhaps you want to, as exercise, try to reproduce the issue (before changing the code).

The following command should work any error (it will prints nothing or sometimes a number)

/var/www/mediawiki# php maintenance/showJobs.php
18

The following command currently fails, but should work the same as the above:

/var/www/mediawiki# php maintenance/showJobs.php --mwdebug

ERROR: mwdebug parameter needs a value after it

The following currently works but the =1 should not be needed. That is what this task is going to fix:

/var/www/mediawiki# php maintenance/showJobs.php --mwdebug=1
18
TheSandDoctor triaged this task as Medium priority.Sep 22 2019, 9:51 PM
TheSandDoctor awarded a token.

@TheSandDoctor Yes, I believe the problem has not yet been resolved.

Perhaps you want to, as exercise, try to reproduce the issue (before changing the code).

The following command should work any error (it will prints nothing or sometimes a number)

/var/www/mediawiki# php maintenance/showJobs.php
18

The following command currently fails, but should work the same as the above:

/var/www/mediawiki# php maintenance/showJobs.php --mwdebug

ERROR: mwdebug parameter needs a value after it

The following currently works but the =1 should not be needed. That is what this task is going to fix:

/var/www/mediawiki# php maintenance/showJobs.php --mwdebug=1
18

Verified the issue. Will work on this in the morning.

TheSandDoctor added a comment.EditedSep 23 2019, 5:28 PM

@Krinkle I tried submitting a patch just now but keep getting the following error. It was working previously for extensions without issue.

$ git review -R
remote: Unauthorized
fatal: Authentication failed for 'https://gerrit.wikimedia.org/r/mediawiki/core.git/'

Change 538679 had a related patch set uploaded (by TheSandDoctor; owner: TheSandDoctor):
[mediawiki/core@master] Maintenance.php: Fix mwdebug error

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

@Krinkle I tried submitting a patch just now but keep getting the following error. It was working previously for extensions without issue.

$ git review -R
remote: Unauthorized
fatal: Authentication failed for 'https://gerrit.wikimedia.org/r/mediawiki/core.git/'

I reset the origin URL and it worked this time. Odd.

Krinkle moved this task from Inbox to Doing on the Performance-Team board.Sep 23 2019, 8:16 PM

Change 538679 merged by jenkins-bot:
[mediawiki/core@master] Maintenance.php: Fix mwdebug error

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

Krinkle closed this task as Resolved.EditedSep 24 2019, 11:19 PM

Thanks @TheSandDoctor.

This will become part of 1.34. Note, I won't be backporting it to older versions as the option was new in 1.31 and never worked. Making it work or changing behaviour retroactively at this point is imho more risk than is worth taking at this point for debug feature like this.

You're welcome @Krinkle ! I'm glad that I could help. This was my first contribution to core :)