git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
Duy Nguyen <pclouds@gmail.com>
Date
Nov 26, 2015, 17:53 UTC
Message-ID
<CACsJy8CNRbvsDY+ucFZGcO4+tE9Ud_QNr1k4D=+utawSCrc+Sg@mail.gmail.com>
In-Reply-To
<CAP8UFD2V3nbY2-abW6cGDtB2PR9Q+sN+d0RgTVJORCPg6TPMcQ@mail.gmail.com>

On Thu, Nov 26, 2015 at 6:21 AM, Christian Couder <christian.couder@gmail.com> wrote:

Show 36 quoted lines
> 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.

Show 16 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)
>> ;)
>
> 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..

Show 6 quoted lines
> 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.

Show 19 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?
>
> 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
Previous: Christian CouderNext: Ævar Arnfjörð Bjarmason
Message 5 of 13 in “config: add core.trustmtime”
  1. config: add core.trustmtimeChristian Couder, Nov 25, 2015
  2. Ævar Arnfjörð BjarmasonNov 25, 2015
  3. Duy NguyenNov 25, 2015
  4. Christian CouderNov 26, 2015
  5. Duy NguyenNov 26, 2015
  6. Ævar Arnfjörð BjarmasonNov 27, 2015
  7. Junio C HamanoNov 30, 2015
  8. Duy NguyenNov 30, 2015
  9. Torsten BögershausenDec 1, 2015
  10. Duy NguyenDec 2, 2015
  11. Christian CouderDec 7, 2015
  12. Johannes SchindelinNov 25, 2015
  13. Christian CouderNov 25, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.