From d81345ce09c121fb0d18dec5c2535ec8d2a67542 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Sun, 5 Feb 2017 13:23:38 -0800 Subject: [PATCH 1/2] difftool: fix bug when printing usage "git difftool -h" reports an error: fatal: BUG: setup_git_env called without repository Defer repository setup so that the help option processing happens before the repository is initialized. Add tests to ensure that the basic usage works inside and outside of a repository. Signed-off-by: David Aguilar Acked-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin/difftool.c | 8 ++++---- t/t7800-difftool.sh | 13 +++++++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/builtin/difftool.c b/builtin/difftool.c index b5e85ab079..d13350ce83 100644 --- a/builtin/difftool.c +++ b/builtin/difftool.c @@ -647,10 +647,6 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) OPT_END() }; - /* 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); - git_config(difftool_config, NULL); symlinks = has_symlinks; @@ -661,6 +657,10 @@ int cmd_difftool(int argc, const char **argv, const char *prefix) if (tool_help) return print_tool_help(); + /* 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); + if (use_gui_tool && diff_gui_tool && *diff_gui_tool) setenv("GIT_DIFF_TOOL", diff_gui_tool, 1); else if (difftool_cmd) { diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 81a2de2b9d..1c81f46cbf 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -23,6 +23,19 @@ prompt_given () test "$prompt" = "Launch 'test-tool' [Y/n]? branch" } +test_expect_success 'basic usage requires no repo' ' + lines=$(git difftool -h | grep ^usage: | wc -l) && + test "$lines" -eq 1 && + # create a ceiling directory to prevent Git from finding a repo + mkdir -p not/repo && + ceiling="$PWD/not" && + lines=$(cd not/repo && + GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | + grep ^usage: | wc -l) && + test "$lines" -eq 1 && + rmdir -p not/repo +' + # Create a file on master and change it on branch test_expect_success 'setup' ' echo master >file && From e66adcadfe63508dfd7410c2253116043894d298 Mon Sep 17 00:00:00 2001 From: David Aguilar Date: Tue, 7 Feb 2017 01:16:59 -0800 Subject: [PATCH 2/2] t7800: simplify basic usage test Use "test_line_count" instead of "wc -l", use "git -C" instead of a subshell, and use test_expect_code when calling difftool. Ease debugging by capturing output into temporary files. Suggested-by: Johannes Schindelin Signed-off-by: David Aguilar Signed-off-by: Junio C Hamano --- t/t7800-difftool.sh | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh index 1c81f46cbf..3d728e296d 100755 --- a/t/t7800-difftool.sh +++ b/t/t7800-difftool.sh @@ -24,16 +24,15 @@ prompt_given () } test_expect_success 'basic usage requires no repo' ' - lines=$(git difftool -h | grep ^usage: | wc -l) && - test "$lines" -eq 1 && + test_expect_code 129 git difftool -h >output && + grep ^usage: output && # create a ceiling directory to prevent Git from finding a repo mkdir -p not/repo && - ceiling="$PWD/not" && - lines=$(cd not/repo && - GIT_CEILING_DIRECTORIES="$ceiling" git difftool -h | - grep ^usage: | wc -l) && - test "$lines" -eq 1 && - rmdir -p not/repo + test_when_finished rm -r not && + test_expect_code 129 \ + env GIT_CEILING_DIRECTORIES="$(pwd)/not" \ + git -C not/repo difftool -h >output && + grep ^usage: output ' # Create a file on master and change it on branch