Page MenuHomePhabricator
Create Task
Maniphest T263500

phan-taint-check-plugin: Undefined constant 'ast\AST_LIST'
Closed, ResolvedPublic
Actions

Description

This is breaking Parsoid's CI: (cf https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/628943)

> phan -p --allow-polyfill-parser --config-file=.phan/standalone.php --long-progress-bar
17:06:48 Parsing files...
[...]
17:06:57 Analyzing methods...
17:06:59 ░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░░ 722MB
17:06:59 Analyzing files...
17:06:59 ERROR: Error: Undefined constant 'ast\AST_LIST' in /workspace/src/services/parsoid/vendor/mediawiki/phan-taint-check-plugin/src/TaintednessBaseVisitor.php:972
17:06:59 Stack trace:
17:06:59 #0 /workspace/src/services/parsoid/vendor/mediawiki/phan-taint-check-plugin/src/TaintednessVisitor.php(527): TaintednessVisitor->getPhanObjsForNode(Object(ast\Node))
17:06:59 #1 /workspace/src/services/parsoid/vendor/mediawiki/phan-taint-check-plugin/src/MWVisitor.php(834): TaintednessVisitor->visitAssign(Object(ast\Node))
17:06:59 #2 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Plugin/ConfigPluginSet.php(1165): MWVisitor->visitAssign(Object(ast\Node))
17:06:59 #3 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Plugin/ConfigPluginSet.php(1093): MWVisitor::Phan\Plugin\{closure}(Object(Phan\CodeBase), Object(Phan\Language\Context), Object(ast\Node), Array)
17:06:59 #4 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Plugin/ConfigPluginSet.php(298): Phan\Plugin\ConfigPluginSet::Phan\Plugin\{closure}(Object(Phan\CodeBase), Object(Phan\Language\Context), Object(ast\Node), Array)
17:06:59 #5 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(2497): Phan\Plugin\ConfigPluginSet->postAnalyzeNode(Object(Phan\CodeBase), Object(Phan\Language\Context), Object(ast\Node), Array)
17:06:59 #6 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(544): Phan\BlockAnalysisVisitor->postOrderAnalyze(Object(Phan\Language\Context), Object(ast\Node))
17:06:59 #7 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/AST/Visitor/KindVisitorImplementation.php(69): Phan\BlockAnalysisVisitor->visit(Object(ast\Node))
17:06:59 #8 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\AST\Visitor\KindVisitorImplementation->visitAssign(Object(ast\Node))
17:06:59 #9 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(233): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #10 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitStmtList(Object(ast\Node))
17:06:59 #11 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(2378): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #12 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitMethod(Object(ast\Node))
17:06:59 #13 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(233): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #14 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitStmtList(Object(ast\Node))
17:06:59 #15 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(1337): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #16 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(2346): Phan\BlockAnalysisVisitor->visitClosedContext(Object(ast\Node))
17:06:59 #17 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(572): Phan\BlockAnalysisVisitor->visitClass(Object(ast\Node))
17:06:59 #18 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/BlockAnalysisVisitor.php(233): Phan\BlockAnalysisVisitor->analyzeAndGetUpdatedContext(Object(Phan\Language\Context), Object(ast\Node), Object(ast\Node))
17:06:59 #19 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/AST/Visitor/KindVisitorImplementation.php(35): Phan\BlockAnalysisVisitor->visitStmtList(Object(ast\Node))
17:06:59 #20 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Analysis.php(560): Phan\AST\Visitor\KindVisitorImplementation->__invoke(Object(ast\Node))
17:06:59 #21 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Phan.php(526): Phan\Analysis::analyzeFile(Object(Phan\CodeBase), 'src/Config/Api/...', NULL, NULL)
17:06:59 #22 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Phan.php(583): Phan\Phan::Phan\{closure}(0, 'src/Config/Api/...', 232)
17:06:59 #23 /workspace/src/services/parsoid/vendor/phan/phan/src/Phan/Phan.php(376): Phan\Phan::finishAnalyzingRemainingStatements(Object(Phan\CodeBase), NULL, Array, Array)
17:06:59 #24 /workspace/src/services/parsoid/vendor/phan/phan/src/phan.php(36): Phan\Phan::analyzeFileList(Object(Phan\CodeBase), Object(Closure))
17:06:59 #25 /workspace/src/services/parsoid/vendor/phan/phan/phan(9): require_once('/workspace/src/...')
17:06:59 #26 {main}
17:06:59 (Phan 2.6.1 crashed due to an uncaught Throwable when parsing/analyzing 'src/Config/Api/ApiHelper.php')
17:06:59 Script phan -p --allow-polyfill-parser --config-file=.phan/standalone.php --long-progress-bar handling the phan event returned with error code 1
17:06:59 Script @phan was called via test
[...]

We keep running into variants of this bug, but this started appearing just today (a new version of phan-taint-check-plugin?). There have been no changes to src/Config/Api/ApiHelper.php since May 21, 2020 and no changes to composer.json since July 23, 2020.

Details

SubjectRepoBranchLines +/-
Release Quibble 0.0.46integration/quibblemaster+74 -2
build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR usemediawiki/services/parsoidREL1_35+2 -2
Only run linters for extensions/skinsintegration/quibblemaster+21 -2
build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR usemediawiki/services/parsoidmaster+2 -2
Customize query in gerrit

Related Objects

Event Timeline

cscott created this task.Sep 21 2020, 9:45 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 21 2020, 9:45 PM
Jdforrester-WMF subscribed.Sep 21 2020, 9:45 PM

Sounds like the base image was changed and no longer has ext-ast?

Daimona subscribed.Sep 21 2020, 9:47 PM

First seen as T251346, perhaps the comeback is related to T263313. Using that job would fix this, as it only happens when ext-ast is not installed (not just avoided with --force-polyfill-parser)

Daimona closed this task as a duplicate of T262451: Release bugfix for ast\AST_LIST in phan-taint-check-plugin to unstuck libup updates on some repos.Sep 21 2020, 9:47 PM
cscott reopened this task as Open.Sep 21 2020, 9:47 PM
cscott updated the task description. (Show Details)
Jdforrester-WMF triaged this task as Unbreak Now! priority.Sep 21 2020, 9:52 PM

Unable to merge production code -> UBN.

cscott added a comment.Sep 21 2020, 9:53 PM
[17:42:21] <subbu> cscott, yes .. i saw a phab email over the weekend .. hashar iirc.
[17:43:50] <subbu> https://phabricator.wikimedia.org/T227352#6474369
[17:45:30] <James_F> cscott: Ah, yes.
[17:45:44] <cscott> https://phabricator.wikimedia.org/T263500 <- parsoid CI breaking
[17:46:26] <wikibugs> (CR) jerkins-bot: [V: -1] WIP: phan: Remove --allow-polyfill-parser [services/parsoid] - https://gerrit.wikimedia.org/r/628952 (owner: C. Scott Ananian)
[17:46:39] <cscott> i removed the --allow-polyfill-parser option in https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/628952 we'll see if that's enough to fix it
[17:47:54] <James_F> It won't, it'll just fail earlier.
[17:49:03] <cscott> James_F: gr8
[17:49:42] <James_F> The polyfill isn't complete, for some reason.
[17:49:46] -*- James_F rolls his eyes.
[17:50:09] <cscott> it's actually failing when running `composer test` AFAICT, but only in the quibble-noselenium case, not in the parsoidsvv-composer-package jobs, etc.
[17:50:30] <James_F> Yeah, the new quibble job is probably mis-built by hashar.
[17:50:43] <James_F> Mark the task as UBN and make it his problem when he wakes up tomorrow?
[17:51:40] <cscott> James_F: Daimona already closed this as a dup of T262451
[17:51:41] <stashbot> T262451: Release bugfix for ast\AST_LIST in phan-taint-check-plugin to unstuck libup updates on some repos - https://phabricator.wikimedia.org/T262451
[17:51:51] <James_F> cscott: Tough.
[17:52:04] <cscott> which is two weeks old
[17:52:38] <James_F> Yeah, that's a minor "nice to have". This is UBN.
cscott assigned this task to hashar.Sep 21 2020, 9:53 PM
Daimona added a comment.Sep 21 2020, 10:07 PM
In T263500#6481769, @Jdforrester-WMF wrote:

Unable to merge production code -> UBN.

Ah, if that's the case yes, and sorry for closing as dupe. I'd still like to investigate the feasibility of T262451 so we won't have this bug return in another couple of months. However, yes, the quick fix is to add php-ast to the image in the meanwhile.

hashar added a comment.Sep 22 2020, 7:35 AM

There are two root causes:

On Monday I have upgraded Quibble on CI with a logic change has to when we run composer test for the repository that triggered the job. Previously we ran composer test when the repository was either an extension or a skin (as determined by its name). Given Parsoid is not under mediawiki/extensions nor under mediawiki/skins, we never ran composer test for it!

In Quibble 0.0.45 the logic has been changed to anything NOT being mediawiki/core bd7c3c94a93a52a7840bbee5cb88bfde90373ab2:

quibble.cmd.py
- if quibble.util.isExtOrSkin(zuul_project):
+ if not is_core:
    run_composer = 'composer-test' in stages

Parsoid comes with mediawiki/phan-taint-check-plugin → 3.0.2, the next minor version includes a commit named Remove reference to AST_LIST ( 4364065f2be876c69515f195168cd4fb8c11f698 ).

@Daimona wrote a very nice commit message with all the references pointing out \ast\AST_LIST has been removed from PHP and from Phan as of of version 0.8.4 (Parsoid has 2.6.1). That got dropped in phan-taint-check-plugin → 3.0.3 T251346

The plugin is installed from mediawiki/mediawiki-phan-config. It has an unreleased commit to bump phan-taint-check-plugin 918be7aee617e5b3223548343a287395a3066c70 and refers again to T251346: ERROR: Error: Undefined constant 'ast\AST_LIST' .

What is needed?

  • cut a new version of mediawiki/mediawiki-phan-config
  • upgrade it in Parsoid composer.json
  • Determine Quibble behavior related to extra repositories such as mediawiki/services/parsoid or mediawiki/tools/api-testing. I guess it should never run composer test or npm test which for those repositories should be handled by standalone jobs instead of by Quibble. For Parsoid that is parsoidsvc-composer-package-php7*-docker jobs.

Determine what the logic should be in Quibble when handling mediawiki/services/parsoid, probably it should just be determined as not core.

hashar added projects: Quibble, Release-Engineering-Team (CI & Testing services), Release-Engineering-Team-TODO (2020-07-01 to 2020-09-30 (Q1)).Sep 22 2020, 7:35 AM
gerritbot added a comment.Sep 22 2020, 8:01 AM

Change 629057 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/quibble@master] tests: plan for mediawiki/services/parsoid

https://gerrit.wikimedia.org/r/629057

gerritbot added a project: Patch-For-Review.Sep 22 2020, 8:01 AM
hashar added a comment.Sep 22 2020, 8:02 AM

TLDR, the new Quibble 0.0.45 now runs those extra steps:

  • Extension and skin tests: composer, npm
  • PHPUnit default standalone suite on services/parsoid
gerritbot added a comment.Sep 22 2020, 8:14 AM

Change 629060 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/tools/phan@master] Release 0.10.3

https://gerrit.wikimedia.org/r/629060

gerritbot added a comment.Sep 22 2020, 2:25 PM

Change 629135 had a related patch set uploaded (by Hashar; owner: Hashar):
[mediawiki/services/parsoid@master] build: update mediawiki-phan-config to 0.10.3

https://gerrit.wikimedia.org/r/629135

hashar added a comment.Sep 22 2020, 2:27 PM

status

The workaround is to upgrade mediawiki-phan-config to 0.10.3 which is https://gerrit.wikimedia.org/r/c/mediawiki/services/parsoid/+/629135
That is pending a git tag in the mediawiki/tools/phan repository and packagist.org to catch up on the tag to make it available.

Thanks @Daimona for the assistance!

I have send a patch for Quibble in order to no more run composer test and npm test for repositories that are neither an extension or a skin. That will definitely address the issue but it needs review, a new Quibble release and an update of all jobs. So that is a bit longer to achieve.

gerritbot added a comment.Sep 22 2020, 4:55 PM

Change 629135 merged by C. Scott Ananian:
[mediawiki/services/parsoid@master] build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR use

https://gerrit.wikimedia.org/r/629135

hashar changed the task status from Open to Stalled.Sep 22 2020, 9:08 PM
hashar lowered the priority of this task from Unbreak Now! to Medium.

No more an UBN since CI lets patches pass now. It is essentially fixed but I am keeping this task around to not forget to get Quibble updated. Marking as stalled meanwhile.

thcipriani moved this task from INBOX to New Work on the Release-Engineering-Team-TODO (2020-07-01 to 2020-09-30 (Q1)) board.Sep 23 2020, 5:29 PM
hashar edited projects, added Release-Engineering-Team-TODO (2020-10-01 to 2020-12-31 (Q2)); removed Release-Engineering-Team-TODO (2020-07-01 to 2020-09-30 (Q1)).Sep 24 2020, 10:24 AM
hashar moved this task from INBOX to Maintenance on the Release-Engineering-Team-TODO (2020-10-01 to 2020-12-31 (Q2)) board.Oct 14 2020, 8:34 AM
gerritbot added a comment.Oct 30 2020, 11:33 AM

Change 629057 merged by jenkins-bot:
[integration/quibble@master] Only run linters for extensions/skins

https://gerrit.wikimedia.org/r/629057

Daimona merged a task: T268860: phan/phan-taint-check-plugin issues on Parsoid/REL1_35.Nov 27 2020, 10:08 AM
Daimona added a subscriber: Reedy.
gerritbot added a comment.Nov 28 2020, 2:59 AM

Change 643953 had a related patch set uploaded (by Reedy; owner: Hashar):
[mediawiki/services/parsoid@REL1_35] build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR use

https://gerrit.wikimedia.org/r/643953

gerritbot added a comment.Dec 16 2020, 11:02 PM

Change 643953 merged by Legoktm:
[mediawiki/services/parsoid@REL1_35] build: update mediawiki-phan-config to 0.10.3 and fix TMPDIR use

https://gerrit.wikimedia.org/r/643953

gerritbot added a comment.Jan 7 2021, 9:40 AM

Change 654804 had a related patch set uploaded (by Hashar; owner: Hashar):
[integration/quibble@master] Release Quibble 0.0.46

https://gerrit.wikimedia.org/r/654804

gerritbot added a comment.Jan 7 2021, 3:35 PM

Change 654804 merged by jenkins-bot:
[integration/quibble@master] Release Quibble 0.0.46

https://gerrit.wikimedia.org/r/654804

Stashbot mentioned this in T266441: MediaWikiUnitTestCase should prevent access to MediaWikiServices.Jan 7 2021, 4:01 PM

Mentioned in SAL (#wikimedia-releng) [2021-01-07T16:01:18Z] <hashar> Tag Quibble 0.0.46 @ df9e75329ab # T225218 T266441 T263500

Stashbot mentioned this in T225218: Consider httpd for quibble instead of php built-in server.Jan 7 2021, 4:01 PM
hashar mentioned this in T274590: Upgrade CI jobs to use Quibble 0.0.46.Feb 11 2021, 8:59 PM
Daimona closed this task as Resolved.Mar 14 2021, 2:37 PM

Everything was done.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL