Page MenuHomePhabricator

[OPS] Jenkins: Package php5-parsekit missing in Trusty
Closed, ResolvedPublic

Description

Trying to provision a Trusty machine with puppet with the roles for integration slaves fails right due. Among the errors is:

E: Unable to locate package php5-parsekit


Version: wmf-deployment
Severity: major
See Also:
https://bugzilla.wikimedia.org/show_bug.cgi?id=40414
https://rt.wikimedia.org/Ticket/Display.html?id=7935

Details

Reference
bz68255

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 3:30 AM
bzimport added a project: Deployments.
bzimport set Reference to bz68255.

I am moving this to deployment component. php5-parsekit is an indirect dependency of scap which lints PHP files:

scap/tasks.py: cmd = '/usr/bin/php -n -dextension=parsekit.so /usr/local/bin/lint.php'

Parsekit need to be imported from pecl and rebuild on Trusty. The last changelog entry for the Debian package being:

php-parsekit (1.3.0-1~precise1) precise-wikimedia; urgency=low

  • Build for precise.
  • Faidon Liambotis <faidon@wikimedia.org> Thu, 11 Oct 2012 13:21:05 +0000

Timo, simply fill a RT to ask parsekit to be rebuild on Trusty and the resulting package uploaded to apt.wikimedia.org :)

We should really find a better way to lint than using a PECL package that hasn't had a release since 2009-10-17. The version we have installed on 12.04 hosts core dumps if you ask it to parse a PHP file that defines a trait.

(In reply to Bryan Davis from comment #2)

We should really find a better way to lint than using a PECL package that
hasn't had a release since 2009-10-17. The version we have installed on
12.04 hosts core dumps if you ask it to parse a PHP file that defines a
trait.

That is unfortunate :-( The whole point of parsekit / lint.php is to avoid PHP startup overhead when one attempts to invoke php -l on each php/inc file. If PHP came with a recursive lint or a function to lint a file, that would let us drop parsekit entirely.

I duplicated the php5-parsekit version in our repo on Trusty. There are a million other deps that won't fall so easily though.

Having looked at this too I see that php5-parsekit may very well be non compilable on trusty. I just tried and got pages upon pages of errors. The first few are:

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c: In function 'php_parsekit_parse_node':
/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:81:66: error: 'union <anonymous>' has no member named 'var'

add_assoc_long(return_value, "var", node->u.var);
                                                               ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:82:92: error: 'union <anonymous>' has no member named 'var'

add_assoc_stringl(return_value, "varname", op_array->vars[node->u.var].name, op_array->vars[node->u.var].name_len, 1);
                                                                                         ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:82:126: error: 'union <anonymous>' has no member named 'var'

add_assoc_stringl(return_value, "varname", op_array->vars[node->u.var].name, op_array->vars[node->u.var].name_len, 1);
                                                                                                                           ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:88:77: error: 'union <anonymous>' has no member named 'var'

snprintf(sop, (sizeof(void *) * 2) + 1, "%X", (unsigned int)node->u.var); 
                                                                          ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:92:67: error: 'union <anonymous>' has no member named 'var'

add_assoc_long(return_value, "var", node->u.var / sizeof(temp_variable));
                                                               ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:117:36: error: 'union <anonymous>' has no member named 'var'

((unsigned int)((char*)node->u.var - (char*)op_array->opcodes))/sizeof(zend_op));
                              ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:126:75: error: 'union <anonymous>' has no member named 'EA'

add_assoc_long(return_value, "EA.type", node->u.EA.type);
                                                                       ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c: In function 'php_parsekit_parse_op':
/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:143:66: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]

add_assoc_long(return_value, "address", (unsigned int)(&(op->opcode)));
                                                                ^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:152:3: warning: passing argument 3 of 'php_parsekit_parse_node' from incompatible pointer type [enabled by default]

php_parsekit_parse_node(result, op_array, &(op->result), flags & PHP_PARSEKIT_RESULT_USED, options TSRMLS_CC);
^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:66:13: note: expected 'struct znode *' but argument is of type 'union znode_op *'
static void php_parsekit_parse_node(zval *return_value, zend_op_array *op_array, znode *node, long flags, long options TSRMLS_DC)

^

/tmp/buildd/php-parsekit-1.3.0/parsekit-1.3.0/parsekit.c:161:3: warning: passing argument 3 of 'php_parsekit_parse_node' from incompatible pointer type [enabled by default]

php_parsekit_parse_node(op1, op_array, &(op->op1), flags & PHP_PARSEKIT_OP1_USED, options TSRMLS_CC);
^

Ideas ?

15:52 <^d> runkit is *scary times*
15:55 <^d> "The runkit extension provides means to modify constants, user-defined functions, and

user-defined classes. It also provides for custom superglobal variables and embeddable 
sub-interpreters via sandboxing."

15:55 <^d> So much scary!
15:57 <^d> godog: I'd want some kind of check for it in either jenkins or scap to keep people from

adding it to their code.

16:06 <godog> ^d: mind if I quote you on the bug? if it is a jenkins check then I think we're fine with

(parallel?) php -l

so slightly off-topic for this bug but I think it is interesting discussion anyway, moving linting from scap and onto jenkins (i.e. no "real time" requirements) would make php -l feasible I think, thoughts?

The nice thing about deploy time linting is that it works for live hacks (yeah they are not supposed to happen, but...), security patches (not put in gerrit until minutes before public disclosure) and things that were +2 verified bypassing Jenkins for whatever reason.

We could go with runkit, just have to make sure it is not loaded by default (i.e. not in a PHP .ini file. Then we could have our lint.php script to load it via -d.

We could try out something like https://github.com/JakubOnderka/PHP-Parallel-Lint where N php procs are run in parallel to speed up the lint time. During a full scap run, we only lint the wmf-config and multiversion directories and trust that the php codebase has been linted already. sync-file doesn't lint at all and sync-dir lints the contents of the directory being synced. We could run some manual tests on tin to see if running some multiple php procs in parallel gives reasonable performance. The current parsekit lint is very fast, but adding even a few seconds to a large sync is not like to make a significant difference in operational wall-clock time.

xargs can runs tasks in parallel. I tried on tin with a warmed up disk cache against a wmf branch and 4 parallel process:

time (find /a/common/php-1.24wmf16 -name '*.php' -or -name '*.inc' -or -name '*.phtml' |xargs -n1 -P 4 -exec php -l)

real 0m57.913s
user 2m9.552s
sys 1m22.673s

Compared to:

time lint /a/common/php-1.24wmf16

real 0m5.389s
user 0m5.088s
sys 0m0.272s

though linting wmf-config and multiversion is significantly less files, yielding ~500ms, I think that's acceptable

filippo@mw1018:~$ time (find /usr/local/apache/common/wmf-config/ /usr/local/apache/common/multiversion/ -name '*.php' -or -name '*.inc' -or -name '*.phtml' |xargs -n1 -P 4 -exec php -l >/dev/null)

real 0m0.524s
user 0m1.116s
sys 0m0.716s

thoughts on going ahead with linting wmf-config and multiversion with parallel php as I suggested above?

(In reply to Filippo Giunchedi from comment #14)

thoughts on going ahead with linting wmf-config and multiversion with
parallel php as I suggested above?

Let's make a patch to scap and try it. I think that it should work fine and hardly be noticeable. It definitely won't impact the wall clock time of a full scap as much as bringing codfw online will. :)

Change 160668 had a related patch set uploaded by BryanDavis:
Check php syntax with parallel php -l

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

Change 160668 merged by jenkins-bot:
Check php syntax with parallel php -l

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

$ scap --verbose 'No code change scap to test scap internal update'
16:45:09 Started scap: No code change scap to test scap internal update
16:45:09 Running command: find '/srv/mediawiki-staging/wmf-config' '/srv/mediawiki-staging/multiversion' -name '*.php' -or -name '*.inc' -or -name '*.phtml' | xargs -n1 -P6 -exec php -l >/dev/null
16:45:09 Copying to tin.eqiad.wmnet from tin.eqiad.wmnet
16:45:09 Running rsync command: sudo -u mwdeploy -n -- /usr/bin/rsync --archive --delete-delay --delay-updates --compress --delete --exclude=**/.svn/lock --exclude=**/.git/objects --exclude=**/.git/**/objects --exclude=**/cache/l10n/*.cdb --no-perms tin.eqiad.wmnet::common /srv/mediawiki
....

Maybe closed too soon? Have we removed the dependency on php5-parsekit from puppet?

The Jenkins job that lint PHP files reused parsekit / phplint.php so I have to adjust the jobs to xargs | php -l as well. Once done we can safely close this bug and forget about php_parsekit entirely.

Change 161748 had a related patch set uploaded by Hashar:
contint: Package 'php5-parsekit' is absent on Trusty, don't require it

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

The JJB macro phplint already uses "xargs -n1 -t php -l"

And on gallium:

$ fgrep -i parsekit /var/lib/jenkins/jobs/*/config.xml
$

$ fgrep -Ri parsekit /srv/deployment/integration/slave-scripts
$

So it is all good for contint.

Change 160691 had a related patch set uploaded by Krinkle:
use scap's embedded linking, remove lint script

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

Change 161748 abandoned by Krinkle:
contint: Package 'php5-parsekit' is absent on Trusty, don't require it

Reason:
Obsolete with I44d33af1ce85.

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

Change 160691 merged by Filippo Giunchedi:
use scap's embedded linking, remove lint script

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

We have removed dependencies upon php5-parsekit which was solely used for linting PHP files.

Instead both scap and the CI script uses something like:

xargs -n1 | php -l

Thank you to everyone that have been involved!