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