Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 3.7.2

Bug #17693 issue with pre-commit hook script with filenames that start with v
Submitted: 2010-08-11 19:18 UTC
From: tfotherby Assigned: squiz
Status: Closed Package: PHP_CodeSniffer (version 1.2.2)
PHP Version: 5.3.2 OS: CentOS 5.5
Roadmaps: (Not assigned)    
Subscription  
Comments Add Comment Add patch


Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know! Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem : 50 - 31 = ?

 
 [2010-08-11 19:18 UTC] tfotherby (Tom Fotherby)
Description: ------------ I have a file called classes/vps_handle_protx_response.php and when I commit it I get the following error from phpcs: svnlook: Path 'ps_handle_protx_response.php' does not exist Note: this is only a problem with the SVN pre-commit hook script (phpcs-svn-pre-commit) not with a normal phpcs run. It also only occurs when the filename starts with "v" AND it is in a sub-folder. It seems the "\v" of the file name is confusing the script. I was able to find the issue on line 136 of phpcs-svn-pre- commit : foreach (preg_split('/\v|\n/', $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { I don't know if this fix is correct but using double-quotes instead of single-quotes allowed me to commit my file that began with v: foreach (preg_split("/\v|\n/", $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { Test script: --------------- Create a directory called dirname and a file that begins with v, e.g. vfilename.php Ensure the new file has invalid code standards Set up the phpcs pre-commit hook commit the new file: php commit dirname/vfilename.php -m "test" Expected result: ---------------- svnlook: Path 'filename.php' does not exist Actual result: -------------- should sniff the file

Comments

 [2010-08-12 08:30 UTC] squiz (Greg Sherwood)
-Status: Open +Status: Feedback -Assigned To: +Assigned To: squiz
I can't replicate this problem, but I can say that sing double quotes instead of single quotes doesn't break anything for me. But before I make a change that I don't understand, are you able to debug a little more? If so, I'd love to see the value in the $contents var so I can put together a small test script and try it outside of the SVN environment and on a few different PHP versions. Any chance you can post that?
 [2010-08-12 11:34 UTC] tfotherby (Tom Fotherby)
Hi, Thanks for looking into this. I played around a bit but couldn't figure out the exact problem. I put the following test code in "/usr/bin/scripts/phpcs-svn-pre-commit": echo "\$contents contains the following: ".print_r($contents,true)."\n"; echo "Test A:\n"; foreach (preg_split('/\v|\n/', $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { echo "A) \$path: $path\n"; } echo "\nTest B:\n"; foreach (preg_split("/\v|\n/", $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { echo "B) \$path: $path\n"; } echo "\nTest C:\n"; foreach (preg_split('/\i|\n/', $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { echo "C) \$path: $path\n"; } I then did the following commit on 4 test files: svn commit classes/vps_handle_protx_response.php top.php library/notifications.php classes/init-dbconnect.php -m "#1 - test commit" This was the result: $contents contains the following: Branches/Staging/classes/init-dbconnect.php Branches/Staging/classes/vps_handle_protx_response.php Branches/Staging/library/notifications.php Branches/Staging/top.php Test A: A) $path: Branches/Staging/classes/init-dbconnect.php A) $path: Branches/Staging/classes/ A) $path: ps_handle_protx_response.php A) $path: Branches/Staging/library/notifications.php A) $path: Branches/Staging/top.php Test B: B) $path: Branches/Staging/classes/init-dbconnect.php B) $path: Branches/Staging/classes/vps_handle_protx_response.php B) $path: Branches/Staging/library/notifications.php B) $path: Branches/Staging/top.php Test C: C) $path: Branches/Stag C) $path: ng/classes/ C) $path: n C) $path: t-dbconnect.php C) $path: Branches/Stag C) $path: ng/classes/vps_handle_protx_response.php C) $path: Branches/Stag C) $path: ng/l C) $path: brary/not C) $path: f C) $path: cat C) $path: ons.php C) $path: Branches/Stag C) $path: ng/top.php I do not know what "\v" is for. I think perhaps "\r" was meant in order to split on CR or LF. Using "\v" seems to split all files with "v" in their name in my version of PHP (5.3.2). Using "\i" splits all files with "i" in their name.
 [2010-08-12 12:56 UTC] squiz (Greg Sherwood)
The \v character matches any vertical whitespace character and was added in 5.2.4 (I think) so it is very strange it is not being picked up. But you've provided some great debug output, so thanks a lot for that. I'll see if I can find a few systems to play around on.
 [2010-08-12 12:56 UTC] squiz (Greg Sherwood)
-Status: Feedback +Status: Assigned
 [2010-08-16 07:38 UTC] squiz (Greg Sherwood)
Well this is pretty strange. I've tested a short script on 5.3.1 and 5.3.2: <?php $contents = 'Branches/Staging/classes/init-dbconnect.php Branches/Staging/classes/vps_handle_protx_response.php Branches/Staging/library/notifications.php Branches/Staging/top.php'; foreach (preg_split('/\v|\n/', $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { echo "$path\n"; } ?> This works fine. The \v character is not splitting on all "v" characters. Are you 100% sure your CLI version is 5.3.2? Also sure there isn't another older version of PHP on the server that SVN is using maybe? Might need to "which php" and check that the path is set correctly in the pre-commit hook.
 [2010-08-16 07:38 UTC] squiz (Greg Sherwood)
-Status: Assigned +Status: Feedback
 [2010-08-23 05:52 UTC] squiz (Greg Sherwood)
-Status: Feedback +Status: No Feedback
Please re-open, or just comment, if you find any more information about your PHP version.
 [2010-08-23 16:00 UTC] tfotherby (Tom Fotherby)
I tried your test script, i.e.: <?php $contents = 'Branches/Staging/classes/init-dbconnect.php Branches/Staging/classes/vps_handle_protx_response.php Branches/Staging/library/notifications.php Branches/Staging/top.php'; foreach (preg_split('/\v|\n/', $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { echo "$path\n"; } ?> It didn't work for me, I got a split at the "/v" string. This is the output: Branches/Staging/classes/init-dbconnect.php Branches/Staging/classes/ ps_handle_protx_response.php Branches/Staging/library/notifications.php Branches/Staging/top.php I think you are right and I must have a config issue rather than the phpcs code having a bug. I'm not sure what I have mis-configured though. I am sure I'm running PHP 5.3.2 > which php /usr/bin/php > /usr/bin/php --version PHP 5.3.2 (cli) (built: Jun 24 2010 17:19:48) Copyright (c) 1997-2010 The PHP Group Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies
 [2010-08-25 09:17 UTC] squiz (Greg Sherwood)
I have absolutely no idea why it is not working when we have the same PHP versions. I also have no idea why using double quotes makes it work for you. I'm obviously missing something but just don't know what.
 [2010-11-23 16:50 UTC] tfotherby (Tom Fotherby)
I migrated phpcs to a new server (i.e. re-installed it) and got this problem again. While trying to commit {{{view_bids.php}}}, {{{showProviderMyMessages.php}}}, phpcs gave a error. {{{ > svn commit ajax/showProviderMyMessages.php view_bids.php -m "#1815(<Client-MyMessages> New Enhancements)-provided all the functionality files" Error: svnlook: Path 'iderMyMessages.php' does not exist Error: svnlook: Path 'iew_bids.php' does not exist Error: Error: FILE: iderMyMessages.php Error: -------------------------------------------------------------------------------- Error: FOUND 0 ERROR(S) AND 1 WARNING(S) AFFECTING 1 LINE(S) Error: -------------------------------------------------------------------------------- Error: 1 | WARNING | No PHP code was found in this file and short open tags are not Error: | | allowed by this install of PHP. This file may be using short Error: | | open tags but PHP does not allow them. Error: -------------------------------------------------------------------------------- }}} It seems, there is a problem with the 'v' char. To fix, I changed single-quotes to double-quotes in the regular expression: {{{ > sudo emacs /usr/bin/scripts/phpcs-svn-pre-commit change foreach (preg_split('/\v|\n/', $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { to foreach (preg_split("/\v|\n/", $contents, -1, PREG_SPLIT_NO_EMPTY) as $path) { }}}
 [2010-11-23 16:58 UTC] tfotherby (Tom Fotherby)
Sorry should have given some details: > php -v PHP 5.3.3 (cli) (built: Aug 20 2010 17:22:27) Copyright (c) 1997-2010 The PHP Group Zend Engine v2.3.0, Copyright (c) 1998-2010 Zend Technologies with Suhosin v0.9.32.1, Copyright (c) 2007-2010, by SektionEins GmbH > cat /etc/redhat-release Red Hat Enterprise Linux Server release 5.5 (Tikanga) phpcs was installed by: > sudo yum install php-pear > sudo pear install PHP_CodeSniffer phpcs was triggered via a line in the pre-commit hook: /usr/bin/scripts/phpcs-svn-pre-commit "$REPOS" -t "$TXN" >&2 || exit 1
 [2010-11-26 05:23 UTC] squiz (Greg Sherwood)
-Status: No Feedback +Status: Assigned
 [2010-12-06 04:46 UTC] squiz (Greg Sherwood)
I've tried a 5.3.2 and 5.3.3 and still can't replicate your problem. I still have absolutely no idea what is causing your issue and why I can't replicate it on any system I've tried on. I think I might just have to use double quotes even though I have no idea why.
 [2010-12-15 09:23 UTC] squiz (Greg Sherwood)
-Status: Assigned +Status: Closed
This bug has been fixed in SVN. If this was a documentation problem, the fix will appear on pear.php.net by the end of next Sunday (CET). If this was a problem with the pear.php.net website, the change should be live shortly. Otherwise, the fix will appear in the package's next release. Thank you for the report and for helping us make PEAR better. I decided to just go ahead and change the single quotes to double quotes. The script was working either way and I can't figure out why single quotes don't work properly on some installs.