From: Junio C Hamano <gitster@pobox.com>
To: Richard Hartmann <richih.mailinglist@gmail.com>
Cc: Git List <git@vger.kernel.org>, Tanay Abhra <tanayabh@gmail.com>,
Matthieu Moy <Matthieu.Moy@imag.fr>
Subject: Re: [BUG?] `config branch.autosetuprebase true` breaks `rev-parse --is-inside-work-tree`
Date: Wed, 08 Oct 2014 10:52:12 -0700 [thread overview]
Message-ID: <xmqqlhoqbgab.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <xmqqppe2bh1p.fsf@gitster.dls.corp.google.com> (Junio C. Hamano's message of "Wed, 08 Oct 2014 10:35:46 -0700")
Junio C Hamano <gitster@pobox.com> writes:
> 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.
next prev parent reply other threads:[~2014-10-08 17:52 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-10-08 11:22 [BUG?] `config branch.autosetuprebase true` breaks `rev-parse --is-inside-work-tree` Richard Hartmann
2014-10-08 17:35 ` Junio C Hamano
2014-10-08 17:52 ` Junio C Hamano [this message]
2014-10-08 19:04 ` Tanay Abhra
2014-10-08 20:09 ` brian m. carlson
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=xmqqlhoqbgab.fsf@gitster.dls.corp.google.com \
--to=gitster@pobox.com \
--cc=Matthieu.Moy@imag.fr \
--cc=git@vger.kernel.org \
--cc=richih.mailinglist@gmail.com \
--cc=tanayabh@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).