From: Christian Couder <christian.couder@gmail.com>
Date: Fri, 08 Jan 2016 17:52:31 GMT
Subject: Re: [PATCH v4 09/10] config: add core.untrackedCache
Message-ID: <CAP8UFD2eJjR4QxtE0x8BbKOAGwi4cwERG7bF89apPbJye356AQ@mail.gmail.com>
In-Reply-To: <xmqqd1thp683.fsf@gitster.mtv.corp.google.com>

On Mon, Jan 4, 2016 at 7:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>>>
>>> The real question is what are the problems in implementing this in
>>> the way Duy suggested in the previous discussion.  The answer may
>>> fall into somewhere between "that approach does not work in such and
>>> such cases, so this is the best I could come up with" and "I know
>>> that approach is far superiour, but I have invested too much in this
>>> inferiour approach and refuse to rework it further to make it better."
>>
>> My question is why should I invest time thinking about and testing
>> another approach when the current approach seems simpler, less bug
>> prone, faster and without any downside?

I just tried Duy's approach by moving into read_index_from() the code
I had put in wt_status_collect_untracked() and it doesn't work because
"git status" calls read_index_from() before calling
git_default_core_config().

The interesting parts of the backtraces for both calls are the following:

#0  read_index_from (istate=0x8660a0 <the_index>, path=0x868b90
".git/index") at read-cache.c:1626
#1  0x0000000000530ba3 in read_index (istate=0x8660a0 <the_index>) at
read-cache.c:1404
#2  0x00000000005743e1 in gitmodules_config () at submodule.c:188
#3  0x0000000000431ed0 in status_init_config (s=0x83aa20 <s>,
fn=0x434edd <git_status_config>) at builtin/commit.c:186

#0  git_default_core_config (var=0x86ac00 "core.untrackedcache",
value=0x86ac60 "true") at config.c:695
#1  0x00000000004be761 in git_default_config (var=0x86ac00
"core.untrackedcache", value=0x86ac60 "true", dummy=0x0) at
config.c:1016
#2  0x00000000004cfeba in git_diff_basic_config (var=0x86ac00
"core.untrackedcache", value=0x86ac60 "true", cb=0x0) at diff.c:277
#3  0x00000000004cfc9b in git_diff_ui_config (var=0x86ac00
"core.untrackedcache", value=0x86ac60 "true", cb=0x0) at diff.c:230
#4  0x000000000043524a in git_status_config (k=0x86ac00
"core.untrackedcache", v=0x86ac60 "true", cb=0x83aa20 <s>) at
builtin/commit.c:1313
#5  0x00000000004bf183 in configset_iter (cs=0x8495e0
<the_config_set>, fn=0x434edd <git_status_config>, data=0x83aa20 <s>)
at config.c:1305
#6  0x00000000004bf206 in git_config (fn=0x434edd <git_status_config>,
data=0x83aa20 <s>) at config.c:1317
#7  0x0000000000431ee3 in status_init_config (s=0x83aa20 <s>,
fn=0x434edd <git_status_config>) at builtin/commit.c:187

You can see that in status_init_config(), gitmodules_config() is
called just before git_config().
And unfortunately gitmodules_config() indirectly calls read_index_from().

So indeed it seems very bug prone to me to rely on read_index_from()
being called before git_default_core_config().