Let `set diff-highlight=true` work when the diff-highlight executable is not on $PATH (#662)

* recast expand_path as path_expand
* recast SIZEOF_CMD as SIZEOF_MED_STR
* define _PATH_DEFPATH, conditionally incl paths.h
* path_search() generalized "which"
* stub apps.c for external applications
* let set diff-highlight=true work when not on $PATH
  * diff-highlight is probably not on the user's $PATH in most installs
  * find it relative to git --exec-path
  * also rescue the case where diff-highlight isn't a prepared executable

14 files changed, 311 insertions, 92 deletions

diff --git a/Makefile b/Makefile
index 81c167c2..5f0c1a12 100644
--- a/Makefile
+++ b/Makefile
@@ -311,6 +311,7 @@ TIG_OBJS = \
 	src/stash.o \
 	src/grep.o \
 	src/ui.o \
+	src/apps.o \
 	$(GRAPH_OBJS) \
 	$(COMPAT_OBJS)
 
diff --git a/configure.ac b/configure.ac
index 4b914e82..b9848b4d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -11,7 +11,7 @@ m4_pattern_forbid([^AX_])
 
 AC_PROG_CC
 
-AC_CHECK_HEADERS([execinfo.h stdint.h stdlib.h string.h sys/time.h unistd.h wordexp.h])
+AC_CHECK_HEADERS([execinfo.h paths.h stdint.h stdlib.h string.h sys/time.h unistd.h wordexp.h])
 AC_CHECK_FUNCS([gettimeofday])
 AC_CHECK_DECLS([environ])
 AC_CHECK_DECLS([errno], [], [], [#include <errno.h>])
diff --git a/include/tig/apps.h b/include/tig/apps.h
new file mode 100644
index 00000000..c90ad6fa
--- /dev/null
+++ b/include/tig/apps.h
@@ -0,0 +1,38 @@
+/* Copyright (c) 2006-2017 Jonas Fonseca <jonas.fonseca@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#ifndef TIG_APPS_H
+#define TIG_APPS_H
+
+#include "tig/tig.h"
+#include "tig/argv.h"
+#include "tig/util.h"
+
+/*
+ * general
+ */
+
+struct app_external {
+	const char *argv[SIZEOF_ARG];
+	char * const env[SIZEOF_ARG];
+};
+
+/*
+ * diff-highlight
+ */
+
+struct app_external *app_diff_highlight_load(const char *query);
+
+#endif
+
+/* vim: set ts=8 sw=8 noexpandtab: */
diff --git a/include/tig/io.h b/include/tig/io.h
index 4649cea9..b2ca0b4d 100644
--- a/include/tig/io.h
+++ b/include/tig/io.h
@@ -41,7 +41,12 @@ extern struct encoding *default_encoding;
  * Path manipulation.
  */
 
-bool expand_path(char *dst, size_t dstlen, const char *src);
+#ifndef _PATH_DEFPATH
+#define _PATH_DEFPATH	"/usr/bin:/bin"
+#endif
+
+bool path_expand(char *dst, size_t dstlen, const char *src);
+bool path_search(char *dst, size_t dstlen, const char *query, const char *colon_path, int access_flags);
 
 /*
  * Executing external commands.
diff --git a/include/tig/tig.h b/include/tig/tig.h
index 20e55769..63970ff9 100644
--- a/include/tig/tig.h
+++ b/include/tig/tig.h
@@ -53,6 +53,7 @@
 #include <sys/file.h>
 #include <time.h>
 #include <fcntl.h>
+#include <libgen.h>
 #include <termios.h>
 
 #include <regex.h>
@@ -65,6 +66,10 @@
 #include <execinfo.h>
 #endif
 
+#ifdef HAVE_PATHS_H
+#include <paths.h>
+#endif
+
 /* ncurses(3): Must be defined to have extended wide-character functions. */
 #define _XOPEN_SOURCE_EXTENDED 1
 
@@ -118,9 +123,9 @@
 #define STRING_SIZE(x)	(sizeof(x) - 1)
 
 #define SIZEOF_STR	1024	/* Default string size. */
+#define SIZEOF_MED_STR	8192	/* Medium string size. */
 #define SIZEOF_REF	256	/* Size of symbolic or SHA1 ID. */
 #define SIZEOF_REV	41	/* Holds a SHA-1 and an ending NUL. */
-#define SIZEOF_CMD	8192	/* Default command string size. */
 
 /* This color name can be used to refer to the default term colors. */
 #define COLOR_DEFAULT	(-1)
diff --git a/src/apps.c b/src/apps.c
new file mode 100644
index 00000000..28a4ac02
--- /dev/null
+++ b/src/apps.c
@@ -0,0 +1,125 @@
+/* Copyright (c) 2006-2017 Jonas Fonseca <jonas.fonseca@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include "tig/tig.h"
+#include "tig/io.h"
+#include "tig/apps.h"
+
+/*
+ * general
+ */
+
+static bool
+app_oneline_buf(char *buf, size_t bufsize, struct app_external *app, const char *dir)
+{
+	struct io io;
+	return io_run(&io, IO_RD, dir, app->env, app->argv) \
+		&& io_read_buf(&io, buf, bufsize, false);
+}
+
+/*
+ * git
+ */
+
+static bool
+app_git_exec_path(char *path, size_t path_len)
+{
+	static char exec_path[SIZEOF_STR] = "";
+	struct app_external app = {
+		{ "git", "--exec-path", NULL },
+		{ "GIT_CONFIG=/dev/null", NULL },
+	};
+
+	if (!*exec_path)
+		app_oneline_buf(exec_path, sizeof(exec_path), &app, NULL);
+
+	if (!*exec_path)
+		return false;
+
+	string_ncopy_do(path, path_len, exec_path, sizeof(exec_path));
+	return true;
+}
+
+/*
+ * diff-highlight
+ */
+
+static bool
+app_diff_highlight_path_search(char *dest, size_t destlen, const char *query)
+{
+	if (!query || !*query)
+		return false;
+
+	if (strchr(query, '/')) {
+		/* can only be interpreted as a fully qualified path */
+		string_ncopy_do(dest, destlen, query, strlen(query));
+		return true;
+	}
+
+	const char *env_path = getenv("PATH");
+	char env_path_plus[SIZEOF_MED_STR];
+	char exec_path[SIZEOF_STR];
+
+	if (!env_path || !*env_path)
+		env_path = _PATH_DEFPATH;
+
+	if (app_git_exec_path(exec_path, sizeof(exec_path)))
+		string_format(env_path_plus, "%s:%s/%s:%s/%s:%s/%s:%s/%s",
+			      env_path,
+			      exec_path, "../../share/git-core/contrib/diff-highlight",
+			      exec_path, "../share/git-core/contrib/diff-highlight",
+			      exec_path, "../../share/git/contrib/diff-highlight",
+			      exec_path, "../share/git/contrib/diff-highlight");
+	else
+		string_ncopy(env_path_plus, env_path, strlen(env_path));
+
+	if (!path_search(dest, destlen, query, env_path_plus, X_OK)
+	    && !strcmp(query, "diff-highlight")
+	    && !path_search(dest, destlen, "diff-highlight.perl", env_path_plus, R_OK))
+		return false;
+
+	return true;
+}
+
+struct app_external
+*app_diff_highlight_load(const char *query)
+{
+	static struct app_external dhlt_app = { { NULL }, { "GIT_CONFIG=/dev/null", NULL } };
+	static bool did_search = false;
+	static char dhlt_path[SIZEOF_STR];
+	static char perl_path[SIZEOF_STR];
+	static char perl_include[SIZEOF_STR];
+
+	if (!did_search
+	    && app_diff_highlight_path_search(dhlt_path, sizeof(dhlt_path), query)
+	    && *dhlt_path) {
+		if (suffixcmp(dhlt_path, strlen(dhlt_path), "/diff-highlight.perl")) {
+			dhlt_app.argv[0] = dhlt_path;
+			dhlt_app.argv[1] = NULL;
+		} else if (path_search(perl_path, sizeof(perl_path), "perl", getenv("PATH"), X_OK)) {
+			/* if the package manager failed to "make install" within the contrib dir, rescue via */
+			/* perl -MDiffHighlight -I/path/containing /path/containing/diff-highlight.perl */
+			string_format(perl_include, "-I%s", dirname(dhlt_path));
+			dhlt_app.argv[0] = perl_path;
+			dhlt_app.argv[1] = "-MDiffHighlight";
+			dhlt_app.argv[2] = perl_include;
+			dhlt_app.argv[3] = dhlt_path;
+			dhlt_app.argv[4] = NULL;
+		}
+	}
+	did_search = true;
+
+	return &dhlt_app;
+}
+
+/* vim: set ts=8 sw=8 noexpandtab: */
diff --git a/src/argv.c b/src/argv.c
index 276d08d7..de32c4ba 100644
--- a/src/argv.c
+++ b/src/argv.c
@@ -286,7 +286,7 @@ struct format_var {
 struct format_context {
 	struct format_var *vars;
 	size_t vars_size;
-	char buf[SIZEOF_CMD];
+	char buf[SIZEOF_MED_STR];
 	size_t bufpos;
 	bool file_filter;
 };
diff --git a/src/diff.c b/src/diff.c
index 583f1e0e..63877a28 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -20,6 +20,7 @@
 #include "tig/pager.h"
 #include "tig/diff.h"
 #include "tig/draw.h"
+#include "tig/apps.h"
 
 static enum status_code
 diff_open(struct view *view, enum open_flags flags)
@@ -45,18 +46,27 @@ diff_open(struct view *view, enum open_flags flags)
 enum status_code
 diff_init_highlight(struct view *view, struct diff_state *state)
 {
-	if (opt_diff_highlight) {
-		const char *argv[] = { opt_diff_highlight, NULL };
-		char * const env[] = { "GIT_CONFIG=/dev/null", NULL };
-		struct io io;
+	if (!opt_diff_highlight || !*opt_diff_highlight)
+		return SUCCESS;
 
-		if (!io_exec(&io, IO_RP, view->dir, env, argv, view->io.pipe))
-			return error("Failed to run %s", opt_diff_highlight);
+	struct app_external *app = app_diff_highlight_load(opt_diff_highlight);
+	struct io io;
 
-		state->view_io = view->io;
-		view->io = io;
-		state->highlight = true;
-	}
+	/* XXX This empty string keeps valgrind happy while preserving earlier
+	 * behavior of test diff/diff-highlight-test:diff-highlight-misconfigured.
+	 * Simpler would be to return error when user misconfigured, though we
+	 * don't want tig to fail when diff-highlight isn't present.  io_exec
+	 * below does not return error when app->argv[0] is empty or null as the
+	 * conditional might suggest. */
+	if (!*app->argv)
+		app->argv[0] = "";
+
+	if (!io_exec(&io, IO_RP, view->dir, app->env, app->argv, view->io.pipe))
+		return error("Failed to run %s", opt_diff_highlight);
+
+	state->view_io = view->io;
+	view->io = io;
+	state->highlight = true;
 
 	return SUCCESS;
 }
diff --git a/src/display.c b/src/display.c
index efc86d75..265a3a05 100644
--- a/src/display.c
+++ b/src/display.c
@@ -53,7 +53,7 @@ open_script(const char *path)
 
 	char buf[SIZEOF_STR];
 
-	if (!expand_path(buf, sizeof(buf), path))
+	if (!path_expand(buf, sizeof(buf), path))
 		return error("Failed to expand path: %s", path);
 
 	return io_open(&script_io, "%s", buf)
diff --git a/src/io.c b/src/io.c
index 9cc30b40..f60e98d4 100644
--- a/src/io.c
+++ b/src/io.c
@@ -136,7 +136,7 @@ get_path_encoding(const char *path, struct encoding *default_encoding)
  */
 
 bool
-expand_path(char *dst, size_t dstlen, const char *src)
+path_expand(char *dst, size_t dstlen, const char *src)
 {
 	if (!src)
 		return false;
@@ -166,6 +166,47 @@ expand_path(char *dst, size_t dstlen, const char *src)
 	return true;
 }
 
+bool
+path_search(char *dst, size_t dstlen, const char *query, const char *colon_path, int access_flags)
+{
+	const char *_colon_path = _PATH_DEFPATH; /* emulate execlp() */
+	char test[SIZEOF_STR];
+	char elt[SIZEOF_STR];
+	size_t elt_len;
+
+	if (!query || !*query)
+		return false;
+
+	if (strchr(query, '/')) {
+		if (access(query, access_flags))
+			return false;
+		string_ncopy_do(dst, dstlen, query, strlen(query));
+		return true;
+	}
+
+	if (colon_path && *colon_path)
+		_colon_path = colon_path;
+
+	while (_colon_path && *_colon_path) {
+		elt_len = strcspn(_colon_path, ":");
+		if (elt_len)
+			string_ncopy(elt, _colon_path, elt_len);
+		else
+			string_ncopy(elt, ".", 1);
+
+		_colon_path += elt_len;
+		if (*_colon_path)
+			_colon_path += 1;
+
+		string_format(test, "%s/%s", elt, query);
+		if (!access(test, access_flags)) {
+			string_ncopy_do(dst, dstlen, test, strlen(test));
+			return true;
+		}
+	}
+	return false;
+}
+
 /*
  * Executing external commands.
  */
diff --git a/src/options.c b/src/options.c
index bcef40c8..cd31c2dc 100644
--- a/src/options.c
+++ b/src/options.c
@@ -986,7 +986,7 @@ load_option_file(const char *path)
 	if (!path || !strlen(path))
 		return SUCCESS;
 
-	if (!expand_path(buf, sizeof(buf), path))
+	if (!path_expand(buf, sizeof(buf), path))
 		return error("Failed to expand path: %s", path);
 
 	/* It's OK that the file doesn't exist. */
diff --git a/src/prompt.c b/src/prompt.c
index 816f52ee..9a7ad1a7 100644
--- a/src/prompt.c
+++ b/src/prompt.c
@@ -1078,7 +1078,7 @@ exec_run_request(struct view *view, struct run_request *req)
 	const char **argv = NULL;
 	bool confirmed = false;
 	enum request request = REQ_NONE;
-	char cmd[SIZEOF_CMD];
+	char cmd[SIZEOF_MED_STR];
 	const char *req_argv[SIZEOF_ARG];
 	int req_argc = 0;
 
diff --git a/test/diff/diff-highlight-test b/test/diff/diff-highlight-test
index 851d71f5..c123faee 100755
--- a/test/diff/diff-highlight-test
+++ b/test/diff/diff-highlight-test
@@ -3,6 +3,8 @@
 . libtest.sh
 . libgit.sh
 
+in_work_dir create_repo_from_tgz "$base_dir/files/scala-js-benchmarks.tgz"
+
 file bin/diff-highlight <<EOF
 #!/bin/sh
 exec sed 's/Add type parameter for js.Dynamic/Changed by diff-highlight/'
@@ -10,27 +12,11 @@ EOF
 
 chmod +x bin/diff-highlight
 
-tigrc <<EOF
-set diff-highlight = true
-EOF
-
-export PATH="./bin:$PATH"
-
-steps '
-	:save-display diff-default.screen
-
-	:set diff-highlight = cat
-	:save-display diff-custom.screen
-
-	:set diff-highlight = program-that-does-not-exist
-	:save-display diff-failure.screen
-'
-
-in_work_dir create_repo_from_tgz "$base_dir/files/scala-js-benchmarks.tgz"
-
-test_tig show master^
-
-assert_equals 'diff-default.screen' <<EOF
+test_case diff-highlight-true \
+	--tigrc='set diff-highlight = true' \
+	--args='show master^' \
+	--subshell='export PATH="./bin:$PATH"' \
+	<<EOF
 commit a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff
 Author:     Jonas Fonseca <jonas.fonseca@gmail.com>
 AuthorDate: Sat Mar 1 15:59:02 2014 -0500
@@ -62,66 +48,74 @@ index 65f914a..3aa4320 100644
 [diff] a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff - line 1 of 24              100%
 EOF
 
-assert_equals 'diff-custom.screen' <<EOF
-commit a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff
-Author:     Jonas Fonseca <jonas.fonseca@gmail.com>
-AuthorDate: Sat Mar 1 15:59:02 2014 -0500
-Commit:     Jonas Fonseca <jonas.fonseca@gmail.com>
-CommitDate: Sat Mar 1 15:59:02 2014 -0500
+test_case diff-highlight-custom \
+	--tigrc='set diff-highlight = wc' \
+	--args='show master^' \
+	<<EOF
+24      85    1067
 
-    Add type parameter for js.Dynamic
----
- common/src/main/scala/org/scalajs/benchmark/Benchmark.scala | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
 
-diff --git a/common/src/main/scala/org/scalajs/benchmark/Benchmark.scala b/commo
-index 65f914a..3aa4320 100644
---- a/common/src/main/scala/org/scalajs/benchmark/Benchmark.scala
-+++ b/common/src/main/scala/org/scalajs/benchmark/Benchmark.scala
-@@ -15,7 +15,7 @@ object Benchmark {
-   val benchmarks = js.Array[Benchmark]()
-   val benchmarkApps = js.Array[BenchmarkApp]()
 
--  val global = js.Dynamic.global.asInstanceOf[js.Dictionary]
-+  val global = js.Dynamic.global.asInstanceOf[js.Dictionary[js.Any]]
-   global("runScalaJSBenchmarks") = runBenchmarks _
-   global("initScalaJSBenchmarkApps") = initBenchmarkApps _
 
 
 
 
 
-[diff] a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff - line 1 of 24              100%
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+[diff] HEAD - line 1 of 1                                                   100%
 EOF
 
-assert_equals 'diff-failure.screen' <<EOF
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
- 
-[diff] a1dcf1aaa11470978db1d5d8bcf9e16201eb70ff                               0%
+test_case diff-highlight-misconfigured \
+	--tigrc='set diff-highlight = program-that-does-not-exist' \
+	--args='show master^' \
+	<<EOF
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+[diff]                                                                        0%
 EOF
+
+run_test_cases
diff --git a/test/tools/valgrind-Darwin.supp b/test/tools/valgrind-Darwin.supp
index 62c48ef0..032edaa0 100644
--- a/test/tools/valgrind-Darwin.supp
+++ b/test/tools/valgrind-Darwin.supp
@@ -3,7 +3,7 @@
 	Memcheck:Cond
 	fun:strstr
 	fun:wordexp
-	fun:expand_path
+	fun:path_expand
 	...
 	fun:main
 }