From: Duy Nguyen <pclouds@gmail.com>
To: David Turner <dturner@twopensource.com>
Cc: git@vger.kernel.org
Subject: Re: [PATCH v2] untracked-cache: fix subdirectory handling
Date: Sat, 15 Aug 2015 14:32:19 +0700 [thread overview]
Message-ID: <20150815073219.GA4496@lanh> (raw)
In-Reply-To: <1438967873-792-1-git-send-email-dturner@twopensource.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
prev parent reply other threads:[~2015-08-15 7:30 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-08-07 17:17 [PATCH v2] untracked-cache: fix subdirectory handling David Turner
2015-08-15 7:32 ` Duy Nguyen [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150815073219.GA4496@lanh \
--to=pclouds@gmail.com \
--cc=dturner@twopensource.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).