From fada767463b599951b37bd544379a1d18dcf9370 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Fri, 29 May 2015 02:49:10 -0400 Subject: setup_git_directory: delay core.bare/core.worktree errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If both core.bare and core.worktree are set, we complain about the bogus config and die. Dying is good, because it avoids commands running and doing damage in a potentially incorrect setup. But dying _there_ is bad, because it means that commands which do not even care about the work tree cannot run. This can make repairing the situation harder: [setup] $ git config core.bare true $ git config core.worktree /some/path [OK, expected.] $ git status fatal: core.bare and core.worktree do not make sense [Hrm...] $ git config --unset core.worktree fatal: core.bare and core.worktree do not make sense [Nope...] $ git config --edit fatal: core.bare and core.worktree do not make sense [Gaaah.] $ git help config fatal: core.bare and core.worktree do not make sense Instead, let's issue a warning about the bogus config when we notice it (i.e., for all commands), but only die when the command tries to use the work tree (by calling setup_work_tree). So we now get: $ git status warning: core.bare and core.worktree do not make sense fatal: unable to set up work tree using invalid config $ git config --unset core.worktree warning: core.bare and core.worktree do not make sense We have to update t1510 to accomodate this; it uses symbolic-ref to check whether the configuration works or not, but of course that command does not use the working tree. Instead, we switch it to use `git status`, as it requires a work-tree, does not need any special setup, and is read-only (so a failure will not adversely affect further tests). In addition, we add a new test that checks the desired behavior (i.e., that running "git config" with the bogus config does in fact work). Reported-by: SZEDER Gábor Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- setup.c | 12 ++++++++++-- t/t1510-repo-setup.sh | 24 ++++++++++++++++-------- 2 files changed, 26 insertions(+), 10 deletions(-) diff --git a/setup.c b/setup.c index 979b13f0c6..9c32aae302 100644 --- a/setup.c +++ b/setup.c @@ -4,6 +4,7 @@ static int inside_git_dir = -1; static int inside_work_tree = -1; +static int work_tree_config_is_bogus; /* * The input parameter must contain an absolute path, and it must already be @@ -286,6 +287,10 @@ void setup_work_tree(void) if (initialized) return; + + if (work_tree_config_is_bogus) + die("unable to set up work tree using invalid config"); + work_tree = get_git_work_tree(); git_dir = get_git_dir(); if (!is_absolute_path(git_dir)) @@ -422,8 +427,11 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, if (work_tree_env) set_git_work_tree(work_tree_env); else if (is_bare_repository_cfg > 0) { - if (git_work_tree_cfg) /* #22.2, #30 */ - die("core.bare and core.worktree do not make sense"); + if (git_work_tree_cfg) { + /* #22.2, #30 */ + warning("core.bare and core.worktree do not make sense"); + work_tree_config_is_bogus = 1; + } /* #18, #26 */ set_git_dir(gitdirenv); diff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh index e1b2a99f10..f6aa3c70f8 100755 --- a/t/t1510-repo-setup.sh +++ b/t/t1510-repo-setup.sh @@ -598,11 +598,20 @@ test_expect_success '#20b/c: core.worktree and core.bare conflict' ' mkdir -p 20b/.git/wt/sub && ( cd 20b/.git && - test_must_fail git symbolic-ref HEAD >/dev/null + test_must_fail git status >/dev/null ) 2>message && grep "core.bare and core.worktree" message ' +test_expect_success '#20d: core.worktree and core.bare OK when working tree not needed' ' + setup_repo 20d non-existent "" true && + mkdir -p 20d/.git/wt/sub && + ( + cd 20d/.git && + git config foo.bar value + ) +' + # Case #21: core.worktree/GIT_WORK_TREE overrides core.bare' ' test_expect_success '#21: setup, core.worktree warns before overriding core.bare' ' setup_repo 21 non-existent "" unset && @@ -611,7 +620,7 @@ test_expect_success '#21: setup, core.worktree warns before overriding core.bare cd 21/.git && GIT_WORK_TREE="$here/21" && export GIT_WORK_TREE && - git symbolic-ref HEAD >/dev/null + git status >/dev/null ) 2>message && ! test -s message @@ -700,13 +709,13 @@ test_expect_success '#22.2: core.worktree and core.bare conflict' ' cd 22/.git && GIT_DIR=. && export GIT_DIR && - test_must_fail git symbolic-ref HEAD 2>result + test_must_fail git status 2>result ) && ( cd 22 && GIT_DIR=.git && export GIT_DIR && - test_must_fail git symbolic-ref HEAD 2>result + test_must_fail git status 2>result ) && grep "core.bare and core.worktree" 22/.git/result && grep "core.bare and core.worktree" 22/result @@ -752,9 +761,8 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' ' setup_repo 28 "$here/28" gitfile true && ( cd 28 && - test_must_fail git symbolic-ref HEAD + test_must_fail git status ) 2>message && - ! grep "^warning:" message && grep "core.bare and core.worktree" message ' @@ -766,7 +774,7 @@ test_expect_success '#29: setup' ' cd 29 && GIT_WORK_TREE="$here/29" && export GIT_WORK_TREE && - git symbolic-ref HEAD >/dev/null + git status ) 2>message && ! test -s message ' @@ -777,7 +785,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version) setup_repo 30 "$here/30" gitfile true && ( cd 30 && - test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result + test_must_fail env GIT_DIR=.git git status 2>result ) && grep "core.bare and core.worktree" 30/result ' -- cgit v1.2.3