Re: bugreport: "git config --global" fails if an invalid .git file present

[Jeff King <peff@peff.net>]

On Wed, Aug 17, 2022 at 01:16:44AM +0900, Jinwook Jeong wrote:

> What did you do before the bug happened? (Steps to reproduce your issue)
> 
> Executing `git config --global ...` and $PWD/.git exists as a file and
> is invalid.
> 
> What did you expect to happen? (Expected behavior)
> 
> Git reads from or writes to the global config store.
> 
> What happened instead? (Actual behavior)
> 
> It prints:
>   fatal: invalid gitfile format: $PWD/.git
> 
> What's different between what you expected and what actually happened?
> 
> Git fails to access the global config even if the current directory's
> `.git` file has nothing to do with the global config.

I agree this isn't entirely friendly, but I think things are working as
designed. Even though you're only asking to change global config, we do
still speculatively check for a repository when running git-config at
all, as various options could impact its behavior. So the backtrace
looks like this:

  0  die (err=0x5555558e5c33 "invalid gitfile format: %s") at usage.c:175
  #1  0x00005555557fbc87 in read_gitfile_error_die (error_code=5, path=0x5555559abbd0 "/home/peff/.git", dir=0x0)
      at setup.c:786
  #2  0x00005555557fbfaa in read_gitfile_gently (path=0x5555559abbd0 "/home/peff/.git", return_error_code=0x0)
      at setup.c:875
  #3  0x00005555557fce4d in setup_git_directory_gently_1 (dir=0x7fffffffe160, gitdir=0x7fffffffe140,
      report=0x7fffffffe120, die_on_error=1) at setup.c:1298
  #4  0x00005555557fd4d6 in setup_git_directory_gently (nongit_ok=0x7fffffffe1ec) at setup.c:1455
  #5  0x0000555555574a32 in run_builtin (p=0x55555596e9f0 <commands+624>, argc=4, argv=0x7fffffffe570) at git.c:436
  #6  0x000055555557505f in handle_builtin (argc=4, argv=0x7fffffffe570) at git.c:720

and when we hit the error, we have not even started running code
specific to git-config, let alone seen that the "--global" option is in
use.

There's another more subtle question lurking here. The config builtin is
marked as RUN_SETUP_GENTLY, so it will run even if we are not inside a
repository at all. But you can see in the backtrace above that we pass
die_on_error=1 to setup_git_directory_gently_1(). That is, the "gentle"
form here means "it is OK not to be in a repository", but if we see
something that indicates we should be in a repository (like a .git
file), but we hit an error examining it, we bail immediately. We _could_
say "well, it's not a valid .git file, let's keep looking".

And that would make your case Just Work. But it's also more dangerous;
if a misconfiguration or transient error caused us to set up an
unexpected environment, then the results could be quite wrong and
confusing. Maybe less so for "git config", but for destructive
operations it's a scarier concept.

So a patch like the one below does what you want, but I'm not sure it's
a good idea for the reasons given above.

In your case, the right resolution is probably to delete the bogus .git
file.

-Peff

---
diff --git a/setup.c b/setup.c
index cefd5f63c4..aafb1e1e99 100644
--- a/setup.c
+++ b/setup.c
@@ -1452,7 +1452,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		die_errno(_("Unable to read current working directory"));
 	strbuf_addbuf(&dir, &cwd);
 
-	switch (setup_git_directory_gently_1(&dir, &gitdir, &report, 1)) {
+	switch (setup_git_directory_gently_1(&dir, &gitdir, &report, !nongit_ok)) {
 	case GIT_DIR_EXPLICIT:
 		prefix = setup_explicit_git_dir(gitdir.buf, &cwd, &repo_fmt, nongit_ok);
 		break;
@@ -1503,6 +1503,11 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		}
 		*nongit_ok = 1;
 		break;
+	case GIT_DIR_INVALID_GITFILE:
+		if (!nongit_ok)
+			BUG("read_gitfile_gently() should have died on error");
+		*nongit_ok = 1;
+		break;
 	case GIT_DIR_NONE:
 		/*
 		 * As a safeguard against setup_git_directory_gently_1 returning