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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 15, 2015, 19:40 UTC
Message-ID: <xmqqh9jjfqk4.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <CACBZZX6=sU2cb_vRn5DAqVEuNTwsk0m7vQ0_WUp5qPWeQq5JhQ@mail.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:

Show 5 quoted lines

> The way the "config decides" patch series deals with this is that if
> you have the UC information in the index and the configuration is set
> to core.untrackedCache=false the UC will be removed from the index.
>
> Otherwise you would indeed easily end up with a stale cache,...

Yeah, that's "correctness" thing; it goes without saying that it would be unacceptable if the series did not get this right.

I still have a problem with the approach from "design cleanliness" point of view, primarily that the index already has a bit to tell us if the user already said that she wants to use the feature, but because you want to make config win, the code needs to always read the config to allow it to disable the feature in the index, just in case the data that is already in the index says otherwise, and the code has to keep doing that even after the data is removed [*1*]. Unfortunately, I do not think you can solve the "design cleanliness" problem unless you give up "we want /etc/gitconfig override".

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.

> Once this series is applied and git is shipped with it, existing
> users that have set "git update-index --untracked-cache" will have
> their UC feature disabled.

Well, the fix to that is merely just a one-shot thing away, so it would not be too much of a hassle, no [*2*]? So I do not think it would be such a big issue.

> We *could* make even that use-case work by detecting the legacy marker
> for the UC in the index (the uname info), then we'd do a one-time "git
> config --local core.untrackedCache true" and remove the marker.

I do not think we want to go there---that would mean you would need to revamp the repository format version because the old tools would be now unusable on the index/config combo your version mucked with.

[Footnote]

*1* I also do not want to see that design pattern imitated and used
    in other parts of the system, and this gives a precedent for
    people to copy.

*2* Yet, those who are broken by this series may say "it is
    unacceptable that we have to survey all existing repositories
    and selectively add the configuration to the ones that have
    enabled the feature before updating.", the same way you complain
    against the "index already knows" approach.