* [PATCH v3] untracked-cache: fix subdirectory handling
@ 2015-08-16 5:17 David Turner
2015-08-16 12:16 ` Duy Nguyen
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
0 siblings, 2 replies; 10+ messages in thread
From: David Turner @ 2015-08-16 5:17 UTC (permalink / raw)
To: git; +Cc: David Turner, Nguyễn Thái Ngọc Duy
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 <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
This version incorporates Duy's version of the dir.c code, and his
suggested test speedup.
---
dir.c | 14 ++++----
t/t7063-status-untracked-cache.sh | 74 +++++++++++++++++++++++++++++++++++++--
2 files changed, 80 insertions(+), 8 deletions(-)
diff --git a/dir.c b/dir.c
index e7b89fe..cd4ac77 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 ff23f4e..6716f69 100755
--- a/t/t7063-status-untracked-cache.sh
+++ b/t/t7063-status-untracked-cache.sh
@@ -402,13 +402,14 @@ 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 --untracked-cache --force-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' '
+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 <<EOF &&
+ M done/two
?? .gitignore
?? done/five
?? dtwo/
@@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with untracked cache' '
GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
git status --porcelain >../status.actual &&
cat >../status.expect <<EOF &&
+ M done/two
?? .gitignore
?? done/five
?? dtwo/
@@ -473,4 +476,71 @@ EOF
test_cmp ../trace.expect ../trace
'
+test_expect_success 'set up for test of subdir and sparse checkouts' '
+ mkdir done/sub &&
+ mkdir done/sub/sub &&
+ echo "sub" > 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 <<EOF &&
+ M done/two
+?? .gitignore
+?? done/five
+?? done/sub/
+?? dtwo/
+EOF
+ test_cmp ../status.expect ../status.actual &&
+ cat >../trace.expect <<EOF &&
+node creation: 2
+gitignore invalidation: 0
+directory invalidation: 1
+opendir: 3
+EOF
+ test_cmp ../trace.expect ../trace
+'
+
+test_expect_success 'verify untracked cache dump (sparse/subdirs)' '
+ test-dump-untracked-cache >../actual &&
+ cat >../expect <<EOF &&
+info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0
+core.excludesfile 0000000000000000000000000000000000000000
+exclude_per_dir .gitignore
+flags 00000006
+/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid
+.gitignore
+dtwo/
+/done/ 1946f0437f90c5005533cbe1736a6451ca301714 recurse valid
+five
+sub/
+/done/sub/ 0000000000000000000000000000000000000000 recurse check_only valid
+sub/
+/done/sub/sub/ 0000000000000000000000000000000000000000 recurse check_only valid
+file
+/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid
+/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid
+two
+EOF
+ test_cmp ../expect ../actual
+'
+
+test_expect_success 'test sparse status again with untracked cache and subdir' '
+ avoid_racy &&
+ : >../trace &&
+ GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
+ git status --porcelain >../status.actual &&
+ test_cmp ../status.expect ../status.actual &&
+ cat >../trace.expect <<EOF &&
+node creation: 0
+gitignore invalidation: 0
+directory invalidation: 0
+opendir: 0
+EOF
+ test_cmp ../trace.expect ../trace
+'
+
test_done
--
2.4.2.622.gac67c30-twtrsrc
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3] untracked-cache: fix subdirectory handling
2015-08-16 5:17 [PATCH v3] untracked-cache: fix subdirectory handling David Turner
@ 2015-08-16 12:16 ` Duy Nguyen
2015-08-19 11:39 ` Duy Nguyen
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
1 sibling, 1 reply; 10+ messages in thread
From: Duy Nguyen @ 2015-08-16 12:16 UTC (permalink / raw)
To: David Turner; +Cc: Git Mailing List
On Sun, Aug 16, 2015 at 12:17 PM, David Turner <dturner@twopensource.com> wrote:
> 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.
Your v2 also fixes untracked_cache_invalidate_path(), which is not
included here. Maybe it's in another patch?
> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> Signed-off-by: David Turner <dturner@twopensource.com>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>
> This version incorporates Duy's version of the dir.c code, and his
> suggested test speedup.
>
> ---
> dir.c | 14 ++++----
> t/t7063-status-untracked-cache.sh | 74 +++++++++++++++++++++++++++++++++++++--
> 2 files changed, 80 insertions(+), 8 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index e7b89fe..cd4ac77 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 ff23f4e..6716f69 100755
> --- a/t/t7063-status-untracked-cache.sh
> +++ b/t/t7063-status-untracked-cache.sh
> @@ -402,13 +402,14 @@ 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 --untracked-cache --force-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' '
> +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 <<EOF &&
> + M done/two
> ?? .gitignore
> ?? done/five
> ?? dtwo/
> @@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with untracked cache' '
> GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
> git status --porcelain >../status.actual &&
> cat >../status.expect <<EOF &&
> + M done/two
> ?? .gitignore
> ?? done/five
> ?? dtwo/
> @@ -473,4 +476,71 @@ EOF
> test_cmp ../trace.expect ../trace
> '
>
> +test_expect_success 'set up for test of subdir and sparse checkouts' '
> + mkdir done/sub &&
> + mkdir done/sub/sub &&
> + echo "sub" > 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 <<EOF &&
> + M done/two
> +?? .gitignore
> +?? done/five
> +?? done/sub/
> +?? dtwo/
> +EOF
> + test_cmp ../status.expect ../status.actual &&
> + cat >../trace.expect <<EOF &&
> +node creation: 2
> +gitignore invalidation: 0
> +directory invalidation: 1
> +opendir: 3
> +EOF
> + test_cmp ../trace.expect ../trace
> +'
> +
> +test_expect_success 'verify untracked cache dump (sparse/subdirs)' '
> + test-dump-untracked-cache >../actual &&
> + cat >../expect <<EOF &&
> +info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0
> +core.excludesfile 0000000000000000000000000000000000000000
> +exclude_per_dir .gitignore
> +flags 00000006
> +/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid
> +.gitignore
> +dtwo/
> +/done/ 1946f0437f90c5005533cbe1736a6451ca301714 recurse valid
> +five
> +sub/
> +/done/sub/ 0000000000000000000000000000000000000000 recurse check_only valid
> +sub/
> +/done/sub/sub/ 0000000000000000000000000000000000000000 recurse check_only valid
> +file
> +/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid
> +/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid
> +two
> +EOF
> + test_cmp ../expect ../actual
> +'
> +
> +test_expect_success 'test sparse status again with untracked cache and subdir' '
> + avoid_racy &&
> + : >../trace &&
> + GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
> + git status --porcelain >../status.actual &&
> + test_cmp ../status.expect ../status.actual &&
> + cat >../trace.expect <<EOF &&
> +node creation: 0
> +gitignore invalidation: 0
> +directory invalidation: 0
> +opendir: 0
> +EOF
> + test_cmp ../trace.expect ../trace
> +'
> +
> test_done
> --
> 2.4.2.622.gac67c30-twtrsrc
>
--
Duy
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3] untracked-cache: fix subdirectory handling
2015-08-16 12:16 ` Duy Nguyen
@ 2015-08-19 11:39 ` Duy Nguyen
0 siblings, 0 replies; 10+ messages in thread
From: Duy Nguyen @ 2015-08-19 11:39 UTC (permalink / raw)
To: David Turner; +Cc: Git Mailing List
On Sun, Aug 16, 2015 at 7:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Aug 16, 2015 at 12:17 PM, David Turner <dturner@twopensource.com> wrote:
>> 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.
>
> Your v2 also fixes untracked_cache_invalidate_path(), which is not
> included here. Maybe it's in another patch?
No I was wrong. Your changes and the original code are effectively the
same (I misread strrchr as strchr). But I think there's a bug
somewhere as I'm writing tests to understand that code..
--
Duy
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 0/3] dt/untracked-subdir
2015-08-16 5:17 [PATCH v3] untracked-cache: fix subdirectory handling David Turner
2015-08-16 12:16 ` Duy Nguyen
@ 2015-08-19 13:01 ` Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 1/3] t7063: use --force-untracked-cache to speed up a bit Nguyễn Thái Ngọc Duy
` (3 more replies)
1 sibling, 4 replies; 10+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-08-19 13:01 UTC (permalink / raw)
To: git; +Cc: David Turner, Nguyễn Thái Ngọc Duy
The first patch is a split from David's v3 patch because it should
belong to dt/untracked-sparse. The second is basically David's v3.
The third patch fixes untracked_cache_invalidate_path(). David fixed
it in v2, but there's another bug in this code.
David Turner (1):
untracked-cache: fix subdirectory handling
Nguyễn Thái Ngọc Duy (2):
t7063: use --force-untracked-cache to speed up a bit
untracked cache: fix entry invalidation
dir.c | 82 +++++++++++++++++++++++-------
t/t7063-status-untracked-cache.sh | 102 ++++++++++++++++++++++++++++++++++++--
2 files changed, 163 insertions(+), 21 deletions(-)
--
2.3.0.rc1.137.g477eb31
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/3] t7063: use --force-untracked-cache to speed up a bit
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
@ 2015-08-19 13:01 ` Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 2/3] untracked-cache: fix subdirectory handling Nguyễn Thái Ngọc Duy
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-08-19 13:01 UTC (permalink / raw)
To: git; +Cc: David Turner, Nguyễn Thái Ngọc Duy
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 <pclouds@gmail.com>
---
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 ff23f4e..744e922 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
--
2.3.0.rc1.137.g477eb31
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] untracked-cache: fix subdirectory handling
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 1/3] t7063: use --force-untracked-cache to speed up a bit Nguyễn Thái Ngọc Duy
@ 2015-08-19 13:01 ` Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 3/3] untracked cache: fix entry invalidation Nguyễn Thái Ngọc Duy
2015-08-19 17:47 ` [PATCH v4 0/3] dt/untracked-subdir Junio C Hamano
3 siblings, 0 replies; 10+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-08-19 13:01 UTC (permalink / raw)
To: git; +Cc: David Turner, Nguyễn Thái Ngọc Duy
From: David Turner <dturner@twopensource.com>
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 <pclouds@gmail.com>
Signed-off-by: David Turner <dturner@twopensource.com>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
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 e7b89fe..cd4ac77 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 744e922..22393b9 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 <<EOF &&
+ M done/two
?? .gitignore
?? done/five
?? dtwo/
@@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with untracked cache' '
GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
git status --porcelain >../status.actual &&
cat >../status.expect <<EOF &&
+ M done/two
?? .gitignore
?? done/five
?? dtwo/
@@ -473,4 +476,71 @@ EOF
test_cmp ../trace.expect ../trace
'
+test_expect_success 'set up for test of subdir and sparse checkouts' '
+ mkdir done/sub &&
+ mkdir done/sub/sub &&
+ echo "sub" > 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 <<EOF &&
+ M done/two
+?? .gitignore
+?? done/five
+?? done/sub/
+?? dtwo/
+EOF
+ test_cmp ../status.expect ../status.actual &&
+ cat >../trace.expect <<EOF &&
+node creation: 2
+gitignore invalidation: 0
+directory invalidation: 1
+opendir: 3
+EOF
+ test_cmp ../trace.expect ../trace
+'
+
+test_expect_success 'verify untracked cache dump (sparse/subdirs)' '
+ test-dump-untracked-cache >../actual &&
+ cat >../expect <<EOF &&
+info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0
+core.excludesfile 0000000000000000000000000000000000000000
+exclude_per_dir .gitignore
+flags 00000006
+/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid
+.gitignore
+dtwo/
+/done/ 1946f0437f90c5005533cbe1736a6451ca301714 recurse valid
+five
+sub/
+/done/sub/ 0000000000000000000000000000000000000000 recurse check_only valid
+sub/
+/done/sub/sub/ 0000000000000000000000000000000000000000 recurse check_only valid
+file
+/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid
+/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid
+two
+EOF
+ test_cmp ../expect ../actual
+'
+
+test_expect_success 'test sparse status again with untracked cache and subdir' '
+ avoid_racy &&
+ : >../trace &&
+ GIT_TRACE_UNTRACKED_STATS="$TRASH_DIRECTORY/trace" \
+ git status --porcelain >../status.actual &&
+ test_cmp ../status.expect ../status.actual &&
+ cat >../trace.expect <<EOF &&
+node creation: 0
+gitignore invalidation: 0
+directory invalidation: 0
+opendir: 0
+EOF
+ test_cmp ../trace.expect ../trace
+'
+
test_done
--
2.3.0.rc1.137.g477eb31
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] untracked cache: fix entry invalidation
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 1/3] t7063: use --force-untracked-cache to speed up a bit Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 2/3] untracked-cache: fix subdirectory handling Nguyễn Thái Ngọc Duy
@ 2015-08-19 13:01 ` Nguyễn Thái Ngọc Duy
2015-08-25 19:25 ` David Turner
2015-08-19 17:47 ` [PATCH v4 0/3] dt/untracked-subdir Junio C Hamano
3 siblings, 1 reply; 10+ messages in thread
From: Nguyễn Thái Ngọc Duy @ 2015-08-19 13:01 UTC (permalink / raw)
To: git; +Cc: David Turner, Nguyễn Thái Ngọc Duy
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>
---
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 cd4ac77..c1edabf 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 22393b9..37a24c1 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 <<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
--
2.3.0.rc1.137.g477eb31
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v4 0/3] dt/untracked-subdir
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
` (2 preceding siblings ...)
2015-08-19 13:01 ` [PATCH v4 3/3] untracked cache: fix entry invalidation Nguyễn Thái Ngọc Duy
@ 2015-08-19 17:47 ` Junio C Hamano
3 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2015-08-19 17:47 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git, David Turner
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> The first patch is a split from David's v3 patch because it should
> belong to dt/untracked-sparse. The second is basically David's v3.
> The third patch fixes untracked_cache_invalidate_path(). David fixed
> it in v2, but there's another bug in this code.
>
> David Turner (1):
> untracked-cache: fix subdirectory handling
>
> Nguyễn Thái Ngọc Duy (2):
> t7063: use --force-untracked-cache to speed up a bit
> untracked cache: fix entry invalidation
>
> dir.c | 82 +++++++++++++++++++++++-------
> t/t7063-status-untracked-cache.sh | 102 ++++++++++++++++++++++++++++++++++++--
> 2 files changed, 163 insertions(+), 21 deletions(-)
They all look reasonable. Will replace v3 with these and wait a bit
to give chance to David and others to comment.
Thanks.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] untracked cache: fix entry invalidation
2015-08-19 13:01 ` [PATCH v4 3/3] untracked cache: fix entry invalidation Nguyễn Thái Ngọc Duy
@ 2015-08-25 19:25 ` David Turner
2015-08-25 20:12 ` Junio C Hamano
0 siblings, 1 reply; 10+ messages in thread
From: David Turner @ 2015-08-25 19:25 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy; +Cc: git
On Wed, 2015-08-19 at 20:01 +0700, Nguyễn Thái Ngọc Duy wrote:
> 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.
LGTM.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] untracked cache: fix entry invalidation
2015-08-25 19:25 ` David Turner
@ 2015-08-25 20:12 ` Junio C Hamano
0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2015-08-25 20:12 UTC (permalink / raw)
To: David Turner; +Cc: Nguyễn Thái Ngọc Duy, git
David Turner <dturner@twopensource.com> writes:
> On Wed, 2015-08-19 at 20:01 +0700, Nguyễn Thái Ngọc Duy wrote:
>> 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.
>
> LGTM.
Thanks, both. Let's move this forward.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2015-08-25 20:13 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-08-16 5:17 [PATCH v3] untracked-cache: fix subdirectory handling David Turner
2015-08-16 12:16 ` Duy Nguyen
2015-08-19 11:39 ` Duy Nguyen
2015-08-19 13:01 ` [PATCH v4 0/3] dt/untracked-subdir Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 1/3] t7063: use --force-untracked-cache to speed up a bit Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 2/3] untracked-cache: fix subdirectory handling Nguyễn Thái Ngọc Duy
2015-08-19 13:01 ` [PATCH v4 3/3] untracked cache: fix entry invalidation Nguyễn Thái Ngọc Duy
2015-08-25 19:25 ` David Turner
2015-08-25 20:12 ` Junio C Hamano
2015-08-19 17:47 ` [PATCH v4 0/3] dt/untracked-subdir Junio C Hamano
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).