Skip to content

Commit

Permalink
setup_git_directory: delay core.bare/core.worktree errors
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jeff King authored and Junio C Hamano committed May 29, 2015
1 parent fdf96a2 commit fada767
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 10 deletions.
12 changes: 10 additions & 2 deletions setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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);
Expand Down
24 changes: 16 additions & 8 deletions t/t1510-repo-setup.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 &&
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
'

Expand All @@ -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
'
Expand All @@ -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
'
Expand Down

0 comments on commit fada767

Please sign in to comment.