[PATCH/RFC] t9500-*.sh: Fix highlight test hang on Linux Mint

Ramsay Jones· Sep 6, 2013, 22:05 UTC · lore

Linux Mint has an implementation of the highlight command (unrelated to the one from http://www.andre-simon.de) that works as a simple filter. The script uses 'sed' to add terminal colour escape codes around text matching a regular expression. When t9500-*.sh attempts to run "highlight --version", the script simply hangs waiting for input. (See https://bugs.launchpad.net/linuxmint/+bug/815005).

The tool required by gitweb can be installed from the 'highlight' package. Unfortunately, given the default $PATH, this leads to the tool having lower precedence than the script.

In order to allow the user to specify the correct tool, introduce the GIT_TEST_HIGHLIGHT_BIN variable. Also, add '</dev/null' to the command line of the highlight invocation; this avoids hanging the test if the filter script is used nonetheless.

Signed-off-by: Ramsay Jones <ramsay@ramsay1.demon.co.uk>
---

Hi Junio,

I recently upgraded my Ubuntu installation to Linux Mint 15 Cinnamon. (Unity makes me want to throw my laptop at the wall!)

Having tickled this bug, I solved the problem by building highlight v3.15 from source and installing in $HOME.

This patch is marked RFC because this bug does not seem to have affected too many people (given that Heiko reported the problem back in 2011) ... :-D

[Also, note that I didn't fix up the form of the conditional.]

ATB, Ramsay Jones

 t/t9500-gitweb-standalone-no-errors.sh | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

Show changes to t/t9500-gitweb-standalone-no-errors.sh +9 −5

diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6fca193..0208c8e 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -683,14 +683,18 @@ test_expect_success \
 # syntax highlighting
 
 
-highlight --version >/dev/null 2>&1
+GIT_TEST_HIGHLIGHT_BIN=${GIT_TEST_HIGHLIGHT_BIN:-highlight}
+highlight_version=$($GIT_TEST_HIGHLIGHT_BIN --version </dev/null 2>/dev/null)
 if [ $? -eq 127 ]; then
-	say "Skipping syntax highlighting test, because 'highlight' was not found"
+	say "Skipping syntax highlighting tests: 'highlight' not found"
+elif test -z "$highlight_version"; then
+	say "Skipping syntax highlighting tests: incorrect 'highlight' found"
+	say "set GIT_TEST_HIGHLIGHT_BIN to full path of highlight program"
 else
 	test_set_prereq HIGHLIGHT
-	cat >>gitweb_config.perl <<-\EOF
-	our $highlight_bin = "highlight";
-	$feature{'highlight'}{'override'} = 1;
+	cat >>gitweb_config.perl <<-EOF
+	our \$highlight_bin = "$GIT_TEST_HIGHLIGHT_BIN";
+	\$feature{'highlight'}{'override'} = 1;
 	EOF
 fi

-- 
1.8.4

Junio C Hamano· Sep 7, 2013, 16:23 UTC · re: Ramsay Jones · lore

Re: [PATCH/RFC] t9500-*.sh: Fix highlight test hang on Linux Mint

Ramsay Jones <ramsay@ramsay1.demon.co.uk> writes:

> In order to allow the user to specify the correct tool, introduce
> the GIT_TEST_HIGHLIGHT_BIN variable. Also, add '</dev/null' to the
> command line of the highlight invocation; this avoids hanging the
> test if the filter script is used nonetheless.

gitweb/Makefile seems to have a rule to set this to a custom location, though. Why do we need a special test-only location? If the user wants the right one during the test, wouldn't the final build and install also want to use the same "right one" that is not just a generic 'highlight' that does not say which path it resides at?

So I think forcing the testers to say the same thing twice like this patch does is a good way forward. Add a mechanism to allow outside world to ask which "highlight" gitweb/Makefile was told to use, use that mechanism to learn the value of HIGHLIGHT_BIN to check if the binary is usable for the test, or something perhaps?

The killing of the input is independently a good idea, I think.

RFC patcht9500-*.sh: Fix highlight test hang on Linux Mint