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

From: Duy Nguyen <pclouds@gmail.com>
Date: Nov 25, 2015, 19:51 UTC
Message-ID: <CACsJy8DhAfu7J=WpPAp8HYGLuFQC5+DZyZj6Hs6vruEJEeVKig@mail.gmail.com>
In-Reply-To: <CACBZZX4YSvfA=BRayk8CC8ipk_nDdvJfznxjyb-upwP9PKRBSA@mail.gmail.com>

On Wed, Nov 25, 2015 at 10:00 AM, Ævar Arnfjörð Bjarmason <avarab@gmail.com> wrote:

Show 19 quoted lines

> 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.

Show 6 quoted lines

> 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) ;)

Show 7 quoted lines

>  * 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?

>  * If you "ctrl+c" git update-index --untracked-cache at an
> inopportune time you'll end up with a mtime-test-XXXXXX directory in
> your working tree. Perhaps this tempdir should be created in the .git
> directory instead?

No because in theory .git could be on a separate file system with different semantics. But we should probably clean those files at ^C.

>  * Maybe we should have a --test-untracked-cache option, so you can
> run the tests without enabling it.

I'd say patches welcome.

> Aside from the slight hassle of enabling this and keeping it enabled
> this feature is great. It's sped up "git status" across the board by
> about 40%. Slightly less than that on faster spinning disks, slightly
> more than that on slower ones.

I'm still waiting for the day when watchman support gets merged and maybe poke Facebook guys to compare performance with Mercurial :) Well, we are probably still behind Mercurial on that day.

Also, there's still work to be done. Right now it's optimized for whole-tree "git status", Doing "git status -- abc" will not benefit from untracked cache, similarly "git add" with pathspec..

-- 
Duy