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

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

From
Christian Couder <christian.couder@gmail.com>
Date
Dec 18, 2015, 22:38 UTC
Message-ID
<CAP8UFD2=hGbUFg_wX-0r6QgW+44ZiX8iJPX5c1FGG-4H8i163A@mail.gmail.com>
In-Reply-To
<CAP8UFD0LAQG+gQ5EhYYLjo5=tpW3_ah6GV-mgRbgTjjgNmdorA@mail.gmail.com>
Sorry I sent this privately to Peff by mistake (once again).
---------- Forwarded message ----------
From: Christian Couder <christian.couder@gmail.com>
Date: Fri, Dec 18, 2015 at 11:09 PM
Subject: Re: [PATCH 7/8] config: add core.untrackedCache
To: Jeff King <peff@peff.net>
On Thu, Dec 17, 2015 at 8:44 AM, Jeff King <peff@peff.net> wrote:
Show 6 quoted lines
> On Tue, Dec 15, 2015 at 10:05:57PM -0800, Junio C Hamano wrote:
>>
>> To put it another way, the "bit" in the index (i.e. the presence of
>> the cached data) is "Am I using the feature now?".  The effect of
>> the feature has to (and is designed to) persist, as it is a cache
>> and you do not want a stale cache to give you wrong answers.

Sorry if I repeat myself or misunderstood something, but in what I implemented we either use the UC fully or we remove it, so it cannot be stale vs git operations (like other changes in the index Duy talks about).

And as we are caching directory mtimes and as we are comparing each cached mtime with the current mtime of the original directory before trusting the cached content related to the directory, a UC that is stale vs working tree operations could result in time lost but not in bad cached content used.

Or maybe if we are very unlucky and have two directories with exactly the same mtime and the same name but not the same content, and if we move away the directory the UC was made from, and then put the other one at the path where the first one was. Yeah in this case you may get bad results because bad UC content is used. But note that this case can already happen now. It is a case that is inherent in using the UC.

Show 21 quoted lines
>> There
>> is no "Do I want to use the feature?" preference, in other words.
>>
>> And I do not mind creating such a preference bit as a configuration.
>>
>> That is why I suggested such a configuration to cause the equivalent
>> of "update-index --untracked-cache" when a new index is created from
>> scratch (as opposed to the case where the previously created cache
>> data is carried forward across read_cache() -> do things to the
>> index -> write_cache() flow).  Doing it that way will not have to
>> involve additional complexity that comes from the desire that
>> setting a single configuration on (or off) has to suddenly change
>> the behaviour of an index file that is already using (or not using)
>> the feature.
>
> I think we may actually be thinking of the same thing. Naively, I would
> expect:
>
>   - if there is untracked cache data in the index, we will make use of
>     it when enumerating untracked files (and my understanding is that if
>     it is stale, we can detect that)

I agree for "git status", but I am not sure that the UC is used in all the code paths that enumerate untracked files. I recall Duy mentioning that it was not used by "git add" and in some other cases, but I might be wrong.

I also agree that we can detect when the UC content should not be used because it is stale vs working tree operations (see above).

Now as Duy says the UC should never be stale vs git operations, but that is easy to do if we just remove the UC when we stop using it.

>   - if core.untrackedCache is set, we will update and write out an
>     untracked cache when we are enumerating the untracked files

In the current patch this happens when "git status" is called, actually in wt_status_collect_untracked(), again I am not sure about all the other code paths.

>   - if there is cache data in the index but that config flag is not set,
>     presumably we would not update it (we could even explicitly drop it,
>     but my understanding is that is not necessary for correctness, but
>     only as a possible optimization).

In the current patch we drop the UC, also in wt_status_collect_untracked(), if the config flag is not set (or set to false).

It is necessary to drop it for correctness for the following reasons:
- git operations on (other parts of) the index must update the UC if
it exists according to Duy who gave the following example in a
previous email:

"... if file "foo" is tracked, then the user does "git rm --cached foo", "foo" may become either untracked or ignored. So if you disable UC while doing this removal, then re-enable UC again, "git-status" may show incorrect output."

- the user may have decided to unset the config flag because the mtime
features we rely on are in fact not well supported by the system.
(Though it's true that the user should have used "git update-index
--test-untracked-cache" before setting the config flag in the first
place, but maybe it was a mistake, or maybe the file system will be
accessed for some time by another system that will mount it or
something.)
> You could have a config option for "if there is a cache there, pretend
> it isn't and ignore it", but I don't see much point.
There is not much point indeed in such an option and it could be dangerous.
Previous: Christian CouderNext: Christian Couder
Message 53 of 54 in “Untracked cache improvements”
  1. 0/8 Untracked cache improvementsChristian Couder, Dec 8, 2015
  2. 1/8 update-index: add untracked cache notificationsChristian Couder, Dec 8, 2015
  3. Junio C HamanoDec 8, 2015
  4. Christian CouderDec 11, 2015
  5. 2/8 update-index: use enum for untracked cache optionsChristian Couder, Dec 8, 2015
  6. Junio C HamanoDec 8, 2015
  7. Christian CouderDec 10, 2015
  8. Junio C HamanoDec 10, 2015
  9. Christian CouderDec 11, 2015
  10. Junio C HamanoDec 11, 2015
  11. Christian CouderDec 12, 2015
  12. 3/8 update-index: add --test-untracked-cacheChristian Couder, Dec 8, 2015
  13. 4/8 update-index: move 'uc' var declarationChristian Couder, Dec 8, 2015
  14. 5/8 dir: add add_untracked_cache()Christian Couder, Dec 8, 2015
  15. Torsten BögershausenDec 9, 2015
  16. Christian CouderDec 11, 2015
  17. 6/8 dir: add remove_untracked_cache()Christian Couder, Dec 8, 2015
  18. Junio C HamanoDec 8, 2015
  19. Torsten BögershausenDec 9, 2015
  20. 7/8 config: add core.untrackedCacheChristian Couder, Dec 8, 2015
  21. Junio C HamanoDec 8, 2015
  22. Junio C HamanoDec 8, 2015
  23. Christian CouderDec 14, 2015
  24. Junio C HamanoDec 14, 2015
  25. Junio C HamanoDec 14, 2015
  26. Christian CouderDec 15, 2015
  27. Torsten BögershausenDec 15, 2015
  28. Christian CouderDec 15, 2015
  29. Duy NguyenDec 15, 2015
  30. Christian CouderDec 15, 2015
  31. Ævar Arnfjörð BjarmasonDec 15, 2015
  32. Christian CouderDec 15, 2015
  33. Junio C HamanoDec 15, 2015
  34. Ævar Arnfjörð BjarmasonDec 15, 2015
  35. Junio C HamanoDec 15, 2015
  36. Ævar Arnfjörð BjarmasonDec 16, 2015
  37. Jeff KingDec 16, 2015
  38. Junio C HamanoDec 16, 2015
  39. Junio C HamanoDec 16, 2015
  40. Jeff KingDec 17, 2015
  41. Duy NguyenDec 17, 2015
  42. Junio C HamanoDec 21, 2015
  43. Duy NguyenDec 22, 2015
  44. Junio C HamanoDec 22, 2015
  45. Junio C HamanoDec 24, 2015
  46. Duy NguyenDec 24, 2015
  47. Junio C HamanoDec 27, 2015
  48. Christian CouderDec 24, 2015
  49. Duy NguyenDec 17, 2015
  50. Christian CouderDec 18, 2015
  51. Torsten BögershausenDec 9, 2015
  52. 8/8 t7063: add tests for core.untrackedCacheChristian Couder, Dec 8, 2015
  53. Fwd: [PATCH 7/8] config: add core.untrackedCacheChristian Couder, Dec 18, 2015
  54. Fwd: [PATCH 7/8] config: add core.untrackedCacheChristian Couder, Dec 18, 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.