v3Add an explicit GIT_DIR to the list of excludes

> diff --git a/dir.c b/dir.c
> index 98bb50f..76969a7 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -1588,6 +1588,26 @@ void setup_standard_excludes(struct dir_struct *dir)
>  {
>  	const char *path;
>  	char *xdg_path;
> +	const char *gitdir = get_git_dir();
> +
> +	/* Add git directory to the ignores first */
> +	if (strcmp(gitdir, ".git") != 0) {

> +		/*
> +		 * See if GIT_DIR is inside the work tree; need to normalize
> +		 * 'gitdir', whereas 'get_git_work_tree()' always appears
> +		 * absolute and normalized
> +		 */
> +		normalize_path_copy(ngit, real_path(absolute_path(gitdir)));
> +
> +		if (dir_inside_of(ngit, get_git_work_tree()) >= 0) {
> +			struct exclude_list *el = add_exclude_list(dir, EXC_CMDL,
> +							"--git-dir option");
> +
> +			add_exclude(gitdir, "", 0, el, 0);
> +		}
> +	}
>  
>  	dir->exclude_per_dir = ".gitignore";
>  	path = git_path("info/exclude");
> diff --git a/t/t2205-add-gitdir.sh b/t/t2205-add-gitdir.sh
> new file mode 100755
> index 0000000..0c99508
> --- /dev/null
> +++ b/t/t2205-add-gitdir.sh
> @@ -0,0 +1,84 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2014 Pasha Bolokhov
> +#
> +
> +test_description='alternative repository path specified by --git-dir is ignored by add and status'
> +
> +. ./test-lib.sh
> +
> +#
> +# Create a tree:
> +#
> +# 	a  b  c  d  subdir/
> +#
> +# subdir:
> +# 	e  f  g  h  meta/  ssubdir/
> +#
> +# subdir/meta:
> +# 	aa
> +#
> +# subdir/ssubdir:
> +# 	meta/
> +#
> +# subdir/ssubdir/meta:
> +# 	aaa
> +#
> +# Name the repository "meta" and see whether or not "git status" includes
> +# or ignores directories named "meta". The slightly deeper hierarchy is
> +# needed in order to be able to put the repository into "../meta", that is,
> +# outside the work tree and still have files called "meta" within the tree
> +#

>> +#
>> +# Create a tree:
>> +#
>> +#    a  b  c  d  subdir/
>> +#
>> +# subdir:
>> +#    e  f  g  h  meta/  ssubdir/
>> +#
>> +# subdir/meta:
>> +#    aa
>> +#
>> +# subdir/ssubdir:
>> +#    meta/
>> +#
>> +# subdir/ssubdir/meta:
>> +#    aaa
>> +#
> It is not quite clear with this large blob of comment what are
> noises and what exactly are being tested.  I think you have two
> directories called "meta", but which one is the repository?  Or do
> you have yet another one next to {a,b,c,d,subdir} called "meta" that
> is not listed above?
>
> Given that the reason why people use --git-dir is so that they can
> put it completely outside the working tree (in which case, the usual
> "start from cwd and go upwards while trying to see if there is .git/
> that governs the working tree" logic would not work), readers would
> not expect to find the directory to be used as GIT_DIR in the
> hierarchy you are creating in the first place.  Because of that, it
> is even more important to clearify which "meta" you mean to use as
> your GIT_DIR if you want to be understood by readers.

> When an explicit '--git-dir' option points to a directory inside
> the work tree, git treats it as if it were any other directory.
> In particular, 'git status' lists it as untracked, while 'git add -A'
> stages the metadata directory entirely
>
> Add GIT_DIR to the list of excludes in setup_standard_excludes(),
> while checking that GIT_DIR is not just '.git', in which case it
> would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE
>
> Although an analogous comparison of any given path against '.git'
> is done in treat_path(), this does not seem to be the right place
> to compare against GIT_DIR. Instead, the excludes provide an
> effective mechanism of ignoring a file/directory, and adding GIT_DIR
> as an exclude is equivalent of putting it into '.gitignore'. Function
> setup_standard_excludes() was chosen because that is the place where
> the excludes are initialized by the commands that are concerned about
> excludes

>> Add GIT_DIR to the list of excludes in setup_standard_excludes(),
>> while checking that GIT_DIR is not just '.git', in which case it
>> would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE
>>
> gives git-grep.txt and git-ls-files.txt. I don't know if we need to
> add something about this extra exclude rule to those .txt. If it's so
> obvious that this should be the expected behavior, then probably not.

>>> Add GIT_DIR to the list of excludes in setup_standard_excludes(),
>>> while checking that GIT_DIR is not just '.git', in which case it
>>> would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE
>>>
>> gives git-grep.txt and git-ls-files.txt. I don't know if we need to
>> add something about this extra exclude rule to those .txt. If it's so
>> obvious that this should be the expected behavior, then probably not.
>
>      I suggest this. There appears to be a notion of "standard
> excludes" both in the code (dir.c) and in the man pages (git-grep,
> git-ls-files). However, it doesn't actually appear to be defined
> strictly speaking. So my suggestion is to define those "standard
> excludes" in one place (say "gitignore(5)"), and make other man pages
> (git-config, git-grip, git-ls-files) have references to that place
> instead of explaining every time in detail what is being excluded.

> W dniu 2014-05-27 19:16, Pasha Bolokhov pisze:
>
>>>> Add GIT_DIR to the list of excludes in setup_standard_excludes(),
>>>> while checking that GIT_DIR is not just '.git', in which case it
>>>> would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE
>>>>
>>> gives git-grep.txt and git-ls-files.txt. I don't know if we need to
>>> add something about this extra exclude rule to those .txt. If it's so
>>> obvious that this should be the expected behavior, then probably not.
>>
>>
>>      I suggest this. There appears to be a notion of "standard
>> excludes" both in the code (dir.c) and in the man pages (git-grep,
>> git-ls-files). However, it doesn't actually appear to be defined
>> strictly speaking. So my suggestion is to define those "standard
>> excludes" in one place (say "gitignore(5)"), and make other man pages
>> (git-config, git-grip, git-ls-files) have references to that place
>> instead of explaining every time in detail what is being excluded.
>
>
> Or define those "standard excludes" in standard-excludes.txt "partial",
> and include said file from git-grep(1), git-ls-files(1), and perhaps
> gitignore(5).
>
> --
> Jakub Narębski
>
>

>>>      I suggest this. There appears to be a notion of "standard
>>> excludes" both in the code (dir.c) and in the man pages (git-grep,
>>> git-ls-files). However, it doesn't actually appear to be defined
>>> strictly speaking. So my suggestion is to define those "standard
>>> excludes" in one place (say "gitignore(5)"), and make other man pages
>>> (git-config, git-grip, git-ls-files) have references to that place
>>> instead of explaining every time in detail what is being excluded.
>>
>>
>> Or define those "standard excludes" in standard-excludes.txt "partial",
>> and include said file from git-grep(1), git-ls-files(1), and perhaps
>> gitignore(5).

> On Sat, May 24, 2014 at 12:33 AM, Pasha Bolokhov
> <pasha.bolokhov@gmail.com> wrote:
>> When an explicit '--git-dir' option points to a directory inside
>> the work tree, git treats it as if it were any other directory.
>> In particular, 'git status' lists it as untracked, while 'git add -A'
>> stages the metadata directory entirely
>>
>> Add GIT_DIR to the list of excludes in setup_standard_excludes(),
>> while checking that GIT_DIR is not just '.git', in which case it
>> would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE
>>
>> Although an analogous comparison of any given path against '.git'
>> is done in treat_path(), this does not seem to be the right place
>> to compare against GIT_DIR. Instead, the excludes provide an
>> effective mechanism of ignoring a file/directory, and adding GIT_DIR
>> as an exclude is equivalent of putting it into '.gitignore'. Function
>> setup_standard_excludes() was chosen because that is the place where
>> the excludes are initialized by the commands that are concerned about
>> excludes
>
> I like this approach. A search of "exclude-standard" in Documentation/
> gives git-grep.txt and git-ls-files.txt. I don't know if we need to
> add something about this extra exclude rule to those .txt. If it's so
> obvious that this should be the expected behavior, then probably not.

>
> The case of "git grep --exclude-standard" is interesting because it's
> intended to work without a repository. First reaction was would
> get_git_dir() return NULL in that case. But it should return ".git" so
> we're good.

> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Sat, May 24, 2014 at 12:33 AM, Pasha Bolokhov
>> <pasha.bolokhov@gmail.com> wrote:
>>> When an explicit '--git-dir' option points to a directory inside
>>> the work tree, git treats it as if it were any other directory.
>>> In particular, 'git status' lists it as untracked, while 'git add -A'
>>> stages the metadata directory entirely
>>>
>>> Add GIT_DIR to the list of excludes in setup_standard_excludes(),
>>> while checking that GIT_DIR is not just '.git', in which case it
>>> would be ignored by default, and that GIT_DIR is inside GIT_WORK_TREE
>>>
>>> Although an analogous comparison of any given path against '.git'
>>> is done in treat_path(), this does not seem to be the right place
>>> to compare against GIT_DIR. Instead, the excludes provide an
>>> effective mechanism of ignoring a file/directory, and adding GIT_DIR
>>> as an exclude is equivalent of putting it into '.gitignore'. Function
>>> setup_standard_excludes() was chosen because that is the place where
>>> the excludes are initialized by the commands that are concerned about
>>> excludes
>>
>> I like this approach. A search of "exclude-standard" in Documentation/
>> gives git-grep.txt and git-ls-files.txt. I don't know if we need to
>> add something about this extra exclude rule to those .txt. If it's so
>> obvious that this should be the expected behavior, then probably not.
>
> OK, so is that an Acked/Reviewed-by?
>
>>
>> The case of "git grep --exclude-standard" is interesting because it's
>> intended to work without a repository. First reaction was would
>> get_git_dir() return NULL in that case. But it should return ".git" so
>> we're good.