Re: [PATCH v3] untracked-cache: fix subdirectory handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Aug 16, 2015 at 12:17 PM, David Turner <dturner@xxxxxxxxxxxxxxxx> 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@xxxxxxxxx>
> Signed-off-by: David Turner <dturner@xxxxxxxxxxxxxxxx>
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@xxxxxxxxx>
> ---
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]