-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
phpcs reads from stdin which requires a terminal, and hangs if there are none. #993
Comments
Many thanks @gsherwood for your efforts trying to reproduce the issue. The bash script I'm using is doing: ...
OUTPUT=$($PHPCS_BIN -s --standard=$PHPCS_CODING_STANDARD $EXTENSIONS $ENCODING $IGNORE $STAGED_FILES)
RETVAL=$?
...
if [ $RETVAL -ne 0 ]; then
echo "$OUTPUT" | less
fi
exit $RETVAL This might be the reason. |
To make this script working one need to use: OUTPUT=$(echo '' | $PHPCS_BIN -s --standard=$PHPCS_CODING_STANDARD $EXTENSIONS $ENCODING $IGNORE $STAGED_FILES)
... To force the existence of stdin (at least that we can open it). |
Strange. I was using the exact same script you just linked to (you linked it in the PR as well) and was getting the result I posted. I also didn't think STDIN disappears when using subshells. |
Just want to share my case I recently update phpcs to 2.6 and I've started to have the same problem with STDIN. I've never had it before. I'm using phing to automate some build steps, and I'm running it from pre-push hook like this
I've following problem
Meanwhile if I'm running here is my phpcs-dev target which I have in build.xml
and it seems that chnages in my build.xml like this |
We're also having the same issue in our CI server which uses docker images. The blank echo above seemed to fix it too! |
I've tried replicating this again (with phing this time) and still can't. It would be great if someone who can replicate it would change their PHPCS command to include "-vv" and grab the output. There is already code that checks if STDIN is just whitespace and ignores it if so, so I'm keen to see what content is actually in STDIN when this happens. It's obviously not whitespace. |
@gsherwood I'm going to try using |
So I added some Note that when git gui freezes it prints on the terminal:
If I BTW, this is on osx, with PHP 5.6.18 (custom built), but I observed the same issue on a linux box (which I don't have access to right now). |
@masterzen Thanks for testing. I've been testing with the 2.6.0 version on OS X, and still can't get Your problem is obviously different from the one reported by @nnnikolay as your one is stuck waiting for input and their one is complaining that it got non-PHP input. So -vv probably isn't going to help you at all anyway. I just wish I could replicate this problem so I could try some fixes. |
Read stdin from the handle opened via `fopen('php://stdin')`, not the original `STDIN` handle which may not have been affected by the `stream_set_blocking()` call. Fixes: squizlabs#993
@gsherwood this unfortunately doesn't work (and I had tested this before). The problem is that Here are the
which itself seems to be connected to the git gui process:
I think that's the issue (probably that one of those pipe is blocking). Now if I use the workaround (ie do phpcs:
bash child (
bash parent (the script that launches phpcs):
git gui itself:
First thing that is noticeable: phpcs STDOUT with the workaround is a PIPE and not directly a terminal. I'm not able to fully understand the way the pipes are connected together in the second tests, but it seems both I don't really understand the issue, but maybe that could help anyone here find a fix :) |
@gsherwood sorry for long silent. the thing is, the problem is really strange, it's not appear every time, it looks like that it depends from your file state or something similar, it might be connected with changes in the shell or the git or both. But if you want I can try to explain with all all details... hopefully today will write it down |
Sorry, no idea how this got auto-closed when I merged the other PR. |
Clone please the following repo
You'll see that
My shell version is:
And git is
|
@nnnikolay Thanks for posting that info. Unfortunately, I can't replicate any problems using either master or a new branch. I get good build results each time. |
@gsherwood Sad :-) But suggest then please how I can debug more here? Thanks |
You could try adding |
@gsherwood have same error ...
[lint:phpcs ] Process token 302 on line 35 [col:5;len:15;lvl:0;]: T_VARIABLE => $renderTemplate
[lint:phpcs ] Process token 303 on line 35 [col:20;len:1;lvl:0;]: T_CLOSE_PARENTHESIS => )
[lint:phpcs ] Process token 304 on line 35 [col:21;len:1;lvl:0;]: T_WHITESPACE => ·
[lint:phpcs ] Process token 305 on line 35 [col:22;len:1;lvl:0;]: T_OPEN_CURLY_BRACKET => {
[lint:phpcs ] => Found scope opener for 299:T_IF
[lint:phpcs ] * level increased *
[lint:phpcs ] * token 299:T_IF added to conditions array *
[lint:phpcs ] Process token 306 on line 35 [col:23;len:0;lvl:1;conds;T_IF;]: T_WHITESPACE => \n
[lint:phpcs ] Process token 307 on line 36 [col:1;len:4;lvl:1;conds;T_IF;]: T_WHITESPACE => ····
[lint:phpcs ] Process token 308 on line 36 [col:5;len:15;lvl:1;conds;T_IF;]: T_STRING => endTemplatePage
[lint:phpcs ] Process token 309 on line 36 [col:20;len:1;lvl:1;conds;T_IF;]: T_OPEN_PARENTHESIS => (
[lint:phpcs ] Process token 310 on line 36 [col:21;len:1;lvl:1;conds;T_IF;]: T_CLOSE_PARENTHESIS => )
[lint:phpcs ] Process token 311 on line 36 [col:22;len:1;lvl:1;conds;T_IF;]: T_SEMICOLON => ;
[lint:phpcs ] Process token 312 on line 36 [col:23;len:0;lvl:1;conds;T_IF;]: T_WHITESPACE => \n
[lint:phpcs ] Process token 313 on line 37 [col:1;len:1;lvl:1;conds;T_IF;]: T_CLOSE_CURLY_BRACKET => }
[lint:phpcs ] => Found scope closer for 305:T_OPEN_CURLY_BRACKET
[lint:phpcs ] * token T_IF removed from conditions array *
[lint:phpcs ] * level decreased *
[lint:phpcs ] Process token 314 on line 37 [col:2;len:0;lvl:0;]: T_WHITESPACE => \n
[lint:phpcs ] Process token 315 on line 38 [col:1;len:2;lvl:0;]: T_CLOSE_TAG => ?>\n
[lint:phpcs ] *** END LEVEL MAP ***
[lint:phpcs ] *** START ADDITIONAL PHP PROCESSING ***
[lint:phpcs ] * token 276 on line 28 changed from T_OPEN_SQUARE_BRACKET to T_OPEN_SHORT_ARRAY
[lint:phpcs ] * token 290 on line 31 changed from T_CLOSE_SQUARE_BRACKET to T_CLOSE_SHORT_ARRAY
[lint:phpcs ] * token 224 on line 20 changed from T_OPEN_SQUARE_BRACKET to T_OPEN_SHORT_ARRAY
[lint:phpcs ] * token 235 on line 20 changed from T_CLOSE_SQUARE_BRACKET to T_CLOSE_SHORT_ARRAY
[lint:phpcs ] * token 156 on line 17 changed from T_OPEN_SQUARE_BRACKET to T_OPEN_SHORT_ARRAY
[lint:phpcs ] * token 161 on line 17 changed from T_CLOSE_SQUARE_BRACKET to T_CLOSE_SHORT_ARRAY
[lint:phpcs ] *** END ADDITIONAL PHP PROCESSING ***
[lint:phpcs ] [PHP => 316 tokens in 38 lines]...
[lint:phpcs ] Processing STDIN
[lint:phpcs ] *** START PHP TOKENIZING ***
[lint:phpcs ] Process token [0]: T_INLINE_HTML => refs/heads/master·790d008c03fa4f0d3032c1eea86d354d2fed2798·refs/heads/master·69d3c2c763012008481956301fc1d67ead83c39c\n
[lint:phpcs ] *** END PHP TOKENIZING ***
[lint:phpcs ] *** START TOKEN MAP ***
[lint:phpcs ] *** END TOKEN MAP ***
[lint:phpcs ] *** START SCOPE MAP ***
[lint:phpcs ] *** END SCOPE MAP ***
[lint:phpcs ] *** START LEVEL MAP ***
[lint:phpcs ] Process token 0 on line 1 [col:1;len:117;lvl:0;]: T_INLINE_HTML => refs/heads/master·790d008c03fa4f0d3032c1eea86d354d2fed2798·refs/heads/master·69d3c2c763012008481956301fc1d67ead83c39c\n
[lint:phpcs ] *** END LEVEL MAP ***
[lint:phpcs ] *** START ADDITIONAL PHP PROCESSING ***
[lint:phpcs ] *** END ADDITIONAL PHP PROCESSING ***
[lint:phpcs ] [PHP => 1 tokens in 1 lines]...
[lint:phpcs ] DONE in 0ms (0 errors, 1 warnings)
[lint:phpcs ]
[lint:phpcs ] FILE: STDIN
[lint:phpcs ] ----------------------------------------------------------------------
[lint:phpcs ] FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
[lint:phpcs ] ----------------------------------------------------------------------
[lint:phpcs ] 1 | WARNING | No PHP code was found in this file and short open tags are
[lint:phpcs ] | | not allowed by this install of PHP. This file may be using
[lint:phpcs ] | | short open tags but PHP does not allow them.
[lint:phpcs ] | | (Internal.NoCodeFound)
[lint:phpcs ] ----------------------------------------------------------------------
[lint:phpcs ]
[lint:phpcs ] Time: 1 mins, 49.89 secs; Memory: 46Mb |
For us as a solution was these lines in ruleset file
|
@nnnikolay thanks! I am wait that says @gsherwood |
The content being passed to PHPCS on STDIN is:
Which appears to be the documented STDIN for the pre-push hook. Adding that change to silence the Edit: The STDIN formats for the hooks are too different to be accurate. It's probably easier if PHPCS just stops generating |
@nnnikolay @evilebottnawi I've pushed a change which should make it unnecessary to mute the This doesn't solve the original issue with PHPCS hanging while looking for STDIN, so I'll need to keep waiting and see if anyone else is able to replicate and debug the issue. |
Blank line is needed to provide STDIN input to phpcs when phpcs is called from the Git pre-push hook context. See squizlabs/PHP_CodeSniffer#993. Otherwise, phpcs outputs the following: vendor/bin/phpcs --standard=phpcs.xml --extensions=php --report=full ./src FILE: STDIN ---------------------------------------------------------------------- FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE ---------------------------------------------------------------------- 1 | WARNING | No PHP code was found in this file and short open tags | | are not allowed by this install of PHP. This file may | | be using short open tags but PHP does not allow them. ----------------------------------------------------------------------
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()` and `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()`, `tty`, and, `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()`, `tty`, and, `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()`, `tty`, and, `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
Non-blocking reading from stdin was introduced in 80b156d because we had problems with auto detecting whether we should read from stdin or not. Combining `posix_isatty()`, `tty`, and, `feof()` can get us back to blocking read. By going back to blocking read from stdin we can avoid the extra check introduced in 23bd023 for Windows. We can also avoid the "read by line" and "usleep" introduced 8d669c5 and 63fafe0. They where introduced trying to solve what is possible the same problem still reported in squizlabs#1472. Fixes squizlabs#993. Fixes squizlabs#1472.
With version 2.6.0,
phpcs
now tries to read content fromstdin
.This of course either requires a terminal or an incoming stream from another process.
When running
phpcs
as part of a git pre-commit hook ingit gui
or any other GUI around git which doesn't allocate a terminal, this completely blocks the pre-commit process and and the GUI itself, the process is stuck in T state on linux or osx.The text was updated successfully, but these errors were encountered: