untracked cache: fix entry invalidation

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 <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Nguyễn Thái Ngọc Duy 2015-08-19 20:01:26 +07:00 committed by Junio C Hamano
parent 2e5910f276
commit 73f9145fbf
2 changed files with 83 additions and 13 deletions

68
dir.c
View File

@ -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,

View File

@ -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 <<EOF &&
M done/two
A dtwo/two
?? .gitignore
?? done/five
?? done/sub/
EOF
test_cmp ../status.expect ../status.actual
'
test_expect_success 'move entry in subdir from cached to untracked' '
git rm --cached dtwo/two &&
git status --porcelain >../status.actual &&
cat >../status.expect <<EOF &&
M done/two
?? .gitignore
?? done/five
?? done/sub/
?? dtwo/
EOF
test_cmp ../status.expect ../status.actual
'
test_done