kvz / ochtra

One Commit Hook To Rule All
MIT License
86 stars 13 forks source link

Does not work on Windows + cygwin (php/js) #7

Open mhujer opened 10 years ago

mhujer commented 10 years ago

js file with error:

$ git commit -m "a"
on branch master
--> jshint syntax checking for test.js
stdin: line 1, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 1, col 9, Missing semicolon.
stdin: line 2, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 2, col 4, Missing semicolon.
stdin: line 3, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 3, col 2, Missing semicolon.
stdin: line 4, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 4, col 2, Missing semicolon.

8 errors
No jshint syntax errors detected in 'test.js'
[master 588c64f] a
 1 file changed, 2 insertions(+), 1 deletion(-)

php file with error:

$ git commit -m "a"
on branch master
--> php syntax checking for test.php
No php syntax errors detected in 'test.php'
[master 4357ee1] a
 1 file changed, 2 insertions(+), 1 deletion(-)

When I run the lint directly:

$ php -l test.php

Parse error: syntax error, unexpected 'adsasdsa' (T_STRING), expecting ',' or ';' in test.php on line 3
Errors parsing test.php

Any hints?

kvz commented 10 years ago

Not sure. I don't have a similar setup at my disposal, but could you try uncommenting the # set -o xtrace fine in the header of the hook? It will show you the full bash trace. Maybe that sheds a light on what happens

mhujer commented 10 years ago
$ git commit -m "b"
+++ dirname .git/hooks/pre-commit
++ cd .git/hooks
+++ pwd
++ echo c:/Users/Martin/Downloads/test/.git/hooks
+ __DIR__=c:/Users/Martin/Downloads/test/.git/hooks
++ basename .git/hooks/pre-commit
+ __BASE__=pre-commit
+ __FILE__=c:/Users/Martin/Downloads/test/.git/hooks/pre-commit
+ '[' -x c:/Users/Martin/Downloads/test/.git/hooks/pre-ochtra ']'
+ against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ git rev-parse --verify HEAD
+ against=HEAD
++ git symbolic-ref -q HEAD
++ awk -F/ '{print $NF}'
+ branch=master
+ echo on branch master
on branch master
+ git diff-index --cached --full-index --diff-filter=ACM HEAD
+ read -r line
++ echo :100644 100644 851503f5aea34191969a49d10f4ccfb475e2f23c 43187387c72ff9893613034b096a0add6ba46ff5 M test.php
++ cut '-d ' -f4
+ sha=43187387c72ff9893613034b096a0add6ba46ff5
++ echo :100644 100644 851503f5aea34191969a49d10f4ccfb475e2f23c 43187387c72ff9893613034b096a0add6ba46ff5 M test.php
++ cut '-d ' -f5
+ sts=M
++ echo :100644 100644 851503f5aea34191969a49d10f4ccfb475e2f23c 43187387c72ff9893613034b096a0add6ba46ff5 M test.php
++ cut '-d ' -f6-
+ pth=test.php
+ ext=php
++ git cat-file -p 43187387c72ff9893613034b096a0add6ba46ff5
++ head -n1
++ awk -F/ '/^#\!/ {print $NF}'
++ sed 's/^env //g'
+ she=
+ out=purge
+ err=red
+ cmd=
+ tmp=
+ '[' php = rb ']'
+ '[' php = erb ']'
+ '[' '' = ruby ']'
+ '[' php = js ']'
+ '[' '' = node ']'
+ '[' php = coffee ']'
+ '[' '' = coffee ']'
+ '[' php = py ']'
+ '[' '' = python ']'
+ '[' php = go ']'
+ '[' '' = bash ']'
+ '[' '' = sh ']'
+ '[' php = php ']'
+ cmd='php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0'
+ '[' -n 'php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0' ']'
++ echo 'php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0'
++ cut '-d ' -f1
+ tool=php
+ paint gray red echo '--> php syntax checking for test.php'
+ set -o pipefail
+ green='s,.*,&,'
+ red='s,.*,&,'
+ gray='s,.*,&,'
+ purge='/.*/d'
+ stdout='s,.*,&,'
+ stderr='s,.*,&,'
+ echo '--> php syntax checking for test.php'
,'sed 's,.*,&
+ sed 's,.*,&,'
--> php syntax checking for test.php
+ which php
+ '[' -n '' ']'
+ git cat-file -p 43187387c72ff9893613034b096a0add6ba46ff5
+ paint purge red php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0
+ set -o pipefail
+ green='s,.*,&,'
+ red='s,.*,&,'
+ gray='s,.*,&,'
+ purge='/.*/d'
+ stdout='/.*/d'
+ stderr='s,.*,&,'
+ php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0
+ sed '/.*/d'
+ sed 's,.*,&,'
+ '[' -n '' ']'
+ paint green red echo 'No php syntax errors detected in '\''test.php'\'''
+ set -o pipefail
+ green='s,.*,&,'
+ red='s,.*,&,'
+ gray='s,.*,&,'
+ purge='/.*/d'
+ stdout='s,.*,&,'
+ stderr='s,.*,&,'
+ echo 'No php syntax errors detected in '\''test.php'\'''
+ sed 's,.*,&,'
+ sed 's,.*,&,'
No php syntax errors detected in 'test.php'
+ read -r line
[master 02ed73c] b
 1 file changed, 1 insertion(+)
mhujer commented 10 years ago

And for js file:

$ git commit -m "JS"
+++ dirname .git/hooks/pre-commit
++ cd .git/hooks
+++ pwd
++ echo c:/Users/Martin/Downloads/test/.git/hooks
+ __DIR__=c:/Users/Martin/Downloads/test/.git/hooks
++ basename .git/hooks/pre-commit
+ __BASE__=pre-commit
+ __FILE__=c:/Users/Martin/Downloads/test/.git/hooks/pre-commit
+ '[' -x c:/Users/Martin/Downloads/test/.git/hooks/pre-ochtra ']'
+ against=4b825dc642cb6eb9a060e54bf8d69288fbee4904
+ git rev-parse --verify HEAD
+ against=HEAD
++ git symbolic-ref -q HEAD
++ awk -F/ '{print $NF}'
+ branch=master
+ echo on branch master
on branch master
+ git diff-index --cached --full-index --diff-filter=ACM HEAD
+ read -r line
++ echo :100644 100644 2d15a6a25d1a814721cf06eb7139ee5beb5b43aa 951112f8c1c4c8912d738e9b796462d9ac96cbfd M test.js
++ cut '-d ' -f4
+ sha=951112f8c1c4c8912d738e9b796462d9ac96cbfd
++ echo :100644 100644 2d15a6a25d1a814721cf06eb7139ee5beb5b43aa 951112f8c1c4c8912d738e9b796462d9ac96cbfd M test.js
++ cut '-d ' -f5
+ sts=M
++ echo :100644 100644 2d15a6a25d1a814721cf06eb7139ee5beb5b43aa 951112f8c1c4c8912d738e9b796462d9ac96cbfd M test.js
++ cut '-d ' -f6-
+ pth=test.js
+ ext=js
++ git cat-file -p 951112f8c1c4c8912d738e9b796462d9ac96cbfd
++ head -n1
++ awk -F/ '/^#\!/ {print $NF}'
++ sed 's/^env //g'
+ she=
+ out=purge
+ err=red
+ cmd=
+ tmp=
+ '[' js = rb ']'
+ '[' js = erb ']'
+ '[' '' = ruby ']'
+ '[' js = js ']'
+ cmd='jshint -'
+ out=red
+ '[' -n 'jshint -' ']'
++ echo 'jshint -'
++ cut '-d ' -f1
+ tool=jshint
+ paint gray red echo '--> jshint syntax checking for test.js'
+ set -o pipefail
+ green='s,.*,&,'
+ red='s,.*,&,'
+ gray='s,.*,&,'
+ purge='/.*/d'
+ stdout='s,.*,&,'
+ stderr='s,.*,&,'
+ echo '--> jshint syntax checking for test.js'
+ sed 's,.*,&,'
+ sed 's,.*,&,'
--> jshint syntax checking for test.js
+ which jshint
+ '[' -n '' ']'
+ git cat-file -p 951112f8c1c4c8912d738e9b796462d9ac96cbfd
+ paint red red jshint -
+ set -o pipefail
+ green='s,.*,&,'
+ red='s,.*,&,'
+ gray='s,.*,&,'
+ purge='/.*/d'
+ stdout='s,.*,&,'
+ stderr='s,.*,&,'
+ jshint -
+ sed 's,.*,&,'
+ sed 's,.*,&,'
stdin: line 1, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 1, col 9, Missing semicolon.
stdin: line 2, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 2, col 4, Missing semicolon.
stdin: line 3, col 1, Expected an assignment or function call and instead saw an expression.
stdin: line 3, col 2, Missing semicolon.
+ '[' -n '' ']'

+ paint green red echo 'No jshint syntax errors detected in '\''test.js'\'''
+ set -o pipefail
+ green='s,.*,&,'
+ red='s,.*,&,'
+ gray='s,.*,&,'
+ purge='/.*/d'
+ stdout='s,.*,&,'
+ stderr='s,.*,&,'
+ echo 'No jshint syntax errors detected in '\''test.js'\'''
+ sed 's,.*,&,'
+ sed 's,.*,&,'
No jshint syntax errors detected in 'test.js'
+ read -r line
[master e914275] JS
 1 file changed, 1 insertion(+), 2 deletions(-)

(the stdin: lines are red)

kvz commented 10 years ago

Maybe something goes wrong with the stdin redirection. Can you try by adding

tmp="${TMPDIR:-/tmp}/${$}.${ext}"

to the php section, e.g. here? https://github.com/kvz/ochtra/blob/master/pre-commit#L93 (just like for python, yaml, etc)

(let's focus on php, once that works, we can get js to work as well)

mhujer commented 10 years ago

Output is now:

+ tmp=/tmp/7036.php
+ cmd='php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/7036.php'
+ '[' -n 'php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/7036.php' ']'
++ echo 'php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/7036.php'
++ cut '-d ' -f1
+ tool=php
+ paint gray red echo '--> php syntax checking for test.php'

But still No php syntax errors detected in 'test.php'.

I also tried to comment out removing of the tmp file and tried this:

$ php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/7036.php

Parse error: syntax error, unexpected 'adsasdsa' (T_STRING), expecting ',' or ';' in C:/Users/Martin/AppData/Local/Temp/7036.php on line 4
Errors parsing C:/Users/Martin/AppData/Local/Temp/7036.php
mhujer commented 10 years ago

Ping @tomasfejfar ? If I can remember, you said it worked for you on Windows, right?

mhujer commented 10 years ago

I just tried yaml file and it works (commit fails for invalid yaml)

tomasfejfar commented 10 years ago

Not sure I use(d) the same version. (I installed by means of http://blog.prskavec.net/2014/01/git-a-pre-commit-hook-pro-kontrolu-syntaxe-v-mnoha-jazycich/) Unfortunatelly (if you remember) I accidentally trashed my .git with FTP sync last week so I can't really tell :(

kvz commented 10 years ago

Maybe pipefail doesn't work. What if you try wihout paint?

e.g. if ! git cat-file -p ${sha} | ${cmd}; then on line 122

Also, what is your bash version? bash --version could tell

mhujer commented 10 years ago
$ bash --version
GNU bash, version 3.1.0(1)-release (i686-pc-msys)
Copyright (C) 2005 Free Software Foundation, Inc.

After changing the line 122, error is reported from php -l (but it is still commited)

+ '[' -n /tmp/3484.php ']'
+ git cat-file -p d81e3e0207a665622eba827dce18ca878bba91dd
+ git cat-file -p d81e3e0207a665622eba827dce18ca878bba91dd
+ php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/3484.php

Parse error: syntax error, unexpected 'adsasdsa' (T_STRING), expecting ',' or ';' in C:/Users/Martin/AppData/Local/Temp/3484.php o
n line 4
Errors parsing C:/Users/Martin/AppData/Local/Temp/3484.php
+ paint green red echo 'No php syntax errors detected in '\''test.php'\'''
+ set -o pipefail
...
...
+ echo 'No php syntax errors detected in '\''test.php'\'''
+ sed 's,.*,&,'
+ sed 's,.*,&,'
No php syntax errors detected in 'test.php'
+ read -r line
[master 1958e7d] x
 1 file changed, 1 insertion(+)
mhujer commented 10 years ago

But this may be the issue:

$ paint
bash": paint: command not found

But the paint package is not available in cygwin (or is it part of some other package?)

kvz commented 10 years ago

paint is a function declared higher up in the file. It's just cosmetics, giving stdout & stderr a color.

I was thinking that it prevented the php exit code from bubling up because pipefail was ignored. That seems not the case. Even without the paint wrapper, it says "No php syntax errors detected" (and you clearly see the errors).

what if you type

echo "exit code; ${?}" right after the php command? take it outside of the if statement, that should reveil the exist code

mhujer commented 10 years ago

That's probably it:

+ php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/4960.php

Parse error: syntax error, unexpected 'adsasdsa' (T_STRING), expecting ',' or ';' in C:/Users/Martin/AppData/Local/Temp/4960.php o
n line 4
Errors parsing C:/Users/Martin/AppData/Local/Temp/4960.php
+ echo 'exit code; 0'
exit code; 0

exit code is 0 eventhough there are errors. So I suppose that cygwin does not return proper error codes when it launches Windows apps.

I also tried this: http://www.cyberciti.biz/faq/shell-how-to-determine-the-exit-status-of-linux-and-unix-command/

$ php -n -l -ddisplay_errors=1 -derror_reporting=E_ALL -dlog_errrors=0 /tmp/4960.php

Parse error: syntax error, unexpected 'adsasdsa' (T_STRING), expecting ',' or ';' in C:/Users/Martin/AppData/Local/Temp/4960.php
n line 4
Errors parsing C:/Users/Martin/AppData/Local/Temp/4960.php

$ echo $?
0

-----------

$ asdasdas
bash": asdasdas: command not found

$ echo $?
127

I will have a look on a cygwin later, to see if I can make the error codes work.