Last modified: 2013-06-18 15:05:26 UTC
E_PARSE errors are never acceptable. But people don't like to test their code. So pre-commit should run 'php -l' on php source files so people can't commit if they mess up. I'll get to this sometime soon-ish.
Done, now checks all .php or .inc or .php5 files changed in a commit for syntax validity. Should output the following on failure: $ svn ci -m 'Try #2' Sending test.php Transmitting file data .svn: Commit failed (details follow): svn: Commit blocked by pre-commit hook (exit code 1) with output: PHP Parse error: syntax error, unexpected T_STRING in - on line 1 *********************************** PHP error in: test.php: Errors parsing test.php ***********************************
Not that I'm adversed to this, but I thought that we didn't want to do a pre-commit hook as it blocks? And would be especially annoying for large (i18n) etc commits...? Also, what if the file already had it in...? (Granted, if it was tested, they would've found it..)
https://bugzilla.wikimedia.org/show_bug.cgi?id=20069 is technically redundant now...
Which php version is used?
Can we unleash checkSyntax.php in its fury on files committed?
PHP 5.3.2-1ubuntu4.5 with Suhosin-Patch (cli) (built: Sep 17 2010 13:49:46) Copyright (c) 1997-2009 The PHP Group Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
(In reply to comment #4) > Which php version is used? 5.3.2. (In reply to comment #2) > Not that I'm adversed to this, but I thought that we didn't want to do a > pre-commit hook as it blocks? > I don't like the idea of doing extensive pre-commit hooks due to that issue. But php -l is typically very fast. > And would be especially annoying for large (i18n) > etc commits...? > It may be, we're not sure yet. Gonna test it later with Raymond. > Also, what if the file already had it in...? (Granted, if it was tested, they > would've found it..) > Then you fix it too, while you're at it ;-)
(In reply to comment #5) > Can we unleash checkSyntax.php in its fury on files committed? checkSyntax.php is slower and checks lots of non-syntax stuff too. That would be better as a post-commit hook.
Commit your script to SVN? In lint.php.. * Recursive directory crawling PHP syntax checker * Uses parsekit, which is much faster than php -l for lots of files due to the
(In reply to comment #9) > Commit your script to SVN? > I need to clean up the pre/post commit hooks and put them somewhere. Puppet makes more sense than SVN, though. > In lint.php.. > > * Recursive directory crawling PHP syntax checker > * Uses parsekit, which is much faster than php -l for lots of files due to the Yes, it's faster, but it's horrendously broken on 5.3.x last I played with it.
(In reply to comment #9) > Commit your script to SVN? maintenance/checkSyntax.php ?
(In reply to comment #8) > checkSyntax.php is slower and checks lots of non-syntax stuff too. That would > be better as a post-commit hook. It isn't much slower on a known list of files. A few regexes is faster than php -l and even parsekit. And some stuff it checks for worth declining on sight, such as common causes of "headers already sent" errors. Also, it avoids using parsekit on PHP 5.3 where it's broken.
(In reply to comment #12) > (In reply to comment #8) > > > checkSyntax.php is slower and checks lots of non-syntax stuff too. That would > > be better as a post-commit hook. > > It isn't much slower on a known list of files. A few regexes is faster than php > -l and even parsekit. And some stuff it checks for worth declining on > sight, such as common causes of "headers already sent" errors. > > Also, it avoids using parsekit on PHP 5.3 where it's broken. We should rewrite it to not depend on MW at all, then it'll be fast :D
It brokeded
Can we fix plox?
This and the empty summary prevention REALLY need fixing :P
Fixed and re-deployed. See /trunk/tools/subversion/hooks.
