Last modified: 2013-07-31 22:06:54 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T47031, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 45031 - Some extensions register their namespaces too late
Some extensions register their namespaces too late
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Site requests (Other open bugs)
wmf-deployment
All All
: High major (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2013-02-15 01:31 UTC by Ori Livneh
Modified: 2013-07-31 22:06 UTC (History)
13 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description Ori Livneh 2013-02-15 01:31:05 UTC
Apologies in advance for the laconic report -- I am exhausted at the moment.

I made a configuration change earlier that should have resulted in the Schema namespace getting enabled on test2wiki. It only half-worked: I was able to create an article in the Schema namespace, and it had the correct value for page_namespace in the database. But the namespace name was getting stripped from the title, transforming 'Schema:Test' into ':Test', as far as most interfaces were concerned. We ran scap after making the configuration change and later on I updating the i18n cache a second time, but that did not seem to fix things.

In PHP, the return value of '$wgContLang->getNamespaceIds()' contained both new namespaces (Schema and Schema_talk). But they did not appear in the 'wgNamespaceIds' object in JavaScript, even several hours after the deployment.

I do not think it's a static asset caching issue: interfaces generated by PHP that one would have expected to display the Schema were not displaying it.

I finally noticed a comment in CommonSettings.php, added by Gerrit change I147c16ecf1b235d4fec514d0c6f2ef10932caef9, that seemed relevant: Fundraising seems to have resorted to declaring the namespace manually, in the CommonSettings.php. I tried the same trick in I8ac1eae2456d845afe989052f188aea20f9f5d38 and it appears to have worked.

Relevant comments from mwalker on #wikimedia-tech (Fri Feb 15 01:29:37 UTC 2013):

17:25 <mwalker> right -- so that was because I was originally adding the namespaces via the $wgExtensionFunctions[] hook system
17:25 <mwalker> which happened too late
17:26 <mwalker> but, CanonicalNamespaces should always happen at the right time
17:26 <ori-l> Hrm.
17:27 <mwalker> I will say that I'm adding my namespace to both the $namespaces param in that hook; and then to all the other fun arrays
17:27 <mwalker> but -- I haven't tested this on test2
17:27 <mwalker> only test

I *do* use CanonicalNamespaces in EventLogging, so perhaps the problems aren't related after all. But for whatever reason adding the namespace manually in CommonSettings.php seems to have fixed my problem, too.
Comment 1 MZMcBride 2013-02-15 06:26:52 UTC
Setting this bug to high/major, as it's needlessly wasting developer time in debugging why the namespaces are somewhat being registered, but not fully being registered.
Comment 2 Antoine "hashar" Musso (WMF) 2013-02-18 20:31:22 UTC
Seems to be related to the site configuration so moving to "Site requests"
Comment 3 Matt Walker 2013-02-20 16:39:16 UTC
When I was poking around trying to solve this I noticed that it might have had something to do with how the Language object cached the canonical namespaces in order for it to cache the localized names. This was caused in CentralNotice by attempting to use the Title object too early (it worked but broke everything later on.)

I also note that the CanonicalNamespaces hook is called multiple times even though it's ostensibly located inside of a static gate -- that one is probably just a fluke though.
Comment 4 Ori Livneh 2013-07-18 19:30:49 UTC
Happened again w/deployment of Campaign NS: see https://test.wikipedia.org/wiki/Special:Contributions/Maintenance_script
Comment 5 Brad Jorsch 2013-07-18 20:12:47 UTC
(In reply to comment #4)
> Happened again w/deployment of Campaign NS: see
> https://test.wikipedia.org/wiki/Special:Contributions/Maintenance_script

At least at the moment, the early call to CanonicalNamespaces seems to be from ShortUrlHooks::setupUrlRouting(), which is called for the WebRequestPathInfoRouter hook, which is being called from the call to $wgRequest->interpolateTitle() in Setup.php.

ShortUrlHooks::setupUrlRouting() calls getPrefixedText() on a Title object for its special page, which eventually calls MWNamespace::getCanonicalNamespaces() to find the local name for the "Special" namespace, which calls the hook and caches the result.

One possibility for a general fix would be to call MWNamespace::getCanonicalNamespaces( true ) from near the bottom of Setup.php, to force the list of canonical namespaces to be recached.
Comment 6 Yuvi Panda 2013-07-18 21:06:31 UTC
https://gerrit.wikimedia.org/r/74523 implements the suggestion in the last comment, but since I am not completely too sure what is happening here, might be completely wrong.
Comment 7 Gerrit Notification Bot 2013-07-18 21:49:01 UTC
Change 74523 had a related patch set uploaded by SuchABot:
Force re-cache of Canonical Namespaces

https://gerrit.wikimedia.org/r/74523
Comment 8 Gerrit Notification Bot 2013-07-19 09:47:51 UTC
Change 74598 had a related patch set uploaded by Reedy:
Force re-cache of Canonical Namespaces

https://gerrit.wikimedia.org/r/74598
Comment 9 Gerrit Notification Bot 2013-07-19 10:09:43 UTC
Change 74598 merged by jenkins-bot:
Force re-cache of Canonical Namespaces

https://gerrit.wikimedia.org/r/74598
Comment 10 Yuvi Panda 2013-07-19 10:23:34 UTC
That change doesn't seem to help. However, disabling ShortUrl does seem to help.
Comment 11 Brad Jorsch 2013-07-19 13:49:07 UTC
(In reply to comment #10)
> That change doesn't seem to help. However, disabling ShortUrl does seem to
> help.

Ah. The Language objects keep their own cache of namespace names, so we'd need to clear that too. Sigh.
Comment 12 Tim Starling 2013-07-24 04:42:28 UTC
(In reply to comment #5)
> At least at the moment, the early call to CanonicalNamespaces seems to be
> from
> ShortUrlHooks::setupUrlRouting(), which is called for the
> WebRequestPathInfoRouter hook, which is being called from the call to
> $wgRequest->interpolateTitle() in Setup.php.

Setup.php line 530 doesn't seem very early to me. Why are namespaces being added after that? Usually, early initialisation problems affect LocalSettings.php.
Comment 13 Ori Livneh 2013-07-24 05:49:32 UTC
(In reply to comment #12)
> Setup.php line 530 doesn't seem very early to me. Why are namespaces being
> added after that? Usually, early initialisation problems affect
> LocalSettings.php.

The CanonicalNamespaces hook handler is registered in a $wgExtensionFunctions[] function. It's a pattern I started with EventLogging. Possibly an anti-pattern.

Yuvi uses it in UploadWizard and Yuri uses it in ZeroRatedMobileAccess and Molly uses it in BookManagerv2. It has possibly spread to another extension in the time it took me to write this comment.

I don't think there's a reason for UploadWizard or BookManagerv2, since these two extensions are either wholly enabled or disabled; they don't branch on the wiki name. EventLogging and ZeroRatedMobileAccess are different: the extensions are active on multiple Wikimedia wikis but select a specific wiki to house the configuration namespace for the entire cluster. (It's metawiki in both cases, but EventLogging also enables the Schema namespace on test2wiki.) The wikis on which the extra namespace is activated are specified via configuration variables.

I'm game for cleaning it up if it's bad pattern. How should this be done?
Comment 14 Tim Starling 2013-07-24 06:43:57 UTC
Why can't you register the CanonicalNamespaces hook in the normal way? Why does it need to be deferred?

$wgExtensionFunctions is meant to be as late as possible in the setup process, I think it's too late for adding namespaces.
Comment 15 Ori Livneh 2013-07-24 06:51:43 UTC
(In reply to comment #14)
> Why can't you register the CanonicalNamespaces hook in the normal way? Why
> does it need to be deferred?

Because there are several things that need to be executed on the target configuration wiki, not just the namespace registration, and it seemed neater to have the activation logic be all in one place rather than sprinkled in multiple places. I suppose I could aggregate it all in the CanonicalNamespaces hook handler, though that looks a bit odd.

> $wgExtensionFunctions is meant to be as late as possible in the setup
> process, I think it's too late for adding namespaces.

Well, OK, but in that case why have the hook at all? If namespace registration shouldn't be deferred, the only interface for adding namespaces should be adding entries to $wgCanonicalNamespaceNames.
Comment 16 Ori Livneh 2013-07-24 07:08:55 UTC
(Tim replied on #wikimedia-dev; edited for clarity and reproduced below. --OL)
---


ZeroRatedMobileAccess sets hooks based on a boolean configuration variable. Obviously you can't do that from the file scope. It should instead have the configuration variable checks inside unconditionally-registered hook functions, e.g.:

    static function onCanonicalNamespaces( array &$namespaces ) {
        if ( !$GLOBALS['wgZeroRatedMobileAccessEnableZeroConfigPages'] ) return;
        ...

But most of the content of zero's onCanonicalNamespaces() can be in the file scope. For example:

    $wgNamespaceContentModels[NS_ZERO] = 'JsonZeroConfig';

This can just be in the file scope. It's harmless to configure a non-existent namespace. There definitely shouldn't be global variable assignments in a CanonicalNamespaces hook handler. It's not guaranteed that the hook will be called before the variables are accessed. What's guaranteed is that there will be no hook calls between execution of the file scope and setting of the configuration variables so you can reference configuration globals from hook functions with confidence.
Comment 17 Gerrit Notification Bot 2013-07-28 05:30:20 UTC
Change 76325 had a related patch set uploaded by Ori.livneh:
Register NS_ZERO early if running on configuration wiki

https://gerrit.wikimedia.org/r/76325
Comment 18 Gerrit Notification Bot 2013-07-28 06:14:56 UTC
Change 76326 had a related patch set uploaded by Ori.livneh:
Register NS_SCHEMA early if running on schema wiki

https://gerrit.wikimedia.org/r/76326
Comment 19 Gerrit Notification Bot 2013-07-28 06:57:10 UTC
Change 76327 had a related patch set uploaded by Ori.livneh:
Declare NS_BOOK namespace unconditionally

https://gerrit.wikimedia.org/r/76327
Comment 20 Gerrit Notification Bot 2013-07-28 07:24:30 UTC
Change 76328 had a related patch set uploaded by Ori.livneh:
Register NS_CAMPAIGN at file level in extension's entry point

https://gerrit.wikimedia.org/r/76328
Comment 21 Gerrit Notification Bot 2013-07-28 07:33:02 UTC
Change 74523 abandoned by Ori.livneh:
Force re-cache of Canonical Namespaces

Reason:
The underlying cause of 45031 was the deferment of namespace registration to an extension function, which is way too late. I committed a fix to each extension which followed this pattern: I2fb2bcaa2 (UploadWizard), I76cfa1d5a (BookManagerv2), Ide5f09206 (EventLogging), and I2fb2bcaa2 (ZeroRatedMobileAccess).

https://gerrit.wikimedia.org/r/74523
Comment 22 Gerrit Notification Bot 2013-07-29 19:01:55 UTC
Change 76327 merged by jenkins-bot:
Declare NS_BOOK namespace unconditionally

https://gerrit.wikimedia.org/r/76327
Comment 23 Gerrit Notification Bot 2013-07-29 19:06:06 UTC
Change 76328 merged by jenkins-bot:
Register NS_CAMPAIGN at file level in extension's entry point

https://gerrit.wikimedia.org/r/76328
Comment 24 Gerrit Notification Bot 2013-07-29 21:43:59 UTC
Change 76326 merged by jenkins-bot:
Register NS_SCHEMA early if running on schema wiki

https://gerrit.wikimedia.org/r/76326
Comment 25 Gerrit Notification Bot 2013-07-31 00:17:35 UTC
Change 76862 had a related patch set uploaded by Ori.livneh:
Register NS_ZERO early if running on configuration wiki

https://gerrit.wikimedia.org/r/76862
Comment 26 Gerrit Notification Bot 2013-07-31 19:03:28 UTC
Change 76325 merged by jenkins-bot:
Register NS_ZERO early if running on configuration wiki

https://gerrit.wikimedia.org/r/76325
Comment 27 MZMcBride 2013-07-31 22:00:25 UTC
I'm not really sure what's going on with Gerrit change #76325 v. Gerrit change #76862, but this bug seems nearly resolved/fixed.
Comment 28 Gerrit Notification Bot 2013-07-31 22:02:32 UTC
Change 76862 abandoned by Ori.livneh:
Register NS_ZERO early if running on configuration wiki

Reason:
Dupe.

https://gerrit.wikimedia.org/r/76862
Comment 29 MZMcBride 2013-07-31 22:06:54 UTC
The four affected extensions (ZeroRatedMobileAccess, EventLogging, BookManagerv2, and UploadWizard) have all been fixed. Marking this bug resolved/fixed accordingly.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links