Greetings,
I found this probable bug: https://gist.github.com/anonymous/01979fd9e6e285df41a2
Cheers, Ingy döt Net
Dennis KaarsemakerRe: GIT_DIR not auto ignored
On za, 2013-11-30 at 23:06 -0800, Ingy dot Net wrote:
> Greetings, > > I found this probable bug: > https://gist.github.com/anonymous/01979fd9e6e285df41a2
Summary:
$ mv .git .foo $ export GIT_DIR=$PWD/.foo $ git status # On branch master # # Initial commit # # Untracked files: # .foo/ nothing added to commit but untracked files present
I checked with 1.8.5 and this still happens. And this also happens:
$ mv .git .foo $ export GIT_DIR=.foo dennis@lightning:~/code/git$ touch .git dennis@lightning:~/code/git$ git status On branch master Untracked files: (use "git add <file>..." to include in what will be committed)
.foo/
nothing added to commit but untracked files present (use "git add" to track)
(Note the absence of .git there)
-- Dennis Kaarsemaker www.kaarsemaker.net
Dennis KaarsemakerRe: GIT_DIR not auto ignored
On zo, 2013-12-01 at 19:08 +0100, Dennis Kaarsemaker wrote:
Show 21 quoted lines
> On za, 2013-11-30 at 23:06 -0800, Ingy dot Net wrote: > > Greetings, > > > > I found this probable bug: > > https://gist.github.com/anonymous/01979fd9e6e285df41a2 > > Summary: > > $ mv .git .foo > $ export GIT_DIR=$PWD/.foo > $ git status > # On branch master > # > # Initial commit > # > # Untracked files: > # .foo/ > nothing added to commit but untracked files present > > > I checked with 1.8.5 and this still happens.
This makes it go away:
diff --git a/dir.c b/dir.c index 23b6de4..884b37d 100644 --- 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) || is_git_directory(path->buf)) return path_none; dtype = DTYPE(de); I'll add a test and submit a proper patch. > And this also happens: > > $ mv .git .foo > $ export GIT_DIR=.foo > dennis@lightning:~/code/git$ touch .git > dennis@lightning:~/code/git$ git status > On branch master > Untracked files: > (use "git add <file>..." to include in what will be committed) > > .foo/ > > nothing added to commit but untracked files present (use "git add" to > track) > > (Note the absence of .git there) Comments in dir.c indicate that this is expected, so I didn't try to "fix" that.
-- Dennis Kaarsemaker www.kaarsemaker.net
Dennis Kaarsemaker[PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
We always ignore anything named .git, but we should also ignore the git directory if the user overrides it by setting $GIT_DIR
Reported-By: Ingy döt Net <ingy@ingy.net> Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net> --- dir.c | 2 +- t/t7508-status.sh | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/dir.c b/dir.c index 23b6de4..884b37d 100644 --- 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) || is_git_directory(path->buf)) return path_none; dtype = DTYPE(de); diff --git a/t/t7508-status.sh b/t/t7508-status.sh index c987b5e..2bd7ef1 100755 --- a/t/t7508-status.sh +++ b/t/t7508-status.sh @@ -198,6 +198,13 @@ test_expect_success 'status -s' ' ' +test_expect_success 'status -s with non-standard $GIT_DIR' ' + mv .git .foo && + GIT_DIR=.foo git status -s >output && + test_cmp expect output && + mv .foo .git +' + test_expect_success 'status with gitignore' ' { echo ".gitignore" &&
-- 1.8.5-386-gb78cb96 -- Dennis Kaarsemaker <dennis@kaarsemaker.net> http://twitter.com/seveas
Duy NguyenRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:
Show 21 quoted lines
> We always ignore anything named .git, but we should also ignore the git > directory if the user overrides it by setting $GIT_DIR > > Reported-By: Ingy döt Net <ingy@ingy.net> > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net> > --- > dir.c | 2 +- > t/t7508-status.sh | 7 +++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/dir.c b/dir.c > index 23b6de4..884b37d 100644 > --- 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) || 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?
Show 31 quoted lines
>
> dtype = DTYPE(de);
> diff --git a/t/t7508-status.sh b/t/t7508-status.sh
> index c987b5e..2bd7ef1 100755
> --- a/t/t7508-status.sh
> +++ b/t/t7508-status.sh
> @@ -198,6 +198,13 @@ test_expect_success 'status -s' '
>
> '
>
> +test_expect_success 'status -s with non-standard $GIT_DIR' '
> + mv .git .foo &&
> + GIT_DIR=.foo git status -s >output &&
> + test_cmp expect output &&
> + mv .foo .git
> +'
> +
> test_expect_success 'status with gitignore' '
> {
> echo ".gitignore" &&
> --
> 1.8.5-386-gb78cb96
>
>
> --
> Dennis Kaarsemaker <dennis@kaarsemaker.net>
> http://twitter.com/seveas
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html-- Duy
Thomas RastRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
Duy Nguyen <pclouds@gmail.com> writes:
> On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker > <dennis@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
[...]
Show 5 quoted lines
>> + 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.
-- Thomas Rast tr@thomasrast.ch
Dennis KaarsemakerRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote:
Show 17 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes: > > > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker > > <dennis@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;
-- Dennis Kaarsemaker www.kaarsemaker.net
Duy NguyenRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On Mon, Dec 2, 2013 at 6:38 AM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:
Show 31 quoted lines
> On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote: >> Duy Nguyen <pclouds@gmail.com> writes: >> >> > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker >> > <dennis@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
Dennis KaarsemakerRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On ma, 2013-12-02 at 07:38 +0700, Duy Nguyen wrote:
Show 40 quoted lines
> On Mon, Dec 2, 2013 at 6:38 AM, Dennis Kaarsemaker > <dennis@kaarsemaker.net> wrote: > > On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote: > >> Duy Nguyen <pclouds@gmail.com> writes: > >> > >> > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker > >> > <dennis@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.
-- Dennis Kaarsemaker www.kaarsemaker.net
Duy NguyenRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On Mon, Dec 2, 2013 at 3:01 PM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:
Show 63 quoted lines
> On ma, 2013-12-02 at 07:38 +0700, Duy Nguyen wrote: >> On Mon, Dec 2, 2013 at 6:38 AM, Dennis Kaarsemaker >> <dennis@kaarsemaker.net> wrote: >> > On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote: >> >> Duy Nguyen <pclouds@gmail.com> writes: >> >> >> >> > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker >> >> > <dennis@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.
-- Duy
Dennis KaarsemakerRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On ma, 2013-12-02 at 16:35 +0700, Duy Nguyen wrote:
Show 79 quoted lines
> On Mon, Dec 2, 2013 at 3:01 PM, Dennis Kaarsemaker > <dennis@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 > >> <dennis@kaarsemaker.net> wrote: > >> > On ma, 2013-12-02 at 00:08 +0100, Thomas Rast wrote: > >> >> Duy Nguyen <pclouds@gmail.com> writes: > >> >> > >> >> > On Mon, Dec 2, 2013 at 2:04 AM, Dennis Kaarsemaker > >> >> > <dennis@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
Duy NguyenRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On Mon, Dec 2, 2013 at 6:40 PM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:
Show 24 quoted lines
>> ~/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
It looks like we try to convert git_dir relative to work_tree (in setup_work_tree) so get_git_dir() probably always returns a path relative to worktree if it's set. I don't know, it looks like so.
-- Duy
Eric SunshineRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
On Sun, Dec 1, 2013 at 2:04 PM, Dennis Kaarsemaker <dennis@kaarsemaker.net> wrote:
Show 13 quoted lines
> diff --git a/t/t7508-status.sh b/t/t7508-status.sh > index c987b5e..2bd7ef1 100755 > --- a/t/t7508-status.sh > +++ b/t/t7508-status.sh > @@ -198,6 +198,13 @@ test_expect_success 'status -s' ' > > ' > > +test_expect_success 'status -s with non-standard $GIT_DIR' ' > + mv .git .foo && > + GIT_DIR=.foo git status -s >output && > + test_cmp expect output && > + mv .foo .git
If test_cmp returns a failure status, the following 'mv' command will never be invoked, thus all subsequent tests in the script will fail since the .git directory will be missing. Instead, use test_when_finished to restore .git from .foo.
Show 7 quoted lines
> +'
> +
> test_expect_success 'status with gitignore' '
> {
> echo ".gitignore" &&
> --
> 1.8.5-386-gb78cb96
Karsten BleesRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
Am 01.12.2013 20:04, schrieb Dennis Kaarsemaker:
Show 15 quoted lines
> We always ignore anything named .git, but we should also ignore the git > directory if the user overrides it by setting $GIT_DIR > > Reported-By: Ingy döt Net <ingy@ingy.net> > Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net> > --- > dir.c | 2 +- > t/t7508-status.sh | 7 +++++++ > 2 files changed, 8 insertions(+), 1 deletion(-) > > diff --git a/dir.c b/dir.c > index 23b6de4..884b37d 100644 > --- a/dir.c > +++ b/dir.c > @@ -1198,7 +1198,7 @@ static enum path_treatment treat_path(struct dir_struct *dir,
The special case for ".git" is hardcoded in many places in git, including the line immediately above this diff hunk. So I figure that GIT_DIR is not meant to _rename_ the ".git" dir, but to point somewhere _outside_ the worktree (or somewhere within the .git dir).
If we want to support the rename case fully, I think there are a few more questions to answer (and a few more places to change), e.g.: - What if GIT_DIR=.foo and someone upstream adds a ".foo" directory? - Should it be possible to track ".git" as a normal file or directory if its not the GIT_DIR? - What about other commands than status, e.g. does 'git clean -df' leave the GIT_DIR alone?
If we don't want to support this, though, I think it would be more approrpiate to issue a warning if GIT_DIR points to a worktree location.
Junio C HamanoRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
Karsten Blees <karsten.blees@gmail.com> writes:
>So I figure that GIT_DIR is not meant to _rename_ the ".git" dir, >but to point somewhere _outside_ the worktree (or somewhere within >the .git dir).
Correct.
> If we don't want to support this, though, I think it would be more > approrpiate to issue a warning if GIT_DIR points to a worktree > location.
But how do tell what is and isn't a "worktree location"? Having the path in the index would be one, but you may find it out only after issuing "git checkout $antient_commit".
Karsten BleesRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
Am 03.12.2013 19:32, schrieb Junio C Hamano:
Show 16 quoted lines
> Karsten Blees <karsten.blees@gmail.com> writes: > >> So I figure that GIT_DIR is not meant to _rename_ the ".git" dir, >> but to point somewhere _outside_ the worktree (or somewhere within >> the .git dir). > > Correct. > >> If we don't want to support this, though, I think it would be more >> approrpiate to issue a warning if GIT_DIR points to a worktree >> location. > > But how do tell what is and isn't a "worktree location"? Having the > path in the index would be one, but you may find it out only after > issuing "git checkout $antient_commit". >
In setup_work_tree(), the result of remove_leading_path(git_dir, work_tree) must be absolute or start with ".." or ".git", otherwise warn?
Jonathan NiederRe: [PATCH] path_treatment: also ignore $GIT_DIR if it's not .git
Junio C Hamano wrote:
> Karsten Blees <karsten.blees@gmail.com> writes:
Show 7 quoted lines
>> If we don't want to support this, though, I think it would be more >> approrpiate to issue a warning if GIT_DIR points to a worktree >> location. > > But how do tell what is and isn't a "worktree location"? Having the > path in the index would be one, but you may find it out only after > issuing "git checkout $antient_commit".
I think the idea was that *any* path under $(git rev-parse --show-toplevel) would not be a valid GIT_DIR, unless its last path component is ".git".
Alas, I don't think that would work smoothly.
- Some people may already be using GIT_DIR=$HOME/dotfiles.git to track some files with a toplevel of $HOME. That is error-prone and it would be cleaner to either use plain .git or keep the $GIT_DIR outside the worktree (for example by tucking dotfiles into a separate $HOME/dotfiles dir), true, but producing a noisy warning with no way out would not serve these people well.
- There is no outside-the-worktree location when GIT_WORK_TREE=/.
So your suggestion of at least noticing when "git checkout" wants to write files that overlap with the GIT_DIR seems simpler.