Re: [BUG?] `config branch.autosetuprebase true` breaks `rev-parse --is-inside-work-tree`

From: Junio C Hamano <gitster@pobox.com>
Date: Oct 8, 2014, 17:52 UTC
Message-ID: <xmqqlhoqbgab.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqppe2bh1p.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 6 quoted lines

> In config.c, git_default_branch_config() must be corrected to set
> git_branch_track and autorebase to BRANCH_TRACK_MALFORMED and
> AUTOREBASE_MALFORMED and the users of these two variables must be
> fixed to deal with the "malformed in the configuration" cases, I
> think.  The error should happen only in the codepath where we need
> the value, and no other places.

Having said that, given that any call git_config_bool() inside a callback function given to the git_config() will stop Git from doing anything even if the variable with a malformed value in quesiton is not used by the operation at all, and there are very many of them (e.g. setting core.filemode to "treu" would break everything), it appears to me that:

 (1) it could be argued that catching obvious typos in the
     configuration file as early as possible, even if the variables
     with typos are not used for the particular operation, is even a
     feature, as long as you can fix the brekage with "git config"
     (and/or your editor);

 (2) it is too much pain to shift the error checking to the site of
     their use from the site of their parsing anyway ;-)

And I suspect Tanay and Matthieu's recent work is taking us to a direction where many code paths do not use the config callbacks (which is what leads us to detect errors at parse time even for variables that are not used) and instead allow the callers that care about the individual variables to diagnose errors at the site of use. So as you stated originally, this may not be something we want to patch up in the current callback based config system.