Output of scap lint.py isn't helpful if it fails the php -l check
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	Reedy
	Jan 22 2021, 11:34 PM

Description

Testing the fixes for T270452: Add more preflight checks to makerelease2

Invalid JSON:

Linting PHP and JSON files for sanity...
Traceback (most recent call last):
  File "/var/www/wiki/mediawiki/tools/release/make-release/scap_lint.py", line 71, in check_valid_json_file
    json.load(json_file)
  File "/usr/lib/python3.8/json/__init__.py", line 293, in load
    return loads(fp.read(),
  File "/usr/lib/python3.8/json/__init__.py", line 357, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python3.8/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python3.8/json/decoder.py", line 353, in raw_decode
    obj, end = self.scan_once(s, idx)
json.decoder.JSONDecodeError: Expecting ',' delimiter: line 7 column 2 (char 168)

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "./makerelease2.py", line 269, in <module>
    main()
  File "./makerelease2.py", line 264, in main
    archive(args.repository, args.tag, args.output_dir, args.previous, sign=args.sign,
  File "./makerelease2.py", line 95, in archive
    scap_lint.check_valid_syntax('.', procs=multiprocessing.cpu_count())
  File "/var/www/wiki/mediawiki/tools/release/make-release/scap_lint.py", line 57, in check_valid_syntax
    check_valid_json_file(abspath)
  File "/var/www/wiki/mediawiki/tools/release/make-release/scap_lint.py", line 73, in check_valid_json_file
    raise ValueError("%s is an invalid JSON file" % path)
ValueError: ./composer.json is an invalid JSON file

That works.

Text before <?php

Linting PHP and JSON files for sanity...
Traceback (most recent call last):
  File "./makerelease2.py", line 269, in <module>
    main()
  File "./makerelease2.py", line 264, in main
    archive(args.repository, args.tag, args.output_dir, args.previous, sign=args.sign,
  File "./makerelease2.py", line 95, in archive
    scap_lint.check_valid_syntax('.', procs=multiprocessing.cpu_count())
  File "/var/www/wiki/mediawiki/tools/release/make-release/scap_lint.py", line 56, in check_valid_syntax
    check_php_opening_tag(abspath)
  File "/var/www/wiki/mediawiki/tools/release/make-release/scap_lint.py", line 115, in check_php_opening_tag
    raise ValueError("%s has content before opening <?php tag" % path)
ValueError: ./index.php has content before opening <?php tag

That works too.

But if we make an invalid php file (index.php again)....

Linting PHP and JSON files for sanity...
Traceback (most recent call last):
  File "./makerelease2.py", line 269, in <module>
    main()
  File "./makerelease2.py", line 264, in main
    archive(args.repository, args.tag, args.output_dir, args.previous, sign=args.sign,
  File "./makerelease2.py", line 95, in archive
    scap_lint.check_valid_syntax('.', procs=multiprocessing.cpu_count())
  File "/var/www/wiki/mediawiki/tools/release/make-release/scap_lint.py", line 50, in check_valid_syntax
    subprocess.check_call(cmd, shell=True)
  File "/usr/lib/python3.8/subprocess.py", line 364, in check_call
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command 'find -O2 '.' -not -type d -name '*.php' -not -name 'autoload_static.php'  -or -name '*.inc' | xargs -n1 -P2 -exec php -l >/dev/null 2>&1' returned non-zero exit status 124.

This error isn't very helpful, beyond knowing something is seemingly wrong :(

Details

Related Changes in Gerrit:

Subject	Repo	Branch	Lines +/-
lint.py: Supply useful output on lint fail	mediawiki/tools/scap	master	+44 -7
[WIP] Better handling of scap_lint errors	mediawiki/tools/scap	master	+4 -2
scap_lint.py: Output errors from `php -l`	mediawiki/tools/release	master	+20 -4

Customize query in gerrit

Related Objects

Mentioned In: T198423: Linting phase in scap doesn't show which file caused the error
Mentioned Here: T270452: Add more preflight checks to makerelease2

Event Timeline

Reedy created this task.Jan 22 2021, 11:34 PM

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 22 2021, 11:34 PM

Reedy updated the task description. (Show Details)Jan 22 2021, 11:37 PM

The "problem" is >/dev/null 2>&1

If we run it without the redirection, we do get something useful:

No syntax errors detected in ./docs/doxygen_first_page.php
No syntax errors detected in ./img_auth.php
PHP Parse error:  syntax error, unexpected '$mediaWiki' (T_VARIABLE) in ./index.php on line 42
Errors parsing ./index.php
No syntax errors detected in ./tests/phpunit/MediaWikiTestCase.php
xargs: php: exited with status 255; aborting

But obviously we don't care about hundreds of lines with No syntax errors detected

We could capture the output in Python and filter it on error?

might just need to remove the 2>&1 part, could use some experimentation.

thcipriani added a project: Release-Engineering-Team.Jan 23 2021, 12:18 AM

Change 657930 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/tools/release@master] scap_lint.py: Output errors from php -l

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

gerritbot added a project: Patch-For-Review.Jan 23 2021, 12:27 AM

The above ends up with

PHP Parse error:  syntax error, unexpected end of file in test.php on line 4
xargs: php: exited with status 255; aborting

The xargs line is a bit of extra noise, but we at least get the error...

PS4

reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ cat test.php
<?php

test
reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ python3 test.py 
PHP Parse error:  syntax error, unexpected end of file in test.php on line 4

reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ cat test.php 
<?php

test
reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ python3 test.py 
Linting PHP and JSON files for sanity...
Failed
PHP Parse error:  syntax error, unexpected end of file in test.php on line 4
reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ nano test.php 
reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ cat test.php 
<?php

reedy@ubuntu64-web-esxi:/var/www/wiki/mediawiki/tools/release/make-release$ python3 test.py 
Linting PHP and JSON files for sanity...
Finished linting PHP and JSON files

Change 657832 had a related patch set uploaded (by Reedy; owner: Reedy):
[mediawiki/tools/scap@master] Better handling of scap_lint errors

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

What version of python is scap/scap tests running on? The patch I've done for tools/release is fine... But isn't gonna work for scap..

In T272760#6770804, @Reedy wrote:

What version of python is scap/scap tests running on? The patch I've done for tools/release is fine... But isn't gonna work for scap..

python 2.7.13

And you're right: subprocess.run didn't exist in python2 so the solution could use some porting to Popen, I suppose.

thcipriani edited projects, added Release-Engineering-Team (Deployment services), Release-Engineering-Team-TODO; removed Release-Engineering-Team.Jan 26 2021, 5:06 PM

thcipriani moved this task from Needs triage to Debt on the Scap board.

thcipriani moved this task from Should be empty (use Release-Engineering-Team) to Later / Need volunteer on the Release-Engineering-Team-TODO board.

• LarsWirzenius subscribed.Jan 26 2021, 5:07 PM

I note that subprocess was added in Python 2.4 and that it's certainly in Python 2.7. In fact, I wish Scap used nothing else to run other programs.

In T272760#6965023, @LarsWirzenius wrote:

I note that subprocess was added in Python 2.4 and that it's certainly in Python 2.7. In fact, I wish Scap used nothing else to run other programs.

Never mind, I misread the previous comment, which is about subprocess.run specifically. That was added on 3.5.

Change 657930 merged by jenkins-bot:

[mediawiki/tools/release@master] scap_lint.py: Output errors from `php -l`

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

thcipriani removed a project: Release-Engineering-Team (Deployment services).Apr 20 2021, 1:10 AM

thcipriani edited projects, added Release-Engineering-Team (thcipriani-workboard-fiddling); removed Release-Engineering-Team-TODO.Apr 20 2021, 3:41 AM

thcipriani moved this task from thcipriani-workboard-fiddling to Seen (ARCHIVE) on the Release-Engineering-Team board.Apr 20 2021, 3:46 AM

thcipriani edited projects, added Release-Engineering-Team; removed Release-Engineering-Team (thcipriani-workboard-fiddling).

thcipriani edited projects, added Release-Engineering-Team (Seen); removed Release-Engineering-Team.Apr 20 2021, 3:23 PM

• LarsWirzenius unsubscribed.Jun 15 2021, 5:03 AM

Change 738024 had a related patch set uploaded (by Ahmon Dancy; author: Ahmon Dancy):

[mediawiki/tools/scap@master] lint.py: Supply useful output on lint fail

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

I ran into this today while experimenting, so I fixed it.

dancy changed the task status from Open to In Progress.Nov 10 2021, 11:12 PM

dancy edited projects, added Release-Engineering-Team (Doing); removed Release-Engineering-Team (Seen).Nov 10 2021, 11:52 PM

In T272760#7497476, @dancy wrote:

I ran into this today while experimenting, so I fixed it.

Thanks!

I was going to say when this is tested/verified/merged, we should sync it back into release tools... But is scap still running on oooold python?

In T272760#7497584, @Reedy wrote:

In T272760#7497476, @dancy wrote:

I ran into this today while experimenting, so I fixed it.

Thanks!

I was going to say when this is tested/verified/merged, we should sync it back into release tools... But is scap still running on oooold python?

Scap 4.0.0+ uses Python 3.

In T272760#7498901, @dancy wrote:

In T272760#7497584, @Reedy wrote:

In T272760#7497476, @dancy wrote:

I ran into this today while experimenting, so I fixed it.

Thanks!

I was going to say when this is tested/verified/merged, we should sync it back into release tools... But is scap still running on oooold python?

Scap 4.0.0+ uses Python 3.

Cool!

So we should be able to replace my hacky version to bring the files back into sync again

In T272760#6990000, @gerritbot wrote:

Change 657930 merged by jenkins-bot:

[mediawiki/tools/release@master] scap_lint.py: Output errors from `php -l`

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