[03:22:56] Krinkle: the point of that is to factor out loading of the config? [03:23:21] that's the main thing I was missing when I saw it before, it took me a while to figure out the point of it [18:12:00] TimStarling: Main point was to try and reduce complexity and enforce a more consistent order across contexts. As a result, some of the issues/dependencies that were previously unclear/vague to me have now been explicitly coded. [18:12:20] Not yet sure if that's an improvement, given it looks quite unpleasant, but it does somewhat reflect the current state of things. [18:12:49] Perhaps after this (if we go with it) we can try and streamline some of those things slowly. [20:07:17] Fatal error: Class undefined: MediaWiki\GlobalUserPage\WikiGlobalUserPagePage in /srv/mediawiki/php-1.31.0-wmf.2/extensions/GlobalUserPage/includes/Hooks.php on line 179 [20:07:28] uhoh [20:07:34] crap [20:07:35] Also Catchable fatal error: Argument 2 passed to PageTranslationHooks::onPageContentLanguage() must be an instance of Language, StubUserLang given in /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php on line 177 [20:07:44] But those only 2 from wmf.2 on group0 so far this week :D [20:07:49] Much better than last 2 ;-) [20:08:25] The latter sounds like it wasn't expected to be called prior to unstubbing [20:09:10] no_justification: https://gerrit.wikimedia.org/r/#/c/382037/ for the first [20:09:30] I think most people don't realize StubUserLang is a thing and just type hint against Language always [20:09:31] I'll get that out asap, thx [20:09:48] To be fair, Stubs are a weird anti-pattern from most other objects in MW [20:10:07] Considering bytecode caches, I'm not entirely sure the overhead (mental & technical) is worth it these days [20:12:06] * legoktm afk --> class [20:15:03] * no_justification waves [20:15:07] Thx for the quick fix on the former [20:28:45] do we even have code paths that don't lead to unstubbing? [20:31:36] All code paths will unstub. The idea though is to delay loading [20:32:22] AIUI years ago from Tim the idea was to delay loading the /actual file from disk/ until as late as possible. [20:32:32] Which is why it was useful even for things with empty constructors [20:32:44] There's an old CR from SVN days from when I tried to rip this all out once before [20:32:45] Lemme find it [20:32:58] ugh stub objects [20:35:45] I killed StubUser years ago [20:35:48] StubContLang is also gone [20:36:09] https://www.mediawiki.org/wiki/Special:Code/MediaWiki/70970 [20:37:44] StubStub [20:37:57] StubUserLang and DeprecatedGlobal only uses in core + extensions [20:38:02] The former we could kill pretty safely [20:40:05] Actually, since we've wrapped all this stuff in RequestContext & friends, we already delay loading via that [20:40:26] Calling RequestContext::getMain()->getUserLanguage() already doesn't invoke Language object construction until called. [20:40:37] The stubiness factor of it only affects code using $wgUserLang directly still [20:40:40] (which they shouldn't) [20:41:28] Er, $wgLang rather [20:41:59] I'm gonna slap a deprecated on it [20:43:36] * brion goes looking for 64-bit xampp build, out of curiosity... [20:43:56] 64-bit php 7.1 does appear to have large integers if i check on the CLI manually [20:44:30] "The point of StubUser is to avoid parsing loading and parsing User.php, which can be very slow on non-APC installations. It's not about object construction, which is fast either way." [20:44:48] Yep [20:44:51] in other words, let's kill this shit with fire [20:45:02] I'm slapping @deprecated notices on this crud now [20:45:08] Or we could just do it in one release [20:45:40] I don't think that removing stubbing requires deprecation process [20:45:51] ideally, users aren't even supposed to notice [20:46:18] for those who do so, we could leave the StubObject class itself [20:46:26] nope, no 64-bit xampp. and i don't feel like configuring apache and php on windows manually, so i'll just document it and move on ;) [20:49:14] brion theres bitanmi wamp :) [20:49:17] it's 64-bit [20:49:38] MaxSem: It's mostly comments. Only a couple of weird calls outside of core [20:49:41] Forced unstubbing, etc [20:50:43] oh nice! thx paladox i'll check it out [20:50:46] lots of extensions copy $wgParser creation from the core blindly [20:50:49] your welcome :) [20:51:03] https://bitnami.com/stack/wamp [20:51:07] MaxSem: https://gerrit.wikimedia.org/r/#/c/382061/ [20:51:09] :) [20:51:09] https://bitnami.com/stack/wamp/installer [20:51:31] MaxSem: BAHAHAHAHAHAHA [20:51:37] Second patch is gonna be even better [20:51:41] \m/ [20:52:51] https://gerrit.wikimedia.org/r/#/c/382063/ [20:54:27] no_justification, use is for IDEs [20:54:42] \m/ [20:54:49] Get a better IDE [20:54:56] One that doesn't require a use statement for a damn comment [20:55:12] :) [20:55:13] eclipse :) [20:55:19] comment is one thing, phpdoc is another :P [20:55:41] though obviously the real problem is functions that accept parameters of multiple types [20:56:38] this is why we need to rewrite everything in rust and use enums [20:57:04] complex types 4evah [21:15:05] MaxSem: Couple of extensions force-call unstub() on the language object [21:15:12] Kinda a messy removal unless we land all of them at once [21:16:13] Actually, a bunch do [21:17:10] no_justification: I wonder what that's for? Given it auto-unstubs [21:17:44] I have nfc [21:17:58] Maybe make it a no-op first. with @deprecated, then we can mass-remove usage after the core commit lands. [21:18:15] https://github.com/wikimedia/mediawiki-extensions-Wikibase/blob/e187932135b70b30bb58c49e9c4602e7d19a36a1/client/includes/Hooks/ParserOutputUpdateHookHandlers.php#L40 [21:18:15] Ahhh, unless you're just wanting to force the code to be loaded, even if it doesn't call a method directly? [21:18:29] Like, a static thing you want? [21:18:41] Like I said, I don't really get it [21:18:48] But a bunch of extensions do it [21:18:56] For $wgContLang too, which isn't a stub anymore even ;-) [21:19:04] https://github.com/wikimedia/mediawiki-extensions-Flow/blob/b96c8d1a3b7be45099708a32b7b92d9cfdc1a439/maintenance/convertToText.php#L179 [21:19:20] Those will handle gracefully if it is a no-op [21:19:25] which as you say is already the case for ContLang [21:19:31] What about this though? https://github.com/wikimedia/mediawiki-extensions-LiquidThreads/blob/4082a70b34c41a722f0264b1bbb8a2af5af729c6/import/import-parsed-discussions.php#L14 [21:19:35] Surely thats already broken [21:19:41] I don't even remember wgOut being a stub [21:19:54] It was [21:20:05] StubOutput, StubUser, StubLang, StubUserLang [21:20:09] Were the 4 iirc [21:21:41] Anything that calls ->unstub() or _unstub() as a method is likely to be broken easily [21:21:45] no_justification, StubObject::unstub() already is a noop if its target is not a stub [21:21:50] unstub() as a static function is safe [21:21:58] Yeah, I'm just talking about this whole system in general [21:22:16] so just remove stubbing then clean up extensions? [21:22:33] unstab() [21:22:54] Well I don't want to remove StubObject & company just yet [21:23:04] DeprecatedGlobal uses it (although could be simplified/refactored after this) [21:23:43] Fuck it, just yank the bandaid [21:25:01] no_justification: That LQT one was the only unguarded one. The others are either with a proper !isRealObject check, or general instanceof check. [21:25:09] Which should work fine if the object is a stub no more. [21:25:33] RE: calls to _unstub() [21:25:44] https://gerrit.wikimedia.org/r/382066 [21:26:11] https://github.com/search?utf8=%E2%9C%93&q=org%3Awikimedia+-repo%3Awikimedia%2Fmediawiki-debian+%22_unstub%22&type=Code looks pretty good otherwise [21:26:52] also, why are we still using LQT? :( [21:28:21] https://gerrit.wikimedia.org/r/#/c/382067/ [21:28:22] Bam [21:29:11] no_justification: Interesting, ExtTab and a few others unstub because the function takes a parameter by reference. Not sure if that matters or not, I guess it doesn't given subobject will still proxy the method call regardless of the global var being unstubbed, right? [21:29:15] no_justification, that thing I get yelled about so often myself :P [21:29:24] release notes! [21:29:54] also, are you sue you don't want to merely unlink and deprecate it? [21:31:13] Lol, release notes [21:31:23] And no, rip the whole thing out [21:33:04] Unrelated, would it be possible for a phpcs sniff that says !( $foo instanceof Bar ) is illegal? [21:33:14] Operate precedence means the parens are extra [21:35:09] Actually, we can't rip it off [21:35:18] I can't ensure those Wikibase patches will land in the next build [21:35:32] So we'll have to wait on the bandaid ripping, or provide a small transition fallback [21:38:13] I actually like parens because nobody remebers the precendence of weird operators, especially when people rutinely develop in several languages at once [21:38:15] no_justification: did stubuserlang break something in particular? [21:38:54] Well they use StubUserLang::unstub() not StubObject::unstub() in wikibase [21:39:16] So unless their patch lands in the next build it'll break them by outright removal [21:39:28] Or do you mean what prompted this discussion and my axe sharpening? [21:39:36] I mean, the reason for removing StubUserLang, did it break something in particular in the past just now? [21:39:48] Catchable fatal error: Argument 2 passed to PageTranslationHooks::onPageContentLanguage() must be an instance of Language, StubUserLang given in /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php on line 177 [21:39:49] Yeah, if not, we can just merge the whole removal at once in a week. [21:41:25] And I figured rather than introduce more stubiness checks let's finally remove the best [21:43:41] So this would be the safest route I think: this week we'll stop making $wgLang a stub object, but keep the class. Can finish up comment removals & checks in core too while we're at [21:43:49] Then we can move on extensions. Once all those land -- then we can finally remove from core [21:44:11] All the calls, minus the LQT you noticed, seem safely guarded enough for this to the be the plan [21:44:30] Otherwise, we'll likely end up with a bunch of messy backports at branch time next week [22:03:43] no_justification: Ah yeah, that works. I don't have a preference between 1) unstub in core, 2) remove no-op usage, 3) remove stub class vs. 1) remove redundant calls, 2) unstub and remove in core. [22:03:47] I suppose the former is safer and cleaner [22:03:48] LGTM :) [22:04:03] Yeah, wikibase ones are the only ones that will lag really [22:08:29] https://gerrit.wikimedia.org/r/#/c/382072/ [22:09:22] Heh, "Stop stubbing StubUserLang" [22:09:28] Should've been "Stop stubbing $wgLang" [22:09:32] yolo [22:09:37] no_justification: Ah, I like this. Now that the unstub happened in core first, the code in Wikidata is much safer to remove. Even if in some way Wikibase methods depend on indirect state set up by wgLang, that is still a given now :) [22:09:39] Very nice [22:10:03] Yeah, so now we can let core proceed, and Wikibae can merge whenever [22:10:17] Wikibae heh [22:10:20] New favorite thing [22:11:43] https://gerrit.wikimedia.org/r/#/q/branch:master+topic:unstub [22:13:24] I think "unstub on purpose" is basically as bandaid for other code that doesn't think of StubWhatever in its codepath [22:13:46] So it's "force it to unstub so other extension doesn't break me" [22:13:49] Or something like that [22:16:13] Wtf is Wigo3? [22:16:17] I've never heard of this until today [22:18:47] Oh man, wfLoadExtensionMessages() too? I wonder if this thing even works [22:21:22] no_justification: Wikibase, Wikibae, Wikivase, we've got all kinds of wikis this week. [22:21:22] https://phabricator.wikimedia.org/T174922#3616688 [22:22:28] no_justification: Yeah, the only manual unstub I can think of that is still needed in the current implementation of StubObject is for type hinting. Everything else shouldn't matter. E.g. pass by reference doesn't matter because even if you keep calling throuhg the sub, it proxies. [22:23:43] no_justification: Why does that have its own wiki page o_O [22:24:00] Tim just imported it recently [22:24:01] June [22:24:04] Apparently [22:24:07] https://gerrit.wikimedia.org/r/#/c/382075/ anyway, cleanup [22:24:38] I was looking for the word "unstub" broadly and came across that one