Merge remote-tracking branch 'p/ticket/10760' into develop-olympus

* p/ticket/10760:
  [ticket/10760] Fix numbering in php fun facts.
  [ticket/10760] Value must be quoted as it might be two words.
  [ticket/10760] PHP is great. This commit is the proof.
  [ticket/10760] Catch both versions of parse error output in php.
  [ticket/10760] Account for display_errors=stderr in pre-commit hook.
  [ticket/10760] Quote PHP_BIN when using it.
  [ticket/10760] Use echo -e only if echo understands -e.
  [ticket/10760] Use externally specified PHP_BIN if any.

1 files changed, 59 insertions, 6 deletions

diff --git a/git-tools/hooks/pre-commit b/git-tools/hooks/pre-commit
index 4d03359773..03babe47cd 100755
--- a/git-tools/hooks/pre-commit
+++ b/git-tools/hooks/pre-commit
@@ -12,8 +12,17 @@
 # ln -s ../../git-tools/hooks/pre-commit \\
 #   .git/hooks/pre-commit
 
-# NOTE: this is run through /usr/bin/env
-PHP_BIN=php
+if [ -z "$PHP_BIN" ]
+then
+	PHP_BIN=php
+fi
+
+if [ "$(echo -e test)" = test ]
+then
+	echo_e="echo -e"
+else
+	echo_e="echo"
+fi
 
 # necessary check for initial commit
 if git rev-parse --verify HEAD >/dev/null 2>&1
@@ -27,7 +36,7 @@ fi
 error=0
 errors=""
 
-if ! which $PHP_BIN >/dev/null 2>&1
+if ! which "$PHP_BIN" >/dev/null 2>&1
 then
 	echo "PHP Syntax check failed:"
 	echo "PHP binary does not exist or is not in path: $PHP_BIN"
@@ -64,7 +73,13 @@ do
 
 	# check the staged file content for syntax errors
 	# using php -l (lint)
-	result=$(git cat-file -p $sha | /usr/bin/env $PHP_BIN -l 2>/dev/null)
+	# note: if display_errors=stderr in php.ini,
+	# parse errors are printed on stderr; otherwise
+	# they are printed on stdout.
+	# we filter everything other than parse errors
+	# with a grep below, therefore it should be safe
+	# to combine stdout and stderr in all circumstances
+	result=$(git cat-file -p $sha | "$PHP_BIN" -l 2>&1)
 	if [ $? -ne 0 ]
 	then
 		error=1
@@ -76,7 +91,45 @@ unset IFS
 
 if [ $error -eq 1 ]
 then
-	echo -e "PHP Syntax check failed:";
-	echo -e "$errors" | grep "^Parse error:"
+	echo "PHP Syntax check failed:"
+	# php "display errors" (display_errors php.ini value)
+	# and "log errors" (log_errors php.ini value).
+	# these are independent settings - see main/main.c in php source.
+	# the "log errors" setting produces output which
+	# starts with "PHP Parse error:"; the "display errors"
+	# setting produces output starting with "Parse error:".
+	# if both are turned on php dumps the parse error twice.
+	# therefore here we try to grep for one version and
+	# if that yields no results grep for the other version.
+	#
+	# other fun php facts:
+	#
+	# 1. in cli, display_errors and log_errors have different
+	#    destinations by default. display_errors prints to
+	#    standard output and log_errors prints to standard error.
+	#    whether these destinations make sense is left
+	#    as an exercise for the reader.
+	# 2. as mentioned above, with all output turned on
+	#    php will print parse errors twice, one time on stdout
+	#    and one time on stderr.
+	# 3. it is possible to set both display_errors and log_errors
+	#    to off. if this is done php will print the text
+	#    "Errors parsing <file>" but will not say what
+	#    the errors are. useful behavior, this.
+	# 4. on my system display_errors defaults to on and
+	#    log_errors defaults to off, therefore providing
+	#    by default one copy of messages. your mileage may vary.
+	# 5. by setting display_errors=stderr and log_errors=on,
+	#    both sets of messages will be printed on stderr.
+	# 6. php-cgi binary, given display_errors=stderr and
+	#    log_errors=on, still prints both sets of messages
+	#    on stderr, but formats one set as an html fragment.
+	# 7. your entry here? ;)
+	$echo_e "$errors" | grep "^Parse error:"
+	if [ $? -ne 0 ]
+	then
+		# match failed
+		$echo_e "$errors" | grep "^PHP Parse error:"
+	fi
 	exit 1
 fi