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

[Duy Nguyen <pclouds@gmail.com>]

First of all, sorry for my long silence. I'm going through my git
inbox now.

> diff --git a/dir.c b/dir.c
> index e7b89fe..314080b 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -631,6 +631,7 @@ static struct untracked_cache_dir *lookup_untracked(struct untracked_cache *uc,
>  	memset(d, 0, sizeof(*d));
>  	memcpy(d->name, name, len);
>  	d->name[len] = '\0';
> +	d->depth = dir->depth + 1;
>  
>  	ALLOC_GROW(dir->dirs, dir->dirs_nr + 1, dir->dirs_alloc);
>  	memmove(dir->dirs + first + 1, dir->dirs + first,
> @@ -1324,7 +1325,19 @@ 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);
> +	if (untracked) {
> +		const char *cur = dirname;
> +		int i;
> +
> +		for (i = 0; i < untracked->depth; i++) {
> +			cur = strchr(cur, '/');
> +			assert(cur);
> +			cur++;
> +		}
> +		untracked = lookup_untracked(dir->untracked, untracked,
> +					     cur,
> +					     len - (cur - dirname));
> +	}

If I read it correctly, the call chain is, treat_path() reconstructs
full path, then it calls treat_one_path -> treat_directory ->
lookup_untracked.

In the same treat_path(), we may also call treat_fast_path ->
read_directory_recursive -> lookup_untracked. In this call chain, we
retain the "baselen" and we can exclude the base path before passing
it to lookup_untracked().

So instead of adding "depth" (and spending some more cycles cutting
path components), perhaps we can do the same for the first call chain,
passing baselen to treat_one_path and treat_directory?  Something like
this passes your new tests

-- 8< --
diff --git a/dir.c b/dir.c
index 07a6642..a4c52bf 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) {
-- 8< --

> +static struct untracked_cache_dir *lookup_untracked_recursive(
> +	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);
> +		return lookup_untracked_recursive(uc, d, rest + 1,
> +						  len - (component_len + 1));
> +	} else {
> +		return dir;
> +	}
> +}
> +
>  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;
> +	d = lookup_untracked_recursive(istate->untracked,
> +				       istate->untracked->root,
> +				       path, strlen(path));

This is totally my fault. It's so obvious, how could I miss
it.. Thanks for fixing.

By the way, while running the tests, I noticed that the test "set up
sparse checkout" in t7063 performs the time-consuming "Testing
...... OK" again. At this point in the test suite, you are already
certain the underlying filesystem supports untracked-cached, maybe do
this to reduce test time for people who do not run tests in parallel?

s/update-index --untracked-cache/update-index --force-untracked-cache/

--
Duy