[05:48:21] grr, everything is so broken [05:48:27] it is distracting [05:48:51] every time I try something in MW these days I see another broken thing [05:55:20] rewrite it! ;-) [05:55:37] in Rust! or maybe Swift! [06:07:49] TimStarling: like what? Is there a pattern to the bugs? [06:08:13] not really, partly it is just me caring about things more [06:08:26] at the moment I am figuring out why the installer is totally broken [06:08:29] which is hard to miss I guess [06:17:05] * robla hasn't tried the installer recently, and is refusing to be nerdsniped [06:35:24] it would probably work for you [06:55:22] I got license sniped on a different channel. I was doomed to be distracted by something [06:58:24] TimStarling: I seem to recall at least one or 2 people complaining about it [06:58:35] Last time I used it, a few months ago, I ended up fixing a few little things, filing a couple of bugs [06:59:04] Actually, I think Daniel Kinzler was one of them... Ended up using the cli installer if I'm not completely mistaken [07:27:44] I wonder if we should have a browser test thing to run through the installer... [07:51:47] MW_NO_SESSION is set, but WebRequest::getSession() still sets up a SessionManager session regardless [07:52:20] which makes a new session ID and causes a call to setcookie() overwriting the session ID [07:52:32] this happens in User::load() [07:52:56] I figured it must not be reproducible since I thought nobody would commit it in such a broken state, but I guess it is reproducible [07:59:23] which is already known at T126177 apparently [08:09:54] Ah, it's a session rewrite issue? [08:20:31] TimStarling: are you using current master? [08:20:40] the fix was merged today, I think [08:22:58] a few hours old I think I'll try updating [08:24:15] Updating 290ca86..7a84c29 [08:24:21] which apparently doesn't include the fix? [08:30:45] do you mean https://gerrit.wikimedia.org/r/#/c/269066/ ? That doesn't match the backtrace I'm seeing [08:33:37] // Having a user with id = 0 safeguards us from DB access via User::loadOptions(). [08:33:37] $wgUser = User::newFromId( 0 ); [08:33:57] you would think [08:35:24] there's a big pile of unmaintained hacks intended to avoid issues like this [08:35:47] we have [08:35:48] $wgLang = Language::factory( $langCode ); [08:36:04] and yet the issue is occuring in RequestContext::getLanguage() [08:36:28] the language is already decided, but the hack to make sure that information is globally known has not been updated [08:36:50] so RequestContext::getLanguage() calls User->getOption() [08:37:14] and we have this $wgUser hack already, but obviously that it not working either because User->loadFromSession() is called [18:02:40] bd808: is there any reason composer on vagrant does not check out dependencies from git? [18:03:59] is there any reason it should? [18:04:10] we don't typically follow HEAD [18:09:13] bd808: yes, so that it's less effort to write patches for them [18:16:32] alrighty, I'm ready to convert all arrays in core to you know what =) who wants to review? gotta be fast cuz merge conflicts [18:17:12] MaxSem: seems like something Reedy or legoktm would love to +2 [18:18:24] Merge all the things [18:18:38] I'm not on a laptop atm [18:18:54] * aude has merged things from my mobile phone :) [18:21:43] I have too... [18:21:52] Gotta log into gerrit and stuff [18:22:03] "Have you reviewed all this?" [18:22:15] "That's what Jenkins is for" [18:24:26] GOGOGOGOG https://gerrit.wikimedia.org/r/#/c/269745/ [18:24:47] decided to do in several steps [18:25:00] 155 files changed, 1255 insertions(+), 1255 deletions(-) [18:29:45] MaxSem: wasn't the outcome of the RfC specifically that we are not going to do that? [18:30:06] nah [18:30:43] someone was actually tasked with mass conversion, I'm just eating their lunch [18:30:44] I'm not finding it in the notes but I think I remember legoktm wanting to add a code check sniff and just mass convert things [18:32:57] woot [18:34:10] well the phab task says "Short array syntax...Updating usage should be permitted in the context of changes that have some other, independent motivation behind them (like fixing a bug or adding a feature), but please let's not destroy our git history with changes that do nothing but update the syntax." [18:34:28] not sure if that's the proposal or the result of the RfC [18:37:01] tgr: I think you're right [18:37:19] personally I think the git history would survive that much, OTOH it's annoying for the people who will need to rebase their patches line by line [18:39:37] please don't do any changes in context of any other changes :/ that's the most annoying thing. separate commits plz [18:40:41] so, who's going to go through the whole codebase to change every current( foo() ) to foo()[0] and every end( foo() ) to foo()[ count(foo())-1 ]? :D [18:40:49] I'm pretty sure that the overeager "upgrade all of the code!" tendency is the reason why Tim was conservative about this. he's seen this behavior before [18:45:22] inconsistency is far, far worse [18:50:26] legoktm, now how do I remove php53 jobs from my extensions/modules? :) [18:51:09] bd808: how do I find out who are gerrit admins? [18:51:29] I need to whine for branch create rights so I can debug [18:52:11] ...is that a sensible way to debug? I want to add debug log calls to a change, except the file where they need to be added is librarized [18:56:48] tgr: admins are https://gerrit.wikimedia.org/r/#/admin/groups/1,members [18:57:04] I'm not sure about your question of what's right though [18:57:07] MatmaRex: "foo()[ count(foo())-1 ]" would be bad because it calls foo() twice. [18:57:54] And "current( foo() ) to foo()[0]" might break if it's an assoc or otherwise not a normal array. [18:59:17] yes, but you know what i mean. [18:59:17] also end() moves the array pointer and [] does not and you probably don't want to review every end() call to make sure that's not a relevant difference [19:05:57] * bd808 steps out to run some errands [21:52:08] anomie: also lots and lots of 'Metadata merge failed: Key "CentralAuthSource" changed' errors [21:53:59] bd808: Do we have any way to filter by "m" domains? [21:55:58] anomie: hmm.. looking [21:56:14] maybe using referrer? [21:57:00] anomie: it looks like 'referrer:"*.m.*"' gets some [21:58:00] bah that doesn't really work [22:03:34] anomie: 'url:"mobile=1"' catches some things but certainly not all [22:03:44] bd808: Spot checking these, I see a lot with referrers coming from Wikipedias that would still be on wmf.12. [22:39:20] anomie: reopening T49647 works for me [22:39:53] anomie: what is the impact for affected users, and is there a way to count how many of those there are? [22:40:19] anomie: I just uploaded v0.4.1 of the firefox plugin. It supports m. domains now -- https://addons.mozilla.org/en-US/firefox/addon/wikimedia-debug-header/versions/?page=1#version-0.4.1 [22:40:53] ori: Direct logins on commons.m.wikimedia.org, meta.m.wikimedia.org, and similar .m.wikimedia.org domains is broken. It works fine for non-'m' domains and non-.wikimedia.org domains, and the CentralAuth auto-login stuff seems to work fine even on the affected domains. [22:41:27] that's pretty bad, IMO [22:41:57] something similar to the no login on loginwiki directly problem it seems with the cookies [22:42:14] Depends on how many people actually log in on those few mobile domains. This was probably present in wmf.11 as well and no one ever reported it. [22:43:15] gah, I remember fixing a similar problem ages ago [22:43:31] anomie: can you make a ballpark guess on how hard it should be to fix? [22:43:37] are we talking hours or days? [22:44:27] Looks like tgr filed it at https://phabricator.wikimedia.org/T126550 [22:44:45] bd808: If we decide to go with the WebResponseSetCookie hack I mentioned in https://phabricator.wikimedia.org/T49647#2017019, someone should be able to make that happen reasonably quickly (less than an hour, probably). [22:45:19] * anomie merges T126550 into T49647 [22:48:12] Why doesn't that hook already allow changing the options? [22:48:20] that seems like an oversite [22:50:01] 14:42 Depends on how many people actually log in on those few mobile domains. This was probably present in wmf.11 as well and no one ever reported it. [22:50:20] I see about 10 reqs / min on a single text varnish to /commons\.m.*UserLogin/ [22:50:37] And yet no one reported it... [22:51:07] roll back commons to wmf12 [22:51:31] fixing this is probably less time than rolling back [22:52:01] wgCentralAuthCookieDomain is not overridden anywhere so just have CASessionProvider use that for now? [22:52:20] anomie, tgr, ori: hook change -- https://gerrit.wikimedia.org/r/#/c/269859/ [22:53:03] bd808: have you audited existing users of the hook to make sure they aren't expecting it to be passed by ref? [22:53:18] bd808: +2ed. Should probably be backported to both wmf.12 and wmf.13 so we don't have to worry about hook signature mismatch in wmf.12. [22:54:01] ori: no audit yet. looking via github now [22:54:14] seems unlikely, but worth a check [22:54:18] is this going to fix the problem? [22:54:29] github has 0 callers for the hook -- https://github.com/search?q=%40wikimedia+WebResponseSetCookie&type=Code&utf8=%E2%9C%93 [22:54:36] nice [22:56:55] bd808: https://gerrit.wikimedia.org/r/269861, seems to work when live-hacked on mw1017 along with your patch. [22:57:25] double-nice [22:57:31] nice. I was just starting to write that patch :) [22:58:32] bd808: I'll leave you to doing the backports and stuff. I have a hungry little boy to cook dinner for. [22:58:43] anomie: thanks [22:58:53] wasn't there some weird PHP issue when you send something as reference, but not define it in the function as such? [22:59:32] although if there are no callers it doesn't matter [22:59:37] there is something about that... AaronSchulz added logging for it right? [23:00:02] but yeah no callers found using a simple search [23:09:48] anomie: if DummySessionManager is in includes/installer then it probably would be in the root namespace, right? [23:10:24] DummySessionProvider rather [23:12:38] * AaronSchulz doesn't quite remember that [23:13:34] I don't think we have good conventions for that sort of thing yet TimStarling [23:13:54] "use your best judgment"? [23:14:15] I think namespaces should follow the directory hierarchy, which they mostly do, except for legacy modules which aren't in namespaces [23:14:26] except that "includes" is removed [23:16:35] we should probably do a bulk conversion to namespaced names, and deprecate the old ones [23:16:49] it's haphazard, introducing them randomly without an RFC [23:21:13] * robla hopes that TimStarling should turn the last few lines he wrote into an RFC :-) [23:26:13] TimStarling: we have PSR-4 [23:26:43] nowaways it is typical for stuff to go into /src instead of includes [23:26:51] and then follow the standard [23:28:35] psr-4 + namespaces would be awesome in core but it will break literally everything [23:28:55] class aliases for the rescue [23:28:58] it looks better than PSR-0 [23:29:24] yeah [23:29:48] when working on other extensions (than wikibase), i always forget to add a new class to $wgAutoloadClasses or autoload.php [23:30:18] because we generally don't need to do that in wikibase [23:30:57] it's not what we're (randomly) doing though, especially "The subdirectory name must match the case of the sub-namespace names" [23:31:23] so nobody is working towards PSR-4 [23:31:31] at least we could start with new stuff [23:31:43] as things are refactored [23:32:37] moving the layout to PSR-4 could be done for things that are already namespaced easily. adding namespaces for all the things at once is what is a bit scary [23:33:01] true :/ [23:34:16] yeah, it's unfortunate that there's no migration feature built into PHP [23:34:29] (there isn't is there?) [23:35:20] there's class_alias [23:35:55] right, yeah that could help [23:36:11] or more hacky, somethign like WikiPage extends Page/WikiPage maybe [23:36:18] for b/c [23:36:43] * aude notes we had some trouble with class_alias when switching to hhvm, but maybe the issues are fixed now [23:36:44] an alias may possibly avoid breaking type hinting [23:36:59] e.g. if an extension type-hints a function as a WikiPage and we pass a Page\WikiPage [23:37:05] https://phabricator.wikimedia.org/T73295 [23:37:49] aude: think they fixed it [23:38:00] https://static-bugzilla.wikimedia.org/bug69196.html [23:38:02] upstream [23:38:08] hoo: i hope so and probably by now [23:38:34] ah adrian fixed at least part of it :) [23:38:43] or reported it [23:44:03] ha [23:44:13] $builder = $this->getMockBuilder( 'DummySessionProvider' ) [23:44:13] ->setMethods( array( 'persistsSessionId', 'sessionIdWasReset' ) ); [23:44:31] actually the unnamespaced name is already used [23:44:35] see, namespaces ftw [23:48:56] is it really scary to do everything at once? Wouldn't any breakage be immediately obvious? [23:49:27] maybe [23:49:43] btw I am committing code right now that is unreachable [23:51:38] it's obvious in code that is covered by unit tests or otherwise tested [23:54:22] ori: immediate, yes. obvious not always. We had some lingering fubars in namespacing OAuth when it had a classname collision with hhvm [23:54:41] mostly exception classes as I remember [23:55:13] things tucked in little used branches can sneak by pretty easily with our typical test coverage ratio [23:58:46] tgr: bd808 : we have a whole scale outage going for ci [23:58:47] ori: class loading is only triggered when the branch is hit so for a rarely used branch it's not that obvious, but the worse problem is that instanceof-style checks will silently fail [23:58:48] sorry :( [23:59:07] bd808: tgr will try to get your pending wmf patch ahead, but feel free to force merge them if that blocks you [23:59:09] as php does not always check whether the class exists there [23:59:29] tgr: anomie may just have found the problem with his config patch [23:59:32] and if you are really unlucky, it does exist, it's just a different class [23:59:39] which is what happened with OAuth