Page MenuHomePhabricator

Scap sync fails because it tries to lint test files
Open, Needs TriagePublic

Description

tgr@deploy1001:/srv/mediawiki-staging$ scap sync-file php-1.32.0-wmf.18/vendor/ 'SWAT: [[gerrit|455770|Update wikimedia/css-sanitizer to 2.0.0 (T197617)]]'
           ___ ____
         ⎛   ⎛ ,----
          \  //==--'
     _//|,.·//==--'    ____________________________
    _OO≣=-  ︶ ᴹw ⎞_§ ______  ___\ ___\ ,\__ \/ __ \
   (∞)_, )  (     |  ______/__  \/ /__ / /_/ / /_/ /
     ¨--¨|| |- (  / ______\____/ \___/ \__^_/  .__/
         ««_/  «_/ jgs/bd808                /_/

11:47:21 Unhandled error:
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/scap/cli.py", line 339, in run
    exit_status = app.main(app.extra_arguments)
  File "/usr/lib/python2.7/dist-packages/scap/main.py", line 681, in main
    return super(SyncFile, self).main(*extra_args)
  File "/usr/lib/python2.7/dist-packages/scap/main.py", line 67, in main
    self._before_cluster_sync()
  File "/usr/lib/python2.7/dist-packages/scap/main.py", line 701, in _before_cluster_sync
    lint.check_valid_syntax(abspath, utils.cpus_for_jobs())
  File "/usr/lib/python2.7/dist-packages/scap/lint.py", line 50, in check_valid_syntax
    subprocess.check_call(cmd, shell=True)
  File "/usr/lib/python2.7/subprocess.py", line 186, in check_call
    raise CalledProcessError(retcode, cmd)
CalledProcessError: Command 'find -O2 '/srv/mediawiki-staging/php-1.32.0-wmf.18/vendor/' -not -type d -name '*.php' -not -name 'autoload_static.php'  -or -name '*.inc' | xargs -n1 -P30 -exec php -l >/dev/null 2>&1' returned non-zero exit status 123
11:47:21 sync-file failed: <CalledProcessError> Command 'find -O2 '/srv/mediawiki-staging/php-1.32.0-wmf.18/vendor/' -not -type d -name '*.php' -not -name 'autoload_static.php'  -or -name '*.inc' | xargs -n1 -P30 -exec php -l >/dev/null 2>&1' returned non-zero exit status 123

tgr@deploy1001:/srv/mediawiki-staging$ find -O2 '/srv/mediawiki-staging/php-1.32.0-wmf.18/vendor/' -not -type d -name '*.php' -not -name 'autoload_static.php'  -or -name '*.inc' | xargs -n1 -P30 -exec php -l

Fatal error: syntax error, unexpected T_CONST, expecting T_VARIABLE in /srv/mediawiki-staging/php-1.32.0-wmf.18/vendor/psy/psysh/test/ClassWithSecrets.php on line 16

Test files usually do not care much about backwards compatibility with older PHP versions so linting them fails (in this case on private class constants which is a PHP 7.1 feature). The vendor repo has a whitelist to work around that. Could scap use that?

Details

Related Gerrit Patches:

Event Timeline

Tgr created this task.Aug 28 2018, 12:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 28 2018, 12:00 PM

Change 455817 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/vendor@master] Add test directories from lint whitelist to gitignore

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

Tgr renamed this task from Scap fails due to lint error to Scap sync fails because it tries to lint test files.Aug 28 2018, 12:05 PM
Tgr updated the task description. (Show Details)

Change 455823 had a related patch set uploaded (by Gergő Tisza; owner: Gergő Tisza):
[mediawiki/vendor@wmf/1.32.0-wmf.18] Add test directories from lint whitelist to gitignore

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

Tgr added a project: Scap.Aug 28 2018, 12:07 PM
Tgr updated the task description. (Show Details)
Tgr updated the task description. (Show Details)
Tgr updated the task description. (Show Details)Aug 28 2018, 12:10 PM

Change 455823 merged by jenkins-bot:
[mediawiki/vendor@wmf/1.32.0-wmf.18] Add test directories from lint whitelist to gitignore

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

Change 455817 merged by jenkins-bot:
[mediawiki/vendor@master] Add test directories from lint whitelist to gitignore

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

Tgr added a comment.Aug 28 2018, 12:43 PM

Solved for now by adding all files from the lint whitelist to gitignore and removing them from the repo. In the longer term, either scap should call composer test (I guess non-ideal security-wise?) or the part about gitignore should be added to the readme file of the vendor repo.