[RFC/PATCH 0/8] Untracked cache improvements

[RFC/PATCH 0/8] Untracked cache improvements

[Christian Couder]

Following the discussions on the "config: add core.trustmtime"
patch I previously sent, here is a patch series that tries to
improve the untracked cache feature.

This patch series implements core.untrackedCache instead of
core.trustmtime. core.untrackedCache is more complex because
basically when it's set to true git should always try to use
the untracked cache, and when set to false git should never
use it.

Patchs 1/8 and 2/8 add some features that are missing.
Patchs 3/8, 4/8 and 5/8 are some refactoring to prepare for
patch 6/8 which implements core.untrackedCache.

Up to patch 6/8 backward compatibility is preserved.
Patchs 7/8 and 8/8 are trying to improve usability by making
the untracked cache cli and config options more in line with
other git cli and config options, but this sacrifies some
backward compatibility.

Christian Couder (8):
  update-index: add untracked cache notifications
  update-index: add --test-untracked-cache
  update-index: move 'uc' var declaration
  dir: add add_untracked_cache()
  dir: add remove_untracked_cache()
  config: add core.untrackedCache
  update-index: prevent --untracked-cache from performing tests
  update-index: make core.untrackedCache a bool

 Documentation/config.txt               | 10 +++++++++
 Documentation/git-update-index.txt     | 30 +++++++++++++++++++-------
 builtin/update-index.c                 | 39 ++++++++++++++++------------------
 cache.h                                |  1 +
 config.c                               |  4 ++++
 contrib/completion/git-completion.bash |  1 +
 dir.c                                  | 22 ++++++++++++++++++-
 dir.h                                  |  2 ++
 environment.c                          |  1 +
 t/t7063-status-untracked-cache.sh      |  2 +-
 wt-status.c                            |  9 ++++++++
 11 files changed, 90 insertions(+), 31 deletions(-)

-- 
2.6.3.391.g95a3a5c

[RFC/PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

Doing:

  cd /tmp
  git --git-dir=/git/somewhere/else/.git update-index --untracked-cache

doesn't work how one would 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.

With this patch "git update-index --untracked-cache" tells the user in
which directory tests are performed and in which working directory the
untracked cache is allowed.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/update-index.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..e568acc 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
 	if (!mkdtemp(mtime_dir.buf))
 		die_errno("Could not make temporary directory");
 
-	fprintf(stderr, _("Testing "));
+	fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
 	atexit(remove_test_directory);
 	xstat_mtime_dir(&st);
 	fill_stat_data(&base, &st);
@@ -1122,9 +1122,11 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		}
 		add_untracked_ident(the_index.untracked);
 		the_index.cache_changed |= UNTRACKED_CHANGED;
+		fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
 	} else if (!untracked_cache && the_index.untracked) {
 		the_index.untracked = NULL;
 		the_index.cache_changed |= UNTRACKED_CHANGED;
+		fprintf(stderr, _("Untracked disabled\n"));
 	}
 
 	if (active_cache_changed) {
-- 
2.6.3.391.g95a3a5c

Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications

[Duy Nguyen]

On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> Doing:
>
>   cd /tmp
>   git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>
> doesn't work how one would 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.
>
> With this patch "git update-index --untracked-cache" tells the user in
> which directory tests are performed and in which working directory the
> untracked cache is allowed.
>
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/update-index.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 7431938..e568acc 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>         if (!mkdtemp(mtime_dir.buf))
>                 die_errno("Could not make temporary directory");
>
> -       fprintf(stderr, _("Testing "));
> +       fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());

We probably should respect --verbose. I know I violated it in the first place.
-- 
Duy

Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications

[Christian Couder]

On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> Doing:
>>
>>   cd /tmp
>>   git --git-dir=/git/somewhere/else/.git update-index --untracked-cache
>>
>> doesn't work how one would 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.
>>
>> With this patch "git update-index --untracked-cache" tells the user in
>> which directory tests are performed and in which working directory the
>> untracked cache is allowed.
>>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>  builtin/update-index.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index 7431938..e568acc 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>>         if (!mkdtemp(mtime_dir.buf))
>>                 die_errno("Could not make temporary directory");
>>
>> -       fprintf(stderr, _("Testing "));
>> +       fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
>
> We probably should respect --verbose. I know I violated it in the first place.

The verbose help is:

    --verbose             report actions to standard output

so yeah, it is not respected first because the output is on by
default, and second because the output is on stderr instead of stdout.
Anyway it can be a separate patch or patch series to make it respect
one or both of these points.

I am not very much interested in doing it myself as I think it's
interesting to have the output by default especially if the above
patch is applied. But if people agree that it would be a good thing, I
will do it.

Re: [RFC/PATCH 1/8] update-index: add untracked cache notifications

[Duy Nguyen]

On Mon, Dec 7, 2015 at 10:08 AM, Christian Couder
<christian.couder@gmail.com> wrote:
> On Wed, Dec 2, 2015 at 8:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:
>>> --- a/builtin/update-index.c
>>> +++ b/builtin/update-index.c
>>> @@ -121,7 +121,7 @@ static int test_if_untracked_cache_is_supported(void)
>>>         if (!mkdtemp(mtime_dir.buf))
>>>                 die_errno("Could not make temporary directory");
>>>
>>> -       fprintf(stderr, _("Testing "));
>>> +       fprintf(stderr, _("Testing mtime in '%s' "), xgetcwd());
>>
>> We probably should respect --verbose. I know I violated it in the first place.
>
> The verbose help is:
>
>     --verbose             report actions to standard output
>
> so yeah, it is not respected first because the output is on by
> default, and second because the output is on stderr instead of stdout.
> Anyway it can be a separate patch or patch series to make it respect
> one or both of these points.
>
> I am not very much interested in doing it myself as I think it's
> interesting to have the output by default especially if the above
> patch is applied. But if people agree that it would be a good thing, I
> will do it.

Then we can leave it out. Not important (until people actually complain).
-- 
Duy

[RFC/PATCH 2/8] update-index: add --test-untracked-cache

[Christian Couder]

It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-update-index.txt | 9 ++++++++-
 builtin/update-index.c             | 8 ++++++--
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 3df9c26..0ff7396 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -17,7 +17,7 @@ SYNOPSIS
 	     [--[no-]assume-unchanged]
 	     [--[no-]skip-worktree]
 	     [--ignore-submodules]
-	     [--[no-|force-]untracked-cache]
+	     [--[no-|test-|force-]untracked-cache]
 	     [--really-refresh] [--unresolve] [--again | -g]
 	     [--info-only] [--index-info]
 	     [-z] [--stdin] [--index-version <n>]
@@ -179,6 +179,13 @@ may not support it yet.
 	system must change `st_mtime` field of a directory if files
 	are added or deleted in that directory.
 
+--test-untracked-cache::
+	Only perform tests on the working directory to make sure
+	untracked cache can be used. You have to manually enable
+	untracked cache using `--force-untracked-cache` (or
+	`--untracked-cache` but this will run the tests again)
+	afterwards if you really want to use it.
+
 --force-untracked-cache::
 	For safety, `--untracked-cache` performs tests on the working
 	directory to make sure untracked cache can be used. These
diff --git a/builtin/update-index.c b/builtin/update-index.c
index e568acc..b7b5108 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			N_("enable or disable split index")),
 		OPT_BOOL(0, "untracked-cache", &untracked_cache,
 			N_("enable/disable untracked cache")),
+		OPT_SET_INT(0, "test-untracked-cache", &untracked_cache,
+			    N_("test if the filesystem supports untracked cache"), 2),
 		OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
-			    N_("enable untracked cache without testing the filesystem"), 2),
+			    N_("enable untracked cache without testing the filesystem"), 3),
 		OPT_END()
 	};
 
@@ -1107,10 +1109,12 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 	if (untracked_cache > 0) {
 		struct untracked_cache *uc;
 
-		if (untracked_cache < 2) {
+		if (untracked_cache < 3) {
 			setup_work_tree();
 			if (!test_if_untracked_cache_is_supported())
 				return 1;
+			if (untracked_cache == 2)
+				return 0;
 		}
 		if (!the_index.untracked) {
 			uc = xcalloc(1, sizeof(*uc));
-- 
2.6.3.391.g95a3a5c

Re: [RFC/PATCH 2/8] update-index: add --test-untracked-cache

[Duy Nguyen]

On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
<christian.couder@gmail.com> wrote:
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index e568acc..b7b5108 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>                         N_("enable or disable split index")),
>                 OPT_BOOL(0, "untracked-cache", &untracked_cache,
>                         N_("enable/disable untracked cache")),
> +               OPT_SET_INT(0, "test-untracked-cache", &untracked_cache,
> +                           N_("test if the filesystem supports untracked cache"), 2),
>                 OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
> -                           N_("enable untracked cache without testing the filesystem"), 2),
> +                           N_("enable untracked cache without testing the filesystem"), 3),
>                 OPT_END()
>         };

I think we got enough numbers to start using enum instead.
-- 
Duy

Re: [RFC/PATCH 2/8] update-index: add --test-untracked-cache

[Christian Couder]

On Wed, Dec 2, 2015 at 8:17 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>> diff --git a/builtin/update-index.c b/builtin/update-index.c
>> index e568acc..b7b5108 100644
>> --- a/builtin/update-index.c
>> +++ b/builtin/update-index.c
>> @@ -996,8 +996,10 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>>                         N_("enable or disable split index")),
>>                 OPT_BOOL(0, "untracked-cache", &untracked_cache,
>>                         N_("enable/disable untracked cache")),
>> +               OPT_SET_INT(0, "test-untracked-cache", &untracked_cache,
>> +                           N_("test if the filesystem supports untracked cache"), 2),
>>                 OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
>> -                           N_("enable untracked cache without testing the filesystem"), 2),
>> +                           N_("enable untracked cache without testing the filesystem"), 3),
>>                 OPT_END()
>>         };
>
> I think we got enough numbers to start using enum instead.

Ok, I will use an enum.

[RFC/PATCH 3/8] update-index: move 'uc' var declaration

[Christian Couder]

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/update-index.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index b7b5108..b54ddc3 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1107,8 +1107,6 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		the_index.cache_changed |= SOMETHING_CHANGED;
 	}
 	if (untracked_cache > 0) {
-		struct untracked_cache *uc;
-
 		if (untracked_cache < 3) {
 			setup_work_tree();
 			if (!test_if_untracked_cache_is_supported())
@@ -1117,7 +1115,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 				return 0;
 		}
 		if (!the_index.untracked) {
-			uc = xcalloc(1, sizeof(*uc));
+			struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
 			strbuf_init(&uc->ident, 100);
 			uc->exclude_per_dir = ".gitignore";
 			/* should be the same flags used by git-status */
-- 
2.6.3.391.g95a3a5c

[RFC/PATCH 4/8] dir: add add_untracked_cache()

[Christian Couder]

This new function will be used in a later patch.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/update-index.c | 11 +----------
 dir.c                  | 14 ++++++++++++++
 dir.h                  |  1 +
 3 files changed, 16 insertions(+), 10 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index b54ddc3..ec67d14 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1114,16 +1114,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			if (untracked_cache == 2)
 				return 0;
 		}
-		if (!the_index.untracked) {
-			struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
-			strbuf_init(&uc->ident, 100);
-			uc->exclude_per_dir = ".gitignore";
-			/* should be the same flags used by git-status */
-			uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
-			the_index.untracked = uc;
-		}
-		add_untracked_ident(the_index.untracked);
-		the_index.cache_changed |= UNTRACKED_CHANGED;
+		add_untracked_cache();
 		fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
 	} else if (!untracked_cache && the_index.untracked) {
 		the_index.untracked = NULL;
diff --git a/dir.c b/dir.c
index d2a8f06..0f7e293 100644
--- a/dir.c
+++ b/dir.c
@@ -1938,6 +1938,20 @@ void add_untracked_ident(struct untracked_cache *uc)
 	strbuf_addch(&uc->ident, 0);
 }
 
+void add_untracked_cache(void)
+{
+	if (!the_index.untracked) {
+		struct untracked_cache *uc = xcalloc(1, sizeof(*uc));
+		strbuf_init(&uc->ident, 100);
+		uc->exclude_per_dir = ".gitignore";
+		/* should be the same flags used by git-status */
+		uc->dir_flags = DIR_SHOW_OTHER_DIRECTORIES | DIR_HIDE_EMPTY_DIRECTORIES;
+		the_index.untracked = uc;
+	}
+	add_untracked_ident(the_index.untracked);
+	the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir,
 						      int base_len,
 						      const struct pathspec *pathspec)
diff --git a/dir.h b/dir.h
index 7b5855d..ee94c76 100644
--- a/dir.h
+++ b/dir.h
@@ -308,4 +308,5 @@ void free_untracked_cache(struct untracked_cache *);
 struct untracked_cache *read_untracked_extension(const void *data, unsigned long sz);
 void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
 void add_untracked_ident(struct untracked_cache *);
+void add_untracked_cache(void);
 #endif
-- 
2.6.3.391.g95a3a5c

[RFC/PATCH 5/8] dir: add remove_untracked_cache()

[Christian Couder]

This new function will be used in a later patch.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 builtin/update-index.c | 3 +--
 dir.c                  | 6 ++++++
 dir.h                  | 1 +
 3 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/builtin/update-index.c b/builtin/update-index.c
index ec67d14..2cbaee0 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1117,8 +1117,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		add_untracked_cache();
 		fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
 	} else if (!untracked_cache && the_index.untracked) {
-		the_index.untracked = NULL;
-		the_index.cache_changed |= UNTRACKED_CHANGED;
+		remove_untracked_cache();
 		fprintf(stderr, _("Untracked disabled\n"));
 	}
 
diff --git a/dir.c b/dir.c
index 0f7e293..ffc0286 100644
--- a/dir.c
+++ b/dir.c
@@ -1952,6 +1952,12 @@ void add_untracked_cache(void)
 	the_index.cache_changed |= UNTRACKED_CHANGED;
 }
 
+void remove_untracked_cache(void)
+{
+	the_index.untracked = NULL;
+	the_index.cache_changed |= UNTRACKED_CHANGED;
+}
+
 static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *dir,
 						      int base_len,
 						      const struct pathspec *pathspec)
diff --git a/dir.h b/dir.h
index ee94c76..3e5114d 100644
--- a/dir.h
+++ b/dir.h
@@ -309,4 +309,5 @@ struct untracked_cache *read_untracked_extension(const void *data, unsigned long
 void write_untracked_extension(struct strbuf *out, struct untracked_cache *untracked);
 void add_untracked_ident(struct untracked_cache *);
 void add_untracked_cache(void);
+void remove_untracked_cache(void);
 #endif
-- 
2.6.3.391.g95a3a5c

[RFC/PATCH 6/8] config: add core.untrackedCache

[Christian Couder]

When we know that mtime is fully supported by the environment, we
might want the untracked cache to be always used by default without
any mtime test or kernel version check being performed.

Also when we know that mtime is not supported by the environment,
for example because the repo is shared over a network file system,
then we might want 'git update-index --untracked-cache' to fail
immediately instead of it testing if it works (because it might
work on some systems using the repo over the network file system
but not others).

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/config.txt               | 10 ++++++++++
 Documentation/git-update-index.txt     | 11 +++++++++--
 builtin/update-index.c                 | 28 ++++++++++++++++++----------
 cache.h                                |  1 +
 config.c                               | 10 ++++++++++
 contrib/completion/git-completion.bash |  1 +
 dir.c                                  |  2 +-
 environment.c                          |  1 +
 wt-status.c                            |  9 +++++++++
 9 files changed, 60 insertions(+), 13 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index b4b0194..bf176ff 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,16 @@ core.trustctime::
 	crawlers and some backup systems).
 	See linkgit:git-update-index[1]. True by default.
 
+core.untrackedCache::
+	If unset or set to 'default' or 'check', untracked cache will
+	not be enabled by default and when
+	'update-index --untracked-cache' is called, Git will test if
+	mtime is working properly before enabling it. If set to false,
+	Git will refuse to enable untracked cache even if
+	'--force-untracked-cache' is used. If set to true, Git will
+	blindly enabled untracked cache by default without testing if
+	it works. See linkgit:git-update-index[1].
+
 core.checkStat::
 	Determines which stat fields to match between the index
 	and work tree. The user can set this to 'default' or
diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 0ff7396..4e6078b 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -177,7 +177,10 @@ may not support it yet.
 	up for commands that involve determining untracked files such
 	as `git status`. The underlying operating system and file
 	system must change `st_mtime` field of a directory if files
-	are added or deleted in that directory.
+	are added or deleted in that directory. If you want to always
+	enable, or always disable, untracked cache, you can set the
+	`core.untrackedCache` configuration variable to 'true' or
+	'false' respectively, (see linkgit:git-config[1]).
 
 --test-untracked-cache::
 	Only perform tests on the working directory to make sure
@@ -190,7 +193,9 @@ may not support it yet.
 	For safety, `--untracked-cache` performs tests on the working
 	directory to make sure untracked cache can be used. These
 	tests can take a few seconds. `--force-untracked-cache` can be
-	used to skip the tests.
+	used to skip the tests. It cannot enable untracked cache if
+	`core.untrackedCache` configuration variable is set to false
+	(see linkgit:git-config[1]).
 
 \--::
 	Do not interpret any more arguments as options.
@@ -406,6 +411,8 @@ It can be useful when the inode change time is regularly modified by
 something outside Git (file system crawlers and backup systems use
 ctime for marking files processed) (see linkgit:git-config[1]).
 
+Untracked cache look at `core.untrackedCache` configuration variable
+(see linkgit:git-config[1]).
 
 SEE ALSO
 --------
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 2cbaee0..bf697a5 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,19 +1106,27 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		the_index.split_index = NULL;
 		the_index.cache_changed |= SOMETHING_CHANGED;
 	}
+	if (untracked_cache == 2 || (untracked_cache == 1 && use_untracked_cache == -1)) {
+		setup_work_tree();
+		if (!test_if_untracked_cache_is_supported())
+			return 1;
+		if (untracked_cache == 2)
+			return 0;
+	}
 	if (untracked_cache > 0) {
-		if (untracked_cache < 3) {
-			setup_work_tree();
-			if (!test_if_untracked_cache_is_supported())
-				return 1;
-			if (untracked_cache == 2)
-				return 0;
-		}
+		if (!use_untracked_cache)
+			die("core.untrackedCache is set to false; "
+			    "the untracked cache will not be enabled");
 		add_untracked_cache();
 		fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
-	} else if (!untracked_cache && the_index.untracked) {
-		remove_untracked_cache();
-		fprintf(stderr, _("Untracked disabled\n"));
+	} else if (!untracked_cache) {
+		if (use_untracked_cache > 0)
+			die("core.untrackedCache is set to true; "
+			    "the untracked cache will not be disabled");
+		if (the_index.untracked) {
+			remove_untracked_cache();
+			fprintf(stderr, _("Untracked disabled\n"));
+		}
 	}
 
 	if (active_cache_changed) {
diff --git a/cache.h b/cache.h
index 736abc0..b27692d 100644
--- a/cache.h
+++ b/cache.h
@@ -601,6 +601,7 @@ extern void set_alternate_index_output(const char *);
 /* Environment bits from configuration mechanism */
 extern int trust_executable_bit;
 extern int trust_ctime;
+extern int use_untracked_cache;
 extern int check_stat;
 extern int quote_path_fully;
 extern int has_symlinks;
diff --git a/config.c b/config.c
index 248a21a..7d50f43 100644
--- a/config.c
+++ b/config.c
@@ -691,6 +691,16 @@ static int git_default_core_config(const char *var, const char *value)
 		trust_ctime = git_config_bool(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "core.untrackedcache")) {
+		if (!strcasecmp(value, "default") || !strcasecmp(value, "check"))
+			use_untracked_cache = -1;
+		else {
+			use_untracked_cache = git_config_maybe_bool(var, value);
+			if (use_untracked_cache == -1)
+				error("unknown core.untrackedCache value '%s'; using default", value);
+		}
+		return 0;
+	}
 	if (!strcmp(var, "core.checkstat")) {
 		if (!strcasecmp(value, "default"))
 			check_stat = 1;
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 482ca84..f23a800 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2038,6 +2038,7 @@ _git_config ()
 		core.sparseCheckout
 		core.symlinks
 		core.trustctime
+		core.untrackedCache
 		core.warnAmbiguousRefs
 		core.whitespace
 		core.worktree
diff --git a/dir.c b/dir.c
index ffc0286..aa07aca 100644
--- a/dir.c
+++ b/dir.c
@@ -2014,7 +2014,7 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
 	if (dir->exclude_list_group[EXC_CMDL].nr)
 		return NULL;
 
-	if (!ident_in_untracked(dir->untracked)) {
+	if (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {
 		warning(_("Untracked cache is disabled on this system."));
 		return NULL;
 	}
diff --git a/environment.c b/environment.c
index 2da7fe2..8f631a1 100644
--- a/environment.c
+++ b/environment.c
@@ -14,6 +14,7 @@
 
 int trust_executable_bit = 1;
 int trust_ctime = 1;
+int use_untracked_cache = -1;
 int check_stat = 1;
 int has_symlinks = 1;
 int minimum_abbrev = 4, default_abbrev = 7;
diff --git a/wt-status.c b/wt-status.c
index 435fc28..3e0fe02 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -586,6 +586,15 @@ static void wt_status_collect_untracked(struct wt_status *s)
 		dir.flags |= DIR_SHOW_IGNORED_TOO;
 	else
 		dir.untracked = the_index.untracked;
+
+	if (!dir.untracked && use_untracked_cache == 1) {
+		add_untracked_cache();
+		dir.untracked = the_index.untracked;
+	} else if (dir.untracked && use_untracked_cache == 0) {
+		remove_untracked_cache();
+		dir.untracked = NULL;
+	}
+
 	setup_standard_excludes(&dir);
 
 	fill_directory(&dir, &s->pathspec);
-- 
2.6.3.391.g95a3a5c

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Torsten Bögershausen]

On 12/01/2015 09:31 PM, Christian Couder wrote:
> When we know that mtime is fully supported by the environment, we
> might want the untracked cache to be always used by default without
> any mtime test or kernel version check being performed.
I'm not sure if ever "we know" ?
How can we know without testing ?
I personaly can not say "I know" in all the different system I am using,
so I always want to test and verify that the untracked cache is working,
before I rely on it.


> Also when we know that mtime is not supported by the environment,
> for example because the repo is shared over a network file system,
> then we might want 'git update-index --untracked-cache' to fail
> immediately instead of it testing if it works (because it might
> work on some systems using the repo over the network file system
> but not others).
Same here.
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>   Documentation/config.txt               | 10 ++++++++++
>   Documentation/git-update-index.txt     | 11 +++++++++--
>   builtin/update-index.c                 | 28 ++++++++++++++++++----------
>   cache.h                                |  1 +
>   config.c                               | 10 ++++++++++
>   contrib/completion/git-completion.bash |  1 +
>   dir.c                                  |  2 +-
>   environment.c                          |  1 +
>   wt-status.c                            |  9 +++++++++
>   9 files changed, 60 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index b4b0194..bf176ff 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -308,6 +308,16 @@ core.trustctime::
>   	crawlers and some backup systems).
>   	See linkgit:git-update-index[1]. True by default.
>   
> +core.untrackedCache::
> +	If unset or set to 'default' or 'check', untracked cache will
> +	not be enabled by default and when
> +	'update-index --untracked-cache' is called, Git will test if
> +	mtime is working properly before enabling it. If set to false,
> +	Git will refuse to enable untracked cache even if
> +	'--force-untracked-cache' is used. If set to true, Git will
> +	blindly enabled untracked cache by default without testing if
> +	it works. See linkgit:git-update-index[1].
> +
Please no.
The command line option should always be able to overwrite any settings
from a config file.


Sorry, I may missing the big picture here.
What exactly should be achieved ?

A config variable that should ask Git to always try to use the untracked 
cache ?
Or a config variable that tells Git to never use the untracked cache ?
Or a combination ?

core.untrackedCache::
  false: Never use the untracked cache ?
  true: Always try to use the untracked cache ?
        Try means: probe, and if the probing fails, record that if fails in the index,
        for this hostname/os/kernel/path (Don't remember all the details)
unset: As today,

   

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Ævar Arnfjörð Bjarmason]

On Wed, Dec 2, 2015 at 8:12 AM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 12/01/2015 09:31 PM, Christian Couder wrote:
>>
>> When we know that mtime is fully supported by the environment, we
>> might want the untracked cache to be always used by default without
>> any mtime test or kernel version check being performed.
>

[Re-arranged some of the quotes for the clarity of my reply....]

[Also: Full disclosure, Christian is working on this for Booking.com,
and I'm managing that project...]

> I always want to test and verify that the untracked cache is working,
> before I rely on it.

Then with this patch you can just not use the core.untrackedCache=true
option, or with the later patches in this series use "git update-index
--test-untracked-cache && git config core.untrackedCache true".

> I'm not sure if ever "we know" ?
> How can we know without testing ?
> I personaly can not say "I know" in all the different system I am using,

Some users of Git can know that their mtime works, just like they know
they deploy it on filesystems where say symlinks work.

The current implementation of turning on this feature needs to be run
on a per-repo basis and without the --force option includes mandatory
tests, which a) makes it inconvenient to deploy across all Git repos
on a set of machines b) Is needlessly paranoid as a default way to
enable it.

>> Also when we know that mtime is not supported by the environment,
>> for example because the repo is shared over a network file system,
>> then we might want 'git update-index --untracked-cache' to fail
>> immediately instead of it testing if it works (because it might
>> work on some systems using the repo over the network file system
>> but not others).
>
> Same here.
>
>> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
>> ---
>>   Documentation/config.txt               | 10 ++++++++++
>>   Documentation/git-update-index.txt     | 11 +++++++++--
>>   builtin/update-index.c                 | 28 ++++++++++++++++++----------
>>   cache.h                                |  1 +
>>   config.c                               | 10 ++++++++++
>>   contrib/completion/git-completion.bash |  1 +
>>   dir.c                                  |  2 +-
>>   environment.c                          |  1 +
>>   wt-status.c                            |  9 +++++++++
>>   9 files changed, 60 insertions(+), 13 deletions(-)
>>
>> diff --git a/Documentation/config.txt b/Documentation/config.txt
>> index b4b0194..bf176ff 100644
>> --- a/Documentation/config.txt
>> +++ b/Documentation/config.txt
>> @@ -308,6 +308,16 @@ core.trustctime::
>>         crawlers and some backup systems).
>>         See linkgit:git-update-index[1]. True by default.
>>   +core.untrackedCache::
>> +       If unset or set to 'default' or 'check', untracked cache will
>> +       not be enabled by default and when
>> +       'update-index --untracked-cache' is called, Git will test if
>> +       mtime is working properly before enabling it. If set to false,
>> +       Git will refuse to enable untracked cache even if
>> +       '--force-untracked-cache' is used. If set to true, Git will
>> +       blindly enabled untracked cache by default without testing if
>> +       it works. See linkgit:git-update-index[1].
>> +
>
> Please no.
> The command line option should always be able to overwrite any settings
> from a config file.

If we keep this patch and not the rest in this series (which I think
should also be applied) you'd either use the update-index way of
changing the setting, or the config option.

> Sorry, I may missing the big picture here.
> What exactly should be achieved ?
>
> A config variable that should ask Git to always try to use the untracked
> cache ?
> Or a config variable that tells Git to never use the untracked cache ?
> Or a combination ?
>
> core.untrackedCache::
>  false: Never use the untracked cache ?
>  true: Always try to use the untracked cache ?
>        Try means: probe, and if the probing fails, record that if fails in
> the index,
>        for this hostname/os/kernel/path (Don't remember all the details)
> unset: As today,

As discussed in the "[RFC/PATCH] config: add core.trustmtime" thread
this feature is IMO needlessly paranoid about enabling itself.

Current state of affairs:

 * Enable on a per-repo basis: git update-index --untracked-cache
 * Disable on a per-repo basis: git update-index --no-cache
 * Enable system-wide: N/A
 * Disable system-wide: N/A

With this patch:

 * Enable on a per-repo basis: git update-index --untracked-cache OR
"git config core.untrackedCache true"
 * Disable on a per-repo basis: git update-index --no-cache OR "git
config core.untrackedCache false"
 * Enable system-wide: git config --global core.untrackedCache true
 * Disable system-wide: git config --global core.untrackedCache false
 * Caveat: The core.untrackedCache config has precidence over "git update-index"

With the rest of the patches in this series:

 * Enable system-wide & per-repo the same, just tweak
core.untrackedCache either for the local .git or --globally
 * If you want to test things either per-repo or globally just use
"git update-index --test-untracked-cache"
 * If you want something exactly like the old --untracked-cache do:
"git update-index --test-untracked-cache && git config
core.untrackedCache true"

I think applying this whole series makes sense. Enabling this feature
doesn't work like anything else in Git, usually you just tweak a
config option and thus can easily enable things either system-wide or
per-repo (or any combination of the two), which makes both system
administration and local configuration easy.

A much saner UI for the CLI tools if we're going to ship git with
tests for filesystem features is to separate the testing from the
enabling of those features.

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Torsten Bögershausen]

[snip all good stuff]

First of all:
Thanks for explaining it so well

I now can see the point in having this patch.
(Do the commit messages reflect all this ? I need to re-read)

The other question is: Do you have patch on a public repo ?

And, of course, can we add 1 or 2 test cases ?


Thanks for pushing this forward.

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Christian Couder]

On Thu, Dec 3, 2015 at 5:10 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> [snip all good stuff]
>
> First of all:
> Thanks for explaining it so well
>
> I now can see the point in having this patch.
> (Do the commit messages reflect all this ? I need to re-read)

Maybe not. I will have a look at improving them, but your re-reading is welcome.

> The other question is: Do you have patch on a public repo ?

Yes, here: https://github.com/chriscool/git/commits/uc-notifs8

> And, of course, can we add 1 or 2 test cases ?

Yes I had planned to add tests for this.
But it would be nice if I could know first if the last two patches are
considered ok even though they are breaking compatibility a bit.
In this case I could squash them with previous patches and only write
tests for the resulting patches.

> Thanks for pushing this forward.

Thanks for your reviews,
Christian.

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Torsten Bögershausen]

> Current state of affairs:
> 
>  * Enable on a per-repo basis: git update-index --untracked-cache
>  * Disable on a per-repo basis: git update-index --no-cache
>  * Enable system-wide: N/A
>  * Disable system-wide: N/A
> 
> With this patch:
> 
>  * Enable on a per-repo basis: git update-index --untracked-cache OR
> "git config core.untrackedCache true"
>  * Disable on a per-repo basis: git update-index --no-cache OR "git
> config core.untrackedCache false"
>  * Enable system-wide: git config --global core.untrackedCache true
>  * Disable system-wide: git config --global core.untrackedCache false
>  * Caveat: The core.untrackedCache config has precidence over "git update-index"
> 
> With the rest of the patches in this series:
> 
>  * Enable system-wide & per-repo the same, just tweak
> core.untrackedCache either for the local .git or --globally
>  * If you want to test things either per-repo or globally just use
> "git update-index --test-untracked-cache"
>  * If you want something exactly like the old --untracked-cache do:
> "git update-index --test-untracked-cache && git config
> core.untrackedCache true"
> 
> I think applying this whole series makes sense. Enabling this feature
> doesn't work like anything else in Git, usually you just tweak a
> config option and thus can easily enable things either system-wide or
> per-repo (or any combination of the two), which makes both system
> administration and local configuration easy.
> 
> A much saner UI for the CLI tools if we're going to ship git with
> tests for filesystem features is to separate the testing from the
> enabling of those features.

My spontanous feeling: squash 6-8 together and add this nice explanation
to the commit message.

Re: [RFC/PATCH 6/8] config: add core.untrackedCache

[Christian Couder]

On Fri, Dec 4, 2015 at 6:54 PM, Torsten Bögershausen <tboegi@web.de> wrote:
>> Current state of affairs:
>>
>>  * Enable on a per-repo basis: git update-index --untracked-cache
>>  * Disable on a per-repo basis: git update-index --no-cache
>>  * Enable system-wide: N/A
>>  * Disable system-wide: N/A
>>
>> With this patch:
>>
>>  * Enable on a per-repo basis: git update-index --untracked-cache OR
>> "git config core.untrackedCache true"
>>  * Disable on a per-repo basis: git update-index --no-cache OR "git
>> config core.untrackedCache false"
>>  * Enable system-wide: git config --global core.untrackedCache true
>>  * Disable system-wide: git config --global core.untrackedCache false
>>  * Caveat: The core.untrackedCache config has precidence over "git update-index"
>>
>> With the rest of the patches in this series:
>>
>>  * Enable system-wide & per-repo the same, just tweak
>> core.untrackedCache either for the local .git or --globally
>>  * If you want to test things either per-repo or globally just use
>> "git update-index --test-untracked-cache"
>>  * If you want something exactly like the old --untracked-cache do:
>> "git update-index --test-untracked-cache && git config
>> core.untrackedCache true"
>>
>> I think applying this whole series makes sense. Enabling this feature
>> doesn't work like anything else in Git, usually you just tweak a
>> config option and thus can easily enable things either system-wide or
>> per-repo (or any combination of the two), which makes both system
>> administration and local configuration easy.
>>
>> A much saner UI for the CLI tools if we're going to ship git with
>> tests for filesystem features is to separate the testing from the
>> enabling of those features.
>
> My spontanous feeling: squash 6-8 together and add this nice explanation
> to the commit message.

Ok, I will do that then.

[RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests

[Christian Couder]

`git update-index --untracked-cache` used to perform tests to
check that the underlying operating system and file system
change `st_mtime` field of a directory if files are added or
deleted in that directory. But those tests take a long time
and there is now `--test-untracked-cache` to perform them.

So to be more consistent with other git commands it's better
to make `--untracked-cache` not perform them, which is the
purpose of this patch.

Note that after this patch there is no difference any more
between `--untracked-cache` and `--force-untracked-cache`.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-update-index.txt | 31 ++++++++++++++++---------------
 builtin/update-index.c             |  7 ++-----
 t/t7063-status-untracked-cache.sh  |  2 +-
 3 files changed, 19 insertions(+), 21 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 4e6078b..5f74cc7 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -175,27 +175,28 @@ may not support it yet.
 --no-untracked-cache::
 	Enable or disable untracked cache extension. This could speed
 	up for commands that involve determining untracked files such
-	as `git status`. The underlying operating system and file
-	system must change `st_mtime` field of a directory if files
-	are added or deleted in that directory. If you want to always
-	enable, or always disable, untracked cache, you can set the
-	`core.untrackedCache` configuration variable to 'true' or
-	'false' respectively, (see linkgit:git-config[1]).
+	as `git status`.
++
+The underlying operating system and file system must change `st_mtime`
+field of a directory if files are added or deleted in that
+directory. You can test that using
+`--test-untracked-cache`. `--untracked-cache` used to test that too
+but it doesn't anymore. If you want to always enable, or always
+disable, untracked cache, you can set the `core.untrackedCache`
+configuration variable to 'true' or 'false' respectively, (see
+linkgit:git-config[1]).
 
 --test-untracked-cache::
 	Only perform tests on the working directory to make sure
 	untracked cache can be used. You have to manually enable
-	untracked cache using `--force-untracked-cache` (or
-	`--untracked-cache` but this will run the tests again)
-	afterwards if you really want to use it.
+	untracked cache using `--untracked-cache` or
+	`--force-untracked-cache` afterwards if you really want to use
+	it.
 
 --force-untracked-cache::
-	For safety, `--untracked-cache` performs tests on the working
-	directory to make sure untracked cache can be used. These
-	tests can take a few seconds. `--force-untracked-cache` can be
-	used to skip the tests. It cannot enable untracked cache if
-	`core.untrackedCache` configuration variable is set to false
-	(see linkgit:git-config[1]).
+	Same as `--untracked-cache`. Note that this option cannot
+	enable untracked cache if `core.untrackedCache` configuration
+	variable is set to false (see linkgit:git-config[1]).
 
 \--::
 	Do not interpret any more arguments as options.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index bf697a5..fb0ea3d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1106,12 +1106,9 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		the_index.split_index = NULL;
 		the_index.cache_changed |= SOMETHING_CHANGED;
 	}
-	if (untracked_cache == 2 || (untracked_cache == 1 && use_untracked_cache == -1)) {
+	if (untracked_cache == 2) {
 		setup_work_tree();
-		if (!test_if_untracked_cache_is_supported())
-			return 1;
-		if (untracked_cache == 2)
-			return 0;
+		return !test_if_untracked_cache_is_supported();
 	}
 	if (untracked_cache > 0) {
 		if (!use_untracked_cache)
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index 0e8d0d4..8c3e703 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -11,7 +11,7 @@ avoid_racy() {
 # It's fine if git update-index returns an error code other than one,
 # it'll be caught in the first test.
 test_lazy_prereq UNTRACKED_CACHE '
-	{ git update-index --untracked-cache; ret=$?; } &&
+	{ git update-index --test-untracked-cache; ret=$?; } &&
 	test $ret -ne 1
 '
 
-- 
2.6.3.391.g95a3a5c

Re: [RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests

[Duy Nguyen]

On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
<christian.couder@gmail.com> wrote:
 diff --git a/t/t7063-status-untracked-cache.sh
b/t/t7063-status-untracked-cache.sh
> index 0e8d0d4..8c3e703 100755
> --- a/t/t7063-status-untracked-cache.sh
> +++ b/t/t7063-status-untracked-cache.sh
> @@ -11,7 +11,7 @@ avoid_racy() {
>  # It's fine if git update-index returns an error code other than one,
>  # it'll be caught in the first test.

Notice this comment. You probably have to chance --test-untr.. for the
first test too if it's stilll true, or delete the comment.

>  test_lazy_prereq UNTRACKED_CACHE '
> -       { git update-index --untracked-cache; ret=$?; } &&
> +       { git update-index --test-untracked-cache; ret=$?; } &&
>         test $ret -ne 1
>  '
>
> --
> 2.6.3.391.g95a3a5c
>



-- 
Duy

Re: [RFC/PATCH 7/8] update-index: prevent --untracked-cache from performing tests

[Christian Couder]

On Wed, Dec 2, 2015 at 8:18 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Tue, Dec 1, 2015 at 9:31 PM, Christian Couder
> <christian.couder@gmail.com> wrote:
>  diff --git a/t/t7063-status-untracked-cache.sh
> b/t/t7063-status-untracked-cache.sh
>> index 0e8d0d4..8c3e703 100755
>> --- a/t/t7063-status-untracked-cache.sh
>> +++ b/t/t7063-status-untracked-cache.sh
>> @@ -11,7 +11,7 @@ avoid_racy() {
>>  # It's fine if git update-index returns an error code other than one,
>>  # it'll be caught in the first test.
>
> Notice this comment. You probably have to chance --test-untr.. for the
> first test too if it's stilll true, or delete the comment.

Ok, I think I will remove the comment and still use "git update-index
--untracked-cache" in the first test.

>>  test_lazy_prereq UNTRACKED_CACHE '
>> -       { git update-index --untracked-cache; ret=$?; } &&
>> +       { git update-index --test-untracked-cache; ret=$?; } &&
>>         test $ret -ne 1
>>  '

Thanks,
Christian.

[RFC/PATCH 8/8] update-index: make core.untrackedCache a bool

[Christian Couder]

Most features in Git can be enabled or disabled using a simple
bool config variable and it would be nice if untracked cache
behaved the same way.

This makes --[no-|force-]untracked-cache change the value of
core.untrackedCache in the repo config file, to avoid making
those options useless and because this avoids the untracked
cache being disabled by a kernel change or a directory
change. Of course this breaks some backward compatibility,
but the simplification and increased useability is worth it.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 Documentation/git-update-index.txt | 13 ++++++-------
 builtin/update-index.c             | 10 ++++------
 config.c                           |  8 +-------
 3 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/Documentation/git-update-index.txt b/Documentation/git-update-index.txt
index 5f74cc7..256b9c5 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -181,10 +181,11 @@ The underlying operating system and file system must change `st_mtime`
 field of a directory if files are added or deleted in that
 directory. You can test that using
 `--test-untracked-cache`. `--untracked-cache` used to test that too
-but it doesn't anymore. If you want to always enable, or always
-disable, untracked cache, you can set the `core.untrackedCache`
-configuration variable to 'true' or 'false' respectively, (see
-linkgit:git-config[1]).
+but it doesn't anymore.
++
+This sets the `core.untrackedCache` configuration variable to 'true'
+or 'false' in the repo config file, (see linkgit:git-config[1]), so
+that the untracked cache stays enabled or disabled.
 
 --test-untracked-cache::
 	Only perform tests on the working directory to make sure
@@ -194,9 +195,7 @@ linkgit:git-config[1]).
 	it.
 
 --force-untracked-cache::
-	Same as `--untracked-cache`. Note that this option cannot
-	enable untracked cache if `core.untrackedCache` configuration
-	variable is set to false (see linkgit:git-config[1]).
+	Same as `--untracked-cache`.
 
 \--::
 	Do not interpret any more arguments as options.
diff --git a/builtin/update-index.c b/builtin/update-index.c
index fb0ea3d..048c293 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1111,15 +1111,13 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		return !test_if_untracked_cache_is_supported();
 	}
 	if (untracked_cache > 0) {
-		if (!use_untracked_cache)
-			die("core.untrackedCache is set to false; "
-			    "the untracked cache will not be enabled");
+		if (!use_untracked_cache && git_config_set("core.untrackedCache", "true"))
+			die("could not set core.untrackedCache to true");
 		add_untracked_cache();
 		fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
 	} else if (!untracked_cache) {
-		if (use_untracked_cache > 0)
-			die("core.untrackedCache is set to true; "
-			    "the untracked cache will not be disabled");
+		if (use_untracked_cache > 0 && git_config_set("core.untrackedCache", "false"))
+			die("could not set core.untrackedCache to false");
 		if (the_index.untracked) {
 			remove_untracked_cache();
 			fprintf(stderr, _("Untracked disabled\n"));
diff --git a/config.c b/config.c
index 7d50f43..f023ee7 100644
--- a/config.c
+++ b/config.c
@@ -692,13 +692,7 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 	if (!strcmp(var, "core.untrackedcache")) {
-		if (!strcasecmp(value, "default") || !strcasecmp(value, "check"))
-			use_untracked_cache = -1;
-		else {
-			use_untracked_cache = git_config_maybe_bool(var, value);
-			if (use_untracked_cache == -1)
-				error("unknown core.untrackedCache value '%s'; using default", value);
-		}
+		use_untracked_cache = git_config_bool(var, value);
 		return 0;
 	}
 	if (!strcmp(var, "core.checkstat")) {
-- 
2.6.3.391.g95a3a5c

Re: [RFC/PATCH 8/8] update-index: make core.untrackedCache a bool

[Torsten Bögershausen]

On 01.12.15 21:31, Christian Couder wrote:
> Most features in Git can be enabled or disabled using a simple
> bool config variable and it would be nice if untracked cache
> behaved the same way.
> 
> This makes --[no-|force-]untracked-cache change the value of
> core.untrackedCache in the repo config file, to avoid making
> those options useless and because this avoids the untracked
> cache being disabled by a kernel change or a directory
> change. Of course this breaks some backward compatibility,
> but the simplification and increased useability is worth it.

Some loose thinking, how the core.untrackedcache and the
command line can work together:

core.untrackedcache = unset
  everything is as today
  if --test-untracked-cache is used on command line,
  the test is run, additionally the result is stored
  in the config variable core.untrackedcache

core.untrackedcache = true
  same as --force-untracked-cache
  if --no-untracked-cache is given on command line,
  this has higher priority

core.untrackedcache = false
  same as --no-untracked-cache
  if --force-untracked-cache is given on command line,
  this has higher priority

Does this support the workflow described by Ævar ?

Re: [RFC/PATCH 8/8] update-index: make core.untrackedCache a bool

[Christian Couder]

(Sorry I already sent a private reply to Tosten by mistake.)

On Sat, Dec 5, 2015 at 1:44 PM, Torsten Bögershausen <tboegi@web.de> wrote:
> On 01.12.15 21:31, Christian Couder wrote:
>> Most features in Git can be enabled or disabled using a simple
>> bool config variable and it would be nice if untracked cache
>> behaved the same way.
>>
>> This makes --[no-|force-]untracked-cache change the value of
>> core.untrackedCache in the repo config file, to avoid making
>> those options useless and because this avoids the untracked
>> cache being disabled by a kernel change or a directory
>> change. Of course this breaks some backward compatibility,
>> but the simplification and increased useability is worth it.
>
> Some loose thinking, how the core.untrackedcache and the
> command line can work together:
>
> core.untrackedcache = unset
>   everything is as today
>   if --test-untracked-cache is used on command line,
>   the test is run, additionally the result is stored
>   in the config variable core.untrackedcache

I don't like storing the result in core.untrackedcache as it means
that --test-untracked-cache would not just test.
And I agree with Aevar that it's a good thing to be able to completely
separate testing and configuring.

> core.untrackedcache = true
>   same as --force-untracked-cache
>   if --no-untracked-cache is given on command line,
>   this has higher priority

I guess you mean that --no-untracked-cache has priority over
"core.untrackedcache = true" without removing this config variable.
This means that --no-untracked-cache would only remove the untracked
cache (UC) from the index.

But what git should then do the next time "git status" is run?
Git sees that the index does not contain any UC, but it doesn't know
that "git update-index --no-untracked-cache" has just been run. It
might be "git config core.untrackedcache true" that has been run last.

If "git config core.untrackedcache true" has been run last, it would
be logical to add the UC to the index and use it.
If "git update-index --no-untracked-cache" has been run last, it would
be logical to not add the UC.

> core.untrackedcache = false
>   same as --no-untracked-cache
>   if --force-untracked-cache is given on command line,
>   this has higher priority

Then the same kind of problem happens as above. There is no clear
solution about what "git status" should do when the state of the index
conflicts with the value of core.untrackedcache.

> Does this support the workflow described by Ævar ?

I don't think so. I think that when we set "core.untrackedcache =
true" we want the UC to be added to the index and used as much as
possible, because we know that our OS/filesystem supports it.

This means that using "git update-index --no-untracked-cache" when
"core.untrackedcache = true" is set can only have two possible
outcomes. It could either just die saying that "core.untrackedcache"
is true, or it could just unset "core.untrackedcache" or set it to
false (which might mean the same thing).