Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Thu, 9 Jul 2009, Junio C Hamano wrote:
>>
>> I was wondering if we could also say that D exists as a directory when we
>> know there is D/F in the index and is up to date.
>
> Yeah, that would probably be a good thing, but is slightly slower to look
> up (we have the name hashing for the case-ignoring code anyway, but that
> only works for exact names, so you can't look up directories that way).
>
> You'd have to use the regular binary search for that (or we'd have to
> change it to hash directories too - which we might want to do for other
> reasons, but don't do now).
>
> Something like this?
Yeah, in Dmitry's response that crossed with this update patch from you,
he says lstat() on directories are still problem---it would be interesting to
hear what he sees after applying this patch and retesting.
> +static int get_index_mode(const char *path, int len)
> +{
> + int pos;
> + struct cache_entry *ce;
> +
> + ce = cache_name_exists(path, len, 0);
> + if (ce) {
> + if (ce_uptodate(ce))
> + return ce->ce_mode;
You return ce->ce_mode for up-to-date entries. I do not remember what
ce_uptodate(ce) says for gitlinks, but ce->ce_mode for them would be
160000 that is not very kosher to give to S_ISDIR(). I realize that this
worry actually applies to your patch from yesterday, the one Dmitry
already tested.
> + return 0;
> + }
> +
> + /* Try to look it up as a directory */
> + pos = cache_name_pos(path, len);
> + if (pos >= 0)
> + return 0;
How can this find an exact entry for the path? Assuming that the name
hash cache_name_exists() is not out of sync? Shouldn't this be a BUG()
instead of "It somehow exists as a blob or submodule, and we'll let the
regular lstat() codepath take care of it by returning 0"?
> + pos = -pos-1;
> + while (pos < active_nr) {
> + ce = active_cache[pos++];
> + if (strncmp(ce->name, path, len))
> + break;
> + if (ce->name[len] > '/')
> + break;
> + if (ce->name[len] < '/')
> + continue;
> + if (!ce_uptodate(ce))
> + break; /* continue? */
I think this should be continue, as the directory D you are interested in
may have two files, one modified, the other uptodate.
> + return S_IFDIR;
> + }
> + return 0;
> +}
> +
> static int get_dtype(struct dirent *de, const char *path, int len)
> {
> int dtype = de ? DTYPE(de) : DT_UNKNOWN;
> - struct cache_entry *ce;
> struct stat st;
>
> if (dtype != DT_UNKNOWN)
> return dtype;
> - ce = cache_name_exists(path, len, 0);
> - if (ce && ce_uptodate(ce))
> - st.st_mode = ce->ce_mode;
> - else if (lstat(path, &st))
> + st.st_mode = get_index_mode(path, len);
> + if (!st.st_mode && lstat(path, &st))
> return dtype;
> if (S_ISREG(st.st_mode))
> return DT_REG;
--
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