[00:17:13] <legoktm>	 MaxSem: wanna take a look at https://gerrit.wikimedia.org/r/#/c/385322/ ?
[00:36:04] <paladox>	 legoktm wondering could you review / merge https://gerrit.wikimedia.org/r/#/c/193434/ please?
[00:36:24] <legoktm>	 paladox: yes, well, it's on my review list for today
[00:36:29] <paladox>	 it is to do with supporting svgs with png fallback
[00:36:44] <paladox>	 ah
[00:36:47] <paladox>	 thanks :) :) :)
[00:38:03] <no_justification>	 legoktm: Couldn't that live in includes/libs since it doesn't really depend on anything else in MW?
[00:38:32] <legoktm>	 no_justification: it depends on Shell::command() which is still dependent upon MW things
[00:38:39] <no_justification>	 Ahh, missed that
[00:38:42] <no_justification>	 Ok no worries
[00:42:45] <no_justification>	 I don't like that we use a wrapped is_executable() + error suppression just for the purposes of also checking if it exists, etc.
[00:42:54] * no_justification shrugs
[00:42:58] <no_justification>	 Not super big deal
[00:47:59] <legoktm>	 no_justification: I'm actually not sure what the error suppression is for, maybe my settings are wrong, but I don't get warnings for is_executable('/bin/foobar')
[00:49:04] <legoktm>	 https://secure.php.net/manual/en/function.is-executable.php  Upon failure, an E_WARNING is emitted. 
[00:49:07] <legoktm>	 sigh, that's dumb
[00:51:14] <legoktm>	 no_justification: so we can check file_exists() first, but we still have to suppress warnings either way
[00:52:28] <no_justification>	 Yeah because file_exists() emits E_WARNING too
[00:52:30] <no_justification>	 Eh, ok
[00:52:38] <no_justification>	 Silly php
[00:52:57] <no_justification>	 Also: do we need an in-memory cache since we've got the stat cache anyway
[00:52:58] <no_justification>	 ?
[00:53:12] <no_justification>	 It's also not like we call this in any tight loops :)
[00:54:10] <legoktm>	 oh. I kind of forgot about statcache
[00:55:03] <legoktm>	 in my follow-up I was going to start calling it for every invocation of Shell::command() if firejail is enabled, but even then that's not very often
[00:55:06] <legoktm>	 I'll get rid of it
[01:04:54] <no_justification>	 I wonder if there's other things we do in-memory caching of that doesn't really need it
[01:05:06] <no_justification>	 Probably a fair bit--pretty common defensive programming mechanism
[01:15:52] <Krinkle>	 no_justification: Note that the warning is for "failure", afaik not being a file or not existing is not a failure for file_exists.
[01:16:00] <Krinkle>	 Must be some kind of other failure
[01:17:57] <Krinkle>	 What about try first and recover later? Checks are prone to race conditions and often redundant with what the real code eventually does somewhere. Python's EAFP guideline is quite nice. Been using it in a few places for ResourceLoader to simplify and speed up c ode.
[01:25:44] <legoktm>	 EAFP?
[02:42:40] <Krinkle>	 Easier to ask for forgiveness than permission
[02:42:59] <Krinkle>	 In contrast to LBYL (look before you leap)