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>

Jeff King committed May 29, 2015 at 02:49 UTC fada767463b599951b37bd544379a1d18dcf9370
2 files changed +26 -10
setup.c
+10 -2
@@ -4,6 +4,7 @@
4
5 static int inside_git_dir = -1;
6 static int inside_work_tree = -1;
7 +static int work_tree_config_is_bogus;
8
9 /*
10 * The input parameter must contain an absolute path, and it must already be
@@ -286,6 +287,10 @@ void setup_work_tree(void)
287
288 if (initialized)
289 return;
290 +
291 + if (work_tree_config_is_bogus)
292 + die("unable to set up work tree using invalid config");
293 +
294 work_tree = get_git_work_tree();
295 git_dir = get_git_dir();
296 if (!is_absolute_path(git_dir))
@@ -422,8 +427,11 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
427 if (work_tree_env)
428 set_git_work_tree(work_tree_env);
429 else if (is_bare_repository_cfg > 0) {
425 - if (git_work_tree_cfg) /* #22.2, #30 */
426 - die("core.bare and core.worktree do not make sense");
430 + if (git_work_tree_cfg) {
431 + /* #22.2, #30 */
432 + warning("core.bare and core.worktree do not make sense");
433 + work_tree_config_is_bogus = 1;
434 + }
435
436 /* #18, #26 */
437 set_git_dir(gitdirenv);
t/t1510-repo-setup.sh
+16 -8
@@ -598,11 +598,20 @@ test_expect_success '#20b/c: core.worktree and core.bare conflict' '
598 mkdir -p 20b/.git/wt/sub &&
599 (
600 cd 20b/.git &&
601 - test_must_fail git symbolic-ref HEAD >/dev/null
601 + test_must_fail git status >/dev/null
602 ) 2>message &&
603 grep "core.bare and core.worktree" message
604 '
605
606 +test_expect_success '#20d: core.worktree and core.bare OK when working tree not needed' '
607 + setup_repo 20d non-existent "" true &&
608 + mkdir -p 20d/.git/wt/sub &&
609 + (
610 + cd 20d/.git &&
611 + git config foo.bar value
612 + )
613 +'
614 +
615 # Case #21: core.worktree/GIT_WORK_TREE overrides core.bare' '
616 test_expect_success '#21: setup, core.worktree warns before overriding core.bare' '
617 setup_repo 21 non-existent "" unset &&
@@ -611,7 +620,7 @@ test_expect_success '#21: setup, core.worktree warns before overriding core.bare
620 cd 21/.git &&
621 GIT_WORK_TREE="$here/21" &&
622 export GIT_WORK_TREE &&
614 - git symbolic-ref HEAD >/dev/null
623 + git status >/dev/null
624 ) 2>message &&
625 ! test -s message
626
@@ -700,13 +709,13 @@ test_expect_success '#22.2: core.worktree and core.bare conflict' '
709 cd 22/.git &&
710 GIT_DIR=. &&
711 export GIT_DIR &&
703 - test_must_fail git symbolic-ref HEAD 2>result
712 + test_must_fail git status 2>result
713 ) &&
714 (
715 cd 22 &&
716 GIT_DIR=.git &&
717 export GIT_DIR &&
709 - test_must_fail git symbolic-ref HEAD 2>result
718 + test_must_fail git status 2>result
719 ) &&
720 grep "core.bare and core.worktree" 22/.git/result &&
721 grep "core.bare and core.worktree" 22/result
@@ -752,9 +761,8 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '
761 setup_repo 28 "$here/28" gitfile true &&
762 (
763 cd 28 &&
755 - test_must_fail git symbolic-ref HEAD
764 + test_must_fail git status
765 ) 2>message &&
757 - ! grep "^warning:" message &&
766 grep "core.bare and core.worktree" message
767 '
768
@@ -766,7 +774,7 @@ test_expect_success '#29: setup' '
774 cd 29 &&
775 GIT_WORK_TREE="$here/29" &&
776 export GIT_WORK_TREE &&
769 - git symbolic-ref HEAD >/dev/null
777 + git status
778 ) 2>message &&
779 ! test -s message
780 '
@@ -777,7 +785,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)
785 setup_repo 30 "$here/30" gitfile true &&
786 (
787 cd 30 &&
780 - test_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result
788 + test_must_fail env GIT_DIR=.git git status 2>result
789 ) &&
790 grep "core.bare and core.worktree" 30/result
791 '