[PATCH 0/8] Untracked cache improvements

classic Classic list List threaded Threaded
54 messages Options
123
Reply | Threaded
Open this post in threaded view
|

[PATCH 0/8] Untracked cache improvements

Christian Couder-2
Following the previous RFC version of this patch series and the
related discussions, here is a new version that tries to improve the
untracked cache feature.

This patch series implements core.untrackedCache as a 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/8 and 3/8 add some features that are missing.

Patch 2/8, which is new, adds an enum as suggested by Duy.

Patchs 4/8, 5/8 and 6/8 are some refactoring to prepare for patch 7/8
which implements core.untrackedCache.

Patch 7/8 is the result of squashing the last 3 patches of the RFC
series. As discussed this sacrifies backward compatibility for a
simpler interface.

Patch 8/8, which is new, add some tests.

So the changes compared to the RFC version are just small bug fixes
and patchs 2/8 and 8/8.

The patch series is also available there:

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

Christian Couder (8):
  update-index: add untracked cache notifications
  update-index: use enum for untracked cache options
  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
  t7063: add tests for core.untrackedCache

 Documentation/config.txt               |  7 +++++
 Documentation/git-update-index.txt     | 31 ++++++++++++++------
 builtin/update-index.c                 | 52 +++++++++++++++++++---------------
 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      | 52 ++++++++++++++++++++++++++++++----
 wt-status.c                            |  9 ++++++
 11 files changed, 145 insertions(+), 37 deletions(-)

--
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Christian Couder-2
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 <[hidden email]>
---
 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..6f6b289 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 cache disabled\n"));
  }
 
  if (active_cache_changed) {
--
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 2/8] update-index: use enum for untracked cache options

Christian Couder-2
In reply to this post by Christian Couder-2
Signed-off-by: Christian Couder <[hidden email]>
---
 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 6f6b289..246b3d3 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 {
+ UNDEF_UC = -1,
+ NO_UC = 0,
+ UC,
+ FORCE_UC
+};
+
 __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 = UNDEF_UC;
  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"), FORCE_UC),
  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 > NO_UC) {
  struct untracked_cache *uc;
 
- if (untracked_cache < 2) {
+ if (untracked_cache < FORCE_UC) {
  setup_work_tree();
  if (!test_if_untracked_cache_is_supported())
  return 1;
@@ -1123,7 +1131,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;
  fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
- } else if (!untracked_cache && the_index.untracked) {
+ } else if (untracked_cache == NO_UC && the_index.untracked) {
  the_index.untracked = NULL;
  the_index.cache_changed |= UNTRACKED_CHANGED;
  fprintf(stderr, _("Untracked cache disabled\n"));
--
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Christian Couder-2
In reply to this post by Christian Couder-2
It is nice to just be able to test if untracked cache is
supported without enabling it.

Signed-off-by: Christian Couder <[hidden email]>
---
 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 246b3d3..ecb685d 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -40,6 +40,7 @@ enum uc_mode {
  UNDEF_UC = -1,
  NO_UC = 0,
  UC,
+ TEST_UC,
  FORCE_UC
 };
 
@@ -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"), TEST_UC),
  OPT_SET_INT(0, "force-untracked-cache", &untracked_cache,
     N_("enable untracked cache without testing the filesystem"), FORCE_UC),
  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 == TEST_UC)
+ return 0;
  }
  if (!the_index.untracked) {
  uc = xcalloc(1, sizeof(*uc));
--
2.6.3.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Christian Couder-2
In reply to this post by Christian Couder-2
Signed-off-by: Christian Couder <[hidden email]>
---
 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 ecb685d..21f74b2 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 > NO_UC) {
- struct untracked_cache *uc;
-
  if (untracked_cache < FORCE_UC) {
  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.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 5/8] dir: add add_untracked_cache()

Christian Couder-2
In reply to this post by Christian Couder-2
This new function will be used in a later patch.

Signed-off-by: Christian Couder <[hidden email]>
---
 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 21f74b2..40530b0 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 == TEST_UC)
  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 == NO_UC && 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.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 6/8] dir: add remove_untracked_cache()

Christian Couder-2
In reply to this post by Christian Couder-2
This new function will be used in a later patch.

Signed-off-by: Christian Couder <[hidden email]>
---
 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 40530b0..e427657 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1126,8 +1126,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 == NO_UC && the_index.untracked) {
- the_index.untracked = NULL;
- the_index.cache_changed |= UNTRACKED_CHANGED;
+ remove_untracked_cache();
  fprintf(stderr, _("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.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Christian Couder-2
In reply to this post by Christian Couder-2
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 preforming 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.

This means that `git update-index --[no-|force-]untracked-cache`,
to be compatible with the new config variable, have to set or
unset it. This new behavior is backward incompatible change, but
that is deliberate.

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.

So to be more consistent with other git commands, this patch
prevents `--untracked-cache` to perform tests. This means that
after this patch there is no difference any more between
`--untracked-cache` and `--force-untracked-cache`.

Signed-off-by: Christian Couder <[hidden email]>
---
 Documentation/config.txt               |  7 +++++++
 Documentation/git-update-index.txt     | 28 ++++++++++++++++++----------
 builtin/update-index.c                 | 23 +++++++++++++----------
 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, 56 insertions(+), 24 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..0fb39db 100644
--- a/Documentation/git-update-index.txt
+++ b/Documentation/git-update-index.txt
@@ -175,22 +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.
+ 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.
++
+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
  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`.
 
 \--::
  Do not interpret any more arguments as options.
@@ -406,6 +412,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 e427657..7fe3a86 100644
--- a/builtin/update-index.c
+++ b/builtin/update-index.c
@@ -1115,19 +1115,22 @@ 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 == TEST_UC) {
+ setup_work_tree();
+ return !test_if_untracked_cache_is_supported();
+ }
  if (untracked_cache > NO_UC) {
- if (untracked_cache < FORCE_UC) {
- setup_work_tree();
- if (!test_if_untracked_cache_is_supported())
- return 1;
- if (untracked_cache == TEST_UC)
- return 0;
- }
+ 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 == NO_UC && the_index.untracked) {
- remove_untracked_cache();
- fprintf(stderr, _("Untracked cache disabled\n"));
+ } else if (untracked_cache == NO_UC) {
+ 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 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 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..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.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH 8/8] t7063: add tests for core.untrackedCache

Christian Couder-2
In reply to this post by Christian Couder-2
Signed-off-by: Christian Couder <[hidden email]>
---
 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.478.g9f95483.dirty

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Christian Couder-2
Christian Couder <[hidden email]> writes:

> 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.

I think your "expectation" needs to be more explicitly spelled out.

"git -C /tmp --git-dir=/git/somewhere/else/.git" is a valid way to
use that repository you have in somewhere else to track things under
/tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the
cwd, i.e. /tmp, is the root level of the working tree), and for such
a usage, the above command works as expected.  Perhaps

    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.

may be an improvement.

The index already implicitly records where the working tree was and
that is not limited to untracked-cache option.  For example, if you
have your repository and its working tree in /git/somewhere/else,
which does not have a path X, then doing:

    cd /tmp && >tmp/X
    git --git-dir=/git/somewhere/else/.git update-index --add X

would store X taken from /tmp in the index, so subsequent use of the
index "knows" about X that was taken outside /git/somewhere/else/
after the above command finishes and the subsequent use is made
without the --git-dir parameter, e.g.

    cd /git/somewhere/else/ && git diff-index --cached HEAD'

would say that you added X, even though /git/somewhere/else/ may not
have that X at all.  And this is not limited to update-index,
either.  You can temporarily use --git-dir with "git add X" and the
result would persist the same way in the index.

I think the moral of the story is that you are not expected to
randomly use git-dir and git-work-tree to point at different places
without knowing what you are doing, and we may need a way to help
people understand what is going on when it is done by a mistake.

The patch to show result from xgetcwd() and get_git_work_tree() may
be a step in the right direction, but if the user is not doing
anything fancy, "Testing mtime in /long/path/to/the/directory" would
be irritatingly verbose.

I wonder if it is easy to tell when the user is doing such an
unnatural thing.  Off the top of my head, when the working tree is
anywhere other than one level above $GIT_DIR, the user is doing
something unusual; I do not know if there are cases where the user
is doing something unnatural if $GIT_WORK_TREE is one level above
$GIT_DIR, though.

> 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 <[hidden email]>
> ---
>  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..6f6b289 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 cache disabled\n"));
>   }
>  
>   if (active_cache_changed) {
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/8] update-index: use enum for untracked cache options

Junio C Hamano
In reply to this post by Christian Couder-2
Christian Couder <[hidden email]> writes:

> Signed-off-by: Christian Couder <[hidden email]>
> ---
>  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 6f6b289..246b3d3 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 {
> + UNDEF_UC = -1,
> + NO_UC = 0,
> + UC,
> + FORCE_UC
> +};
> +

With these, the code is much easier to read than with the mystery
constants, but did you consider making UC_ a common prefix for
further ease-of-reading?  E.g.

    UC_UNSPECIFIED
    UC_DONTUSE
    UC_USE
    UC_FORCE

or something?

>  __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 = UNDEF_UC;
>   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"), FORCE_UC),
>   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 > NO_UC) {
>   struct untracked_cache *uc;
>  
> - if (untracked_cache < 2) {
> + if (untracked_cache < FORCE_UC) {
>   setup_work_tree();
>   if (!test_if_untracked_cache_is_supported())
>   return 1;
> @@ -1123,7 +1131,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;
>   fprintf(stderr, _("Untracked cache enabled for '%s'\n"), get_git_work_tree());
> - } else if (!untracked_cache && the_index.untracked) {
> + } else if (untracked_cache == NO_UC && the_index.untracked) {
>   the_index.untracked = NULL;
>   the_index.cache_changed |= UNTRACKED_CHANGED;
>   fprintf(stderr, _("Untracked cache disabled\n"));
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/8] dir: add remove_untracked_cache()

Junio C Hamano
In reply to this post by Christian Couder-2
Christian Couder <[hidden email]> writes:

> This new function will be used in a later patch.
>
> Signed-off-by: Christian Couder <[hidden email]>
> ---

Up to this step I looked at and they made sense (I am not saying the
remainder of the series do not make sense).

I however wonder where the memory used for untracked cache goes when
this is called?

>  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 40530b0..e427657 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -1126,8 +1126,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 == NO_UC && the_index.untracked) {
> - the_index.untracked = NULL;
> - the_index.cache_changed |= UNTRACKED_CHANGED;
> + remove_untracked_cache();
>   fprintf(stderr, _("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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
In reply to this post by Christian Couder-2
Christian Couder <[hidden email]> writes:

> 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 preforming 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.
>
> This means that `git update-index --[no-|force-]untracked-cache`,
> to be compatible with the new config variable, have to set or
> unset it. This new behavior is backward incompatible change, but
> that is deliberate.

The logic in this paragraph is fuzzy to me.  Shouldn't the config
give a sensible default, that is overriden by command line options?
I agree that it is insane to do a runtime check when the user says
"update-index --untracked-cache" to enable it, as the user _knows_
that enabling it would help (or the user _knows_ that she wants to
play with it).  Similarly, shouldn't the config be ignored when the
user says "update-index --no-untracked-cache" (hence removing the
untracked cache from the resulting index no matter the config is set
to)?  Why is it a good idea to allow an explicit operation from the
command line to muck with the configuration?  If the user wants to
change the configuration permanently, "git config" has been there
for the purpose for all the configuration variables to do so for
almost forever.  Why is it a good idea to make this variable so
special that the user does not have to use "git config" but disable
or enable it in some other way?

> 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.
>
> So to be more consistent with other git commands, this patch
> prevents `--untracked-cache` to perform tests.

Good change.

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

A tip to write the explanation.  Every time you say "This means
that...", you are (perhaps unconsciously) admitting that what you
wrote immedidately before that may not be understandable and what
comes after it may be a better explanation.  Discard the sentence
before "This means that", and try to formulate your explanation
around what you wrote after it, adding information in the discarded
earlier sentence back to the later one.  Doing this exercise often
helps the result read much better.

>
> Signed-off-by: Christian Couder <[hidden email]>
> ---
>  Documentation/config.txt               |  7 +++++++
>  Documentation/git-update-index.txt     | 28 ++++++++++++++++++----------
>  builtin/update-index.c                 | 23 +++++++++++++----------
>  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, 56 insertions(+), 24 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..0fb39db 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -175,22 +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.
> + 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.
> ++
> +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
>   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`.
>  
>  \--::
>   Do not interpret any more arguments as options.
> @@ -406,6 +412,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 e427657..7fe3a86 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -1115,19 +1115,22 @@ 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 == TEST_UC) {
> + setup_work_tree();
> + return !test_if_untracked_cache_is_supported();
> + }
>   if (untracked_cache > NO_UC) {
> - if (untracked_cache < FORCE_UC) {
> - setup_work_tree();
> - if (!test_if_untracked_cache_is_supported())
> - return 1;
> - if (untracked_cache == TEST_UC)
> - return 0;
> - }
> + 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 == NO_UC && the_index.untracked) {
> - remove_untracked_cache();
> - fprintf(stderr, _("Untracked cache disabled\n"));
> + } else if (untracked_cache == NO_UC) {
> + 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 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 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..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);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Junio C Hamano
Junio C Hamano <[hidden email]> writes:

> Christian Couder <[hidden email]> writes:
>
>> 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 preforming tests (because it might work on
>> some systems using the repo over the network file system but not
>> others).
>> ...
> The logic in this paragraph is fuzzy to me.  Shouldn't the config
> give a sensible default, that is overriden by command line options?
> I agree that it is insane to do a runtime check when the user says
> "update-index --untracked-cache" to enable it, as the user _knows_
> that enabling it would help (or the user _knows_ that she wants to
> play with it).  Similarly, shouldn't the config be ignored when the
> user says "update-index --no-untracked-cache" (hence removing the
> untracked cache from the resulting index no matter the config is set
> to)?  ...

As I think about this more, it really seems to me that we shouldn't
need to make this configuration variable that special.  Because I
think it is a *BUG* in the current implementation to do the runtime
check even when the user explicitly tells us to use untracked-cache,
I'd imagine something along the lines of the following would be a
lot simpler, easier to understand and a generally more useful
bugfix:

 1 Add one boolean configuration variable, core.untrackedCache, that
   defaults to 'false'.

 2 When creating an index file in an empty repository, enable the
   untracked cache in the index (even without the user runninng
   "update-index --untracked-cache") iff the configuration is set to
   'true'.  No runtime check necessary.

 3 When working on an existing index file, unless the operation is
   "update-index --[no-]untracked-cache", keep the current setting,
   regardless of this configuration variable.  No runtime check
   necessary.

 4 "update-index --[no-]untracked-cache" should enable or disable
   the untracked cache as the user tells us, regardless of the
   configuration variable.  No runtime check necessary.

It is OK to then add an "auto-detect" on top of the above, that
would only affect the second bullet point, like so:

 2a When creating an index file in an empty repository, if the
    configuration is set to 'auto', do the lengthy runtime check and
    enable the untracked cache in the index (even without the user
    runninng "update-index --untracked-cache").

without changing any other in the first 4-bullet list.

Am I missing some other requirements?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 5/8] dir: add add_untracked_cache()

Torsten Bögershausen-2
In reply to this post by Christian Couder-2
On 08.12.15 18:15, Christian Couder wrote:
> This new function will be used in a later patch.
May be
Factor out code into add_untracked_cache(), which will be used in the next commit.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 6/8] dir: add remove_untracked_cache()

Torsten Bögershausen-2
In reply to this post by Christian Couder-2
On 08.12.15 18:15, Christian Couder wrote:
> This new function will be used in a later patch.
Why not combine 5/8 with 6/8 into a single patch ?

(And please consider to mark the series with v2)


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Torsten Bögershausen-2
In reply to this post by Christian Couder-2
On 08.12.15 18:15, 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.
>
For the people which didn't follow the discussion, or read this in
a year or 2:
A short description what "mtime is fully supported by the environment" means
would be nice:
When a file system object is added or deleted in a directory,
and stat(dirname, &st) returns an updated st.st_mtime.
> 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 preforming tests (because it might work on
                         performing
> 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.
If this is the case, can we remove some code?
e.g add_untracked_ident() in dir.c, do we still need it?
And probably some other functions as well.
Or would it be better to say
"false" is false,
"true" is true
"unset" is as before (Where when different machines/OS/mount points
  access the same repo over a network file system, some use the UT, some don't)

>
> When "git status" is run, it now adds or removes the untracked
> cache in the index to respect the value of this variable.
>
> This means that `git update-index --[no-|force-]untracked-cache`,
> to be compatible with the new config variable, have to set or
> unset it.
what does unset mean ? Set to false ?
 This new behavior is backward incompatible change, but
> that is deliberate.

>
> 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.
>
> So to be more consistent with other git commands, this patch
> prevents `--untracked-cache` to perform tests. This means that
> after this patch there is no difference any more between
> `--untracked-cache` and `--force-untracked-cache`.
>
> Signed-off-by: Christian Couder <[hidden email]>
> ---
>  Documentation/config.txt               |  7 +++++++
>  Documentation/git-update-index.txt     | 28 ++++++++++++++++++----------
>  builtin/update-index.c                 | 23 +++++++++++++----------
>  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, 56 insertions(+), 24 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.
set to what ? true ? false ?

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.
> +
isn't this what "git update-index --test-untracked-cached" is good for?

>  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..0fb39db 100644
> --- a/Documentation/git-update-index.txt
> +++ b/Documentation/git-update-index.txt
> @@ -175,22 +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. You can test that using
> +`--test-untracked-cache`.

`--untracked-cache` used to test that too
> +but it doesn't anymore.
Do we need this historical comment ?

> ++
> +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
>   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.
This seems confusing, at least to me.
Do you mean:
  --test-untracked-cache::
  Perform tests on the working directory and tells the user if
  untracked cache can be used.

>  
>  --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`.
>  
>  \--::
>   Do not interpret any more arguments as options.
> @@ -406,6 +412,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 e427657..7fe3a86 100644
> --- a/builtin/update-index.c
> +++ b/builtin/update-index.c
> @@ -1115,19 +1115,22 @@ 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 == TEST_UC) {
> + setup_work_tree();
> + return !test_if_untracked_cache_is_supported();
> + }
>   if (untracked_cache > NO_UC) {
> - if (untracked_cache < FORCE_UC) {
> - setup_work_tree();
> - if (!test_if_untracked_cache_is_supported())
> - return 1;
> - if (untracked_cache == TEST_UC)
> - return 0;
> - }
> + 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 == NO_UC && the_index.untracked) {
> - remove_untracked_cache();
> - fprintf(stderr, _("Untracked cache disabled\n"));
> + } else if (untracked_cache == NO_UC) {
> + 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 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 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..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;
> + }

If we say core.untrackedCache = unset is the same as false,
this code can be simplified:

> + if (!dir.untracked && use_untracked_cache) {
> + add_untracked_cache();
> + dir.untracked = the_index.untracked;
> + } else if (dir.untracked && !use_untracked_cache) {
> + remove_untracked_cache();
> + dir.untracked = NULL;
> + }



--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/8] update-index: use enum for untracked cache options

Christian Couder-2
In reply to this post by Junio C Hamano
On Tue, Dec 8, 2015 at 8:11 PM, Junio C Hamano <[hidden email]> wrote:

> Christian Couder <[hidden email]> writes:
>
>> Signed-off-by: Christian Couder <[hidden email]>
>> ---
>>  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 6f6b289..246b3d3 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 {
>> +     UNDEF_UC = -1,
>> +     NO_UC = 0,
>> +     UC,
>> +     FORCE_UC
>> +};
>> +
>
> With these, the code is much easier to read than with the mystery
> constants, but did you consider making UC_ a common prefix for
> further ease-of-reading?  E.g.
>
>     UC_UNSPECIFIED
>     UC_DONTUSE
>     UC_USE
>     UC_FORCE
>
> or something?

Ok, I will use what you suggest.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 2/8] update-index: use enum for untracked cache options

Junio C Hamano
Christian Couder <[hidden email]> writes:

>>> +/* Untracked cache mode */
>>> +enum uc_mode {
>>> +     UNDEF_UC = -1,
>>> +     NO_UC = 0,
>>> +     UC,
>>> +     FORCE_UC
>>> +};
>>> +
>>
>> With these, the code is much easier to read than with the mystery
>> constants, but did you consider making UC_ a common prefix for
>> further ease-of-reading?  E.g.
>>
>>     UC_UNSPECIFIED
>>     UC_DONTUSE
>>     UC_USE
>>     UC_FORCE
>>
>> or something?
>
> Ok, I will use what you suggest.

As you know I am bad at bikeshedding; the only suggestion in the
above is to have common UC_ prefix ;-)  Don't take what follow UC_
as my suggestion.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

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

Christian Couder-2
In reply to this post by Junio C Hamano
On Tue, Dec 8, 2015 at 8:03 PM, Junio C Hamano <[hidden email]> wrote:

> Christian Couder <[hidden email]> writes:
>
>> 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.
>
> I think your "expectation" needs to be more explicitly spelled out.
>
> "git -C /tmp --git-dir=/git/somewhere/else/.git" is a valid way to
> use that repository you have in somewhere else to track things under
> /tmp/ (as you are only passing GIT_DIR but not GIT_WORK_TREE, the
> cwd, i.e. /tmp, is the root level of the working tree), and for such
> a usage, the above command works as expected.  Perhaps
>
>     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.
>
> may be an improvement.

Yeah, I agree that it is better. I have included your explanations in
the next version I will send. Thanks.

> The index already implicitly records where the working tree was and
> that is not limited to untracked-cache option.  For example, if you
> have your repository and its working tree in /git/somewhere/else,
> which does not have a path X, then doing:
>
>     cd /tmp && >tmp/X
>     git --git-dir=/git/somewhere/else/.git update-index --add X
>
> would store X taken from /tmp in the index, so subsequent use of the
> index "knows" about X that was taken outside /git/somewhere/else/
> after the above command finishes and the subsequent use is made
> without the --git-dir parameter, e.g.
>
>     cd /git/somewhere/else/ && git diff-index --cached HEAD'
>
> would say that you added X, even though /git/somewhere/else/ may not
> have that X at all.  And this is not limited to update-index,
> either.  You can temporarily use --git-dir with "git add X" and the
> result would persist the same way in the index.
>
> I think the moral of the story is that you are not expected to
> randomly use git-dir and git-work-tree to point at different places
> without knowing what you are doing, and we may need a way to help
> people understand what is going on when it is done by a mistake.

Yeah, I agree, and I think displaying more information might be a good way.

> The patch to show result from xgetcwd() and get_git_work_tree() may
> be a step in the right direction, but if the user is not doing
> anything fancy, "Testing mtime in /long/path/to/the/directory" would
> be irritatingly verbose.

Yeah, but after this series only "--test-untracked-cache" does any
testing, and the 10 seconds time it takes are probably more irritating
than its output.

> I wonder if it is easy to tell when the user is doing such an
> unnatural thing.  Off the top of my head, when the working tree is
> anywhere other than one level above $GIT_DIR, the user is doing
> something unusual; I do not know if there are cases where the user
> is doing something unnatural if $GIT_WORK_TREE is one level above
> $GIT_DIR, though.

Yeah, it could only print a warning in case something unusual is done.
I am not sure it is worth it though.

Thanks,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
123