[PATCH v2 00/10] Untracked cache improvements

[PATCH v2 00/10] Untracked cache improvements

[Christian Couder]

Here is a new version of a patch series to improve the untracked cache
feature.

This v2 still implements core.untrackedCache as a simple bool config
variable. When it's true, Git should always try to use the untracked
cache, and when false, Git should never use it.

Patchs 1/10 to 3/10 add some features that are missing. Patch 3/10 has
been moved after the two other patches and has been changed a bit
according to Duy's and Junio's suggestions. In patch 2/10 the enum
names have been changed as discussed with Junio.

Patchs 4/10, 5/10 and 6/10, which have not been changed, are some
refactoring to prepare for patch 8/10 which implements
core.untrackedCache.

Patch 7/10 is a small bug fix suggested by Junio.

Patch 8/10, which adds core.untrackedCache, contains many
documentation and commit message improvements, some by AEvar.

Patch 9/10 has not been changed.

Patch 10/10 is new and removes code that is now useless.

So the changes compared to v1 are mostly small updates, and patchs
7/10 and 10/10.

The patch series is also available there:

https://github.com/chriscool/git/tree/uc-notifs25

Thanks to the reviewers and helpers.

Christian Couder (10):
  update-index: use enum for untracked cache options
  update-index: add --test-untracked-cache
  update-index: add untracked cache notifications
  update-index: move 'uc' var declaration
  dir: add add_untracked_cache()
  dir: add remove_untracked_cache()
  dir: free untracked cache when removing it
  config: add core.untrackedCache
  t7063: add tests for core.untrackedCache
  dir: do not use untracked cache ident anymore

 Documentation/config.txt               |  7 ++++
 Documentation/git-update-index.txt     | 61 ++++++++++++++++++++++++++++------
 builtin/update-index.c                 | 54 +++++++++++++++++-------------
 cache.h                                |  1 +
 config.c                               |  4 +++
 contrib/completion/git-completion.bash |  1 +
 dir.c                                  | 53 +++++++++++++----------------
 dir.h                                  |  4 ++-
 environment.c                          |  1 +
 t/t7063-status-untracked-cache.sh      | 52 ++++++++++++++++++++++++++---
 wt-status.c                            |  9 +++++
 11 files changed, 178 insertions(+), 69 deletions(-)

-- 
2.6.3.479.g8eb29d4

[PATCH v2 01/10] update-index: use enum for untracked cache options

[Christian Couder]

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

diff --git a/builtin/update-index.c b/builtin/update-index.c
index 7431938..2430a68 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
 #define UNMARK_FLAG 2
 static struct strbuf mtime_dir = STRBUF_INIT;
 
+/* Untracked cache mode */
+enum uc_mode {
+	UC_UNSPECIFIED = -1,
+	UC_DISABLE = 0,
+	UC_ENABLE,
+	UC_FORCE
+};
+
 __attribute__((format (printf, 1, 2)))
 static void report(const char *fmt, ...)
 {
@@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
 int cmd_update_index(int argc, const char **argv, const char *prefix)
 {
 	int newfd, entries, has_errors = 0, line_termination = '\n';
-	int untracked_cache = -1;
+	enum uc_mode untracked_cache = UC_UNSPECIFIED;
 	int read_from_stdin = 0;
 	int prefix_length = prefix ? strlen(prefix) : 0;
 	int preferred_index_format = 0;
@@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		OPT_BOOL(0, "untracked-cache", &untracked_cache,
 			N_("enable/disable untracked cache")),
 		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"), UC_FORCE),
 		OPT_END()
 	};
 
@@ -1104,10 +1112,10 @@ 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 > 0) {
+	if (untracked_cache > UC_DISABLE) {
 		struct untracked_cache *uc;
 
-		if (untracked_cache < 2) {
+		if (untracked_cache < UC_FORCE) {
 			setup_work_tree();
 			if (!test_if_untracked_cache_is_supported())
 				return 1;
@@ -1122,7 +1130,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		}
 		add_untracked_ident(the_index.untracked);
 		the_index.cache_changed |= UNTRACKED_CHANGED;
-	} else if (!untracked_cache && the_index.untracked) {
+	} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
 		the_index.untracked = NULL;
 		the_index.cache_changed |= UNTRACKED_CHANGED;
 	}
-- 
2.6.3.479.g8eb29d4

[PATCH v2 02/10] 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             | 5 +++++
 2 files changed, 13 insertions(+), 1 deletion(-)

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 2430a68..e747a6c 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -40,6 +40,7 @@ enum uc_mode {
 	UC_UNSPECIFIED = -1,
 	UC_DISABLE = 0,
 	UC_ENABLE,
+	UC_TEST,
 	UC_FORCE
 };
 
@@ -1004,6 +1005,8 @@ 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"), UC_TEST),
 		OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
 			    N_("enable untracked cache without testing the filesystem"), UC_FORCE),
 		OPT_END()
@@ -1119,6 +1122,8 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			setup_work_tree();
 			if (!test_if_untracked_cache_is_supported())
 				return 1;
+			if (untracked_cache == UC_TEST)
+				return 0;
 		}
 		if (!the_index.untracked) {
 			uc = xcalloc(1, sizeof(*uc));
-- 
2.6.3.479.g8eb29d4

[PATCH v2 03/10] update-index: add untracked cache notifications

[Christian Couder]

Attempting to flip the untracked-cache feature on for a random index
file with

    cd /random/unrelated/place
    git --git-dir=/somewhere/else/.git update-index --untracked-cache

would not work as you might expect. Because flipping the feature on
in the index also records the location of the corresponding working
tree (/random/unrelated/place in the above example), when the index
is subsequently used to keep track of files in the working tree in
/somewhere/else, the feature is disabled.

With this patch "git update-index --[test-]untracked-cache" tells the
user in which directory tests are performed. This makes it easy to
spot any problem.

Also in verbose mode, let's tell the user when the cache is enabled
or disabled.

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

diff --git a/builtin/update-index.c b/builtin/update-index.c
index e747a6c..e84674f 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -130,7 +130,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);
@@ -1135,9 +1135,13 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		}
 		add_untracked_ident(the_index.untracked);
 		the_index.cache_changed |= UNTRACKED_CHANGED;
+		if (verbose)
+			printf(_("Untracked cache enabled\n"));
 	} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
 		the_index.untracked = NULL;
 		the_index.cache_changed |= UNTRACKED_CHANGED;
+		if (verbose)
+			printf(_("Untracked cache disabled\n"));
 	}
 
 	if (active_cache_changed) {
-- 
2.6.3.479.g8eb29d4

[PATCH v2 04/10] 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 e84674f..fffad79 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1116,8 +1116,6 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		the_index.cache_changed |= SOMETHING_CHANGED;
 	}
 	if (untracked_cache > UC_DISABLE) {
-		struct untracked_cache *uc;
-
 		if (untracked_cache < UC_FORCE) {
 			setup_work_tree();
 			if (!test_if_untracked_cache_is_supported())
@@ -1126,7 +1124,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.479.g8eb29d4

[PATCH v2 05/10] dir: add add_untracked_cache()

[Christian Couder]

Factor out code into add_untracked_cache(), which will be used
in a later commit.

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 fffad79..5f009cf 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1123,16 +1123,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 			if (untracked_cache == UC_TEST)
 				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();
 		if (verbose)
 			printf(_("Untracked cache enabled\n"));
 	} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
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.479.g8eb29d4

[PATCH v2 06/10] dir: add remove_untracked_cache()

[Christian Couder]

Factor out code into remove_untracked_cache(), which will be used
in a later commit.

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 5f009cf..4ca6d94 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1127,8 +1127,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
 		if (verbose)
 			printf(_("Untracked cache enabled\n"));
 	} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
-		the_index.untracked = NULL;
-		the_index.cache_changed |= UNTRACKED_CHANGED;
+		remove_untracked_cache();
 		if (verbose)
 			printf(_("Untracked cache 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.479.g8eb29d4

[PATCH v2 07/10] dir: free untracked cache when removing it

[Christian Couder]

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 dir.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/dir.c b/dir.c
index ffc0286..3b83cc0 100644
--- a/dir.c
+++ b/dir.c
@@ -1954,6 +1954,7 @@ void add_untracked_cache(void)
 
 void remove_untracked_cache(void)
 {
+	free_untracked_cache(the_index.untracked);
 	the_index.untracked = NULL;
 	the_index.cache_changed |= UNTRACKED_CHANGED;
 }
-- 
2.6.3.479.g8eb29d4

[PATCH v2 08/10] 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 performing tests (because it might work on
some systems using the repo over the network file system but not
others).

The normal way to achieve the above in Git is to use a config
variable. That's why this patch introduces "core.untrackedCache".

To keep things simple, this variable is a bool which default to
false.

When "git status" is run, it now adds or removes the untracked
cache in the index to respect the value of this variable.

The job of `git update-index --[no-|force-]untracked-cache` was
to add or remove the untracked cache from the index. This was a
kind of configuration because this was persistant across git
commands. To make this kind of configuration compatible with the
new config variable, the simple thing to do, and what this patch
does, is to make `git update-index --[no-|force-]untracked-cache`
set or unset this config option.

This new behavior is a backward incompatible change, but that is
deliberate. The untracked cache feature has been experimental
and is very unlikely used by beginners.

When people will upgrade, this will remove any untracked cache
they used unless they set "core.untrackedCache" before upgrading.
This should be stated in the release notes.

Also `--untracked-cache` used 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.

That's why, to be more consistent with other git commands, this
patch prevents `--untracked-cache` to perform tests, so that
after this patch there is no difference any more between
`--untracked-cache` and `--force-untracked-cache`.

All the changes to `--[no-|force-]untracked-cache` make it
possible to deprecate those options in the future.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt               |  7 ++++
 Documentation/git-update-index.txt     | 58 +++++++++++++++++++++++++++-------
 builtin/update-index.c                 | 25 ++++++++-------
 cache.h                                |  1 +
 config.c                               |  4 +++
 contrib/completion/git-completion.bash |  1 +
 dir.c                                  |  2 +-
 environment.c                          |  1 +
 t/t7063-status-untracked-cache.sh      |  4 +--
 wt-status.c                            |  9 ++++++
 10 files changed, 85 insertions(+), 27 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 2d06b11..94820eb 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -308,6 +308,13 @@ core.trustctime::
 	crawlers and some backup systems).
 	See linkgit:git-update-index[1]. True by default.
 
+core.untrackedCache::
+	Determines if untracked cache will be enabled. Using
+	'git update-index --[no-|force-]untracked-cache' will set
+	this variable. Before setting it to true, you should check
+	that mtime is working properly on your system.
+	See linkgit:git-update-index[1]. False by default.
+
 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..cd02de4 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -173,24 +173,29 @@ may not support it yet.
 
 --untracked-cache::
 --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.
+	Enable or disable untracked cache extension. Please use
+	`--test-untracked-cache` before enabling it.
++
+These options are mostly aliases for setting the `core.untrackedCache`
+configuration variable to 'true' or 'false' in the local config file
+(see linkgit:git-config[1]). You can equivalently just set those
+configuration values directly. These options are just provided for
+backwards compatibility with the older versions of Git where this was
+the only way to enable or disable the untracked cache extension.
 
 --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` or the `core.untrackedCache`
+	configuration variable 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.
+	Same as `--untracked-cache`. Provided for backwards
+	compatibility with older versions of Git where
+	`--untracked-cache` used to imply `--test-untracked-cache` but
+	this option would enable the extension unconditionally.
 
 \--::
 	Do not interpret any more arguments as options.
@@ -381,6 +386,33 @@ Although this bit looks similar to assume-unchanged bit, its goal is
 different from assume-unchanged bit's. Skip-worktree also takes
 precedence over assume-unchanged bit when both are set.
 
+Untracked cache
+---------------
+
+This cache could speed up commands that involve determining untracked
+files such as `git status`.
+
+This feature works by recording the mtime of the working tree
+directories and then omitting reading directories and stat calls
+against files in those directories whose mtime hasn't changed. For
+this to work the underlying operating system and file system must
+change the `st_mtime` field of directories if files in the directory
+are added, modified or deleted.
+
+You can test whether the filesystem supports that with the
+`--test-untracked-cache` option. The `--untracked-cache` option used
+to implicitly perform that test in older versions of Git, but that's
+no longer the case.
+
+It is recommended to use the `core.untrackedCache` configuration
+variable (see linkgit:git-config[1]) to enable or disable this feature
+instead of using the `--[no-|force-]untracked-cache` which are going
+to set or unset this configuration variable anyway.
+
+When the `core.untrackedCache` configuration variable is changed, the
+untracked cache is added or removed from the index the next time "git
+status" is run, while when `--[no-|force-]untracked-cache` are used,
+the untracked cache is immediately added to the index.
 
 Configuration
 -------------
@@ -406,6 +438,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]).
 
+The untracked cache extension is enabled by the `core.untrackedCache`
+configuration variable (see linkgit:git-config[1]).
 
 SEE ALSO
 --------
diff --git a/builtin/update-index.c b/builtin/update-index.c
index 4ca6d94..98e3bac 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1115,21 +1115,24 @@ 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 == UC_TEST) {
+		setup_work_tree();
+		return !test_if_untracked_cache_is_supported();
+	}
 	if (untracked_cache > UC_DISABLE) {
-		if (untracked_cache < UC_FORCE) {
-			setup_work_tree();
-			if (!test_if_untracked_cache_is_supported())
-				return 1;
-			if (untracked_cache == UC_TEST)
-				return 0;
-		}
+		if (!use_untracked_cache && git_config_set("core.untrackedCache", "true"))
+			die("could not set core.untrackedCache to true");
 		add_untracked_cache();
 		if (verbose)
 			printf(_("Untracked cache enabled\n"));
-	} else if (untracked_cache == UC_DISABLE && the_index.untracked) {
-		remove_untracked_cache();
-		if (verbose)
-			printf(_("Untracked cache disabled\n"));
+	} else if (untracked_cache == UC_DISABLE) {
+		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();
+			if (verbose)
+				printf(_("Untracked cache disabled\n"));
+		}
 	}
 
 	if (active_cache_changed) {
diff --git a/cache.h b/cache.h
index 2a9e902..0cc2c2f 100644
--- a/cache.h
+++ b/cache.h
@@ -619,6 +619,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..f023ee7 100644
--- a/config.c
+++ b/config.c
@@ -691,6 +691,10 @@ 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")) {
+		use_untracked_cache = git_config_bool(var, 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 111b053..b7e5736 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2054,6 +2054,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 3b83cc0..0b07ba7 100644
--- a/dir.c
+++ b/dir.c
@@ -2015,7 +2015,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..9ca71b1 100644
--- a/environment.c
+++ b/environment.c
@@ -14,6 +14,7 @@
 
 int trust_executable_bit = 1;
 int trust_ctime = 1;
+int use_untracked_cache;
 int check_stat = 1;
 int has_symlinks = 1;
 int minimum_abbrev = 4, default_abbrev = 7;
diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index 0e8d0d4..253160a 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -8,10 +8,8 @@ avoid_racy() {
 	sleep 1
 }
 
-# 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
 '
 
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.479.g8eb29d4

[PATCH v2 09/10] t7063: add tests for core.untrackedCache

[Christian Couder]

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 t/t7063-status-untracked-cache.sh | 48 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 46 insertions(+), 2 deletions(-)

diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh
index 253160a..f0af41c 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -18,6 +18,10 @@ if ! test_have_prereq UNTRACKED_CACHE; then
 	test_done
 fi
 
+test_expect_success 'core.untrackedCache is unset' '
+	test_must_fail git config --get core.untrackedCache
+'
+
 test_expect_success 'setup' '
 	git init worktree &&
 	cd worktree &&
@@ -28,6 +32,11 @@ test_expect_success 'setup' '
 	git update-index --untracked-cache
 '
 
+test_expect_success 'core.untrackedCache is true' '
+	UC=$(git config core.untrackedCache) &&
+	test "$UC" = "true"
+'
+
 test_expect_success 'untracked cache is empty' '
 	test-dump-untracked-cache >../actual &&
 	cat >../expect <<EOF &&
@@ -506,7 +515,7 @@ EOF
 
 test_expect_success 'verify untracked cache dump (sparse/subdirs)' '
 	test-dump-untracked-cache >../actual &&
-	cat >../expect <<EOF &&
+	cat >../expect-from-test-dump <<EOF &&
 info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0
 core.excludesfile 0000000000000000000000000000000000000000
 exclude_per_dir .gitignore
@@ -525,7 +534,7 @@ file
 /dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid
 two
 EOF
-	test_cmp ../expect ../actual
+	test_cmp ../expect-from-test-dump ../actual
 '
 
 test_expect_success 'test sparse status again with untracked cache and subdir' '
@@ -569,4 +578,39 @@ EOF
 	test_cmp ../status.expect ../status.actual
 '
 
+test_expect_success '--no-untracked-cache removes the cache' '
+	git update-index --no-untracked-cache &&
+	UC=$(git config core.untrackedCache) &&
+	test "$UC" = "false" &&
+	test-dump-untracked-cache >../actual &&
+	echo "no untracked cache" >../expect &&
+	test_cmp ../expect ../actual
+'
+
+test_expect_success 'git status does not change anything' '
+	git status &&
+	test-dump-untracked-cache >../actual &&
+	test_cmp ../expect ../actual &&
+	UC=$(git config core.untrackedCache) &&
+	test "$UC" = "false"
+'
+
+test_expect_success 'setting core.untrackedCache and using git status creates the cache' '
+	git config core.untrackedCache true &&
+	test-dump-untracked-cache >../actual &&
+	test_cmp ../expect ../actual &&
+	git status &&
+	test-dump-untracked-cache >../actual &&
+	test_cmp ../expect-from-test-dump ../actual
+'
+
+test_expect_success 'unsetting core.untrackedCache and using git status removes the cache' '
+	git config --unset core.untrackedCache &&
+	test-dump-untracked-cache >../actual &&
+	test_cmp ../expect-from-test-dump ../actual &&
+	git status &&
+	test-dump-untracked-cache >../actual &&
+	test_cmp ../expect ../actual
+'
+
 test_done
-- 
2.6.3.479.g8eb29d4

[PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Christian Couder]

A previous commit disabled the check to see if the untracked cache
ident field was matching the current environment. So this field is
now useless and we can remove some related code.

We don't remove the ident field from "struct untracked_cache" as
it would break compatibility with old indexes that already have an
untracked cache with this field.

Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
---
 dir.c | 38 +++++---------------------------------
 dir.h |  2 +-
 2 files changed, 6 insertions(+), 34 deletions(-)

diff --git a/dir.c b/dir.c
index 0b07ba7..94fba2a 100644
--- a/dir.c
+++ b/dir.c
@@ -1904,36 +1904,13 @@ static int treat_leading_path(struct dir_struct *dir,
 	return rc;
 }
 
-static const char *get_ident_string(void)
-{
-	static struct strbuf sb = STRBUF_INIT;
-	struct utsname uts;
-
-	if (sb.len)
-		return sb.buf;
-	if (uname(&uts) < 0)
-		die_errno(_("failed to get kernel name and information"));
-	strbuf_addf(&sb, "Location %s, system %s %s %s", get_git_work_tree(),
-		    uts.sysname, uts.release, uts.version);
-	return sb.buf;
-}
-
-static int ident_in_untracked(const struct untracked_cache *uc)
-{
-	const char *end = uc->ident.buf + uc->ident.len;
-	const char *p   = uc->ident.buf;
-
-	for (p = uc->ident.buf; p < end; p += strlen(p) + 1)
-		if (!strcmp(p, get_ident_string()))
-			return 1;
-	return 0;
-}
-
+/*
+ * We used to save the location of the work tree and the kernel version,
+ * but it was not a good idea, so we now just save an empty string.
+ */
 void add_untracked_ident(struct untracked_cache *uc)
 {
-	if (ident_in_untracked(uc))
-		return;
-	strbuf_addstr(&uc->ident, get_ident_string());
+	strbuf_addstr(&uc->ident, "");
 	/* this strbuf contains a list of strings, save NUL too */
 	strbuf_addch(&uc->ident, 0);
 }
@@ -2015,11 +1992,6 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d
 	if (dir->exclude_list_group[EXC_CMDL].nr)
 		return NULL;
 
-	if (use_untracked_cache != 1 && !ident_in_untracked(dir->untracked)) {
-		warning(_("Untracked cache is disabled on this system."));
-		return NULL;
-	}
-
 	if (!dir->untracked->root) {
 		const int len = sizeof(*dir->untracked->root);
 		dir->untracked->root = xmalloc(len);
diff --git a/dir.h b/dir.h
index 3e5114d..1935b76 100644
--- a/dir.h
+++ b/dir.h
@@ -127,7 +127,7 @@ struct untracked_cache {
 	struct sha1_stat ss_info_exclude;
 	struct sha1_stat ss_excludes_file;
 	const char *exclude_per_dir;
-	struct strbuf ident;
+	struct strbuf ident; /* unused now */
 	/*
 	 * dir_struct#flags must match dir_flags or the untracked
 	 * cache is ignored.
-- 
2.6.3.479.g8eb29d4

Re: [PATCH v2 07/10] dir: free untracked cache when removing it

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  dir.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/dir.c b/dir.c
> index ffc0286..3b83cc0 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1954,6 +1954,7 @@ void add_untracked_cache(void)
>  
>  void remove_untracked_cache(void)
>  {
> +	free_untracked_cache(the_index.untracked);
>  	the_index.untracked = NULL;
>  	the_index.cache_changed |= UNTRACKED_CHANGED;
>  }

Up to this point the series makes sense (again, I am not saying the
remainder does not ;-)).  But shouldn't this step, as a bugfix,
appear a lot earlier in the series?

Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> +/*
> + * We used to save the location of the work tree and the kernel version,
> + * but it was not a good idea, so we now just save an empty string.
> + */

I do agree that storing the kernel version (or hostname or whatever
specific to the machine) was not a good idea.  I however suspect
that you must save and check the location of the working tree,
though, for correctness.  If you use one GIT_DIR and GIT_WORK_TREE
to do "git add" or whatever, and then use the same GIT_DIR but a
different GIT_WORK_TREE, you should be able to notice that a
directory D in the old GIT_WORK_TREE whose modification time you
recorded is different from the directory D with the same name but in
the new GIT_WORK_TREE, no?

Re: [PATCH v2 01/10] update-index: use enum for untracked cache options

[Torsten Bögershausen]

On 15.12.15 17:28, Christian Couder wrote:
> Signed-off-by: Christian Couder <chriscool@tuxfamily.org>
> ---
>  builtin/update-index.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/builtin/update-index.c b/builtin/update-index.c
> index 7431938..2430a68 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -35,6 +35,14 @@ static int mark_skip_worktree_only;
>  #define UNMARK_FLAG 2
>  static struct strbuf mtime_dir = STRBUF_INIT;
>  
> +/* Untracked cache mode */
> +enum uc_mode {
> +	UC_UNSPECIFIED = -1,
> +	UC_DISABLE = 0,
> +	UC_ENABLE,
> +	UC_FORCE
> +};
> +
>  __attribute__((format (printf, 1, 2)))
>  static void report(const char *fmt, ...)
>  {
> @@ -902,7 +910,7 @@ static int reupdate_callback(struct parse_opt_ctx_t *ctx,
>  int cmd_update_index(int argc, const char **argv, const char *prefix)
>  {
>  	int newfd, entries, has_errors = 0, line_termination = '\n';
> -	int untracked_cache = -1;
> +	enum uc_mode untracked_cache = UC_UNSPECIFIED;
>  	int read_from_stdin = 0;
>  	int prefix_length = prefix ? strlen(prefix) : 0;
>  	int preferred_index_format = 0;
> @@ -997,7 +1005,7 @@ int cmd_update_index(int argc, const char **argv, const char *prefix)
>  		OPT_BOOL(0, "untracked-cache", &untracked_cache,
>  			N_("enable/disable untracked cache")),
>  		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"), UC_FORCE),
>  		OPT_END()
>  	};
>  
> @@ -1104,10 +1112,10 @@ 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 > 0) {
> +	if (untracked_cache > UC_DISABLE) {
I think the "correct way" for a comparison between an enum and an int is to cast the enum
into an int.
But, if we have an enum, these kind of comparision should go away, and replaced
with a switch-case (or if) statemant.
>  		struct untracked_cache *uc;
>  
> -		if (untracked_cache < 2) {
> +		if (untracked_cache < UC_FORCE) {

Same here

Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Christian Couder]

On Tue, Dec 15, 2015 at 8:49 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> +/*
>> + * We used to save the location of the work tree and the kernel version,
>> + * but it was not a good idea, so we now just save an empty string.
>> + */
>
> I do agree that storing the kernel version (or hostname or whatever
> specific to the machine) was not a good idea.  I however suspect
> that you must save and check the location of the working tree,
> though, for correctness.  If you use one GIT_DIR and GIT_WORK_TREE
> to do "git add" or whatever, and then use the same GIT_DIR but a
> different GIT_WORK_TREE, you should be able to notice that a
> directory D in the old GIT_WORK_TREE whose modification time you
> recorded is different from the directory D with the same name but in
> the new GIT_WORK_TREE, no?

Yeah, if people use many worktrees made using "git worktree", the code
above should be fine because there is one index per worktree, but if
they just use one GIT_DIR with many GIT_WORK_TREE then there will be
only one index used.

I am wondering about the case when the worktree is moved and then
GIT_WORK_TREE is changed to reflect the new path were the worktree has
been moved.

In the "git worktree" documentation there is:

"If you move a linked working tree to another file system, or within a
file system that does not support hard links, you need to run at least
one git command inside the linked working tree (e.g. git status) in
order to update its administrative files in the repository so that
they do not get automatically pruned."

It looks like git can detect when a worktree created with "git
worktree" has been moved and I wonder if it would be possible to
detect if the main worktree pointed to by GIT_WORK_TREE as moved.

Re: [PATCH v2 02/10] update-index: add --test-untracked-cache

[David Turner]

On Tue, 2015-12-15 at 17:28 +0100, Christian Couder wrote: 
> +--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.

It would be nice if this said how the result would be reported (by exit
code, it appears).

Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Junio C Hamano]

Christian Couder <christian.couder@gmail.com> writes:

> In the "git worktree" documentation there is:
>
> "If you move a linked working tree to another file system, or within a
> file system that does not support hard links, you need to run at least
> one git command inside the linked working tree (e.g. git status) in
> order to update its administrative files in the repository so that
> they do not get automatically pruned."
>
> It looks like git can detect when a worktree created with "git
> worktree" has been moved and I wonder if it would be possible to
> detect if the main worktree pointed to by GIT_WORK_TREE as moved.

As I personally do not find "moving a working tree" a very
compelling use case, I'd be fine if cached information is not used
when the actual worktree and the root of the cached untracked paths
are different.

If you are going to change the in-index data of untracked cache
anyway (like you attempted with 10/10 patch), I think a lot more
sensible simplification may be to make the mechanism _always_ keep
track of the worktree that is rooted one level above the index, and
not use the cache in all other cases.  That way, if you move the
working tree in its entirety (i.e. $foo/{Makefile,.git/,untracked}
all move to $bar/. at the same time), the untracked cache data that
was in $foo/.git/index, which knew about $foo/untracked, will now
know about $bar/untracked when the index is moved to $bar/.git/index
automatically.

Re: [PATCH v2 10/10] dir: do not use untracked cache ident anymore

[Christian Couder]

On Thu, Dec 17, 2015 at 7:33 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Christian Couder <christian.couder@gmail.com> writes:
>
>> In the "git worktree" documentation there is:
>>
>> "If you move a linked working tree to another file system, or within a
>> file system that does not support hard links, you need to run at least
>> one git command inside the linked working tree (e.g. git status) in
>> order to update its administrative files in the repository so that
>> they do not get automatically pruned."
>>
>> It looks like git can detect when a worktree created with "git
>> worktree" has been moved and I wonder if it would be possible to
>> detect if the main worktree pointed to by GIT_WORK_TREE as moved.
>
> As I personally do not find "moving a working tree" a very
> compelling use case, I'd be fine if cached information is not used
> when the actual worktree and the root of the cached untracked paths
> are different.

Yeah, I could just discard and recreate the UC from scratch if the
actual worktree and the root of the UC paths are different.

> If you are going to change the in-index data of untracked cache
> anyway (like you attempted with 10/10 patch), I think a lot more
> sensible simplification may be to make the mechanism _always_ keep
> track of the worktree that is rooted one level above the index, and
> not use the cache in all other cases.  That way, if you move the
> working tree in its entirety (i.e. $foo/{Makefile,.git/,untracked}
> all move to $bar/. at the same time), the untracked cache data that
> was in $foo/.git/index, which knew about $foo/untracked, will now
> know about $bar/untracked when the index is moved to $bar/.git/index
> automatically.

I am ok with that, though I worry a bit about some people having a
setup where they always use a worktree that is not one level above the
.git directory.