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

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


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


On Thu, Dec 17, 2015 at 1:26 PM, Duy Nguyen [off-list ref] wrote:

On Thu, Dec 17, 2015 at 2:44 PM, Jeff King [off-list ref] wrote:

quoted

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?