From f49fc7c7fd05bc402a34def6fdb94c0f062f7df5 Mon Sep 17 00:00:00 2001 From: Roland Walker Date: Sun, 20 May 2018 00:13:42 -0400 Subject: [PATCH] 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 --- Makefile | 1 + configure.ac | 2 +- include/tig/apps.h | 38 +++++++++ include/tig/io.h | 7 +- include/tig/tig.h | 7 +- src/apps.c | 125 ++++++++++++++++++++++++++++ src/argv.c | 2 +- src/diff.c | 30 ++++--- src/display.c | 2 +- src/io.c | 43 +++++++++- src/options.c | 2 +- src/prompt.c | 2 +- test/diff/diff-highlight-test | 140 +++++++++++++++----------------- test/tools/valgrind-Darwin.supp | 2 +- 14 files changed, 311 insertions(+), 92 deletions(-) create mode 100644 include/tig/apps.h create mode 100644 src/apps.c diff --git a/Makefile b/Makefile index 81c167c2f..5f0c1a12a 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 4b914e821..b9848b4db 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 ]) diff --git a/include/tig/apps.h b/include/tig/apps.h new file mode 100644 index 000000000..c90ad6fa2 --- /dev/null +++ b/include/tig/apps.h @@ -0,0 +1,38 @@ +/* Copyright (c) 2006-2017 Jonas Fonseca + * + * 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 4649cea99..b2ca0b4d0 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 20e55769f..63970ff97 100644 --- a/include/tig/tig.h +++ b/include/tig/tig.h @@ -53,6 +53,7 @@ #include #include #include +#include #include #include @@ -65,6 +66,10 @@ #include #endif +#ifdef HAVE_PATHS_H +#include +#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 000000000..28a4ac028 --- /dev/null +++ b/src/apps.c @@ -0,0 +1,125 @@ +/* Copyright (c) 2006-2017 Jonas Fonseca + * + * 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 276d08d70..de32c4bac 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 583f1e0e0..63877a286 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 efc86d751..265a3a056 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 9cc30b401..f60e98d4e 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 bcef40c86..cd31c2dc8 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 816f52ee4..9a7ad1a7b 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 851d71f56..c123faeed 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 < 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' < -AuthorDate: Sat Mar 1 15:59:02 2014 -0500 -Commit: Jonas Fonseca -CommitDate: Sat Mar 1 15:59:02 2014 -0500 +test_case diff-highlight-custom \ + --tigrc='set diff-highlight = wc' \ + --args='show master^' \ + <