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

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 8, 2015, 22:43 UTC
Message-ID: <xmqqfuzck1dp.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <xmqqsi3ckadi.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 22 quoted lines

> Christian Couder <christian.couder@gmail.com> writes:
>
>> When we know that mtime is fully supported by the environment, we
>> might want the untracked cache to be always used by default without
>> any mtime test or kernel version check being performed.
>>
>> Also when we know that mtime is not supported by the environment,
>> for example because the repo is shared over a network file system,
>> then we might want 'git update-index --untracked-cache' to fail
>> immediately instead of preforming tests (because it might work on
>> some systems using the repo over the network file system but not
>> others).
>> ...
> The logic in this paragraph is fuzzy to me.  Shouldn't the config
> give a sensible default, that is overriden by command line options?
> I agree that it is insane to do a runtime check when the user says
> "update-index --untracked-cache" to enable it, as the user _knows_
> that enabling it would help (or the user _knows_ that she wants to
> play with it).  Similarly, shouldn't the config be ignored when the
> user says "update-index --no-untracked-cache" (hence removing the
> untracked cache from the resulting index no matter the config is set
> to)?  ...

As I think about this more, it really seems to me that we shouldn't need to make this configuration variable that special. Because I think it is a *BUG* in the current implementation to do the runtime check even when the user explicitly tells us to use untracked-cache, I'd imagine something along the lines of the following would be a lot simpler, easier to understand and a generally more useful bugfix:

 1 Add one boolean configuration variable, core.untrackedCache, that
   defaults to 'false'.

 2 When creating an index file in an empty repository, enable the
   untracked cache in the index (even without the user runninng
   "update-index --untracked-cache") iff the configuration is set to
   'true'.  No runtime check necessary.

 3 When working on an existing index file, unless the operation is
   "update-index --[no-]untracked-cache", keep the current setting,
   regardless of this configuration variable.  No runtime check
   necessary.

 4 "update-index --[no-]untracked-cache" should enable or disable
   the untracked cache as the user tells us, regardless of the
   configuration variable.  No runtime check necessary.

It is OK to then add an "auto-detect" on top of the above, that would only affect the second bullet point, like so:

 2a When creating an index file in an empty repository, if the
    configuration is set to 'auto', do the lengthy runtime check and
    enable the untracked cache in the index (even without the user
    runninng "update-index --untracked-cache").

without changing any other in the first 4-bullet list.

Am I missing some other requirements?