{"thread":{"id":"39436","subject":"Recovering from 'fatal: core.bare and core.worktree do not make sense'","startedAt":"2015-05-27T10:31:47Z","lastAt":"2015-05-29T06:49:10Z","messageCount":2,"participants":["SZEDER Gábor","Jeff King"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"262222","messageId":"20150527123147.Horde.GqzoX-7JvXiOGBlB5moP4A8@webmail.informatik.kit.edu","threadId":"39436","inReplyTo":null,"subject":"Recovering from 'fatal: core.bare and core.worktree do not make sense'","fromName":"SZEDER Gábor","fromEmail":"szeder@ira.uka.de","sentAt":"2015-05-27T10:31:47Z","receivedAt":"2015-05-27T10:31:47Z","isPatch":false,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"\nHi,\n\nthe other day I said 'git config core.worktree /somewhere' in a bare\nrepo while thinking I was in a regular one, user error.  The 'fatal:\ncore.bare and core.worktree do not make sense' error from the next\ncommand made me realize immediately that I was wrong, that's good.\nHowever...\n\nOK, let's have a look and recover from the situation:\n\n    $ git config --edit\n    fatal: core.bare and core.worktree do not make sense\n\nWell, all was well before I set 'core.worktree', so let's unset it:\n\n    $ git config --unset core.worktree\n    fatal: core.bare and core.worktree do not make sense\n\nHmph, not expecting much, but how about unsetting the other\nvariable?\n\n    $ git config --unset core.bare\n    fatal: core.bare and core.worktree do not make sense\n\nGood, at least it's pretty consistent, though I still don't get what\n'git config' has to do with the worktree that is so important that\nit has to bail out.  Time to look for help:\n\n    $ git help config\n    fatal: core.bare and core.worktree do not make sense\n\nWTF :)\nAlright, I give up:\n\n    $ vim config\n    $ # happy\n\nIt was two days later that I had a bit of a lightbulb moment,\nreproduced the situation and just for fun tried this:\n\n    $ git -c core.bare=false config --unset core.bare\n\nI didn't expect, but it worked!  Great.\n\nSome thoughts:\n\n    1) Perhaps 'git config' should be more careful in the first place\n       and refuse to set 'core.worktree' when 'core.bare' is already\n       true and vice versa.\n\n    2) The damage was done with 'git config', so I expected that I can\n       repair it with \"plain\" 'git config' (i.e. without 'git -c') as\n       well.  'git config' has nothing to do with the path to the\n       worktree after all.  And 'git config --edit' should work\n       regardless of the mess that might be in the config file.\n\n    3) 'git help <cmd>' should always work, shouldn't it?  (Though\n       that's the easiest to remedy, just cd out of the repo, or fire\n       up a new terminal window.)\n\n\nGábor\n"},{"id":"262389","messageId":"20150529064910.GA2987@peff.net","threadId":"39436","inReplyTo":"20150527123147.Horde.GqzoX-7JvXiOGBlB5moP4A8@webmail.informatik.kit.edu","subject":"[PATCH] setup_git_directory: delay core.bare/core.worktree errors","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-05-29T06:49:10Z","receivedAt":"2015-05-29T06:49:10Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 27, 2015 at 12:31:47PM +0200, SZEDER Gábor wrote:\n\n> Some thoughts:\n> \n>    1) Perhaps 'git config' should be more careful in the first place\n>       and refuse to set 'core.worktree' when 'core.bare' is already\n>       true and vice versa.\n\nRight now git-config doesn't know anything about the meaning of the\nconfig variables; that logic is spread throughout the code, at the\nactual users of the variables. So I'm a little hesitant to special-case\nthis one problem (especially because I think we can fix this in a\ndifferent way).\n\n>    2) The damage was done with 'git config', so I expected that I can\n>       repair it with \"plain\" 'git config' (i.e. without 'git -c') as\n>       well.  'git config' has nothing to do with the path to the\n>       worktree after all.  And 'git config --edit' should work\n>       regardless of the mess that might be in the config file.\n\nThe problem is that setup_git_directory is overly eager to die in this\ncase (even when we use the \"gentle\" form to indicate we can handle being\noutside of a repo). Patch is below.\n\n>    3) 'git help <cmd>' should always work, shouldn't it?  (Though\n>       that's the easiest to remedy, just cd out of the repo, or fire\n>       up a new terminal window.)\n\nIt looks in the repo (gently) so that it can find repo-specific config\nlike help.format. And thus gets caught in the same problem as\ngit-config.\n\n-- >8 --\nSubject: setup_git_directory: delay core.bare/core.worktree errors\n\nIf both core.bare and core.worktree are set, we complain\nabout the bogus config and die. Dying is good, because it\navoids commands running and doing damage in a potentially\nincorrect setup. But dying _there_ is bad, because it means\nthat commands which do not even care about the work tree\ncannot run. This can make repairing the situation harder:\n\n  [setup]\n  $ git config core.bare true\n  $ git config core.worktree /some/path\n\n  [OK, expected.]\n  $ git status\n  fatal: core.bare and core.worktree do not make sense\n\n  [Hrm...]\n  $ git config --unset core.worktree\n  fatal: core.bare and core.worktree do not make sense\n\n  [Nope...]\n  $ git config --edit\n  fatal: core.bare and core.worktree do not make sense\n\n  [Gaaah.]\n  $ git help config\n  fatal: core.bare and core.worktree do not make sense\n\nInstead, let's issue a warning about the bogus config when\nwe notice it (i.e., for all commands), but only die when the\ncommand tries to use the work tree (by calling setup_work_tree).\nSo we now get:\n\n  $ git status\n  warning: core.bare and core.worktree do not make sense\n  fatal: unable to set up work tree using invalid config\n\n  $ git config --unset core.worktree\n  warning: core.bare and core.worktree do not make sense\n\nWe have to update t1510 to accomodate this; it uses\nsymbolic-ref to check whether the configuration works or\nnot, but of course that command does not use the working\ntree. Instead, we switch it to use `git status`, as it\nrequires a work-tree, does not need any special setup, and\nis read-only (so a failure will not adversely affect further\ntests).\n\nIn addition, we add a new test that checks the desired\nbehavior (i.e., that running \"git config\" with the bogus\nconfig does in fact work).\n\nReported-by: SZEDER Gábor <szeder@ira.uka.de>\nSigned-off-by: Jeff King <peff@peff.net>\n---\nThe message for \"git status\" is a little clunky (first we warn, then we\nsay \"wait, that warning was important!\"). I also considered dropping the\nwarning (so you'd see nothing in the \"git config\" case) and then just\nmoving the die() to setup_work_tree().\n\n setup.c               | 12 ++++++++++--\n t/t1510-repo-setup.sh | 24 ++++++++++++++++--------\n 2 files changed, 26 insertions(+), 10 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex 863ddfd..82c0cc2 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -4,6 +4,7 @@\n \n static int inside_git_dir = -1;\n static int inside_work_tree = -1;\n+static int work_tree_config_is_bogus;\n \n /*\n  * The input parameter must contain an absolute path, and it must already be\n@@ -327,6 +328,10 @@ void setup_work_tree(void)\n \n \tif (initialized)\n \t\treturn;\n+\n+\tif (work_tree_config_is_bogus)\n+\t\tdie(\"unable to set up work tree using invalid config\");\n+\n \twork_tree = get_git_work_tree();\n \tgit_dir = get_git_dir();\n \tif (!is_absolute_path(git_dir))\n@@ -495,8 +500,11 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \tif (work_tree_env)\n \t\tset_git_work_tree(work_tree_env);\n \telse if (is_bare_repository_cfg > 0) {\n-\t\tif (git_work_tree_cfg) /* #22.2, #30 */\n-\t\t\tdie(\"core.bare and core.worktree do not make sense\");\n+\t\tif (git_work_tree_cfg) {\n+\t\t\t/* #22.2, #30 */\n+\t\t\twarning(\"core.bare and core.worktree do not make sense\");\n+\t\t\twork_tree_config_is_bogus = 1;\n+\t\t}\n \n \t\t/* #18, #26 */\n \t\tset_git_dir(gitdirenv);\ndiff --git a/t/t1510-repo-setup.sh b/t/t1510-repo-setup.sh\nindex 33c1a58..13ae12d 100755\n--- a/t/t1510-repo-setup.sh\n+++ b/t/t1510-repo-setup.sh\n@@ -599,11 +599,20 @@ test_expect_success '#20b/c: core.worktree and core.bare conflict' '\n \tmkdir -p 20b/.git/wt/sub &&\n \t(\n \t\tcd 20b/.git &&\n-\t\ttest_must_fail git symbolic-ref HEAD >/dev/null\n+\t\ttest_must_fail git status >/dev/null\n \t) 2>message &&\n \tgrep \"core.bare and core.worktree\" message\n '\n \n+test_expect_success '#20d: core.worktree and core.bare OK when working tree not needed' '\n+\tsetup_repo 20d non-existent \"\" true &&\n+\tmkdir -p 20d/.git/wt/sub &&\n+\t(\n+\t\tcd 20d/.git &&\n+\t\tgit config foo.bar value\n+\t)\n+'\n+\n # Case #21: core.worktree/GIT_WORK_TREE overrides core.bare' '\n test_expect_success '#21: setup, core.worktree warns before overriding core.bare' '\n \tsetup_repo 21 non-existent \"\" unset &&\n@@ -612,7 +621,7 @@ test_expect_success '#21: setup, core.worktree warns before overriding core.bare\n \t\tcd 21/.git &&\n \t\tGIT_WORK_TREE=\"$here/21\" &&\n \t\texport GIT_WORK_TREE &&\n-\t\tgit symbolic-ref HEAD >/dev/null\n+\t\tgit status >/dev/null\n \t) 2>message &&\n \t! test -s message\n \n@@ -701,13 +710,13 @@ test_expect_success '#22.2: core.worktree and core.bare conflict' '\n \t\tcd 22/.git &&\n \t\tGIT_DIR=. &&\n \t\texport GIT_DIR &&\n-\t\ttest_must_fail git symbolic-ref HEAD 2>result\n+\t\ttest_must_fail git status 2>result\n \t) &&\n \t(\n \t\tcd 22 &&\n \t\tGIT_DIR=.git &&\n \t\texport GIT_DIR &&\n-\t\ttest_must_fail git symbolic-ref HEAD 2>result\n+\t\ttest_must_fail git status 2>result\n \t) &&\n \tgrep \"core.bare and core.worktree\" 22/.git/result &&\n \tgrep \"core.bare and core.worktree\" 22/result\n@@ -753,9 +762,8 @@ test_expect_success '#28: core.worktree and core.bare conflict (gitfile case)' '\n \tsetup_repo 28 \"$here/28\" gitfile true &&\n \t(\n \t\tcd 28 &&\n-\t\ttest_must_fail git symbolic-ref HEAD\n+\t\ttest_must_fail git status\n \t) 2>message &&\n-\t! grep \"^warning:\" message &&\n \tgrep \"core.bare and core.worktree\" message\n '\n \n@@ -767,7 +775,7 @@ test_expect_success '#29: setup' '\n \t\tcd 29 &&\n \t\tGIT_WORK_TREE=\"$here/29\" &&\n \t\texport GIT_WORK_TREE &&\n-\t\tgit symbolic-ref HEAD >/dev/null\n+\t\tgit status\n \t) 2>message &&\n \t! test -s message\n '\n@@ -778,7 +786,7 @@ test_expect_success '#30: core.worktree and core.bare conflict (gitfile version)\n \tsetup_repo 30 \"$here/30\" gitfile true &&\n \t(\n \t\tcd 30 &&\n-\t\ttest_must_fail env GIT_DIR=.git git symbolic-ref HEAD 2>result\n+\t\ttest_must_fail env GIT_DIR=.git git status 2>result\n \t) &&\n \tgrep \"core.bare and core.worktree\" 30/result\n '\n-- \n2.4.2.668.gc3b1ade.dirty\n"}]}