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

From: Christian Couder <christian.couder@gmail.com>
Date: Dec 18, 2015, 22:40 UTC
Message-ID: <CAP8UFD1kWd9Y9pLfz2aAHNQjtxOrhLk25ExZPU7p0OFfiixuow@mail.gmail.com>
In-Reply-To: <CAP8UFD0S_rWKjWiq_enkN+QVtvnq9fuwAxuuVTXTxu-F1mw4dg@mail.gmail.com>

(Sorry I sent this one privately to Duy by mistake too.)

---------- Forwarded message ----------
From: Christian Couder <christian.couder@gmail.com>
Date: Fri, Dec 18, 2015 at 11:35 PM
Subject: Re: [PATCH 7/8] config: add core.untrackedCache
To: Duy Nguyen <pclouds@gmail.com>

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

Show 15 quoted lines

> On Thu, Dec 17, 2015 at 2:44 PM, Jeff King <peff@peff.net> wrote:
>> I think we may actually be thinking of the same thing. Naively, I would
>> expect:
>>
>> ..
>>   - if there is cache data in the index but that config flag is not set,
>>     presumably we would not update it (we could even explicitly drop it,
>>     but my understanding is that is not necessary for correctness, but
>>     only as a possible optimization).
>
> No, if somebody adds or removes something from the index, we either
> update or drop it, or it's stale. There's the invalidate_untracked()
> or something in dir.c that we can hook in, check config var and do
> that. And because config is cached recently, it should be a cheap
> operation.

I think I understand what you are saying but it took me a long time, and I am not sure it is very clear to others.

What I understand is that you are talking about validate_untracked_cache() in dir.c. And you suggest to check there if the core.untrackedCache config var is set, and, if it is not set, then to drop the UC there. And the reason for that is that git operations on other parts of the index should update the UC if it exists otherwise the UC content could be wrong as you explained previously in your "git rm --cached foo" example.

In the current patch, the code to create or remove the UC to reflect the state of the core.untrackedCache config var is in wt_status_collect_untracked(). I think it works well there, but if you are saying that it's better if it is in validate_untracked_cache(), I am willing to consider moving it to validate_untracked_cache(). Could you tell a bit though about why you think it's better in validate_untracked_cache()?

> Apart from that the idea is fine.

Ok so, except maybe the part about wt_status_collect_untracked() vs validate_untracked_cache(), what the patch does is ok for you?