On Mon, Dec 2, 2013 at 6:38 AM, Dennis Kaarsemaker <den...@kaarsemaker.net> wrote: > On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote: >> Duy Nguyen <pclo...@gmail.com> writes: >> >> > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker >> > <den...@kaarsemaker.net> wrote: >> >> We always ignore anything named .git, but we should also ignore the git >> >> directory if the user overrides it by setting $GIT_DIR >> [...] >> >> + if (simplify_away(path->buf, path->len, simplify) || >> >> is_git_directory(path->buf)) >> >> return path_none; >> > >> > this adds 2 access, 1 lstat, 1 open, 1 read, 1 close to _every_ path >> > we check. Is it worth the cost? >> >> Moreover it is a much more inclusive check than what the commit message >> claims: it will ignore anything that looks like a .git directory, >> regardless of the name. In particular GIT_DIR doesn't have anything to >> do with it. > > Ah, yes thanks, that's rather incorrect indeed. How about the following > instead? Passes all tests, including the new one. > > --- a/dir.c > +++ b/dir.c > @@ -1198,7 +1198,7 @@ static enum path_treatment treat_path(struct dir_struct > *dir, > return path_none; > strbuf_setlen(path, baselen); > strbuf_addstr(path, de->d_name); > - if (simplify_away(path->buf, path->len, simplify)) > + if (simplify_away(path->buf, path->len, simplify) || > !strncmp(get_git_dir(), path->buf, path->len)) > return path_none;
get_git_dir() may return a relative or absolute path, depending on GIT_DIR/GIT_WORK_TREE. path->buf is always relative. You'll pass one case with this (relative vs relative) and fail another. It might be simpler to just add get_git_dir(), after converting to relative path and check if it's in worktree, to the exclude list and let the current exclude mechanism handle it. -- Duy -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
