[PATCH 2/2] Add support for GIT_CEILING_DIRS

[PATCH 2/2] Add support for GIT_CEILING_DIRS

[David Reiss]

Make git recognize a new environment variable that prevents it from
chdir'ing up into specified directories when looking for a GIT_DIR.
Useful for avoiding slow network directories.

Signed-off-by: David Reiss <dreiss@facebook.com>
---
Just a bit of context about the motivation for this.  I use git in an
environment where homedirs are automounted and "ls /home/nonexistent"
takes about 9 seconds.  I use the git bash completion, which runs
"git help -a".  As of Git 1.5.5, this calls setup_git_directory_gently,
which searches /home/.git and /home/objects, which takes forever,
delaying all of my logins.  This patch lets me set
"GIT_CEILING_DIRS=/home", which makes this operation fast.  As a bonus,
the "git symbolic-ref HEAD" in my shell prompt now works quickly when
I'm in a non-git directory.

 Documentation/git.txt   |    8 ++++
 cache.h                 |    1 +
 setup.c                 |   67 +++++++++++++++++++++++++-----
 t/t1504-ceiling-dirs.sh |  104 +++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh           |    1 +
 5 files changed, 170 insertions(+), 11 deletions(-)
 create mode 100755 t/t1504-ceiling-dirs.sh

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6f445b1..8aea331 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -415,6 +415,14 @@ git so take care if using Cogito etc.
 	This can also be controlled by the '--work-tree' command line
 	option and the core.worktree configuration variable.
 
+'GIT_CEILING_DIRS'::
+	This should be a colon-separated list of absolute paths.
+	If set, it is a list of directories that git should not chdir
+	up into while looking for a repository directory.
+	It will not exclude the current working directory or
+	a GIT_DIR set on the command line or in the environment.
+	(Useful for excluding slow-loading network directories.)
+
 git Commits
 ~~~~~~~~~~~
 'GIT_AUTHOR_NAME'::
diff --git a/cache.h b/cache.h
index 9cee9a5..8300acc 100644
--- a/cache.h
+++ b/cache.h
@@ -300,6 +300,7 @@ static inline enum object_type object_type(unsigned int mode)
 #define CONFIG_ENVIRONMENT "GIT_CONFIG"
 #define CONFIG_LOCAL_ENVIRONMENT "GIT_CONFIG_LOCAL"
 #define EXEC_PATH_ENVIRONMENT "GIT_EXEC_PATH"
+#define CEILING_DIRS_ENVIRONMENT "GIT_CEILING_DIRS"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
diff --git a/setup.c b/setup.c
index c54f2b6..d7d986c 100644
--- a/setup.c
+++ b/setup.c
@@ -359,10 +359,11 @@ const char *read_gitfile_gently(const char *path)
 const char *setup_git_directory_gently(int *nongit_ok)
 {
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
+	const char *env_ceiling_dirs = getenv(CEILING_DIRS_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv;
 	const char *gitfile_dir;
-	int len, offset;
+	int len, offset, min_offset = -1;
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -414,6 +415,37 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
 
+	// Compute min_offset based on GIT_CEILING_DIRS.
+	if (env_ceiling_dirs) {
+		char *ceils, *ceil, *colon;
+		ceil = ceils = xstrdup(env_ceiling_dirs);
+		for (;;) {
+			int len;
+
+			if ((colon = strchr(ceil, ':')))
+				*colon = '\0';
+			len = strlen(ceil);
+
+			// "" would otherwise be treated like "/".
+			if (len) {
+				// Trim trailing slashes.
+				while (len && ceil[len-1] == '/')
+					ceil[--len] = '\0';
+
+				if (!strncmp(cwd, ceil, len) &&
+				    cwd[len] == '/' &&
+				    len > min_offset) {
+					min_offset = len;
+				}
+			}
+
+			if (!colon)
+				break;
+			ceil = colon + 1;
+		}
+		free(ceils);
+	}
+
 	/*
 	 * Test in the following order (relative to the cwd):
 	 * - .git (file containing "gitdir: <path>")
@@ -427,6 +459,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 */
 	offset = len = strlen(cwd);
 	for (;;) {
+		// Check the current directory (.git first).
 		gitfile_dir = read_gitfile_gently(DEFAULT_GIT_DIR_ENVIRONMENT);
 		if (gitfile_dir) {
 			if (set_git_dir(gitfile_dir))
@@ -443,17 +476,29 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}
-		do {
-			if (!offset) {
-				if (nongit_ok) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
-					*nongit_ok = 1;
-					return NULL;
-				}
-				die("Not a git repository");
+
+		// Did we just check the root dir?
+		if (!offset) {
+		not_a_repo:
+			if (nongit_ok) {
+				if (chdir(cwd))
+					die("Cannot come back to cwd");
+				*nongit_ok = 1;
+				return NULL;
 			}
-		} while (cwd[--offset] != '/');
+			die("Not a git repository");
+		}
+
+		while (cwd[--offset] != '/') {
+			assert(offset > 0);
+		}
+
+		// Don't chdir into the ceiling.
+		if (offset <= min_offset) {
+			assert(offset == min_offset);
+			goto not_a_repo;
+		}
+
 		chdir("..");
 	}
 
diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
new file mode 100755
index 0000000..d4eaa13
--- /dev/null
+++ b/t/t1504-ceiling-dirs.sh
@@ -0,0 +1,104 @@
+#!/bin/sh
+
+test_description='test GIT_CEILING_DIRS'
+. ./test-lib.sh
+
+test_prefix() {
+	test_expect_success "$1" \
+	"test '$2' = \"\$(git rev-parse --show-prefix)\""
+	shift
+	[ $# -eq 0 ] && return
+}
+
+test_fail() {
+	test_expect_code 128 "$1: prefix" \
+	"git rev-parse --show-prefix"
+	shift
+	[ $# -eq 0 ] && return
+}
+
+TRASH_ROOT="$(pwd)"
+ROOT_PARENT=$(dirname "$TRASH_ROOT")
+
+
+unset GIT_CEILING_DIRS
+test_prefix no_ceil ""
+
+export GIT_CEILING_DIRS=""
+test_prefix ceil_empty ""
+
+export GIT_CEILING_DIRS="$ROOT_PARENT"
+test_prefix ceil_at_parent ""
+
+export GIT_CEILING_DIRS="$ROOT_PARENT/"
+test_prefix ceil_at_parent_slash ""
+
+export GIT_CEILING_DIRS="$TRASH_ROOT"
+test_prefix ceil_at_trash ""
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/"
+test_prefix ceil_at_trash_slash ""
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub"
+test_prefix ceil_at_sub ""
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub/"
+test_prefix ceil_at_sub_slash ""
+
+
+mkdir -p sub/dir || exit 1
+cd sub/dir || exit 1
+
+unset GIT_CEILING_DIRS
+test_prefix subdir_no_ceil "sub/dir/"
+
+export GIT_CEILING_DIRS=""
+test_prefix subdir_ceil_empty "sub/dir/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT"
+test_fail subdir_ceil_at_trash
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/"
+test_fail subdir_ceil_at_trash_slash
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub"
+test_fail subdir_ceil_at_sub
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub/"
+test_fail subdir_ceil_at_sub_slash
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub/dir"
+test_prefix subdir_ceil_at_subdir "sub/dir/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub/dir/"
+test_prefix subdir_ceil_at_subdir_slash "sub/dir/"
+
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/su"
+test_prefix subdir_ceil_at_su "sub/dir/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/su/"
+test_prefix subdir_ceil_at_su_slash "sub/dir/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/subdi"
+test_prefix subdir_ceil_at_subdi "sub/dir/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/subdi"
+test_prefix subdir_ceil_at_subdi_slash "sub/dir/"
+
+
+export GIT_CEILING_DIRS="foo:$TRASH_ROOT/sub"
+test_fail second_of_two
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub:bar"
+test_fail first_of_two
+
+export GIT_CEILING_DIRS="foo:$TRASH_ROOT/sub:bar"
+test_fail second_of_three
+
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub"
+export GIT_DIR=../../.git
+test_prefix git_dir_specified ""
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 7c2a8ba..22899c1 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -35,6 +35,7 @@ unset GIT_WORK_TREE
 unset GIT_EXTERNAL_DIFF
 unset GIT_INDEX_FILE
 unset GIT_OBJECT_DIRECTORY
+unset GIT_CEILING_DIRS
 unset SHA1_FILE_DIRECTORIES
 unset SHA1_FILE_DIRECTORY
 GIT_MERGE_VERBOSITY=5
-- 
1.5.4

Re: [PATCH 2/2] Add support for GIT_CEILING_DIRS

[Johannes Schindelin]

Hi,

On Tue, 13 May 2008, David Reiss wrote:

> Make git recognize a new environment variable that prevents it from 
> chdir'ing up into specified directories when looking for a GIT_DIR. 
> Useful for avoiding slow network directories.
> 
> Signed-off-by: David Reiss <dreiss@facebook.com>
> ---
> Just a bit of context about the motivation for this.  I use git in an 
> environment where homedirs are automounted and "ls /home/nonexistent" 
> takes about 9 seconds.

I think this is a very strong argument in favor of your patch, so it 
belongs into the commit message.

> diff --git a/setup.c b/setup.c
> index c54f2b6..d7d986c 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -359,10 +359,11 @@ const char *read_gitfile_gently(const char *path)
>  const char *setup_git_directory_gently(int *nongit_ok)
>  {
>  	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
> +	const char *env_ceiling_dirs = getenv(CEILING_DIRS_ENVIRONMENT);
>  	static char cwd[PATH_MAX+1];
>  	const char *gitdirenv;
>  	const char *gitfile_dir;
> -	int len, offset;
> +	int len, offset, min_offset = -1;

AFAIR we have min_offset in the mingw branch, too, but I think it is 
initialized to 0.

> @@ -414,6 +415,37 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  	if (!getcwd(cwd, sizeof(cwd)-1))
>  		die("Unable to read current working directory");
>  
> +	// Compute min_offset based on GIT_CEILING_DIRS.

We do not like C99 style comments.  Remember, there are people who compile 
Git on something else than the super-latest Linux with cutting-edge GCC.

> +	if (env_ceiling_dirs) {
> +		char *ceils, *ceil, *colon;
> +		ceil = ceils = xstrdup(env_ceiling_dirs);

I think it is quite possible to do this without data copying.

Besides, I think that this whole block should be a function in path.c, if 
only to _document_ what it does: it is much easier to read for people who 
did not write the code.

Something like

	int longest_prefix(const char *path, const char *prefix_list)
	{
		int max_length = 0, length = 0, i;

		for (i = 0; *prefix_list; i++)
			if (prefix_list[i] == ':') {
				if (length > max_length)
					max_length = length;
				length = 0;
			}
			else if (length >= 0) {
				if (prefix_list[i] == path[length])
					length++;
				else {
					if (length > max_length)
						max_length = length;
					length = -1;
				}
			}

		return max_length;
	}

(Completely untested, though.)

> @@ -427,6 +459,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  	 */
>  	offset = len = strlen(cwd);
>  	for (;;) {
> +		// Check the current directory (.git first).

Again, C99 comments are a no-go.

> @@ -443,17 +476,29 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  			check_repository_format_gently(nongit_ok);
>  			return NULL;
>  		}
> -		do {
> -			if (!offset) {
> -				if (nongit_ok) {
> -					if (chdir(cwd))
> -						die("Cannot come back to cwd");
> -					*nongit_ok = 1;
> -					return NULL;
> -				}
> -				die("Not a git repository");
> +
> +		// Did we just check the root dir?
> +		if (!offset) {
> +		not_a_repo:
> +			if (nongit_ok) {
> +				if (chdir(cwd))
> +					die("Cannot come back to cwd");
> +				*nongit_ok = 1;
> +				return NULL;
>  			}
> -		} while (cwd[--offset] != '/');
> +			die("Not a git repository");
> +		}
> +
> +		while (cwd[--offset] != '/') {
> +			assert(offset > 0);
> +		}
> +
> +		// Don't chdir into the ceiling.
> +		if (offset <= min_offset) {
> +			assert(offset == min_offset);
> +			goto not_a_repo;
> +		}
> +
>  		chdir("..");

Why this big, ugly change in the code?  I would have expected you to 
simply change the "if (!offset)" to "if (offset <= min_offset)" (together 
with the initialization to 0).

> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
> new file mode 100755
> index 0000000..d4eaa13
> --- /dev/null
> +++ b/t/t1504-ceiling-dirs.sh
> @@ -0,0 +1,104 @@
> +#!/bin/sh
> +
> +test_description='test GIT_CEILING_DIRS'
> +. ./test-lib.sh
> +
> +test_prefix() {
> +	test_expect_success "$1" \
> +	"test '$2' = \"\$(git rev-parse --show-prefix)\""
> +	shift
> +	[ $# -eq 0 ] && return
> +}
> +
> +test_fail() {
> +	test_expect_code 128 "$1: prefix" \
> +	"git rev-parse --show-prefix"
> +	shift
> +	[ $# -eq 0 ] && return
> +}

I think it would make more sense to write these out.  Besides, I think 
that the "&& return" is not necessary.

And your tests are, uhm,  quite extensive.

Ciao,
Dscho

Re: [PATCH 2/2] Add support for GIT_CEILING_DIRS

[Johannes Sixt]

Johannes Schindelin schrieb:
> On Tue, 13 May 2008, David Reiss wrote:
>> diff --git a/setup.c b/setup.c
>> index c54f2b6..d7d986c 100644
>> --- a/setup.c
>> +++ b/setup.c
>> @@ -359,10 +359,11 @@ const char *read_gitfile_gently(const char *path)
>>  const char *setup_git_directory_gently(int *nongit_ok)
>>  {
>>  	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
>> +	const char *env_ceiling_dirs = getenv(CEILING_DIRS_ENVIRONMENT);
>>  	static char cwd[PATH_MAX+1];
>>  	const char *gitdirenv;
>>  	const char *gitfile_dir;
>> -	int len, offset;
>> +	int len, offset, min_offset = -1;
> 
> AFAIR we have min_offset in the mingw branch, too, but I think it is 
> initialized to 0.

Yes, indeed, and even for much the same purpose (i.e. to stop before the
beginning of the path is reached - on Windows we don't want to look at
"C:" prefixes). Please look at this commit:

http://repo.or.cz/w/git/mingw/j6t.git?a=commitdiff;h=4ab30965f520bd444f3eefe1dee4622181933f90

-- Hannes

Re: [PATCH 2/2] Add support for GIT_CEILING_DIRS

[Kevin Ballard]

On May 14, 2008, at 3:23 AM, Johannes Schindelin wrote:

>> @@ -414,6 +415,37 @@ const char *setup_git_directory_gently(int  
>> *nongit_ok)
>> 	if (!getcwd(cwd, sizeof(cwd)-1))
>> 		die("Unable to read current working directory");
>>
>> +	// Compute min_offset based on GIT_CEILING_DIRS.
>
> We do not like C99 style comments.  Remember, there are people who  
> compile
> Git on something else than the super-latest Linux with cutting-edge  
> GCC.

Out of curiosity, what environment these days doesn't allow C99  
comments? I think I started programming roughly 10 years ago (granted,  
I didn't start with C, but I did C programming at least 8 years ago,  
if not earlier) and every environment I've ever used has supported  
those comments even in C89 mode.

I mean, I fully understand that git wants to be portable across all  
platforms, but it still makes me wonder if there's any environment  
that anybody actually uses these days that doesn't understand those  
comments when compiling C.

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com

Re: [PATCH 2/2] Add support for GIT_CEILING_DIRS

[Johannes Schindelin]

Hi,

On Wed, 14 May 2008, Kevin Ballard wrote:

> On May 14, 2008, at 3:23 AM, Johannes Schindelin wrote:
> 
> > >@@ -414,6 +415,37 @@ const char *setup_git_directory_gently(int *nongit_ok)
> > > if (!getcwd(cwd, sizeof(cwd)-1))
> > >  die("Unable to read current working directory");
> > >
> > >+	// Compute min_offset based on GIT_CEILING_DIRS.
> >
> >We do not like C99 style comments.  Remember, there are people who compile
> >Git on something else than the super-latest Linux with cutting-edge GCC.
> 
> Out of curiosity, what environment these days doesn't allow C99 comments?

On an SGI/IRIX machine I was working some time ago, GCC was too big for my 
quota.  And the admin was not willing to install it.

But I have to wonder: why argue something as C99 comments, when it is _no 
problem_ whatsoever to replace them with C89-style comments, especially 
given the fact that this makes our source code more consistent and thus 
easier on the eye?

Ciao,
Dscho

Re: [PATCH 2/2] Add support for GIT_CEILING_DIRS

[Kevin Ballard]

On May 14, 2008, at 12:02 PM, Johannes Schindelin wrote:

> On Wed, 14 May 2008, Kevin Ballard wrote:
>
>> On May 14, 2008, at 3:23 AM, Johannes Schindelin wrote:
>>
>>>> @@ -414,6 +415,37 @@ const char *setup_git_directory_gently(int  
>>>> *nongit_ok)
>>>> if (!getcwd(cwd, sizeof(cwd)-1))
>>>> die("Unable to read current working directory");
>>>>
>>>> +	// Compute min_offset based on GIT_CEILING_DIRS.
>>>
>>> We do not like C99 style comments.  Remember, there are people who  
>>> compile
>>> Git on something else than the super-latest Linux with cutting- 
>>> edge GCC.
>>
>> Out of curiosity, what environment these days doesn't allow C99  
>> comments?
>
> On an SGI/IRIX machine I was working some time ago, GCC was too big  
> for my
> quota.  And the admin was not willing to install it.
>
> But I have to wonder: why argue something as C99 comments, when it  
> is _no
> problem_ whatsoever to replace them with C89-style comments,  
> especially
> given the fact that this makes our source code more consistent and  
> thus
> easier on the eye?

Oh I wasn't intending to argue, I just was curious. I'm always happy  
to adopt the style of the surrounding code (except when people use the  
One True Brace Style, ugh).

Granted, I personally prefer C99 comments for one-liners and C89  
comments for multi-liners. It's especially helpful if the comment goes  
off the right side of the screen, because I don't have to hunt for the  
closing */ (source code highlighting generally solves this issue, but  
I don't always read code in an environment that can understand it,  
e.g. email).

-Kevin Ballard

-- 
Kevin Ballard
http://kevin.sb.org
kevin@sb.org
http://www.tildesoft.com