Re: [RFC/PATCH] config: add core.trustmtime

[Duy Nguyen <pclouds@gmail.com>]

On Thu, Nov 26, 2015 at 6:21 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> Hi Duy,
>
> On Wed, Nov 25, 2015 at 8:51 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>> On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason
>> <avarab@gmail.com> wrote:
>>> On Wed, Nov 25, 2015 at 7:35 AM, Christian Couder
>>> <christian.couder@gmail.com> wrote:
>>>> At Booking.com we know that mtime works everywhere and we don't
>>>> want the untracked cache to stop working when a kernel is upgraded
>>>> or when the repo is copied to a machine with a different kernel.
>>>> I will add tests later if people are ok with this.
>>>
>>> I bit more info: I rolled Git out internally with this patch:
>>> https://github.com/avar/git/commit/c63f7c12c2664631961add7cf3da901b0b6aa2f2
>>>
>>> The --untracked-cache feature hardcodes the equivalent of:
>>>
>>>     pwd; uname --kernel-name --kernel-release --kernel-version
>>>
>>> Into the index. If any of those change it prints out the "cache is
>>> disabled" warning.
>>>
>>> This patch will make it stop being so afraid of itself to the point of
>>> disabling itself on minor kernel upgrades :)
>>
>> The problem is, there's no way to teach git to know it's a "minor"
>> upgrade.. but if there is a config key to say "don't be paranoid, I
>> know what I'm doing", then we can skip that check, or just warn
>> instead of disabling the cache.
>
> Yeah, in my patch if core.trustmtime is set to true or false the check
> is skipped.
>
> I am wondering why you didn't make it by default run the mtime checks
> when a kernel change is detected. Maybe that would be better than
> disabling itself.

It takes about 10 seconds to go through the mtime check. Imagine you
have to wait 10s for some random "git status".. Plus I didn't want to
do anything fancy.

>
>>> A few other issues with this feature I've noticed:
>>>
>>>  * There's no way to just enable it globally via the config. Makes it
>>> a bit of a hassle to use it. I wanted to have a config option to
>>> enable it via the config, how about "index.untracked_cache = true" for
>>> the config variable name?
>>
>> If you haven't noticed, all these experimental features have no real
>> UI (update-index is plumbing). I have been waiting for someone like
>> you to start using it and figure out the best UI (then implement it)
>> ;)
>
> Ok, we are happy to do that (including implementing it) :-)
>
> I will take a look at something like index.untracked_cache. It will

Nit. untrackedCache (underscores are avoided for config keys) and it's
probably core.untrackedCache instead..

> probably also be a tristate like this:
>
> - true: always enable it; die if core.trustmtime is false otherwise
> warn if it is not true
> - default/unset: same as current behavior
> - false: die if it is enabled or when trying to enable to it

There could be a fourth option: let a hook do the checking. If the
hook returns ok, all is good.

>>>  * Doing "cd /tmp: git --git-dir=/git/somewhere/else/.git update-index
>>> --untracked-cache" doesn't work how I'd expect. It hardcodes "/tmp" as
>>> the directory that "works" into the index, so if you use the working
>>> tree you'll never use the untracked cache. I spotted this because I
>>> carry out a bunch of git maintenance commands with --git-dir instead
>>> of cd-ing to the relevant directories. This works for most other
>>> things in git, is it a bug that it doesn't work here?
>>
>> It needs the current directory at --untrack-cache time to test if the
>> directory satisfies the requirements. So either you cd to that
>> worktree, or you have to specify --worktree as well. Or am I missing
>> something?
>
> Maybe it could print out a message saying "Testing mtime in directory
> $(pwd)" and if that works then "Untracked cache is enabled for
> $(pwd)". That would make it clear that it will not work in other
> directories.
>
> Also maybe the mtime checks could be run when a directory change is detected.

Yeah.. and we could also have a hook to do this test your own way too,
if you already know your system supports it, so you wait no time at
all.
-- 
Duy