Re: [PATCH 7/8] config: add core.untrackedCache

From: Christian Couder <christian.couder@gmail.com>
Date: Dec 18, 2015, 23:24 UTC
Message-ID: <CAP8UFD16HbwN71ugTKek_CdT65571M1meisaBajjSFgoZePg8g@mail.gmail.com>
In-Reply-To: <CACsJy8B2tFqhfKPE17cPpp1uDYPa4OTB9zLHPY0j2QF9zuxAtQ@mail.gmail.com>

On Thu, Dec 17, 2015 at 1:36 PM, Duy Nguyen <pclouds@gmail.com> wrote:

Show 32 quoted lines

> On Wed, Dec 16, 2015 at 4:53 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> On Tue, Dec 15, 2015 at 8:40 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
>>> I still have a problem with the approach from "design cleanliness"
>>> point of view[...]
>>>
>>> In any case I think we already have agreed to disagree on this
>>> point, so there is no use discussing it any longer from my side.  I
>>> am not closing the door to this series, but I am not convinced,
>>> either.  At least not yet.
>>
>> In general the fantastic thing about the git configuration facility is
>> that it provides both systems administrators and normal users with
>> what they want. It's possible to configure things system-wide and
>> override those on a user or repository basis.
>>
>> Of course hindsight is 20/20, but I think that given what's been
>> covered in this thread it's been established that it's categorically
>> better that if we introduce features like these that they be
>> configured through the normal configuration facility rather than the
>> configuration being sticky to the index.
>
> A minor note for implementers. We need to check that config is loaded
> first. read-cache.c, being part of the core, does not bother itself
> with config loading. And I think so far it has not used any config
> vars. If a command forgets (*) to load the config, the cache may be
> deleted (if we do it the safe way).
>
> (*) is there any command deliberately avoid loading config? git-clone
> and git-init are special, but for this particular case it's probably
> fine.

Thanks for this note.

Looking at the current patch, the global variable in which the value of the core.untrackedCache config var is stored is "use_untracked_cache". It is used in the following places:

- wt_status_collect_untracked() in wt-status.c which is called only by
"git status" and "git commit" after the config has been loaded.

- cmd_update_index() in builtin/update-index.c which loads the config
before using it

- validate_untracked_cache() in dir.c where it is used in:

       if (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {
                warning(_("Untracked cache is disabled on this system."));
                return NULL;
        }

but this "if" and its contents are removed by patch 10/10 in v2.

So at the end of this patch series, there is no risk of use_untracked_cache not being properly setup.