From 7687252f3f7def11a862a0fd118c531ee630bd05 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 31 Jul 2015 13:35:01 -0400 Subject: [PATCH 1/4] untracked-cache: support sparse checkout Remove a check that would disable the untracked cache for sparse checkouts. Add tests that ensure that the untracked cache works with sparse checkouts -- specifically considering the case that a file foo/bar is checked out, but foo/.gitignore is not. Signed-off-by: David Turner Signed-off-by: Junio C Hamano --- dir.c | 17 +---- t/t7063-status-untracked-cache.sh | 119 ++++++++++++++++++++++++++++++ 2 files changed, 122 insertions(+), 14 deletions(-) diff --git a/dir.c b/dir.c index 8209f8b8af..e7b89fe6fe 100644 --- a/dir.c +++ b/dir.c @@ -1078,10 +1078,9 @@ static void prep_exclude(struct dir_struct *dir, const char *base, int baselen) (!untracked || !untracked->valid || /* * .. and .gitignore does not exist before - * (i.e. null exclude_sha1 and skip_worktree is - * not set). Then we can skip loading .gitignore, - * which would result in ENOENT anyway. - * skip_worktree is taken care in read_directory() + * (i.e. null exclude_sha1). Then we can skip + * loading .gitignore, which would result in + * ENOENT anyway. */ !is_null_sha1(untracked->exclude_sha1))) { /* @@ -1880,7 +1879,6 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d const struct pathspec *pathspec) { struct untracked_cache_dir *root; - int i; if (!dir->untracked || getenv("GIT_DISABLE_UNTRACKED_CACHE")) return NULL; @@ -1932,15 +1930,6 @@ static struct untracked_cache_dir *validate_untracked_cache(struct dir_struct *d if (dir->exclude_list_group[EXC_CMDL].nr) return NULL; - /* - * An optimization in prep_exclude() does not play well with - * CE_SKIP_WORKTREE. It's a rare case anyway, if a single - * entry has that bit set, disable the whole untracked cache. - */ - for (i = 0; i < active_nr; i++) - if (ce_skip_worktree(active_cache[i])) - return NULL; - if (!ident_in_untracked(dir->untracked)) { warning(_("Untracked cache is disabled on this system.")); return NULL; diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index bd4806c12a..ff23f4e94e 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -354,4 +354,123 @@ EOF test_cmp ../expect ../actual ' +test_expect_success 'set up for sparse checkout testing' ' + echo two >done/.gitignore && + echo three >>done/.gitignore && + echo two >done/two && + git add -f done/two done/.gitignore && + git commit -m "first commit" +' + +test_expect_success 'status after commit' ' + : >../trace && + GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + git status --porcelain >../actual && + cat >../status.expect <../trace.expect <../actual && + cat >../expect <.git/info/sparse-checkout && + test_config core.sparsecheckout true && + git checkout master && + git update-index --untracked-cache && + git status --porcelain >/dev/null && # prime the cache + test_path_is_missing done/.gitignore && + test_path_is_file done/one +' + +test_expect_success 'create files, some of which are gitignored' ' + echo three >done/three && # three is gitignored + echo four >done/four && # four is gitignored at a higher level + echo five >done/five # five is not gitignored +' + +test_expect_success 'test sparse status with untracked cache' ' + : >../trace && + avoid_racy && + GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + git status --porcelain >../status.actual && + cat >../status.expect <../trace.expect <../actual && + cat >../expect <../trace && + GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + git status --porcelain >../status.actual && + cat >../status.expect <../trace.expect < Date: Wed, 19 Aug 2015 20:01:24 +0700 Subject: [PATCH 2/4] t7063: use --force-untracked-cache to speed up a bit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When in the middle of t7063, we are sure untracked cache is supported, so we can use --force-untracked-cache to skip the support detection phase and save a few seconds. It's also good that --force-untracked-cache is exercised in the test suite. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- t/t7063-status-untracked-cache.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index ff23f4e94e..744e922699 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -402,7 +402,7 @@ test_expect_success 'set up sparse checkout' ' echo "done/[a-z]*" >.git/info/sparse-checkout && test_config core.sparsecheckout true && git checkout master && - git update-index --untracked-cache && + git update-index --force-untracked-cache && git status --porcelain >/dev/null && # prime the cache test_path_is_missing done/.gitignore && test_path_is_file done/one From 2e5910f27695b1b24e8a6870940bfac2a03ef0d7 Mon Sep 17 00:00:00 2001 From: David Turner Date: Wed, 19 Aug 2015 20:01:25 +0700 Subject: [PATCH 3/4] untracked-cache: fix subdirectory handling MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, some calls lookup_untracked would pass a full path. But lookup_untracked assumes that the portion of the path up to and including to the untracked_cache_dir has been removed. So lookup_untracked would be looking in the untracked_cache for 'foo' for 'foo/bar' (instead of just looking for 'bar'). This would cause untracked cache corruption. Instead, treat_directory learns to track the base length of the parent directory, so that only the last path component is passed to lookup_untracked. Helped-by: Nguyễn Thái Ngọc Duy Signed-off-by: David Turner Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- dir.c | 14 +++--- t/t7063-status-untracked-cache.sh | 72 ++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 7 deletions(-) diff --git a/dir.c b/dir.c index e7b89fe6fe..cd4ac779a6 100644 --- a/dir.c +++ b/dir.c @@ -1297,7 +1297,7 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len) */ static enum path_treatment treat_directory(struct dir_struct *dir, struct untracked_cache_dir *untracked, - const char *dirname, int len, int exclude, + const char *dirname, int len, int baselen, int exclude, const struct path_simplify *simplify) { /* The "len-1" is to strip the final '/' */ @@ -1324,7 +1324,8 @@ static enum path_treatment treat_directory(struct dir_struct *dir, if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES)) return exclude ? path_excluded : path_untracked; - untracked = lookup_untracked(dir->untracked, untracked, dirname, len); + untracked = lookup_untracked(dir->untracked, untracked, + dirname + baselen, len - baselen); return read_directory_recursive(dir, dirname, len, untracked, 1, simplify); } @@ -1444,6 +1445,7 @@ static int get_dtype(struct dirent *de, const char *path, int len) static enum path_treatment treat_one_path(struct dir_struct *dir, struct untracked_cache_dir *untracked, struct strbuf *path, + int baselen, const struct path_simplify *simplify, int dtype, struct dirent *de) { @@ -1495,8 +1497,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir, return path_none; case DT_DIR: strbuf_addch(path, '/'); - return treat_directory(dir, untracked, path->buf, path->len, exclude, - simplify); + return treat_directory(dir, untracked, path->buf, path->len, + baselen, exclude, simplify); case DT_REG: case DT_LNK: return exclude ? path_excluded : path_untracked; @@ -1557,7 +1559,7 @@ static enum path_treatment treat_path(struct dir_struct *dir, return path_none; dtype = DTYPE(de); - return treat_one_path(dir, untracked, path, simplify, dtype, de); + return treat_one_path(dir, untracked, path, baselen, simplify, dtype, de); } static void add_untracked(struct untracked_cache_dir *dir, const char *name) @@ -1827,7 +1829,7 @@ static int treat_leading_path(struct dir_struct *dir, break; if (simplify_away(sb.buf, sb.len, simplify)) break; - if (treat_one_path(dir, NULL, &sb, simplify, + if (treat_one_path(dir, NULL, &sb, baselen, simplify, DT_DIR, NULL) == path_none) break; /* do not recurse into it */ if (len <= baselen) { diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 744e922699..22393b9511 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -408,7 +408,8 @@ test_expect_success 'set up sparse checkout' ' test_path_is_file done/one ' -test_expect_success 'create files, some of which are gitignored' ' +test_expect_success 'create/modify files, some of which are gitignored' ' + echo two bis >done/two && echo three >done/three && # three is gitignored echo four >done/four && # four is gitignored at a higher level echo five >done/five # five is not gitignored @@ -420,6 +421,7 @@ test_expect_success 'test sparse status with untracked cache' ' GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ git status --porcelain >../status.actual && cat >../status.expect <../status.actual && cat >../status.expect < done/sub/sub/file +' + +test_expect_success 'test sparse status with untracked cache and subdir' ' + avoid_racy && + : >../trace && + GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + git status --porcelain >../status.actual && + cat >../status.expect <../trace.expect <../actual && + cat >../expect <../trace && + GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \ + git status --porcelain >../status.actual && + test_cmp ../status.expect ../status.actual && + cat >../trace.expect < Date: Wed, 19 Aug 2015 20:01:26 +0700 Subject: [PATCH 4/4] untracked cache: fix entry invalidation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First, the current code in untracked_cache_invalidate_path() is wrong because it can only handle paths "a" or "a/b", not "a/b/c" because lookup_untracked() only looks for entries directly under the given directory. In the last case, it will look for the entry "b/c" in directory "a" instead. This means if you delete or add an entry in a subdirectory, untracked cache may become out of date because it does not invalidate properly. This is noticed by David Turner. The second problem is about invalidation inside a fully untracked/excluded directory. In this case we may have to invalidate back to root. See the comment block for detail. Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- dir.c | 68 +++++++++++++++++++++++++------ t/t7063-status-untracked-cache.sh | 28 ++++++++++++- 2 files changed, 83 insertions(+), 13 deletions(-) diff --git a/dir.c b/dir.c index cd4ac779a6..c1edabf34d 100644 --- a/dir.c +++ b/dir.c @@ -2616,23 +2616,67 @@ done2: return uc; } +static void invalidate_one_directory(struct untracked_cache *uc, + struct untracked_cache_dir *ucd) +{ + uc->dir_invalidated++; + ucd->valid = 0; + ucd->untracked_nr = 0; +} + +/* + * Normally when an entry is added or removed from a directory, + * invalidating that directory is enough. No need to touch its + * ancestors. When a directory is shown as "foo/bar/" in git-status + * however, deleting or adding an entry may have cascading effect. + * + * Say the "foo/bar/file" has become untracked, we need to tell the + * untracked_cache_dir of "foo" that "bar/" is not an untracked + * directory any more (because "bar" is managed by foo as an untracked + * "file"). + * + * Similarly, if "foo/bar/file" moves from untracked to tracked and it + * was the last untracked entry in the entire "foo", we should show + * "foo/" instead. Which means we have to invalidate past "bar" up to + * "foo". + * + * This function traverses all directories from root to leaf. If there + * is a chance of one of the above cases happening, we invalidate back + * to root. Otherwise we just invalidate the leaf. There may be a more + * sophisticated way than checking for SHOW_OTHER_DIRECTORIES to + * detect these cases and avoid unnecessary invalidation, for example, + * checking for the untracked entry named "bar/" in "foo", but for now + * stick to something safe and simple. + */ +static int invalidate_one_component(struct untracked_cache *uc, + struct untracked_cache_dir *dir, + const char *path, int len) +{ + const char *rest = strchr(path, '/'); + + if (rest) { + int component_len = rest - path; + struct untracked_cache_dir *d = + lookup_untracked(uc, dir, path, component_len); + int ret = + invalidate_one_component(uc, d, rest + 1, + len - (component_len + 1)); + if (ret) + invalidate_one_directory(uc, dir); + return ret; + } + + invalidate_one_directory(uc, dir); + return uc->dir_flags & DIR_SHOW_OTHER_DIRECTORIES; +} + void untracked_cache_invalidate_path(struct index_state *istate, const char *path) { - const char *sep; - struct untracked_cache_dir *d; if (!istate->untracked || !istate->untracked->root) return; - sep = strrchr(path, '/'); - if (sep) - d = lookup_untracked(istate->untracked, - istate->untracked->root, - path, sep - path); - else - d = istate->untracked->root; - istate->untracked->dir_invalidated++; - d->valid = 0; - d->untracked_nr = 0; + invalidate_one_component(istate->untracked, istate->untracked->root, + path, strlen(path)); } void untracked_cache_remove_from_index(struct index_state *istate, diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh index 22393b9511..37a24c1312 100755 --- a/t/t7063-status-untracked-cache.sh +++ b/t/t7063-status-untracked-cache.sh @@ -375,7 +375,7 @@ EOF node creation: 0 gitignore invalidation: 0 directory invalidation: 0 -opendir: 1 +opendir: 2 EOF test_cmp ../trace.expect ../trace ' @@ -543,4 +543,30 @@ EOF test_cmp ../trace.expect ../trace ' +test_expect_success 'move entry in subdir from untracked to cached' ' + git add dtwo/two && + git status --porcelain >../status.actual && + cat >../status.expect <../status.actual && + cat >../status.expect <