[00:37:23] legoktm: regarding https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/480419/ ... [00:37:47] * legoktm looks [00:37:57] can you think of any way that early assignment to $wgAutoloadClasses in CommonSettings.php might be overwritten later? [00:39:08] I merged that change yesterday, it didn't work in beta from mediawiki-staging, there was a "class not found" error from logging.php which uses the new XWikimediaDebug class [00:39:10] is that before DefaultSettings.php? [00:39:14] no [00:39:49] so I commented out the reference in logging.php, and then it was able to get into maintenance/eval.php [00:40:04] weirdly, since there is also a reference to XWikimediaDebug in CommonSettings.php [00:40:25] but class_exists() then returned false and the entry was absent from the $wgAutoloadClasses array [00:40:38] hmm [00:40:55] oh, I should have dumped get_included_files() [00:41:27] does using the autoloader provide any benefit here? [00:41:33] the class seems to be called unconditionally [00:42:28] the file is already included from PhpAutoPrepend.php [00:42:58] so the autoloader shouldn't be invoked unless PHP was started without PhpAutoPrepend.php [00:43:59] we could do require_once, I'm just not sure whether it is faster than adding an entry to the autoloader array [00:44:08] at least it'll definitely work, I guess [00:44:54] ServiceConfig, which I am introducing in the subsequent commit https://gerrit.wikimedia.org/r/#/c/operations/mediawiki-config/+/477956/ , is much the same [00:45:07] I did [00:45:07] since we have to load the class unconditionally it seems like loading it right away would be slightly faster (but significantly more straightforward) instead of going through the autoloader and then loading it [00:45:11] AutoLoader::$psr4Namespaces['Wikimedia\MWConfig'] = __DIR__ . '/../src'; [00:45:15] which you might find hackish [00:45:55] yes, very [00:46:01] it's marked @private [00:46:16] yeah but there was no other way to add a PSR-4 namespace from config [00:46:18] we already have a composer autoloader in that repo, why not use that? [00:46:50] use it how? [00:47:16] https://getcomposer.org/doc/01-basic-usage.md#autoloading [00:48:01] right now we're loading vendor/autoload.php if some specific conditions are met in wmf-config/profiler.php [00:48:44] the normal case will be PhpAutoPrepend reading both these files with require_once, and then CommonSettings.php will not do anything and the autoloader will not be used [00:49:19] in what cases is the auto prepend file not used? [00:50:52] CLI [00:51:26] this wasn't clear from how the code was already, but auto_prepend_file is currently only set in HHVM's server.ini, not in php.ini [00:52:18] it is the same in PHP 7.2 [00:52:30] it is in fpm/php.ini but not cli/php.ini [00:55:05] doing require_once twice is probably fastest [00:55:34] if there were 100 classes in that src directory, maybe not [00:56:06] ClassLoader::addPsr4() is implemented using array_merge() on the existing PSR-4 array, which won't be fast [00:57:09] sorry that is in the case that the namespace is already registered [00:57:10] tangentially, I'm getting more concerned about the amount of code in that repo given the repository isn't really set up for it to be so complex [00:57:21] still, there are extra lines of code [00:58:27] you can see with these patches that I'm trying to write code with a level of complexity that the repo deserves, instead of having require_once spaghetti [00:58:41] having everything in the file scope is getting old [01:00:11] looks like we have a 1:1 scheduled now [06:08:22] <_joe_> TimStarling: uhm didn't I add the auto_prepend_file to cli too? [06:08:29] <_joe_> if that's the case, I just forgot [06:09:00] <_joe_> (I'll read backlog better after a coffee) [07:23:32] <_joe_> I have a question for people with more recent php experience than me: why is "a: " . $a preferred to sprintf("a: %s", $a) ? [07:23:55] <_joe_> is it more performant or just more "php-like"? [07:42:17] it's definitely more "php-like" to use string concatenation rather than sprintf [07:48:39] <_joe_> ok [07:49:30] <_joe_> what can one expect of a language with truly equal and truly not equal operators [07:49:32] <_joe_> :P [07:49:50] <_joe_> yes, I just had to do an if strpos() !== 0 [09:52:38] Reedy: For when you have a little bit of time: https://gerrit.wikimedia.org/r/c/mediawiki/core/+/480655 [10:15:47] Thanks [10:16:17] I was like, how did we miss that [10:16:34] Then I noticed I picked it up in the original CR, but obviously didn't check the updated patch in so much detal :D [20:06:26] anomie: does https://gerrit.wikimedia.org/r/c/mediawiki/core/+/480785 need a backport? I don't remember when that stuff landed [20:07:39] legoktm: I don't see the problematic code in ApiBlock.php in REL1_32. [20:08:22] Looks like 0813c46daaa7ca1b52008633a5eb8a769df67137 was first in 1.33.0-wmf.2. [20:11:12] ack, thanks [20:15:01] after digging around MDN, https://developer.mozilla.org/en-US/docs/Glossary/Forbidden_header_name says "Note: The User-Agent header is no longer forbidden..." :) [20:23:40] Yeah, that's where I first found that some spec was changed. After you pointed out the spec had a repo on github, I tracked down the actual change as https://github.com/whatwg/fetch/commit/dab09b0c483c46324082df1e54b29ed4c9c02162. [21:54:10] TimStarling, so, are we missing some option to the HTMLFormatter or Serializer (remex).. since we are seeing {"href":".\/Niccol\u00f2_de'_Conti"} (remex) instead of {"href":"./Niccolò_de'_Conti"} (domino) .. see the \u00f2 there. [22:22:11] Something funky with AbuseFilter and/or MCR? T212352 [22:22:11] T212352: Fatal error from https://en.wikipedia.org/wiki/Special:AbuseLog/8229629 - https://phabricator.wikimedia.org/T212352 [22:33:07] bd808: Wild guess: AbuseFilter serialized the whole Article object, and it fails to work now that Revision has become a wrapper around RevisionRecord. There might be another task about that same issue that that one could be marked as a duplicate of. [22:35:13] looks like it could be T187153? [22:35:14] T187153: Special:Abuselog throws when viewing details or examining (BadMethodCallException: Call get getId() on null) - https://phabricator.wikimedia.org/T187153 [22:35:23] T187153 [22:35:31] Ha, you beat me to it [23:36:31] bpirkle: noticed the issue fixed by https://gerrit.wikimedia.org/r/#/c/mediawiki/core/+/479899/ still in the logs [23:36:38] Is it scheduled for deploy yet? [23:37:15] Oh, I guess wmf.9 tomorrow also fixes it.