diff mbox series

[v2] dir: force untracked cache with core.untrackedCache

Message ID pull.1058.v2.git.1645131630140.gitgitgadget@gmail.com (mailing list archive)
State Accepted
Commit 26b89464219d3cfb0af7dc2274751eff641dea8d
Headers show
Series [v2] dir: force untracked cache with core.untrackedCache | expand

Commit Message

Derrick Stolee Feb. 17, 2022, 9 p.m. UTC 
From: Derrick Stolee <derrickstolee@github.com>

The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked
cache more frequently than the core.untrackedCache config variable. This
is due to how read_directory() handles the creation of an untracked
cache.

Before this change, Git would not create the untracked cache extension
for an index that did not already have one. Users would need to run a
command such as 'git update-index --untracked-cache' before the index
would actually contain an untracked cache.

In particular, users noticed that the untracked cache would not appear
even with core.untrackedCache=true. Some users reported setting
GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to
ensure the untracked cache would be created.

The decision to not write the untracked cache without an environment
variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty
for changes to the untracked cache, 2018-02-05). The motivation of that
change is that writing the index is expensive, and if the untracked
cache is the only thing that needs to be written, then it is more
expensive than the benefit of the cache. However, this also means that
the untracked cache never gets populated, so the user who enabled it via
config does not actually get the extension until running 'git
update-index --untracked-cache' manually or using the environment
variable.

We have had a version of this change in the microsoft/git fork for a few
major releases now. It has been working well to get users into a good
state. Yes, that first index write is slow, but the remaining index
writes are much faster than they would be without this change.

Signed-off-by: Derrick Stolee <derrickstolee@github.com>
---
    dir: force untracked cache with core.untrackedCache
    
    We have seen users in the wild that have had core.untrackedCache
    enabled, but never actually have an untracked cache created for them. We
    have a test in t7063 that shows git status should write the untracked
    cache, so I'm not exactly sure how users are in this state for long.
    
    This patch fixes the situation. I also know of another group that sets
    GIT_FORCE_UNTRACKED_CACHE=1 in their developer environment in order to
    get this behavior.
    
    -Stolee
    
    
    Update in v2
    ============
    
     * Edited the commit message to be clearer.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-1058%2Fderrickstolee%2Funtracked-cache-write-more-v2
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-1058/derrickstolee/untracked-cache-write-more-v2
Pull-Request: https://github.com/gitgitgadget/git/pull/1058

Range-diff vs v1:

 1:  061ee9a379d ! 1:  8d132bc5566 dir: force untracked cache with core.untrackedCache
     @@ Commit message
          The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked
          cache more frequently than the core.untrackedCache config variable. This
          is due to how read_directory() handles the creation of an untracked
     -    cache. The old mechanism required using something like 'git update-index
     -    --untracked-cache' before the index would actually contain an untracked
     -    cache. This was noted as a performance problem on macOS in the past, and
     -    this is a resolution for that issue.
     +    cache.
     +
     +    Before this change, Git would not create the untracked cache extension
     +    for an index that did not already have one. Users would need to run a
     +    command such as 'git update-index --untracked-cache' before the index
     +    would actually contain an untracked cache.
     +
     +    In particular, users noticed that the untracked cache would not appear
     +    even with core.untrackedCache=true. Some users reported setting
     +    GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to
     +    ensure the untracked cache would be created.
      
          The decision to not write the untracked cache without an environment
          variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty


 dir.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: b80121027d1247a0754b3cc46897fee75c050b44

Comments

Junio C Hamano Feb. 17, 2022, 10:51 p.m. UTC | #1 
"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
>
> The GIT_FORCE_UNTRACKED_CACHE environment variable writes the untracked
> cache more frequently than the core.untrackedCache config variable. This
> is due to how read_directory() handles the creation of an untracked
> cache.
>
> Before this change, Git would not create the untracked cache extension
> for an index that did not already have one. Users would need to run a
> command such as 'git update-index --untracked-cache' before the index
> would actually contain an untracked cache.
>
> In particular, users noticed that the untracked cache would not appear
> even with core.untrackedCache=true. Some users reported setting
> GIT_FORCE_UNTRACKED_CACHE=1 in their engineering system environment to
> ensure the untracked cache would be created.
>
> The decision to not write the untracked cache without an environment
> variable tracks back to fc9ecbeb9 (dir.c: don't flag the index as dirty
> for changes to the untracked cache, 2018-02-05). The motivation of that
> change is that writing the index is expensive, and if the untracked
> cache is the only thing that needs to be written, then it is more
> expensive than the benefit of the cache. However, this also means that
> the untracked cache never gets populated, so the user who enabled it via
> config does not actually get the extension until running 'git
> update-index --untracked-cache' manually or using the environment
> variable.
>
> We have had a version of this change in the microsoft/git fork for a few
> major releases now. It has been working well to get users into a good
> state. Yes, that first index write is slow, but the remaining index
> writes are much faster than they would be without this change.
>
> Signed-off-by: Derrick Stolee <derrickstolee@github.com>
> ---

Will queue.

Thanks.
diff mbox series

Patch

diff --git a/dir.c b/dir.c
index d91295f2bcd..79a5f6918c8 100644
--- a/dir.c
+++ b/dir.c
@@ -2936,7 +2936,9 @@  int read_directory(struct dir_struct *dir, struct index_state *istate,
 
 		if (force_untracked_cache < 0)
 			force_untracked_cache =
-				git_env_bool("GIT_FORCE_UNTRACKED_CACHE", 0);
+				git_env_bool("GIT_FORCE_UNTRACKED_CACHE", -1);
+		if (force_untracked_cache < 0)
+			force_untracked_cache = (istate->repo->settings.core_untracked_cache == UNTRACKED_CACHE_WRITE);
 		if (force_untracked_cache &&
 			dir->untracked == istate->untracked &&
 		    (dir->untracked->dir_opened ||