On ma, 2013-12-02 at 16:35 +0700, Duy Nguyen wrote: > On Mon, Dec 2, 2013 at 3:01 PM, Dennis Kaarsemaker > <den...@kaarsemaker.net> wrote: > > On ma, 2013-12-02 at 07:38 +0700, Duy Nguyen wrote: > >> 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. > > > > 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. > > ~/w/git $ cd t > ~/w/git/t $ GIT_TRACE_SETUP=1 ../git --git-dir=../.git --work-tree=.. > --no-pager status > setup: git_dir: /home/pclouds/w/git/.git > setup: worktree: /home/pclouds/w/git > setup: cwd: /home/pclouds/w/git > setup: prefix: t/ > On branch exclude-pathspec > Your branch and 'origin/master' have diverged, > and have 2 and 5 different commits each, respectively. > > I can't say this is the only case though. One has to audit to all > possible setup cases in setup_git_directory() to make that claim.
I'm probably missing something, but that's the same as my second example, and works. I also tried running it from completely outside the repo: dennis@lightning:~$ code/git/git --git-dir=code/git/.foo --work-tree=code/git status On branch master nothing to commit, working directory clean dennis@lightning:~$ code/git/git --git-dir=/home/dennis/code/git/.foo --work-tree=code/git status On branch master nothing to commit, working directory clean -- Dennis Kaarsemaker www.kaarsemaker.net -- 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
