{"thread":{"id":"40109","subject":"[PATCH v3] untracked-cache: fix subdirectory handling","startedAt":"2015-08-16T05:17:07Z","lastAt":"2015-08-25T20:12:56Z","messageCount":10,"participants":["David Turner","Duy Nguyen","Nguyễn Thái Ngọc Duy","Junio C Hamano"],"isPatch":true,"patchVersion":3,"patchTotal":null},"messages":[{"id":"268143","messageId":"1439702227-15453-1-git-send-email-dturner@twopensource.com","threadId":"40109","inReplyTo":null,"subject":"[PATCH v3] untracked-cache: fix subdirectory handling","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-16T05:17:07Z","receivedAt":"2015-08-16T05:17:07Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"Previously, some calls lookup_untracked would pass a full path.  But\nlookup_untracked assumes that the portion of the path up to and\nincluding to the untracked_cache_dir has been removed.  So\nlookup_untracked would be looking in the untracked_cache for 'foo' for\n'foo/bar' (instead of just looking for 'bar').  This would cause\nuntracked cache corruption.\n\nInstead, treat_directory learns to track the base length of the parent\ndirectory, so that only the last path component is passed to\nlookup_untracked.\n\nHelped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: David Turner <dturner@twopensource.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n\nThis version incorporates Duy's version of the dir.c code, and his\nsuggested test speedup.\n\n---\n dir.c                             | 14 ++++----\n t/t7063-status-untracked-cache.sh | 74 +++++++++++++++++++++++++++++++++++++--\n 2 files changed, 80 insertions(+), 8 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex e7b89fe..cd4ac77 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1297,7 +1297,7 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)\n  */\n static enum path_treatment treat_directory(struct dir_struct *dir,\n \tstruct untracked_cache_dir *untracked,\n-\tconst char *dirname, int len, int exclude,\n+\tconst char *dirname, int len, int baselen, int exclude,\n \tconst struct path_simplify *simplify)\n {\n \t/* The \"len-1\" is to strip the final '/' */\n@@ -1324,7 +1324,8 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \tif (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))\n \t\treturn exclude ? path_excluded : path_untracked;\n \n-\tuntracked = lookup_untracked(dir->untracked, untracked, dirname, len);\n+\tuntracked = lookup_untracked(dir->untracked, untracked,\n+\t\t\t\t     dirname + baselen, len - baselen);\n \treturn read_directory_recursive(dir, dirname, len,\n \t\t\t\t\tuntracked, 1, simplify);\n }\n@@ -1444,6 +1445,7 @@ static int get_dtype(struct dirent *de, const char *path, int len)\n static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\t\t\t\t  struct untracked_cache_dir *untracked,\n \t\t\t\t\t  struct strbuf *path,\n+\t\t\t\t\t  int baselen,\n \t\t\t\t\t  const struct path_simplify *simplify,\n \t\t\t\t\t  int dtype, struct dirent *de)\n {\n@@ -1495,8 +1497,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\treturn path_none;\n \tcase DT_DIR:\n \t\tstrbuf_addch(path, '/');\n-\t\treturn treat_directory(dir, untracked, path->buf, path->len, exclude,\n-\t\t\tsimplify);\n+\t\treturn treat_directory(dir, untracked, path->buf, path->len,\n+\t\t\t\t       baselen, exclude, simplify);\n \tcase DT_REG:\n \tcase DT_LNK:\n \t\treturn exclude ? path_excluded : path_untracked;\n@@ -1557,7 +1559,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n \t\treturn path_none;\n \n \tdtype = DTYPE(de);\n-\treturn treat_one_path(dir, untracked, path, simplify, dtype, de);\n+\treturn treat_one_path(dir, untracked, path, baselen, simplify, dtype, de);\n }\n \n static void add_untracked(struct untracked_cache_dir *dir, const char *name)\n@@ -1827,7 +1829,7 @@ static int treat_leading_path(struct dir_struct *dir,\n \t\t\tbreak;\n \t\tif (simplify_away(sb.buf, sb.len, simplify))\n \t\t\tbreak;\n-\t\tif (treat_one_path(dir, NULL, &sb, simplify,\n+\t\tif (treat_one_path(dir, NULL, &sb, baselen, simplify,\n \t\t\t\t   DT_DIR, NULL) == path_none)\n \t\t\tbreak; /* do not recurse into it */\n \t\tif (len <= baselen) {\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex ff23f4e..6716f69 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -402,13 +402,14 @@ test_expect_success 'set up sparse checkout' '\n \techo \"done/[a-z]*\" >.git/info/sparse-checkout &&\n \ttest_config core.sparsecheckout true &&\n \tgit checkout master &&\n-\tgit update-index --untracked-cache &&\n+\tgit update-index --untracked-cache --force-untracked-cache &&\n \tgit status --porcelain >/dev/null && # prime the cache\n \ttest_path_is_missing done/.gitignore &&\n \ttest_path_is_file done/one\n '\n \n-test_expect_success 'create files, some of which are gitignored' '\n+test_expect_success 'create/modify files, some of which are gitignored' '\n+\techo two bis >done/two &&\n \techo three >done/three && # three is gitignored\n \techo four >done/four && # four is gitignored at a higher level\n \techo five >done/five # five is not gitignored\n@@ -420,6 +421,7 @@ test_expect_success 'test sparse status with untracked cache' '\n \tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n \tgit status --porcelain >../status.actual &&\n \tcat >../status.expect <<EOF &&\n+ M done/two\n ?? .gitignore\n ?? done/five\n ?? dtwo/\n@@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with untracked cache' '\n \tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n \tgit status --porcelain >../status.actual &&\n \tcat >../status.expect <<EOF &&\n+ M done/two\n ?? .gitignore\n ?? done/five\n ?? dtwo/\n@@ -473,4 +476,71 @@ EOF\n \ttest_cmp ../trace.expect ../trace\n '\n \n+test_expect_success 'set up for test of subdir and sparse checkouts' '\n+\tmkdir done/sub &&\n+\tmkdir done/sub/sub &&\n+\techo \"sub\" > done/sub/sub/file\n+'\n+\n+test_expect_success 'test sparse status with untracked cache and subdir' '\n+\tavoid_racy &&\n+\t: >../trace &&\n+\tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n+\tgit status --porcelain >../status.actual &&\n+\tcat >../status.expect <<EOF &&\n+ M done/two\n+?? .gitignore\n+?? done/five\n+?? done/sub/\n+?? dtwo/\n+EOF\n+\ttest_cmp ../status.expect ../status.actual &&\n+\tcat >../trace.expect <<EOF &&\n+node creation: 2\n+gitignore invalidation: 0\n+directory invalidation: 1\n+opendir: 3\n+EOF\n+\ttest_cmp ../trace.expect ../trace\n+'\n+\n+test_expect_success 'verify untracked cache dump (sparse/subdirs)' '\n+\ttest-dump-untracked-cache >../actual &&\n+\tcat >../expect <<EOF &&\n+info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0\n+core.excludesfile 0000000000000000000000000000000000000000\n+exclude_per_dir .gitignore\n+flags 00000006\n+/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid\n+.gitignore\n+dtwo/\n+/done/ 1946f0437f90c5005533cbe1736a6451ca301714 recurse valid\n+five\n+sub/\n+/done/sub/ 0000000000000000000000000000000000000000 recurse check_only valid\n+sub/\n+/done/sub/sub/ 0000000000000000000000000000000000000000 recurse check_only valid\n+file\n+/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid\n+/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid\n+two\n+EOF\n+\ttest_cmp ../expect ../actual\n+'\n+\n+test_expect_success 'test sparse status again with untracked cache and subdir' '\n+\tavoid_racy &&\n+\t: >../trace &&\n+\tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n+\tgit status --porcelain >../status.actual &&\n+\ttest_cmp ../status.expect ../status.actual &&\n+\tcat >../trace.expect <<EOF &&\n+node creation: 0\n+gitignore invalidation: 0\n+directory invalidation: 0\n+opendir: 0\n+EOF\n+\ttest_cmp ../trace.expect ../trace\n+'\n+\n test_done\n-- \n2.4.2.622.gac67c30-twtrsrc\n"},{"id":"268151","messageId":"CACsJy8C=rnqrJHqMBSQcBsD1ihSvztVCnQ46N2mzoeh=HFM6qw@mail.gmail.com","threadId":"40109","inReplyTo":"1439702227-15453-1-git-send-email-dturner@twopensource.com","subject":"Re: [PATCH v3] untracked-cache: fix subdirectory handling","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-16T12:16:24Z","receivedAt":"2015-08-16T12:16:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Aug 16, 2015 at 12:17 PM, David Turner <dturner@twopensource.com> wrote:\n> Previously, some calls lookup_untracked would pass a full path.  But\n> lookup_untracked assumes that the portion of the path up to and\n> including to the untracked_cache_dir has been removed.  So\n> lookup_untracked would be looking in the untracked_cache for 'foo' for\n> 'foo/bar' (instead of just looking for 'bar').  This would cause\n> untracked cache corruption.\n>\n> Instead, treat_directory learns to track the base length of the parent\n> directory, so that only the last path component is passed to\n> lookup_untracked.\n\nYour v2 also fixes untracked_cache_invalidate_path(), which is not\nincluded here. Maybe it's in another patch?\n\n> Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> Signed-off-by: David Turner <dturner@twopensource.com>\n> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n> ---\n>\n> This version incorporates Duy's version of the dir.c code, and his\n> suggested test speedup.\n>\n> ---\n>  dir.c                             | 14 ++++----\n>  t/t7063-status-untracked-cache.sh | 74 +++++++++++++++++++++++++++++++++++++--\n>  2 files changed, 80 insertions(+), 8 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index e7b89fe..cd4ac77 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -1297,7 +1297,7 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)\n>   */\n>  static enum path_treatment treat_directory(struct dir_struct *dir,\n>         struct untracked_cache_dir *untracked,\n> -       const char *dirname, int len, int exclude,\n> +       const char *dirname, int len, int baselen, int exclude,\n>         const struct path_simplify *simplify)\n>  {\n>         /* The \"len-1\" is to strip the final '/' */\n> @@ -1324,7 +1324,8 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n>         if (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))\n>                 return exclude ? path_excluded : path_untracked;\n>\n> -       untracked = lookup_untracked(dir->untracked, untracked, dirname, len);\n> +       untracked = lookup_untracked(dir->untracked, untracked,\n> +                                    dirname + baselen, len - baselen);\n>         return read_directory_recursive(dir, dirname, len,\n>                                         untracked, 1, simplify);\n>  }\n> @@ -1444,6 +1445,7 @@ static int get_dtype(struct dirent *de, const char *path, int len)\n>  static enum path_treatment treat_one_path(struct dir_struct *dir,\n>                                           struct untracked_cache_dir *untracked,\n>                                           struct strbuf *path,\n> +                                         int baselen,\n>                                           const struct path_simplify *simplify,\n>                                           int dtype, struct dirent *de)\n>  {\n> @@ -1495,8 +1497,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n>                 return path_none;\n>         case DT_DIR:\n>                 strbuf_addch(path, '/');\n> -               return treat_directory(dir, untracked, path->buf, path->len, exclude,\n> -                       simplify);\n> +               return treat_directory(dir, untracked, path->buf, path->len,\n> +                                      baselen, exclude, simplify);\n>         case DT_REG:\n>         case DT_LNK:\n>                 return exclude ? path_excluded : path_untracked;\n> @@ -1557,7 +1559,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n>                 return path_none;\n>\n>         dtype = DTYPE(de);\n> -       return treat_one_path(dir, untracked, path, simplify, dtype, de);\n> +       return treat_one_path(dir, untracked, path, baselen, simplify, dtype, de);\n>  }\n>\n>  static void add_untracked(struct untracked_cache_dir *dir, const char *name)\n> @@ -1827,7 +1829,7 @@ static int treat_leading_path(struct dir_struct *dir,\n>                         break;\n>                 if (simplify_away(sb.buf, sb.len, simplify))\n>                         break;\n> -               if (treat_one_path(dir, NULL, &sb, simplify,\n> +               if (treat_one_path(dir, NULL, &sb, baselen, simplify,\n>                                    DT_DIR, NULL) == path_none)\n>                         break; /* do not recurse into it */\n>                 if (len <= baselen) {\n> diff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\n> index ff23f4e..6716f69 100755\n> --- a/t/t7063-status-untracked-cache.sh\n> +++ b/t/t7063-status-untracked-cache.sh\n> @@ -402,13 +402,14 @@ test_expect_success 'set up sparse checkout' '\n>         echo \"done/[a-z]*\" >.git/info/sparse-checkout &&\n>         test_config core.sparsecheckout true &&\n>         git checkout master &&\n> -       git update-index --untracked-cache &&\n> +       git update-index --untracked-cache --force-untracked-cache &&\n>         git status --porcelain >/dev/null && # prime the cache\n>         test_path_is_missing done/.gitignore &&\n>         test_path_is_file done/one\n>  '\n>\n> -test_expect_success 'create files, some of which are gitignored' '\n> +test_expect_success 'create/modify files, some of which are gitignored' '\n> +       echo two bis >done/two &&\n>         echo three >done/three && # three is gitignored\n>         echo four >done/four && # four is gitignored at a higher level\n>         echo five >done/five # five is not gitignored\n> @@ -420,6 +421,7 @@ test_expect_success 'test sparse status with untracked cache' '\n>         GIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n>         git status --porcelain >../status.actual &&\n>         cat >../status.expect <<EOF &&\n> + M done/two\n>  ?? .gitignore\n>  ?? done/five\n>  ?? dtwo/\n> @@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with untracked cache' '\n>         GIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n>         git status --porcelain >../status.actual &&\n>         cat >../status.expect <<EOF &&\n> + M done/two\n>  ?? .gitignore\n>  ?? done/five\n>  ?? dtwo/\n> @@ -473,4 +476,71 @@ EOF\n>         test_cmp ../trace.expect ../trace\n>  '\n>\n> +test_expect_success 'set up for test of subdir and sparse checkouts' '\n> +       mkdir done/sub &&\n> +       mkdir done/sub/sub &&\n> +       echo \"sub\" > done/sub/sub/file\n> +'\n> +\n> +test_expect_success 'test sparse status with untracked cache and subdir' '\n> +       avoid_racy &&\n> +       : >../trace &&\n> +       GIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n> +       git status --porcelain >../status.actual &&\n> +       cat >../status.expect <<EOF &&\n> + M done/two\n> +?? .gitignore\n> +?? done/five\n> +?? done/sub/\n> +?? dtwo/\n> +EOF\n> +       test_cmp ../status.expect ../status.actual &&\n> +       cat >../trace.expect <<EOF &&\n> +node creation: 2\n> +gitignore invalidation: 0\n> +directory invalidation: 1\n> +opendir: 3\n> +EOF\n> +       test_cmp ../trace.expect ../trace\n> +'\n> +\n> +test_expect_success 'verify untracked cache dump (sparse/subdirs)' '\n> +       test-dump-untracked-cache >../actual &&\n> +       cat >../expect <<EOF &&\n> +info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0\n> +core.excludesfile 0000000000000000000000000000000000000000\n> +exclude_per_dir .gitignore\n> +flags 00000006\n> +/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid\n> +.gitignore\n> +dtwo/\n> +/done/ 1946f0437f90c5005533cbe1736a6451ca301714 recurse valid\n> +five\n> +sub/\n> +/done/sub/ 0000000000000000000000000000000000000000 recurse check_only valid\n> +sub/\n> +/done/sub/sub/ 0000000000000000000000000000000000000000 recurse check_only valid\n> +file\n> +/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid\n> +/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid\n> +two\n> +EOF\n> +       test_cmp ../expect ../actual\n> +'\n> +\n> +test_expect_success 'test sparse status again with untracked cache and subdir' '\n> +       avoid_racy &&\n> +       : >../trace &&\n> +       GIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n> +       git status --porcelain >../status.actual &&\n> +       test_cmp ../status.expect ../status.actual &&\n> +       cat >../trace.expect <<EOF &&\n> +node creation: 0\n> +gitignore invalidation: 0\n> +directory invalidation: 0\n> +opendir: 0\n> +EOF\n> +       test_cmp ../trace.expect ../trace\n> +'\n> +\n>  test_done\n> --\n> 2.4.2.622.gac67c30-twtrsrc\n>\n\n\n\n-- \nDuy\n"},{"id":"268296","messageId":"CACsJy8DUUmT0xvBtMiC41qt+f=a02W1F-+i5gWjounNmpLA1tQ@mail.gmail.com","threadId":"40109","inReplyTo":"CACsJy8C=rnqrJHqMBSQcBsD1ihSvztVCnQ46N2mzoeh=HFM6qw@mail.gmail.com","subject":"Re: [PATCH v3] untracked-cache: fix subdirectory handling","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-19T11:39:37Z","receivedAt":"2015-08-19T11:39:37Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Sun, Aug 16, 2015 at 7:16 PM, Duy Nguyen <pclouds@gmail.com> wrote:\n> On Sun, Aug 16, 2015 at 12:17 PM, David Turner <dturner@twopensource.com> wrote:\n>> Previously, some calls lookup_untracked would pass a full path.  But\n>> lookup_untracked assumes that the portion of the path up to and\n>> including to the untracked_cache_dir has been removed.  So\n>> lookup_untracked would be looking in the untracked_cache for 'foo' for\n>> 'foo/bar' (instead of just looking for 'bar').  This would cause\n>> untracked cache corruption.\n>>\n>> Instead, treat_directory learns to track the base length of the parent\n>> directory, so that only the last path component is passed to\n>> lookup_untracked.\n>\n> Your v2 also fixes untracked_cache_invalidate_path(), which is not\n> included here. Maybe it's in another patch?\n\nNo I was wrong. Your changes and the original code are effectively the\nsame (I misread strrchr as strchr). But I think there's a bug\nsomewhere as I'm writing tests to understand that code..\n-- \nDuy\n"},{"id":"268298","messageId":"1439989286-24355-1-git-send-email-pclouds@gmail.com","threadId":"40109","inReplyTo":"1439702227-15453-1-git-send-email-dturner@twopensource.com","subject":"[PATCH v4 0/3] dt/untracked-subdir","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-19T13:01:23Z","receivedAt":"2015-08-19T13:01:23Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"The first patch is a split from David's v3 patch because it should\nbelong to dt/untracked-sparse. The second is basically David's v3.\nThe third patch fixes untracked_cache_invalidate_path(). David fixed\nit in v2, but there's another bug in this code.\n\nDavid Turner (1):\n  untracked-cache: fix subdirectory handling\n\nNguyễn Thái Ngọc Duy (2):\n  t7063: use --force-untracked-cache to speed up a bit\n  untracked cache: fix entry invalidation\n\n dir.c                             |  82 +++++++++++++++++++++++-------\n t/t7063-status-untracked-cache.sh | 102 ++++++++++++++++++++++++++++++++++++--\n 2 files changed, 163 insertions(+), 21 deletions(-)\n\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"268300","messageId":"1439989286-24355-2-git-send-email-pclouds@gmail.com","threadId":"40109","inReplyTo":"1439989286-24355-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 1/3] t7063: use --force-untracked-cache to speed up a bit","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-19T13:01:24Z","receivedAt":"2015-08-19T13:01:24Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"When in the middle of t7063, we are sure untracked cache is supported,\nso we can use --force-untracked-cache to skip the support detection\nphase and save a few seconds. It's also good that --force-untracked-cache\nis exercised in the test suite.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n t/t7063-status-untracked-cache.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex ff23f4e..744e922 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -402,7 +402,7 @@ test_expect_success 'set up sparse checkout' '\n \techo \"done/[a-z]*\" >.git/info/sparse-checkout &&\n \ttest_config core.sparsecheckout true &&\n \tgit checkout master &&\n-\tgit update-index --untracked-cache &&\n+\tgit update-index --force-untracked-cache &&\n \tgit status --porcelain >/dev/null && # prime the cache\n \ttest_path_is_missing done/.gitignore &&\n \ttest_path_is_file done/one\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"268299","messageId":"1439989286-24355-3-git-send-email-pclouds@gmail.com","threadId":"40109","inReplyTo":"1439989286-24355-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 2/3] untracked-cache: fix subdirectory handling","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-19T13:01:25Z","receivedAt":"2015-08-19T13:01:25Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"From: David Turner <dturner@twopensource.com>\n\nPreviously, some calls lookup_untracked would pass a full path.  But\nlookup_untracked assumes that the portion of the path up to and\nincluding to the untracked_cache_dir has been removed.  So\nlookup_untracked would be looking in the untracked_cache for 'foo' for\n'foo/bar' (instead of just looking for 'bar').  This would cause\nuntracked cache corruption.\n\nInstead, treat_directory learns to track the base length of the parent\ndirectory, so that only the last path component is passed to\nlookup_untracked.\n\nHelped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\nSigned-off-by: David Turner <dturner@twopensource.com>\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c                             | 14 ++++----\n t/t7063-status-untracked-cache.sh | 72 ++++++++++++++++++++++++++++++++++++++-\n 2 files changed, 79 insertions(+), 7 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex e7b89fe..cd4ac77 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1297,7 +1297,7 @@ static enum exist_status directory_exists_in_index(const char *dirname, int len)\n  */\n static enum path_treatment treat_directory(struct dir_struct *dir,\n \tstruct untracked_cache_dir *untracked,\n-\tconst char *dirname, int len, int exclude,\n+\tconst char *dirname, int len, int baselen, int exclude,\n \tconst struct path_simplify *simplify)\n {\n \t/* The \"len-1\" is to strip the final '/' */\n@@ -1324,7 +1324,8 @@ static enum path_treatment treat_directory(struct dir_struct *dir,\n \tif (!(dir->flags & DIR_HIDE_EMPTY_DIRECTORIES))\n \t\treturn exclude ? path_excluded : path_untracked;\n \n-\tuntracked = lookup_untracked(dir->untracked, untracked, dirname, len);\n+\tuntracked = lookup_untracked(dir->untracked, untracked,\n+\t\t\t\t     dirname + baselen, len - baselen);\n \treturn read_directory_recursive(dir, dirname, len,\n \t\t\t\t\tuntracked, 1, simplify);\n }\n@@ -1444,6 +1445,7 @@ static int get_dtype(struct dirent *de, const char *path, int len)\n static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\t\t\t\t  struct untracked_cache_dir *untracked,\n \t\t\t\t\t  struct strbuf *path,\n+\t\t\t\t\t  int baselen,\n \t\t\t\t\t  const struct path_simplify *simplify,\n \t\t\t\t\t  int dtype, struct dirent *de)\n {\n@@ -1495,8 +1497,8 @@ static enum path_treatment treat_one_path(struct dir_struct *dir,\n \t\treturn path_none;\n \tcase DT_DIR:\n \t\tstrbuf_addch(path, '/');\n-\t\treturn treat_directory(dir, untracked, path->buf, path->len, exclude,\n-\t\t\tsimplify);\n+\t\treturn treat_directory(dir, untracked, path->buf, path->len,\n+\t\t\t\t       baselen, exclude, simplify);\n \tcase DT_REG:\n \tcase DT_LNK:\n \t\treturn exclude ? path_excluded : path_untracked;\n@@ -1557,7 +1559,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,\n \t\treturn path_none;\n \n \tdtype = DTYPE(de);\n-\treturn treat_one_path(dir, untracked, path, simplify, dtype, de);\n+\treturn treat_one_path(dir, untracked, path, baselen, simplify, dtype, de);\n }\n \n static void add_untracked(struct untracked_cache_dir *dir, const char *name)\n@@ -1827,7 +1829,7 @@ static int treat_leading_path(struct dir_struct *dir,\n \t\t\tbreak;\n \t\tif (simplify_away(sb.buf, sb.len, simplify))\n \t\t\tbreak;\n-\t\tif (treat_one_path(dir, NULL, &sb, simplify,\n+\t\tif (treat_one_path(dir, NULL, &sb, baselen, simplify,\n \t\t\t\t   DT_DIR, NULL) == path_none)\n \t\t\tbreak; /* do not recurse into it */\n \t\tif (len <= baselen) {\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 744e922..22393b9 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -408,7 +408,8 @@ test_expect_success 'set up sparse checkout' '\n \ttest_path_is_file done/one\n '\n \n-test_expect_success 'create files, some of which are gitignored' '\n+test_expect_success 'create/modify files, some of which are gitignored' '\n+\techo two bis >done/two &&\n \techo three >done/three && # three is gitignored\n \techo four >done/four && # four is gitignored at a higher level\n \techo five >done/five # five is not gitignored\n@@ -420,6 +421,7 @@ test_expect_success 'test sparse status with untracked cache' '\n \tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n \tgit status --porcelain >../status.actual &&\n \tcat >../status.expect <<EOF &&\n+ M done/two\n ?? .gitignore\n ?? done/five\n ?? dtwo/\n@@ -459,6 +461,7 @@ test_expect_success 'test sparse status again with untracked cache' '\n \tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n \tgit status --porcelain >../status.actual &&\n \tcat >../status.expect <<EOF &&\n+ M done/two\n ?? .gitignore\n ?? done/five\n ?? dtwo/\n@@ -473,4 +476,71 @@ EOF\n \ttest_cmp ../trace.expect ../trace\n '\n \n+test_expect_success 'set up for test of subdir and sparse checkouts' '\n+\tmkdir done/sub &&\n+\tmkdir done/sub/sub &&\n+\techo \"sub\" > done/sub/sub/file\n+'\n+\n+test_expect_success 'test sparse status with untracked cache and subdir' '\n+\tavoid_racy &&\n+\t: >../trace &&\n+\tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n+\tgit status --porcelain >../status.actual &&\n+\tcat >../status.expect <<EOF &&\n+ M done/two\n+?? .gitignore\n+?? done/five\n+?? done/sub/\n+?? dtwo/\n+EOF\n+\ttest_cmp ../status.expect ../status.actual &&\n+\tcat >../trace.expect <<EOF &&\n+node creation: 2\n+gitignore invalidation: 0\n+directory invalidation: 1\n+opendir: 3\n+EOF\n+\ttest_cmp ../trace.expect ../trace\n+'\n+\n+test_expect_success 'verify untracked cache dump (sparse/subdirs)' '\n+\ttest-dump-untracked-cache >../actual &&\n+\tcat >../expect <<EOF &&\n+info/exclude 13263c0978fb9fad16b2d580fb800b6d811c3ff0\n+core.excludesfile 0000000000000000000000000000000000000000\n+exclude_per_dir .gitignore\n+flags 00000006\n+/ e6fcc8f2ee31bae321d66afd183fcb7237afae6e recurse valid\n+.gitignore\n+dtwo/\n+/done/ 1946f0437f90c5005533cbe1736a6451ca301714 recurse valid\n+five\n+sub/\n+/done/sub/ 0000000000000000000000000000000000000000 recurse check_only valid\n+sub/\n+/done/sub/sub/ 0000000000000000000000000000000000000000 recurse check_only valid\n+file\n+/dthree/ 0000000000000000000000000000000000000000 recurse check_only valid\n+/dtwo/ 0000000000000000000000000000000000000000 recurse check_only valid\n+two\n+EOF\n+\ttest_cmp ../expect ../actual\n+'\n+\n+test_expect_success 'test sparse status again with untracked cache and subdir' '\n+\tavoid_racy &&\n+\t: >../trace &&\n+\tGIT_TRACE_UNTRACKED_STATS=\"$TRASH_DIRECTORY/trace\" \\\n+\tgit status --porcelain >../status.actual &&\n+\ttest_cmp ../status.expect ../status.actual &&\n+\tcat >../trace.expect <<EOF &&\n+node creation: 0\n+gitignore invalidation: 0\n+directory invalidation: 0\n+opendir: 0\n+EOF\n+\ttest_cmp ../trace.expect ../trace\n+'\n+\n test_done\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"268301","messageId":"1439989286-24355-4-git-send-email-pclouds@gmail.com","threadId":"40109","inReplyTo":"1439989286-24355-1-git-send-email-pclouds@gmail.com","subject":"[PATCH v4 3/3] untracked cache: fix entry invalidation","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2015-08-19T13:01:26Z","receivedAt":"2015-08-19T13:01:26Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"First, the current code in untracked_cache_invalidate_path() is wrong\nbecause it can only handle paths \"a\" or \"a/b\", not \"a/b/c\" because\nlookup_untracked() only looks for entries directly under the given\ndirectory. In the last case, it will look for the entry \"b/c\" in\ndirectory \"a\" instead. This means if you delete or add an entry in a\nsubdirectory, untracked cache may become out of date because it does not\ninvalidate properly. This is noticed by David Turner.\n\nThe second problem is about invalidation inside a fully untracked/excluded\ndirectory. In this case we may have to invalidate back to root. See the\ncomment block for detail.\n\nSigned-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>\n---\n dir.c                             | 68 ++++++++++++++++++++++++++++++++-------\n t/t7063-status-untracked-cache.sh | 28 +++++++++++++++-\n 2 files changed, 83 insertions(+), 13 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex cd4ac77..c1edabf 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -2616,23 +2616,67 @@ done2:\n \treturn uc;\n }\n \n+static void invalidate_one_directory(struct untracked_cache *uc,\n+\t\t\t\t     struct untracked_cache_dir *ucd)\n+{\n+\tuc->dir_invalidated++;\n+\tucd->valid = 0;\n+\tucd->untracked_nr = 0;\n+}\n+\n+/*\n+ * Normally when an entry is added or removed from a directory,\n+ * invalidating that directory is enough. No need to touch its\n+ * ancestors. When a directory is shown as \"foo/bar/\" in git-status\n+ * however, deleting or adding an entry may have cascading effect.\n+ *\n+ * Say the \"foo/bar/file\" has become untracked, we need to tell the\n+ * untracked_cache_dir of \"foo\" that \"bar/\" is not an untracked\n+ * directory any more (because \"bar\" is managed by foo as an untracked\n+ * \"file\").\n+ *\n+ * Similarly, if \"foo/bar/file\" moves from untracked to tracked and it\n+ * was the last untracked entry in the entire \"foo\", we should show\n+ * \"foo/\" instead. Which means we have to invalidate past \"bar\" up to\n+ * \"foo\".\n+ *\n+ * This function traverses all directories from root to leaf. If there\n+ * is a chance of one of the above cases happening, we invalidate back\n+ * to root. Otherwise we just invalidate the leaf. There may be a more\n+ * sophisticated way than checking for SHOW_OTHER_DIRECTORIES to\n+ * detect these cases and avoid unnecessary invalidation, for example,\n+ * checking for the untracked entry named \"bar/\" in \"foo\", but for now\n+ * stick to something safe and simple.\n+ */\n+static int invalidate_one_component(struct untracked_cache *uc,\n+\t\t\t\t    struct untracked_cache_dir *dir,\n+\t\t\t\t    const char *path, int len)\n+{\n+\tconst char *rest = strchr(path, '/');\n+\n+\tif (rest) {\n+\t\tint component_len = rest - path;\n+\t\tstruct untracked_cache_dir *d =\n+\t\t\tlookup_untracked(uc, dir, path, component_len);\n+\t\tint ret =\n+\t\t\tinvalidate_one_component(uc, d, rest + 1,\n+\t\t\t\t\t\t len - (component_len + 1));\n+\t\tif (ret)\n+\t\t\tinvalidate_one_directory(uc, dir);\n+\t\treturn ret;\n+\t}\n+\n+\tinvalidate_one_directory(uc, dir);\n+\treturn uc->dir_flags & DIR_SHOW_OTHER_DIRECTORIES;\n+}\n+\n void untracked_cache_invalidate_path(struct index_state *istate,\n \t\t\t\t     const char *path)\n {\n-\tconst char *sep;\n-\tstruct untracked_cache_dir *d;\n \tif (!istate->untracked || !istate->untracked->root)\n \t\treturn;\n-\tsep = strrchr(path, '/');\n-\tif (sep)\n-\t\td = lookup_untracked(istate->untracked,\n-\t\t\t\t     istate->untracked->root,\n-\t\t\t\t     path, sep - path);\n-\telse\n-\t\td = istate->untracked->root;\n-\tistate->untracked->dir_invalidated++;\n-\td->valid = 0;\n-\td->untracked_nr = 0;\n+\tinvalidate_one_component(istate->untracked, istate->untracked->root,\n+\t\t\t\t path, strlen(path));\n }\n \n void untracked_cache_remove_from_index(struct index_state *istate,\ndiff --git a/t/t7063-status-untracked-cache.sh b/t/t7063-status-untracked-cache.sh\nindex 22393b9..37a24c1 100755\n--- a/t/t7063-status-untracked-cache.sh\n+++ b/t/t7063-status-untracked-cache.sh\n@@ -375,7 +375,7 @@ EOF\n node creation: 0\n gitignore invalidation: 0\n directory invalidation: 0\n-opendir: 1\n+opendir: 2\n EOF\n \ttest_cmp ../trace.expect ../trace\n '\n@@ -543,4 +543,30 @@ EOF\n \ttest_cmp ../trace.expect ../trace\n '\n \n+test_expect_success 'move entry in subdir from untracked to cached' '\n+\tgit add dtwo/two &&\n+\tgit status --porcelain >../status.actual &&\n+\tcat >../status.expect <<EOF &&\n+ M done/two\n+A  dtwo/two\n+?? .gitignore\n+?? done/five\n+?? done/sub/\n+EOF\n+\ttest_cmp ../status.expect ../status.actual\n+'\n+\n+test_expect_success 'move entry in subdir from cached to untracked' '\n+\tgit rm --cached dtwo/two &&\n+\tgit status --porcelain >../status.actual &&\n+\tcat >../status.expect <<EOF &&\n+ M done/two\n+?? .gitignore\n+?? done/five\n+?? done/sub/\n+?? dtwo/\n+EOF\n+\ttest_cmp ../status.expect ../status.actual\n+'\n+\n test_done\n-- \n2.3.0.rc1.137.g477eb31\n"},{"id":"268330","messageId":"xmqq37zfb25x.fsf@gitster.dls.corp.google.com","threadId":"40109","inReplyTo":"1439989286-24355-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v4 0/3] dt/untracked-subdir","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-19T17:47:54Z","receivedAt":"2015-08-19T17:47:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:\n\n> The first patch is a split from David's v3 patch because it should\n> belong to dt/untracked-sparse. The second is basically David's v3.\n> The third patch fixes untracked_cache_invalidate_path(). David fixed\n> it in v2, but there's another bug in this code.\n>\n> David Turner (1):\n>   untracked-cache: fix subdirectory handling\n>\n> Nguyễn Thái Ngọc Duy (2):\n>   t7063: use --force-untracked-cache to speed up a bit\n>   untracked cache: fix entry invalidation\n>\n>  dir.c                             |  82 +++++++++++++++++++++++-------\n>  t/t7063-status-untracked-cache.sh | 102 ++++++++++++++++++++++++++++++++++++--\n>  2 files changed, 163 insertions(+), 21 deletions(-)\n\nThey all look reasonable.  Will replace v3 with these and wait a bit\nto give chance to David and others to comment.\n\nThanks.\n"},{"id":"268683","messageId":"1440530701.7859.3.camel@twopensource.com","threadId":"40109","inReplyTo":"1439989286-24355-4-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH v4 3/3] untracked cache: fix entry invalidation","fromName":"David Turner","fromEmail":"dturner@twopensource.com","sentAt":"2015-08-25T19:25:01Z","receivedAt":"2015-08-25T19:25:01Z","isPatch":true,"sender":{"key":"novalis@novalis.org","avatar":"https://avatars.githubusercontent.com/u/77003?v=4"},"body":"On Wed, 2015-08-19 at 20:01 +0700, Nguyễn Thái Ngọc Duy wrote:\n> First, the current code in untracked_cache_invalidate_path() is wrong\n> because it can only handle paths \"a\" or \"a/b\", not \"a/b/c\" because\n> lookup_untracked() only looks for entries directly under the given\n> directory. In the last case, it will look for the entry \"b/c\" in\n> directory \"a\" instead. This means if you delete or add an entry in a\n> subdirectory, untracked cache may become out of date because it does not\n> invalidate properly. This is noticed by David Turner.\n> \n> The second problem is about invalidation inside a fully untracked/excluded\n> directory. In this case we may have to invalidate back to root. See the\n> comment block for detail.\n\nLGTM.\n"},{"id":"268688","messageId":"xmqqr3mrxh2v.fsf@gitster.dls.corp.google.com","threadId":"40109","inReplyTo":"1440530701.7859.3.camel@twopensource.com","subject":"Re: [PATCH v4 3/3] untracked cache: fix entry invalidation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2015-08-25T20:12:56Z","receivedAt":"2015-08-25T20:12:56Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"David Turner <dturner@twopensource.com> writes:\n\n> On Wed, 2015-08-19 at 20:01 +0700, Nguyễn Thái Ngọc Duy wrote:\n>> First, the current code in untracked_cache_invalidate_path() is wrong\n>> because it can only handle paths \"a\" or \"a/b\", not \"a/b/c\" because\n>> lookup_untracked() only looks for entries directly under the given\n>> directory. In the last case, it will look for the entry \"b/c\" in\n>> directory \"a\" instead. This means if you delete or add an entry in a\n>> subdirectory, untracked cache may become out of date because it does not\n>> invalidate properly. This is noticed by David Turner.\n>> \n>> The second problem is about invalidation inside a fully untracked/excluded\n>> directory. In this case we may have to invalidate back to root. See the\n>> comment block for detail.\n>\n> LGTM.\n\nThanks, both.  Let's move this forward.\n"}]}