Re: [PATCH v3] Add an explicit GIT_DIR to the list of excludes

[Junio C Hamano <gitster@pobox.com>]

Pasha Bolokhov <pasha.bolokhov@gmail.com> writes:

> 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) {

It is more idiomatic and common to say

	if (strcmp(gitdir, ".git")) {

around here, I think.

>     /* "--git-dir" has been given */

... or it could have come from GIT_DIR environment, no?

Does this "additional exclude" need to kick in if GIT_DIR is set to
"/home/pasha/w/.git"?  That is, when gitdir is ".git" or ends with
"/.git"?

> +		char ngit[PATH_MAX + 1];

Hmmm.  As I recall that recently we had flurry of changes to
eradicate PATH_MAX from our codebase, I am not happy to see an
introduction of a new buffer that is fixed-size.

> +		/*
> +		 * 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
> +#

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.