setup_git_directory: delay core.bare/core.worktree errors

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 <szeder@ira.uka.de>
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/setup.c b/setup.c
index 979b13f0c6cd6bc3c265187e8ea79628bfe99f87..9c32aae3020174523e9638cb605b71840c60f71d 100644 (file)
--- 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 e1b2a99f105f8700e72974428c126ea049247194..f6aa3c70f8ade06b94d28e340d7060a81718dcca 100755 (executable)
--- 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
 '