Re: [PATCH v3 0/9] Fix the early config

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 8, 2017, 17:09 UTC
Message-ID: <xmqqy3wf3dbo.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170308073035.pt5ysp6bzpozoqps@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 11 quoted lines

> Good catch. Another "non-gentle" thing I noticed here while looking at
> another thread: the repository-format version check uses the config
> parser, which will die() in certain circumstances. So for instance:
>
>   $ git init
>   $ git rev-parse && echo ok
>   ok
>
>   $ echo '[core]repositoryformatversion = 10' >.git/config
>   $ git rev-parse && echo ok
>   fatal: Expected git repo version <= 1, found 10

Just to set my expectation straight. Do you expect/wish this not to fail because of this in cmd_rev_parse()?

	/* No options; just report on whether we're in a git repo or not. */
	if (argc == 1) {
		setup_git_directory();
		git_config(git_default_config, NULL);
		return 0;
	}

Because we do not have anything other than yes/no to the question "Are we in Git repository?", I'd expect that the expected answer to the question would be "no" (if we could say "Yes, we are in a Git repository but its version and layout is unknown to us so we are not supposed to look at or touch it", that is a different matter).

So "fatal:" may be bad, but I think not seeing "ok" is what we want to happen in this case.

Having said that, I am not sure asking for default-config is what we wanted to do in the above code. Perhaps a more modern way to write the above code would be to do the "gently" version of setup, without calling git_config() ourselves, and return the resulting value returned in *nongit_ok? If we can do so without triggering "fatal:" and still return "no, we are not in a Git repository we are supposed to touch", that would be good.

Or are you discussing a more general issue, iow, anything that can work without repository (i.e. those who do _gently version of the setup and act on *nongit_ok) should pretend as if there were no (broken) repository and take the "no we are not in a repository" codepath?