Add support for GIT_CEILING_DIRS

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.

For example, I use git in an environment where homedirs are automounted and "ls /home/nonexistent" takes about 9 seconds. Setting GIT_CEILING_DIRS="/home" allows "git help -a" (for bash completion) and "git symbolic-ref" (for my shell prompt) to run in a reasonable time.

This also moves the chdir call to after computing the new cwd. This should be a no-op because the cwd is not read in the interim and any nonlocal exits either chdir to an absolute path or die.

---
Regarding initializing min_offset to -1, it is necessary, given that code to have the root directory be a git repository. Setting min_offset, to 0 would prevent "/.git" and "/objects" from being checked.

Removed all C99-style comments. Old habits die hard. :)

I moved the prefix-calculating code into a separate function, as you suggested, and removed the allocation (which actually wasn't really necessary to begin with). This also has the nice effect that it is much easier to test it in isolation. I added a test for it, #if 0'ed out. I wasn't able to find a convention for this sort of test.

Regarding the "big, ugly change", your suggestion has an off-by-one error. With that code, as soon as "cwd[--offset] != '/'" is false, the loop ends, and we would not detect that offset is now equal to min_offset. Getting this loop right was really tricky because it is basically checking three things at once (beginning of string, ceiling, directory boundary). The key to getting it to look like you suggested was to add 1 to min_offset in all cases. I renamed it ceil_offset to reflect the fact that it is one less than the miinimum offset we are willing to test and added a comment explaining exactly what it means (because it is a little counter-intuitive). I also added a bunch of tests to make sure that this behavior doesn't break down with single-character directories.

For my tests, you said "I think it would make more sense to write these out." Could you explain what you mean by this? Also, I removed the "&& return". I copied these from t1500, and I obviously wasn't paying close attention.

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

index 6f445b1..8aea331 100644
@@ -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'::

index 9cee9a5..8300acc 100644
@@ -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]"

index 7c2a8ba..22899c1 100644
@@ -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

This is what the new version of the patch does.

It seems like that would cause it to continue on with the outer loop, rather than aborting (which is what the current version does) when you hit the ceiling. Or maybe I'm misunderstanding something.

--David

Ok, I see. I'm proposing that the meaning of ceil_offset should be "do not look at this many characters at the beginning of cwd". And we always have cwd[ceil_offset] == '/'. Something like this on top of your patch. Further tweaking will be necessary (I'm just illustrating my point), in particular you could name it min_offset again.

-- Hannes

index de9e7f1..5dbc080 100644
@@ -368,13 +368,13 @@ const char *read_gitfile_gently(const char *path)
 static int longest_ancestor_length(const char *path, const char *prefix_list)
 {
 	const char *ceil, *colon;
-	int max_len = -1;
+	int max_len = 0;
 
 	if (prefix_list == NULL)
-		return -1;
-	/* "/" is a tricky edge case.  It should always return -1, though. */
+		return 0;
+	/* "/" is a tricky edge case.  It should always return 0, though. */
 	if (!strcmp(path, "/"))
-		return -1;
+		return 0;
 
 	ceil = prefix_list;
 	for (;;) {

@@ -522,7 +522,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 * Putting it so far to the right is necessary in order to bail out of the
 	 * "--offset" loop early enough.
 	 */
-	ceil_offset = 1 + longest_ancestor_length(cwd, env_ceiling_dirs);
+	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
 
 	/*
 	 * Test in the following order (relative to the cwd):

@@ -553,17 +553,16 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}
-		do {
-			if (offset <= ceil_offset) {
-				if (nongit_ok) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
-					*nongit_ok = 1;
-					return NULL;
-				}
-				die("Not a git repository");
+		do { } while (offset > ceil_offset && cwd[--offset] != '/');
+		if (offset <= ceil_offset) {
+			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");
+		}
 		chdir("..");
 	}

Hi,

			; /* do nothing */

The problem with this implementation is that it does not distinguish between GIT_CEILING_DIRS being unset and GIT_CEILING_DIRS="/". For example...

cd /
sudo git init
cd /home
git rev-parse --show-prefix

That series of commands works with either version of my patch, but fails with "fatal: Not a git repository" if I apply this change. I am certainly open to changing this code, but I think we will always need two separate values of ceil_offset to represent "unset" and "/". It's just a question of whether they should be -1 and 0 or 0 and 1.

--David

Hi,

You are much more familiar with the code, but I suspect that a simple change would fix that:

Just use "--offset >= 0 && cwd[--offset] != '/'" here. And maybe ceil_offset = -1 again.

I cannot quickly test, since I am short on time, and it would be too cumbersome to find which patches to apply first.

But I strongly believe that it is not beyond your capabilities to adjust Hannes' patch for your command series, keeping the elegance.

Ciao,
Dscho