bogus config file vs. 'git config --edit'

bogus config file vs. 'git config --edit'

[SZEDER Gábor]

Let's suppose I somehow ended up with a bogus config file:

  $ tail -n2 .git/config 
  [section]
          foo bar baz

and now I try to rectify the situation, but I know that poking around
in the .git directory is a no-no, so instead of 'vim .git/config' I
try:

  $ git config --edit
  fatal: bad config line 81 in file .git/config

Uh-oh.

Furthermore, if I don't remember quite clearly the finer points of the
syntax of the config file, then I might want to look it up:

  $ git help config
  fatal: bad config line 81 in file .git/config

I think bith 'git config --edit' and 'git help ...' should just work,
no matter what nonsense might be in the config file, even if they then
launch a different editor or pager than what are set in the
configuration.

Re: bogus config file vs. 'git config --edit'

[Jeff King]

On Fri, Dec 27, 2019 at 12:04:31PM +0100, SZEDER Gábor wrote:

> I think bith 'git config --edit' and 'git help ...' should just work,
> no matter what nonsense might be in the config file, even if they then
> launch a different editor or pager than what are set in the
> configuration.

Agreed. This has come up before (e.g., [1]) but I think doing it right
would need several changes:

  - the config code is too eager to die on a syntax error; it should
    return an error up the stack

  - the stack looks like this when we first die():

      #0  die (err=0x55555582436f "%s") at usage.c:165
      #1  0x0000555555678ab6 in git_parse_source (fn=0x5555557719c4 <check_repo_format>, data=0x7fffffffe1e0, opts=0x0)
          at config.c:849
      #2  0x000055555567a9c5 in do_config_from (top=0x7fffffffdfc0, fn=0x5555557719c4 <check_repo_format>, 
          data=0x7fffffffe1e0, opts=0x0) at config.c:1546
      #3  0x000055555567aab4 in do_config_from_file (fn=0x5555557719c4 <check_repo_format>, origin_type=CONFIG_ORIGIN_FILE, 
          name=0x555555913220 ".git/config", path=0x555555913220 ".git/config", f=0x5555559117c0, data=0x7fffffffe1e0, 
          opts=0x0) at config.c:1574
      #4  0x000055555567ab80 in git_config_from_file_with_options (fn=0x5555557719c4 <check_repo_format>, 
          filename=0x555555913220 ".git/config", data=0x7fffffffe1e0, opts=0x0) at config.c:1594
      #5  0x000055555567abc5 in git_config_from_file (fn=0x5555557719c4 <check_repo_format>, 
          filename=0x555555913220 ".git/config", data=0x7fffffffe1e0) at config.c:1603
      #6  0x0000555555771e0b in read_repository_format (format=0x7fffffffe1e0, path=0x555555913220 ".git/config")
          at setup.c:523
      #7  0x0000555555771bb5 in check_repository_format_gently (gitdir=0x555555911750 ".git", candidate=0x7fffffffe1e0, 
          nongit_ok=0x7fffffffe2cc) at setup.c:460
      #8  0x000055555577272f in setup_discovered_git_dir (gitdir=0x555555911750 ".git", cwd=0x5555558c90b0 <cwd>, 
          offset=19, repo_fmt=0x7fffffffe1e0, nongit_ok=0x7fffffffe2cc) at setup.c:770
      #9  0x0000555555773490 in setup_git_directory_gently (nongit_ok=0x7fffffffe2cc) at setup.c:1100
      #10 0x00005555555706c8 in run_builtin (p=0x5555558bb980 <commands+576>, argc=2, argv=0x7fffffffe538) at git.c:416
      #11 0x0000555555570baf in handle_builtin (argc=2, argv=0x7fffffffe538) at git.c:674
      #12 0x00005555555711b2 in cmd_main (argc=2, argv=0x7fffffffe538) at git.c:842
      #13 0x000055555563412e in main (argc=2, argv=0x7fffffffe538) at common-main.c:52

     So we'd need to teach read_repository_format() to handle the error
     and just assume we're not in a Git repo. But then how would "git
     config --edit" realize it needs to edit the repo config file?

  - assuming we get around the above problem, I suspect we'd run into
    other things that want to read the config (e.g., loading default
    config like the editor). We could be more lenient everywhere, but I
    suspect that most of the other commands _do_ want to die on bogus
    config (rather than do something unexpected). I wouldn't want to
    change every git_config() call to handle errors, but maybe we could
    have a lenient form and use it in just enough call-sites. Or maybe
    we could detect early in git-config that we are in "--edit" mode,
    and set a global for "be lenient when reading the config". I dunno.

So I think it's definitely do-able and would be much better, but it's
probably not entirely trivial.

-Peff

[1] https://lore.kernel.org/git/A5CDBB91-E889-4849-953A-2C1DB4A04513@gmail.com/