[01:06:55] Reedy: oh, right, I dont' remember doing that at all xD [13:08:42] xSavitar: Do you want to break MW Core? :D [13:44:20] Why would phan be complaining about files not touched... never mind complaining at all at just renaming a folder in the file path? [13:59:35] Reedy, heh, I don't want to break core :D [14:00:28] yeah Phan is complaining, I will get back to working on those patches again [14:00:45] But I wonder why even for the tiniest renames I've done so far causes phan to complain [14:01:10] I was even thinking of doing it one file at a time but that can take forever to complete [14:13:36] yep, tried doing one directory (https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1199787) and it still causes issues [14:13:39] lovely phan [14:36:30] zabe, interesting. So even 1 dir also annoys phan. Okay! So we may have to do it file by file? That will be painful as Reedy already saw the future :D [14:36:52] how many PHP files do we have in MW core? :D [14:37:00] 3000 ish? [14:41:05] I did used `cloc` [14:41:11] mwcli [master] cloc --include-lang=PHP --exclude-dir=vendor,extensions . [14:41:11] 14154 text files. [14:41:12] 13460 unique files. [14:41:12] 8851 files ignored. [14:41:12] github.com/AlDanial/cloc v 2.06 T=13.86 s (400.1 files/s, 82649.5 lines/s) [14:41:12] ------------------------------------------------------------------------------- [14:41:12] Language files blank comment code [14:41:13] ------------------------------------------------------------------------------- [14:41:14] PHP 5546 118754 280783 746016 [14:41:15] ------------------------------------------------------------------------------- [14:41:17] SUM: 5546 118754 280783 746016 [14:41:19] ------------------------------------------------------------------------------- [14:41:26] It looks like we have over 5000? Hm [14:42:24] Maybe I should also exclude maint scripts [14:42:33] Also language directory [14:42:51] iirc its ~3000 in includes and ~2000 in tests or something like that [14:43:28] Yes, I think you're right, I did `cloc --include-lang=PHP --exclude-dir=vendor,extensions,languages,maintenance,tests .` [14:43:35] Got [14:43:36] Language files blank comment code [14:43:36] ------------------------------------------------------------------------------- [14:43:37] PHP 3061 69566 221379 381278 [14:44:03] ~3,000 patches is going to be wild [14:44:50] just turn off phan for a while? xD [14:45:40] That's an idea for sure but that could also mean if there are things that Phan can help us catch, we won't be able to catch them in the process [14:46:22] But it's just renaming so maybe worth a try [14:47:48] I would take the slow route of renaming file by file over a period of time if there is an active reviewer to +2 things. But we'll need more hands to make it faster [14:47:56] Why is node failing on yours too? [14:49:04] >The best way to prevent Phan from analyzing a specific file or directory is to add it to the exclude_file_list in your configuration file. This is the recommended approach for files that are moved, removed, or are otherwise out of sync with your project's state during a migration. [14:49:07] Hmm... [14:49:28] Patch for a patch exclude... patch... patch to revert the first? [14:49:55] Annoying, but arguably better than doing it file by file renames [14:50:44] And you can +2 the three in a stack and just merge the lot in one go [14:50:53] rather than having extended periods with phan disabled [14:51:12] re node failing, they look like warnings and legit ones by the way. [14:51:28] the file by file renames, I didn't want to disable phan with that strategy [14:51:35] yeah, but why does it result in an actual fail? [14:52:02] I remember node will fail with warnings in CI right? Or are my missing something? [14:52:23] The thing I don't understand is why does renaming a PHP file(s) cause JS warnings all over the place [14:52:30] It shouldn't... but you're also not even touching those files [14:52:38] exactly [14:52:51] it doesn't fail on zabe's patch.. so I wonder if one of the api changes is actually breaking something more silently [14:52:53] We don't have those warnings today failing CI do we? [14:53:14] maybe it is breaking something [14:53:36] but knowing exactly which file now is going to be non-trivial [14:53:53] I guess you could do auth seperate to api for example [14:54:47] Ack! [14:55:13] if we know it's just the api code being even more annoying... [14:56:12] I guess we need to be careful to have includes/api and includes/Api simultaneously as that will upset some peoples setups depending on the OS file system case sensitivity [15:02:12] looks like phan still doesn't like it [15:02:15] The phan failures are persistant, https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1199803 [15:03:04] does phan do some caching? [15:04:30] https://phabricator.wikimedia.org/T408700 filed... :( [15:04:46] Reedy, from my little understanding, yes. Phan relies heavily (by default I think) on caching when analysing, parsing and building the AST. [15:05:13] But I don't know how things are configured in production [15:05:35] And caching can also be disabled too (I've tried it locally), but it makes it very slow since it has to redo everything everytime [15:14:45] Reedy, zabe, majority of the failures are SecurityCheck-* and it seems like I've seen this before. Trying to find the task. [15:15:29] Daimona or Matmarex may have a clue on what is happening or atleast how we can work around it [15:17:31] https://phabricator.wikimedia.org/T401748 [15:18:43] That ticket relates to a patch that didn't touch a file and Phan started reporting about it [15:19:18] Then matmarex filed a task and the issue got addressed. So maybe there is a similarity between that and other parts of the codebase (triggered during renames)? [15:19:42] Or maybe anything that causes phan to re-analyze the file? [15:37:58] There's maybe some smaller batches we can try (sub folders?) in the meantime... [15:58:07] while we are talking about phan, may I convince someone to review https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikimediaMaintenance/+/1196953? :) [15:59:05] maybe when CI +2s :P [15:59:53] ah, its slow [16:00:40] now CI +2'ed [16:03:39] gonna need a few doc updates on wikitech [17:13:14] Updated the places I found: [18:02:44] zabe: there are scheduled maintenance scripts as well, at least for blameStartupRegistry [18:02:55] might be hard to deploy given different wiki versions on different wikis [18:03:04] maybe leave a symlink for a week? [18:03:33] sounds like a plan [18:05:32] from what I can tell it is only blameStartupRegistry and sendVerifyEmailReminderNotification [18:05:38] and scap is using dumpInterwiki [18:10:31] https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikimediaMaintenance/+/1199834 [18:20:47] zabe: I confirmed that symlinks work via mw-experimental + https://phabricator.wikimedia.org/T405688#11302125 [18:21:21] I moved blameStartup and WikimediaMaint to a maint subfolder and confirmed the previous invokation fails with not found, and that creating the symlink makes it work again [18:21:37] wasn't sure whether symlinks work through all the layers involved there [22:45:34] Reedy: I can do a general cleanup of RELEASE-NOTES-1.45 while I'm adding a note for that library if you like