From 7491173c8c0935d283de671552e5e157537d3bd7 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Tue, 21 Feb 2017 15:20:15 +0100 Subject: [PATCH] TEMPORARY: re-add opt-in for builtin difftool Upstream Git already picked up the builtin difftool (it took only six iterations this time to get our changes into upstream) and it will be part of v2.12.0. In the meantime, to make maintenance of Git for Windows easier, let's just add back the opt-in, and then drop this patch as soon as it is time to release Git for Windows v2.12.0. Signed-off-by: Johannes Schindelin --- .gitignore | 1 + Makefile | 1 + builtin/difftool.c | 47 ++++++++++++++++++- ...-difftool.perl => git-legacy-difftool.perl | 0 git.c | 7 ++- t/t7800-difftool.sh | 41 +++++++++++++++- 6 files changed, 93 insertions(+), 4 deletions(-) rename contrib/examples/git-difftool.perl => git-legacy-difftool.perl (100%) mode change 100755 => 100644 diff --git a/.gitignore b/.gitignore index 6722f78f9a..5555ae025b 100644 --- a/.gitignore +++ b/.gitignore @@ -76,6 +76,7 @@ /git-init-db /git-interpret-trailers /git-instaweb +/git-legacy-difftool /git-log /git-ls-files /git-ls-remote diff --git a/Makefile b/Makefile index 71a4273f1d..4137a9915b 100644 --- a/Makefile +++ b/Makefile @@ -527,6 +527,7 @@ SCRIPT_LIB += git-sh-setup SCRIPT_LIB += git-sh-i18n SCRIPT_PERL += git-add--interactive.perl +SCRIPT_PERL += git-legacy-difftool.perl SCRIPT_PERL += git-archimport.perl SCRIPT_PERL += git-cvsexportcommit.perl SCRIPT_PERL += git-cvsimport.perl diff --git a/builtin/difftool.c b/builtin/difftool.c index d13350ce83..a3510caf13 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -616,10 +616,34 @@ static int run_file_diff(int prompt, const char *prefix, exit(ret); } +/* + * NEEDSWORK: this function can go once the legacy-difftool Perl script is + * retired. + * + * We intentionally avoid reading the config directly here, to avoid messing up + * the GIT_* environment variables when we need to fall back to exec()ing the + * Perl script. + */ +static int use_builtin_difftool(void) { + struct child_process cp = CHILD_PROCESS_INIT; + struct strbuf out = STRBUF_INIT; + int ret; + + argv_array_pushl(&cp.args, + "config", "--bool", "difftool.usebuiltin", NULL); + cp.git_cmd = 1; + if (capture_command(&cp, &out, 6)) + return 0; + strbuf_trim(&out); + ret = !strcmp("true", out.buf); + strbuf_release(&out); + return ret; +} + int cmd_difftool(int argc, const char **argv, const char *prefix) { int use_gui_tool = 0, dir_diff = 0, prompt = -1, symlinks = 0, - tool_help = 0; + tool_help = 0, nongit; static char *difftool_cmd = NULL, *extcmd = NULL; struct option builtin_difftool_options[] = { OPT_BOOL('g', "gui", &use_gui_tool, @@ -647,6 +671,25 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) OPT_END() }; + /* + * NEEDSWORK: Once the builtin difftool has been tested enough + * and git-legacy-difftool.perl is retired to contrib/, this preamble + * can be removed. + */ + if (!use_builtin_difftool()) { + const char *path = mkpath("%s/git-legacy-difftool", + git_exec_path()); + + if (sane_execvp(path, (char **)argv) < 0) + die_errno("could not exec %s", path); + + return 0; + } + prefix = setup_git_directory_gently(&nongit); + trace_repo_setup(prefix); + if (!nongit) + setup_work_tree(); + git_config(difftool_config, NULL); symlinks = has_symlinks; @@ -657,6 +700,8 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) if (tool_help) return print_tool_help(); + if (nongit) + die("Not a git repository (or any of the parent directories): .git"); /* NEEDSWORK: once we no longer spawn anything, remove this */ setenv(GIT_DIR_ENVIRONMENT, absolute_path(get_git_dir()), 1); setenv(GIT_WORK_TREE_ENVIRONMENT, absolute_path(get_git_work_tree()), 1); diff --git a/contrib/examples/git-difftool.perl b/git-legacy-difftool.perl old mode 100755 new mode 100644 similarity index 100% rename from contrib/examples/git-difftool.perl rename to git-legacy-difftool.perl diff --git a/git.c b/git.c index d2d843d824..519a4c6f82 100644 --- a/git.c +++ b/git.c @@ -424,7 +424,12 @@ static struct cmd_struct commands[] = { { "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE }, { "diff-index", cmd_diff_index, RUN_SETUP }, { "diff-tree", cmd_diff_tree, RUN_SETUP }, - { "difftool", cmd_difftool, RUN_SETUP | NEED_WORK_TREE }, + /* + * NEEDSWORK: Once the redirection to git-legacy-difftool.perl in + * builtin/difftool.c has been removed, this entry should be changed to + * RUN_SETUP | NEED_WORK_TREE + */ + { "difftool", cmd_difftool }, { "fast-export", cmd_fast_export, RUN_SETUP }, { "fetch", cmd_fetch, RUN_SETUP }, { "fetch-pack", cmd_fetch_pack, RUN_SETUP }, diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 97bae54d83..e7ec09d6f3 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -23,13 +23,41 @@ prompt_given () test "$prompt" = "Launch 'test-tool' [Y/n]? branch" } +for use_builtin_difftool in false true +do + +case "$use_builtin_difftool" in +true) + GIT_CONFIG_PARAMETERS="'difftool.usebuiltin=true'" + export GIT_CONFIG_PARAMETERS + ;; +false) + test_have_prereq PERL || { + say 'Skipping scripted difftool (missing PERL)' + continue + } + ;; +esac + +test_expect_success 'verify we are running the correct difftool' ' + if test true = '$use_builtin_difftool' + then + test_must_fail ok=129 git difftool -h >help && + grep "g, --gui" help + else + git difftool -h >help && + grep "g|--gui" help + fi +' + test_expect_success 'basic usage requires no repo' ' - test_expect_code 129 git difftool -h >output && + code='$(test "$use_builtin_difftool" = true && echo 129 || echo 0)' && + test_expect_code $code git difftool -h >output && grep ^usage: output && # create a ceiling directory to prevent Git from finding a repo mkdir -p not/repo && test_when_finished rm -r not && - test_expect_code 129 \ + test_expect_code $code \ env GIT_CEILING_DIRECTORIES="$(pwd)/not" \ git -C not/repo difftool -h >output && grep ^usage: output @@ -616,4 +644,13 @@ test_expect_success SYMLINKS 'difftool --dir-diff symlinked directories' ' ) ' +test true != $use_builtin_difftool || break + +test_expect_success 'tear down for re-run' ' + rm -rf * .[a-z]* && + git init +' + +done + test_done