Jeff King <p...@peff.net> writes:

> A minor nit, but please use something like:
>
>   if (git_env_bool("GIT_TEST_UNTRACKED_CACHE", 0) && ...
>
> so that:
>
>   GIT_TEST_UNTRACKED_CACHE=false
>
> does what one might expect, and not the opposite.
>
> Two other thoughts:
>
>   - it may be worth memo-izing it with a static variable to avoid
>     repeatedly calling the possibly-slow getenv()
>
>   - I agree with the sentiment elsewhere that something like
>     GIT_FORCE_UNTRACKED_CACHE is probably a better name
>
> (The idea itself seems sound to me, but it's not really my area).

Somehow this topic has been hanging without getting further
attention for too long.  It's time to wrap it up and moving it
forward.  How about this?

-- >8 --
From: Junio C Hamano <gits...@pobox.com>
Date: Wed, 28 Feb 2018 13:21:09 -0800
Subject: [PATCH] untracked cache: use git_env_bool() not getenv() for 
customization

GIT_DISABLE_UNTRACKED_CACHE and GIT_TEST_UNTRACKED_CACHE are only
sensed for their presense by using getenv(); use git_env_bool()
instead so that GIT_DISABLE_UNTRACKED_CACHE=false would work as
naïvely expected.

Also rename GIT_TEST_UNTRACKED_CACHE to GIT_FORCE_UNTRACKED_CACHE
to express what it does more honestly.  Forcing its use may be one
useful thing to do while testing the feature, but testing does not
have to be the only use of the knob.

While at it, avoid repeated calls to git_env_bool() by capturing the
return value from the first call in a static variable.

Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
 dir.c                             | 14 ++++++++++++--
 t/t7063-status-untracked-cache.sh |  4 ++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/dir.c b/dir.c
index da93374f0c..d445d77e62 100644
--- a/dir.c
+++ b/dir.c
@@ -2164,8 +2164,13 @@ static struct untracked_cache_dir 
*validate_untracked_cache(struct dir_struct *d
                                                      const struct pathspec 
*pathspec)
 {
        struct untracked_cache_dir *root;
+       static int untracked_cache_disabled = -1;
 
-       if (!dir->untracked || getenv("GIT_DISABLE_UNTRACKED_CACHE"))
+       if (!dir->untracked)
+               return NULL;
+       if (untracked_cache_disabled < 0)
+               untracked_cache_disabled = 
git_env_bool("GIT_DISABLE_UNTRACKED_CACHE", 0);
+       if (untracked_cache_disabled)
                return NULL;
 
        /*
@@ -2287,7 +2292,12 @@ int read_directory(struct dir_struct *dir, struct 
index_state *istate,
        }
 
        if (dir->untracked) {
+               static int force_untracked_cache = -1;
                static struct trace_key trace_untracked_stats = 
TRACE_KEY_INIT(UNTRACKED_STATS);
+
+               if (force_untracked_cache < 0)
+                       force_untracked_cache =
+                               git_env_bool("GIT_FORCE_UNTRACKED_CACHE", 0);
                trace_printf_key(&trace_untracked_stats,
                                 "node creation: %u\n"
                                 "gitignore invalidation: %u\n"
@@ -2297,7 +2307,7 @@ int read_directory(struct dir_struct *dir, struct 
index_state *istate,
                                 dir->untracked->gitignore_invalidated,
                                 dir->untracked->dir_invalidated,
                                 dir->untracked->dir_opened);
-               if (getenv("GIT_TEST_UNTRACKED_CACHE") &&
+               if (force_untracked_cache &&
                        dir->untracked == istate->untracked &&
                    (dir->untracked->dir_opened ||
                     dir->untracked->gitignore_invalidated ||
diff --git a/t/t7063-status-untracked-cache.sh 
b/t/t7063-status-untracked-cache.sh
index 6ef520e823..9cb16ca36d 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -14,8 +14,8 @@ test_description='test untracked cache'
 # See <20160803174522.5571-1-pclo...@gmail.com> if you want to know
 # more.
 
-GIT_TEST_UNTRACKED_CACHE=true
-export GIT_TEST_UNTRACKED_CACHE
+GIT_FORCE_UNTRACKED_CACHE=true
+export GIT_FORCE_UNTRACKED_CACHE
 
 sync_mtime () {
        find . -type d -ls >/dev/null
-- 
2.16.2-325-g2fc74f41c5

Reply via email to