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

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Dec 2, 2015 at 8:12 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 12/01/2015 09:31 PM, Christian Couder wrote:
>>
>> 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.
>

[Re-arranged some of the quotes for the clarity of my reply....]

[Also: Full disclosure, Christian is working on this for Booking.com,
and I'm managing that project...]

> I always want to test and verify that the untracked cache is working,
> before I rely on it.

Then with this patch you can just not use the core.untrackedCache=true
option, or with the later patches in this series use "git update-index
--test-untracked-cache && git config core.untrackedCache true".

> I'm not sure if ever "we know" ?
> How can we know without testing ?
> I personaly can not say "I know" in all the different system I am using,

Some users of Git can know that their mtime works, just like they know
they deploy it on filesystems where say symlinks work.

The current implementation of turning on this feature needs to be run
on a per-repo basis and without the --force option includes mandatory
tests, which a) makes it inconvenient to deploy across all Git repos
on a set of machines b) Is needlessly paranoid as a default way to
enable it.

>> 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 it testing if it works (because it might
>> work on some systems using the repo over the network file system
>> but not others).
>
> Same here.
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>   Documentation/config.txt               | 10 ++++++++++
>>   Documentation/git-update-index.txt     | 11 +++++++++--
>>   builtin/update-index.c                 | 28 ++++++++++++++++++----------
>>   cache.h                                |  1 +
>>   config.c                               | 10 ++++++++++
>>   contrib/completion/git-completion.bash |  1 +
>>   dir.c                                  |  2 +-
>>   environment.c                          |  1 +
>>   wt-status.c                            |  9 +++++++++
>>   9 files changed, 60 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index b4b0194..bf176ff 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -308,6 +308,16 @@ core.trustctime::
>>         crawlers and some backup systems).
>>         See linkgit:git-update-index[1]. True by default.
>>   +core.untrackedCache::
>> +       If unset or set to 'default' or 'check', untracked cache will
>> +       not be enabled by default and when
>> +       'update-index --untracked-cache' is called, Git will test if
>> +       mtime is working properly before enabling it. If set to false,
>> +       Git will refuse to enable untracked cache even if
>> +       '--force-untracked-cache' is used. If set to true, Git will
>> +       blindly enabled untracked cache by default without testing if
>> +       it works. See linkgit:git-update-index[1].
>> +
>
> Please no.
> The command line option should always be able to overwrite any settings
> from a config file.

If we keep this patch and not the rest in this series (which I think
should also be applied) you'd either use the update-index way of
changing the setting, or the config option.

> Sorry, I may missing the big picture here.
> What exactly should be achieved ?
>
> A config variable that should ask Git to always try to use the untracked
> cache ?
> Or a config variable that tells Git to never use the untracked cache ?
> Or a combination ?
>
> core.untrackedCache::
>  false: Never use the untracked cache ?
>  true: Always try to use the untracked cache ?
>        Try means: probe, and if the probing fails, record that if fails in
> the index,
>        for this hostname/os/kernel/path (Don't remember all the details)
> unset: As today,

As discussed in the "[RFC/PATCH] config: add core.trustmtime" thread
this feature is IMO needlessly paranoid about enabling itself.

Current state of affairs:

 * Enable on a per-repo basis: git update-index --untracked-cache
 * Disable on a per-repo basis: git update-index --no-cache
 * Enable system-wide: N/A
 * Disable system-wide: N/A

With this patch:

 * Enable on a per-repo basis: git update-index --untracked-cache OR
"git config core.untrackedCache true"
 * Disable on a per-repo basis: git update-index --no-cache OR "git
config core.untrackedCache false"
 * Enable system-wide: git config --global core.untrackedCache true
 * Disable system-wide: git config --global core.untrackedCache false
 * Caveat: The core.untrackedCache config has precidence over "git update-index"

With the rest of the patches in this series:

 * Enable system-wide & per-repo the same, just tweak
core.untrackedCache either for the local .git or --globally
 * If you want to test things either per-repo or globally just use
"git update-index --test-untracked-cache"
 * If you want something exactly like the old --untracked-cache do:
"git update-index --test-untracked-cache && git config
core.untrackedCache true"

I think applying this whole series makes sense. Enabling this feature
doesn't work like anything else in Git, usually you just tweak a
config option and thus can easily enable things either system-wide or
per-repo (or any combination of the two), which makes both system
administration and local configuration easy.

A much saner UI for the CLI tools if we're going to ship git with
tests for filesystem features is to separate the testing from the
enabling of those features.