git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tanay Abhra <tanayabh@gmail.com>
To: Junio C Hamano <gitster@pobox.com>,
	Richard Hartmann <richih.mailinglist@gmail.com>
Cc: Git List <git@vger.kernel.org>
Subject: Re: [BUG?] `config branch.autosetuprebase true` breaks `rev-parse --is-inside-work-tree`
Date: Thu, 09 Oct 2014 00:34:16 +0530	[thread overview]
Message-ID: <54358AB0.3060002@gmail.com> (raw)
In-Reply-To: <xmqqppe2bh1p.fsf@gitster.dls.corp.google.com>

On 10/8/2014 11:05 PM, Junio C Hamano wrote:
> Richard Hartmann <richih.mailinglist@gmail.com> writes:
> 
>> So this is not a real bug report, more of a "is this intended this way?"
>> richih@titanium  ~/git_test % git rev-parse --is-inside-work-tree
>> error: Malformed value for branch.autosetuprebase
>> fatal: bad config file line 8 in .git/config
>> richih@titanium  ~/git_test % cat .git/config
>> ...
>> [branch]
>> autosetuprebase = true
> 
> It does not seem to be limited to rev-parse but having a malformed
> configuration for that variable would break everything Git, which
> certainly is not how it is supposed to work.  It also seems that the
> breakage dates back very far into the past (I checked 1.7.0 and it
> seems to be broken the same way).
> 
> The same breakage exists for branch.autosetupmerge, I think, e.g.
> 
> 	[branch]
>                 autosetupmerge = garbage
> 
> 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.

Supporting Junio's claim, there is a function called git_default_config()
which checks and sets a whole load of config values which may or maynot
be relevant to the codepath that called it. (branch.autosetuprebase is a
part of it) So an error may occur printing a seemingly unrelated config value
as the malformed variable as happened in your case.

There are currently 72 callers of git_default_config() in the codebase,
so a malformed config value breaks most of git commands. The only path
to correct this behavior would be either correct the config variable in
the file or we could decouple the huge monolithic function that
git_default_config() has become and use the git_config_get_value() in the
code paths that really need them. This part is doable, albeit slowly. All
the config variables in git_default_config() can be rewritten using the
new non callback based functions easily as demonstrated in an earlier
RFC patch.

  parent reply	other threads:[~2014-10-08 19:04 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
2014-10-08 19:04   ` Tanay Abhra [this message]
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=54358AB0.3060002@gmail.com \
    --to=tanayabh@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=richih.mailinglist@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).