Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

From: Johannes Sixt <j.sixt@viscovery.net>
Date: May 19, 2008, 07:55 UTC
Message-ID: <48313258.5010208@viscovery.net>
In-Reply-To: <alpine.DEB.1.00.0805170117000.30431@racer>

Johannes Schindelin schrieb:

Show 41 quoted lines

> Hi,
> 
> On Fri, 16 May 2008, David Reiss wrote:
> 
>> Sorry I missed this before.  As you said, in Johannes's version the 
>> ceiling directories are the last directories we look in, whereas in my 
>> implementation, they are the first directories we do not look in.  I 
>> made this choice because it makes more sense for me to set my ceiling to 
>> "/home", rather than "/home/dreiss", so it will work even if I am in 
>> another user's homedir.
>>
>> There is also a difference in how they handle the case where the cwd is 
>> a ceiling directory, but I think it is worth sorting out the first issue 
>> first.
> 
> Please do not top-post.
> 
> This is the interdiff to the last squashed patch:
> 
> -- snip --
> diff --git a/setup.c b/setup.c
> index cece3e4..2f7a17a 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -441,6 +441,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  	 */
>  	offset = len = strlen(cwd);
>  	for (;;) {
> +		if (offset <= min_offset)
> +			goto non_git;
>  		gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
>  		if (gitfile_dir) {
>  			if (set_git_dir(gitfile_dir))
> @@ -460,6 +462,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  		chdir("..");
>  		do {
>  			if (offset <= min_offset) {
> +non_git:
>  				if (nongit_ok) {
>  					if (chdir(cwd))
>  						die("Cannot come back to cwd");

Hmm... If the implementation needs a 'goto', then I have the strong suspicion that there's already something wrong at the concept level.

I actually like the previous version better because of its clearer semantics:

- The current directory is always checked.
- GIT_CEILING_DIRECTORIES are checked. Consequently, setting the variable
to the empty string is equivalent to not setting it at all.

(but it means that David can't have what he wants, i.e. he must set GIT_CEILING_DIRECTORIES=/home/dreiss.)

This implementation:

- Never checks the root directory, even if it is the current directory.
- Otherwise always checks the current directory, even if it is mentioned
in GIT_CEILING_DIRECTORIES.

[That said, I'm not in strong support of this feature in general - I'm just caring because *if* it goes in, it will have conflicts with the mingw branch.]

-- Hannes