On ma, 2013-12-02 at 07:38 +0700, Duy Nguyen wrote: > On Mon, Dec 2, 2013 at 6:38 AM, Dennis Kaarsemaker > <dennis@xxxxxxxxxxxxxxx> wrote: > > On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote: > >> Duy Nguyen <pclouds@xxxxxxxxx> writes: > >> > >> > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker > >> > <dennis@xxxxxxxxxxxxxxx> 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. This type of invocation really only works from the root of the workdir anyway and both a relative and absolute path work just fine: dennis@lightning:~/code/git$ GIT_DIR=$(pwd)/.foo ./git status On branch master nothing to commit, working directory clean dennis@lightning:~/code/git$ GIT_DIR=./.foo ./git status On branch master nothing to commit, working directory clean Well, unless you set GIT_WORK_TREE as well, but then it still works: dennis@lightning:~/code/git/t$ GIT_DIR=$(pwd)/../.foo GIT_WORK_TREE=.. ../git status On branch master nothing to commit, working directory clean dennis@lightning:~/code/git/t$ GIT_DIR=../.foo GIT_WORK_TREE=.. ../git status On branch master nothing to commit, working directory clean So I'm wondering when you think this will fail. Because then I can add a test for that case too. -- Dennis Kaarsemaker www.kaarsemaker.net -- 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