[PATCH v3] Add support for GIT_CEILING_DIRS

[PATCH v3] 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.

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.

Signed-off-by: David Reiss <dreiss@facebook.com>
---
 Documentation/git.txt   |    8 +++
 cache.h                 |    1 +
 setup.c                 |  127 ++++++++++++++++++++++++++++++++++----
 t/t1504-ceiling-dirs.sh |  156 +++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh           |    1 +
 5 files changed, 281 insertions(+), 12 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 b8fd476..fdcfae1 100644
--- a/setup.c
+++ b/setup.c
@@ -353,16 +353,118 @@ const char *read_gitfile_gently(const char *path)
 }
 
 /*
+ * path = Canonical absolute path
+ * prefix_list = Colon-separated list of canonical absolute paths
+ *
+ * Determines, for each path in parent_list, whether the "prefix" really
+ * is an ancestor directory of path.  Returns the length of the longest
+ * ancestor directory, excluding any trailing slashes, or -1 if no prefix
+ * is an ancestry.  (Note that this means 0 is returned if prefix_list
+ * contains "/".)  "/foo" is not considered an ancestor of "/foobar".
+ * Directories are not considered to be their own ancestors.  Paths must
+ * be in a canonical form: empty components, or "." or ".." components
+ * are not allowed.  prefix_list may be null, which is like "".
+ */
+static int longest_ancestor_length(const char *path, const char *prefix_list)
+{
+	const char *ceil, *colon;
+	int max_len = -1;
+
+	if (prefix_list == NULL)
+		return -1;
+	/* "/" is a tricky edge case.  It should always return -1, though. */
+	if (!strcmp(path, "/"))
+		return -1;
+
+	ceil = prefix_list;
+	for (;;) {
+		int len;
+
+		/* Add strchrnul to compat? */
+		colon = strchr(ceil, ':');
+		if (colon)
+			len = colon - ceil;
+		else
+			len = strlen(ceil);
+
+		/* "" would otherwise be treated like "/". */
+		if (len) {
+			/* Trim trailing slashes. */
+			while (len && ceil[len-1] == '/')
+				len--;
+
+			if (!strncmp(path, ceil, len) &&
+					path[len] == '/' &&
+					len > max_len) {
+				max_len = len;
+			}
+		}
+
+		if (!colon)
+			break;
+		ceil = colon + 1;
+	}
+
+	return max_len;
+}
+
+#if 0
+static void test_longest_ancestor_length()
+{
+	assert(longest_ancestor_length("/", NULL           ) == -1);
+	assert(longest_ancestor_length("/", ""             ) == -1);
+	assert(longest_ancestor_length("/", "/"            ) == -1);
+
+	assert(longest_ancestor_length("/foo", NULL           ) == -1);
+	assert(longest_ancestor_length("/foo", ""             ) == -1);
+	assert(longest_ancestor_length("/foo", ":"            ) == -1);
+	assert(longest_ancestor_length("/foo", "/"            ) ==  0);
+	assert(longest_ancestor_length("/foo", "/fo"          ) == -1);
+	assert(longest_ancestor_length("/foo", "/foo"         ) == -1);
+	assert(longest_ancestor_length("/foo", "/foo/"        ) == -1);
+	assert(longest_ancestor_length("/foo", "/bar"         ) == -1);
+	assert(longest_ancestor_length("/foo", "/bar/"        ) == -1);
+	assert(longest_ancestor_length("/foo", "/foo/bar"     ) == -1);
+	assert(longest_ancestor_length("/foo", "/foo:/bar/"   ) == -1);
+	assert(longest_ancestor_length("/foo", "/foo/:/bar/"  ) == -1);
+	assert(longest_ancestor_length("/foo", "/foo::/bar/"  ) == -1);
+	assert(longest_ancestor_length("/foo", "/:/foo:/bar/" ) ==  0);
+	assert(longest_ancestor_length("/foo", "/foo:/:/bar/" ) ==  0);
+	assert(longest_ancestor_length("/foo", "/:/bar/:/foo" ) ==  0);
+
+	assert(longest_ancestor_length("/foo/bar", NULL           ) == -1);
+	assert(longest_ancestor_length("/foo/bar", ""             ) == -1);
+	assert(longest_ancestor_length("/foo/bar", "/"            ) ==  0);
+	assert(longest_ancestor_length("/foo/bar", "/fo"          ) == -1);
+	assert(longest_ancestor_length("/foo/bar", "/foo"         ) ==  4);
+	assert(longest_ancestor_length("/foo/bar", "/foo/"        ) ==  4);
+	assert(longest_ancestor_length("/foo/bar", "/foo/ba"      ) == -1);
+	assert(longest_ancestor_length("/foo/bar", "/:/fo"        ) ==  0);
+	assert(longest_ancestor_length("/foo/bar", "/foo:/foo/ba" ) ==  4);
+	assert(longest_ancestor_length("/foo/bar", "/bar"         ) == -1);
+	assert(longest_ancestor_length("/foo/bar", "/bar/"        ) == -1);
+	assert(longest_ancestor_length("/foo/bar", "/fo:"         ) == -1);
+	assert(longest_ancestor_length("/foo/bar", ":/fo"         ) == -1);
+	assert(longest_ancestor_length("/foo/bar", "/foo:/bar/"   ) ==  4);
+	assert(longest_ancestor_length("/foo/bar", "/:/foo:/bar/" ) ==  4);
+	assert(longest_ancestor_length("/foo/bar", "/foo:/:/bar/" ) ==  4);
+	assert(longest_ancestor_length("/foo/bar", "/:/bar/:/fo"  ) ==  0);
+	assert(longest_ancestor_length("/foo/bar", "/:/bar/"      ) ==  0);
+}
+#endif
+
+/*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
  */
 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, ceil_offset;
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -414,6 +516,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
 
+	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);
+
 	/*
 	 * Test in the following order (relative to the cwd):
 	 * - .git (file containing "gitdir: <path>")
@@ -443,18 +547,17 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}
-		chdir("..");
-		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");
+		while (--offset > ceil_offset && cwd[offset] != '/') /* EMPTY */;
+		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("..");
 	}
 
 	inside_git_dir = 0;
diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
new file mode 100755
index 0000000..091baad
--- /dev/null
+++ b/t/t1504-ceiling-dirs.sh
@@ -0,0 +1,156 @@
+#!/bin/sh
+
+test_description='test GIT_CEILING_DIRS'
+. ./test-lib.sh
+
+test_prefix() {
+	test_expect_success "$1" \
+	"test '$2' = \"\$(git rev-parse --show-prefix)\""
+}
+
+test_fail() {
+	test_expect_code 128 "$1: prefix" \
+	"git rev-parse --show-prefix"
+}
+
+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/sub/di"
+test_prefix subdir_ceil_at_sub_di "sub/dir/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sub/di"
+test_prefix subdir_ceil_at_sub_di_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 ""
+unset GIT_DIR
+
+
+cd ../.. || exit 1
+mkdir -p s/d || exit 1
+cd s/d || exit 1
+
+unset GIT_CEILING_DIRS
+test_prefix sd_no_ceil "s/d/"
+
+export GIT_CEILING_DIRS=""
+test_prefix sd_ceil_empty "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT"
+test_fail sd_ceil_at_trash
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/"
+test_fail sd_ceil_at_trash_slash
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/s"
+test_fail sd_ceil_at_s
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/s/"
+test_fail sd_ceil_at_s_slash
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/s/d"
+test_prefix sd_ceil_at_sd "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/s/d/"
+test_prefix sd_ceil_at_sd_slash "s/d/"
+
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/su"
+test_prefix sd_ceil_at_su "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/su/"
+test_prefix sd_ceil_at_su_slash "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/s/di"
+test_prefix sd_ceil_at_s_di "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/s/di"
+test_prefix sd_ceil_at_s_di_slash "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sdi"
+test_prefix sd_ceil_at_sdi "s/d/"
+
+export GIT_CEILING_DIRS="$TRASH_ROOT/sdi"
+test_prefix sd_ceil_at_sdi_slash "s/d/"
+
+
+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 v3] Add support for GIT_CEILING_DIRS

[Johannes Schindelin]

Hi,

On Thu, 15 May 2008, David Reiss wrote:

>  cache.h                 |    1 +
>  setup.c                 |  127 ++++++++++++++++++++++++++++++++++----
>  t/t1504-ceiling-dirs.sh |  156 +++++++++++++++++++++++++++++++++++++++++++++++

By now, I strongly believe that these changes are too large.  I am 
convinced that what you desire can be expressed much simpler, and thus 
less error-prone.

Also, I think that your test cases are too extensive.  While it is usually 
good to have exhaustive tests, running them takes time.  And if it takes 
so much time that hardly anybody bothers with running the test suite, 
there are _too_ many tests.

> diff --git a/setup.c b/setup.c
> index b8fd476..fdcfae1 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -353,16 +353,118 @@ const char *read_gitfile_gently(const char *path)
>  }
>  
>  /*
> + * path = Canonical absolute path
> + * prefix_list = Colon-separated list of canonical absolute paths
> + *
> + * Determines, for each path in parent_list, whether the "prefix" really
> + * is an ancestor directory of path.  Returns the length of the longest
> + * ancestor directory, excluding any trailing slashes, or -1 if no prefix
> + * is an ancestry.  (Note that this means 0 is returned if prefix_list
> + * contains "/".)  "/foo" is not considered an ancestor of "/foobar".
> + * Directories are not considered to be their own ancestors.  Paths must
> + * be in a canonical form: empty components, or "." or ".." components
> + * are not allowed.  prefix_list may be null, which is like "".
> + */
> +static int longest_ancestor_length(const char *path, const char *prefix_list)
> +{
> +	const char *ceil, *colon;
> +	int max_len = -1;
> +
> +	if (prefix_list == NULL)
> +		return -1;
> +	/* "/" is a tricky edge case.  It should always return -1, though. */
> +	if (!strcmp(path, "/"))
> +		return -1;
> +
> +	ceil = prefix_list;
> +	for (;;) {
> +		int len;
> +
> +		/* Add strchrnul to compat? */
> +		colon = strchr(ceil, ':');
> +		if (colon)
> +			len = colon - ceil;
> +		else
> +			len = strlen(ceil);
> +
> +		/* "" would otherwise be treated like "/". */
> +		if (len) {
> +			/* Trim trailing slashes. */
> +			while (len && ceil[len-1] == '/')
> +				len--;
> +
> +			if (!strncmp(path, ceil, len) &&
> +					path[len] == '/' &&
> +					len > max_len) {
> +				max_len = len;
> +			}
> +		}
> +
> +		if (!colon)
> +			break;
> +		ceil = colon + 1;
> +	}
> +
> +	return max_len;
> +}

You know, I wonder why I even bothered writing those responses, if you 
just ignore them.

> +#if 0
> +static void test_longest_ancestor_length()
> +{
> +	assert(longest_ancestor_length("/", NULL           ) == -1);
> +	assert(longest_ancestor_length("/", ""             ) == -1);
> +	assert(longest_ancestor_length("/", "/"            ) == -1);
> +
> +	assert(longest_ancestor_length("/foo", NULL           ) == -1);
> +	assert(longest_ancestor_length("/foo", ""             ) == -1);
> +	assert(longest_ancestor_length("/foo", ":"            ) == -1);
> +	assert(longest_ancestor_length("/foo", "/"            ) ==  0);
> +	assert(longest_ancestor_length("/foo", "/fo"          ) == -1);
> +	assert(longest_ancestor_length("/foo", "/foo"         ) == -1);
> +	assert(longest_ancestor_length("/foo", "/foo/"        ) == -1);
> +	assert(longest_ancestor_length("/foo", "/bar"         ) == -1);
> +	assert(longest_ancestor_length("/foo", "/bar/"        ) == -1);
> +	assert(longest_ancestor_length("/foo", "/foo/bar"     ) == -1);
> +	assert(longest_ancestor_length("/foo", "/foo:/bar/"   ) == -1);
> +	assert(longest_ancestor_length("/foo", "/foo/:/bar/"  ) == -1);
> +	assert(longest_ancestor_length("/foo", "/foo::/bar/"  ) == -1);
> +	assert(longest_ancestor_length("/foo", "/:/foo:/bar/" ) ==  0);
> +	assert(longest_ancestor_length("/foo", "/foo:/:/bar/" ) ==  0);
> +	assert(longest_ancestor_length("/foo", "/:/bar/:/foo" ) ==  0);
> +
> +	assert(longest_ancestor_length("/foo/bar", NULL           ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", ""             ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", "/"            ) ==  0);
> +	assert(longest_ancestor_length("/foo/bar", "/fo"          ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", "/foo"         ) ==  4);
> +	assert(longest_ancestor_length("/foo/bar", "/foo/"        ) ==  4);
> +	assert(longest_ancestor_length("/foo/bar", "/foo/ba"      ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", "/:/fo"        ) ==  0);
> +	assert(longest_ancestor_length("/foo/bar", "/foo:/foo/ba" ) ==  4);
> +	assert(longest_ancestor_length("/foo/bar", "/bar"         ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", "/bar/"        ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", "/fo:"         ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", ":/fo"         ) == -1);
> +	assert(longest_ancestor_length("/foo/bar", "/foo:/bar/"   ) ==  4);
> +	assert(longest_ancestor_length("/foo/bar", "/:/foo:/bar/" ) ==  4);
> +	assert(longest_ancestor_length("/foo/bar", "/foo:/:/bar/" ) ==  4);
> +	assert(longest_ancestor_length("/foo/bar", "/:/bar/:/fo"  ) ==  0);
> +	assert(longest_ancestor_length("/foo/bar", "/:/bar/"      ) ==  0);
> +}
> +#endif

This has _no_ place in the Git source code.  Have you looked around, and 
found similar dead code?  No?  That's because Git's source code is not a 
graveyard of useless code bits, but it is a collection of elegant code.  
Mostly, at least.

Instead of wasting my time further, I will try to come up with a better 
implementation, as is the way of Open Source.

Ciao,
Dscho

Re: [PATCH v3] Add support for GIT_CEILING_DIRS

[David Reiss]

> By now, I strongly believe that these changes are too large.  I am
> convinced that what you desire can be expressed much simpler, and thus
> less error-prone.
Most of the code is in the one function to parse out the colon-separated
environment variable value and compute the longest directory prefix.
I'm not convinced this can be made much simpler.  (Using strtok_r could
help, but would require an allocation.)  Most of the rest of the changes
are test code and indentation.
 
> Also, I think that your test cases are too extensive.  While it is usually
> good to have exhaustive tests, running them takes time.  And if it takes
> so much time that hardly anybody bothers with running the test suite,
> there are _too_ many tests.
I am more than happy to remove most of them, leaving only basic sanity
checks.  However, I would prefer to leave them in but comment them out,
so that if I or someone else wants to modify this code later, they would
be able to run a more extensive test suite.  I'll submit a modified
patch with this change.

> You know, I wonder why I even bothered writing those responses, if you
> just ignore them.
I must say that I am very confused.  I thought I followed all of your
responses to the letter.  Could you please be more specific about the
ones I missed?  I'm happy to make further changes.

> This has _no_ place in the Git source code.  Have you looked around, and
> found similar dead code?  No?  That's because Git's source code is not a
> graveyard of useless code bits, but it is a collection of elegant code. 
> Mostly, at least.
As I stated in "PATCH v2", I was unsure what the convention was for unit
tests like this.  Most of the git code is (obviously) functional tests,
but it is impossible to test how this code would behave with a git
directory under "/" using a functional test, unless it was run as root.
Someone just pointed out to me that there are some C-based tests (like
test-sha1) that are run from "make test".  I guess I can move the test
function to a new one of those, but it will require making
longest_ancestor_length extern.

> Instead of wasting my time further, I will try to come up with a better
> implementation, as is the way of Open Source.
I am sorry if this has wasted your time.  I really have been trying to
incorporate your feedback into my patch, and the code has definitely
improved as a result.  However, my main goal is simply to get this
feature working (I have already patched it into my own git build, and it
has saved me a lot of time), so if you come up with a better
implementation, that would be great!

--David

Re: [PATCH v3] Add support for GIT_CEILING_DIRS

[Junio C Hamano]

David Reiss <dreiss@facebook.com> writes:

> + * ....  Paths must
> + * be in a canonical form: empty components, or "." or ".." components
> + * are not allowed.  prefix_list may be null, which is like "".

The caller starts from cwd[] and chomps, so you can safely assume that it
would not feed anything problematic.  But prefix_list comes from user's
environment, and it is easy to make mistakes like doubled slashes (which
you seem to take care) and also is tempting to use ".." when specifying
the ceiling (e.g. "CEIL=$HOME/..").  Perhaps canonicalizing the ceiling
would make this easier to use for end users?


> +#if 0
> +static void test_longest_ancestor_length()
> +{
> ...
> +	assert(longest_ancestor_length("/foo/bar", "/:/bar/"      ) ==  0);

No test for nonsense/invalid input, like "::/foo" for prefix_list?

> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
> new file mode 100755
> index 0000000..091baad
> --- /dev/null
> +++ b/t/t1504-ceiling-dirs.sh
> @@ -0,0 +1,156 @@
> +#!/bin/sh
> +
> +test_description='test GIT_CEILING_DIRS'
> +. ./test-lib.sh
> +
> +test_prefix() {
> +	test_expect_success "$1" \
> +	"test '$2' = \"\$(git rev-parse --show-prefix)\""
> +}
> +
> +test_fail() {
> +	test_expect_code 128 "$1: prefix" \
> +	"git rev-parse --show-prefix"
> +}
> +
> +TRASH_ROOT="$(pwd)"
> +ROOT_PARENT=$(dirname "$TRASH_ROOT")
> +
> +
> +unset GIT_CEILING_DIRS
> +test_prefix no_ceil ""
> +
> +export GIT_CEILING_DIRS=""

Portability.  Instead write this as two separate statements, please.

        VAR=val
        export VAR

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

You may also check stuff like "sub//dir" and "::sub/dir/".

How well would this colon separated list work with msys folks?

[PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

In certain setups, trying to access a non-existing .git/ can take quite
some time, for example when the directory is an automount directory.

Allow the user to specify directories where Git should stop looking for
a .git/ directory: GIT_CEILING_DIRECTORIES, if set, is expected to be
a colon delimited list of such barrier directories.

Note: if GIT_CEILING_DIRECTORIES=/a/b and your current working directory 
is /a, Git will _not_ stop looking.

Note2: you must not specify the directories with trailing slashes.

Initial implementation by David Reiss.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	Show me the bugs!

	I just checked: the "/" issue you were referring to is most likely
	the fact that "git rev-parse --git-dir" would return "//.git"
	instead of "/.git" (if that is the appropriate GIT_DIR).

	This is the original behavior (without this patch), and IMO a 
	separate issue, which might not even need fixing.

 Documentation/git.txt          |    6 +++++
 cache.h                        |    2 +
 path.c                         |   19 ++++++++++++++++
 setup.c                        |   11 +++++++-
 t/t1504-ceiling-directories.sh |   46 ++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                  |    1 +
 6 files changed, 83 insertions(+), 2 deletions(-)
 create mode 100644 t/t1504-ceiling-directories.sh

diff --git a/Documentation/git.txt b/Documentation/git.txt
index adcd3e0..a12d1f8 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -415,6 +415,12 @@ 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'::
+	If set (to a colon delimited list of absolute directories), Git
+	will refuse to look for the .git/ directory further when hitting
+	one of those directories (otherwise it would traverse the parent
+	directories until hitting the root directory).
+
 git Commits
 ~~~~~~~~~~~
 'GIT_AUTHOR_NAME'::
diff --git a/cache.h b/cache.h
index a8638b1..c31b4c7 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_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
@@ -522,6 +523,7 @@ static inline int is_absolute_path(const char *path)
 	return path[0] == '/';
 }
 const char *make_absolute_path(const char *path);
+int longest_prefix(const char *path, const char *prefix_list);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index b7c24a2..c0d7364 100644
--- a/path.c
+++ b/path.c
@@ -357,3 +357,22 @@ const char *make_absolute_path(const char *path)
 
 	return buf;
 }
+
+int longest_prefix(const char *path, const char *prefix_list)
+{
+	int max_length = 0, length = 0, i;
+
+	for (i = 0; prefix_list[i]; 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
+				length = -1;
+		}
+	return max_length > length ? max_length : length;
+}
diff --git a/setup.c b/setup.c
index 9e9a2b1..cece3e4 100644
--- a/setup.c
+++ b/setup.c
@@ -365,10 +365,13 @@ 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 *ceiling_directories =
+		getenv(CEILING_DIRECTORIES_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv;
 	const char *gitfile_dir;
 	int len, offset;
+	int min_offset = 0;
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -422,6 +425,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
 
+	if (ceiling_directories)
+		min_offset = longest_prefix(cwd, ceiling_directories);
+
 	/*
 	 * Test in the following order (relative to the cwd):
 	 * - .git (file containing "gitdir: <path>")
@@ -453,7 +459,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		}
 		chdir("..");
 		do {
-			if (!offset) {
+			if (offset <= min_offset) {
 				if (nongit_ok) {
 					if (chdir(cwd))
 						die("Cannot come back to cwd");
@@ -462,7 +468,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				}
 				die("Not a git repository");
 			}
-		} while (cwd[--offset] != '/');
+		} while (offset > min_offset &&
+				--offset >=0 && cwd[offset] != '/');
 	}
 
 	inside_git_dir = 0;
diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
new file mode 100644
index 0000000..1d8ef0b
--- /dev/null
+++ b/t/t1504-ceiling-directories.sh
@@ -0,0 +1,46 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='test limiting with GIT_CEILING_DIRECTORIES'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	CWD="$(pwd -P)" &&
+	mkdir subdir
+
+'
+
+test_expect_success 'without GIT_CEILING_DIRECTORIES' '
+
+	test .git = "$(git rev-parse --git-dir)" &&
+	(cd subdir && git rev-parse --git-dir) &&
+	echo "$CWD" &&
+	test "$CWD/.git" = "$(cd subdir && git rev-parse --git-dir)"
+
+'
+
+test_expect_success 'with non-matching ceiling directory' '
+
+	test "$(GIT_CEILING_DIRECTORIES="$CWD/X" \
+		git rev-parse --git-dir)" = .git
+
+'
+
+test_expect_success 'with matching ceiling directories' '
+
+	GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	git rev-parse --git-dir &&
+	GIT_CEILING_DIRECTORIES="$CWD/subdir:$CWD/X" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	git rev-parse --git-dir
+
+'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5002fb0..c3a3167 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_DIRECTORIES
 unset SHA1_FILE_DIRECTORIES
 unset SHA1_FILE_DIRECTORY
 GIT_MERGE_VERBOSITY=5
-- 
1.5.5.1.425.g5f464

Re: [PATCH v3] Add support for GIT_CEILING_DIRS

[Johannes Schindelin]

Hi,

On Thu, 15 May 2008, Junio C Hamano wrote:

> David Reiss <dreiss@facebook.com> writes:
> 
> > + * ....  Paths must
> > + * be in a canonical form: empty components, or "." or ".." components
> > + * are not allowed.  prefix_list may be null, which is like "".
> 
> The caller starts from cwd[] and chomps, so you can safely assume that 
> it would not feed anything problematic.  But prefix_list comes from 
> user's environment, and it is easy to make mistakes like doubled slashes 
> (which you seem to take care) and also is tempting to use ".." when 
> specifying the ceiling (e.g. "CEIL=$HOME/..").  Perhaps canonicalizing 
> the ceiling would make this easier to use for end users?

Is this not going too far?  I mean, CEILING_DIRECTORIES is already a very 
special case.

> How well would this colon separated list work with msys folks?

Not well at all.  At least for the moment, I think setting this variable 
would fail (since it would be rewritten into a Windows-PATH-style string).  
Though honestly, I have no idea what getcwd() does on MinGW.

Steffen is working on that rewriting stuff, so maybe it will be a 
non-issue, eventually.

Ciao,
Dscho

Re: [PATCH] Add support for GIT_CEILING_DIRECTORIES

[David Reiss]

longest_prefix is just a textual check.  It doesn't verify that the prefix
is actually a full directory component of the cwd.  For example...

dreiss@dreiss-vmware:/tmp$ mkdir test
dreiss@dreiss-vmware:/tmp$ cd test
dreiss@dreiss-vmware:/tmp/test$ git init
Initialized empty Git repository in .git/
dreiss@dreiss-vmware:master:test$ mkdir abcd
dreiss@dreiss-vmware:master:test$ cd abcd
dreiss@dreiss-vmware:master:test/abcd$ git rev-parse --show-prefix
abcd/
dreiss@dreiss-vmware:master:test/abcd$ ~/git-ceil/git rev-parse --show-prefix
abcd/
dreiss@dreiss-vmware:master:test/abcd$ GIT_CEILING_DIRECTORIES=/tmp/test/ab ~/git-ceil/git rev-parse --show-prefix
fatal: Not a git repository
dreiss@dreiss-vmware:master:test/abcd:128$ 

"/tmp/test/ab" is a textual prefix of the cwd, but it should not prevent
"/tmp/test/.git" from being discovered.  For what it's worth, my test cases
check this behavior.  Ironically, I think this could be fixed by requiring
the ceiling directories to have trailing slashes.

Also, I think it is better to move the 'chdir("..")' after the do loop,
so that git won't even chdir up into the ceiling directory.  This actually
doesn't matter to me, but I figured that it might be nice for someone.

Finally, just a small thing.  The documentation still says "GIT_CEILING_DIRS".

--David

Johannes Schindelin wrote:
> 
> In certain setups, trying to access a non-existing .git/ can take quite
> some time, for example when the directory is an automount directory.
> 
> Allow the user to specify directories where Git should stop looking for
> a .git/ directory: GIT_CEILING_DIRECTORIES, if set, is expected to be
> a colon delimited list of such barrier directories.
> 
> Note: if GIT_CEILING_DIRECTORIES=/a/b and your current working directory
> is /a, Git will _not_ stop looking.
> 
> Note2: you must not specify the directories with trailing slashes.
> 
> Initial implementation by David Reiss.
> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> 
>         Show me the bugs!
> 
>         I just checked: the "/" issue you were referring to is most likely
>         the fact that "git rev-parse --git-dir" would return "//.git"
>         instead of "/.git" (if that is the appropriate GIT_DIR).
> 
>         This is the original behavior (without this patch), and IMO a
>         separate issue, which might not even need fixing.
> 
>  Documentation/git.txt          |    6 +++++
>  cache.h                        |    2 +
>  path.c                         |   19 ++++++++++++++++
>  setup.c                        |   11 +++++++-
>  t/t1504-ceiling-directories.sh |   46
> ++++++++++++++++++++++++++++++++++++++++
>  t/test-lib.sh                  |    1 +
>  6 files changed, 83 insertions(+), 2 deletions(-)
>  create mode 100644 t/t1504-ceiling-directories.sh
> 
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index adcd3e0..a12d1f8 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -415,6 +415,12 @@ 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'::
> +       If set (to a colon delimited list of absolute directories), Git
> +       will refuse to look for the .git/ directory further when hitting
> +       one of those directories (otherwise it would traverse the parent
> +       directories until hitting the root directory).
> +
>  git Commits
>  ~~~~~~~~~~~
>  'GIT_AUTHOR_NAME'::
> diff --git a/cache.h b/cache.h
> index a8638b1..c31b4c7 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_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
>  #define GITATTRIBUTES_FILE ".gitattributes"
>  #define INFOATTRIBUTES_FILE "info/attributes"
>  #define ATTRIBUTE_MACRO_PREFIX "[attr]"
> @@ -522,6 +523,7 @@ static inline int is_absolute_path(const char *path)
>         return path[0] == '/';
>  }
>  const char *make_absolute_path(const char *path);
> +int longest_prefix(const char *path, const char *prefix_list);
> 
>  /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
>  extern int sha1_object_info(const unsigned char *, unsigned long *);
> diff --git a/path.c b/path.c
> index b7c24a2..c0d7364 100644
> --- a/path.c
> +++ b/path.c
> @@ -357,3 +357,22 @@ const char *make_absolute_path(const char *path)
> 
>         return buf;
>  }
> +
> +int longest_prefix(const char *path, const char *prefix_list)
> +{
> +       int max_length = 0, length = 0, i;
> +
> +       for (i = 0; prefix_list[i]; 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
> +                               length = -1;
> +               }
> +       return max_length > length ? max_length : length;
> +}
> diff --git a/setup.c b/setup.c
> index 9e9a2b1..cece3e4 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -365,10 +365,13 @@ 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 *ceiling_directories =
> +               getenv(CEILING_DIRECTORIES_ENVIRONMENT);
>         static char cwd[PATH_MAX+1];
>         const char *gitdirenv;
>         const char *gitfile_dir;
>         int len, offset;
> +       int min_offset = 0;
> 
>         /*
>          * Let's assume that we are in a git repository.
> @@ -422,6 +425,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
>         if (!getcwd(cwd, sizeof(cwd)-1))
>                 die("Unable to read current working directory");
> 
> +       if (ceiling_directories)
> +               min_offset = longest_prefix(cwd, ceiling_directories);
> +
>         /*
>          * Test in the following order (relative to the cwd):
>          * - .git (file containing "gitdir: <path>")
> @@ -453,7 +459,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>                 }
>                 chdir("..");
>                 do {
> -                       if (!offset) {
> +                       if (offset <= min_offset) {
>                                 if (nongit_ok) {
>                                         if (chdir(cwd))
>                                                 die("Cannot come back to
> cwd");
> @@ -462,7 +468,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
>                                 }
>                                 die("Not a git repository");
>                         }
> -               } while (cwd[--offset] != '/');
> +               } while (offset > min_offset &&
> +                               --offset >=0 && cwd[offset] != '/');
>         }
> 
>         inside_git_dir = 0;
> diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
> new file mode 100644
> index 0000000..1d8ef0b
> --- /dev/null
> +++ b/t/t1504-ceiling-directories.sh
> @@ -0,0 +1,46 @@
> +#!/bin/sh
> +#
> +# Copyright (c) 2007 Johannes E. Schindelin
> +#
> +
> +test_description='test limiting with GIT_CEILING_DIRECTORIES'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +
> +       CWD="$(pwd -P)" &&
> +       mkdir subdir
> +
> +'
> +
> +test_expect_success 'without GIT_CEILING_DIRECTORIES' '
> +
> +       test .git = "$(git rev-parse --git-dir)" &&
> +       (cd subdir && git rev-parse --git-dir) &&
> +       echo "$CWD" &&
> +       test "$CWD/.git" = "$(cd subdir && git rev-parse --git-dir)"
> +
> +'
> +
> +test_expect_success 'with non-matching ceiling directory' '
> +
> +       test "$(GIT_CEILING_DIRECTORIES="$CWD/X" \
> +               git rev-parse --git-dir)" = .git
> +
> +'
> +
> +test_expect_success 'with matching ceiling directories' '
> +
> +       GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
> +       export GIT_CEILING_DIRECTORIES &&
> +       (cd subdir && test_must_fail git rev-parse --git-dir) &&
> +       git rev-parse --git-dir &&
> +       GIT_CEILING_DIRECTORIES="$CWD/subdir:$CWD/X" &&
> +       export GIT_CEILING_DIRECTORIES &&
> +       (cd subdir && test_must_fail git rev-parse --git-dir) &&
> +       git rev-parse --git-dir
> +
> +'
> +
> +test_done
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index 5002fb0..c3a3167 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_DIRECTORIES
>  unset SHA1_FILE_DIRECTORIES
>  unset SHA1_FILE_DIRECTORY
>  GIT_MERGE_VERBOSITY=5
> --
> 1.5.5.1.425.g5f464
> 

Re: [PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

Hi,

On Thu, 15 May 2008, David Reiss wrote:

> longest_prefix is just a textual check.  It doesn't verify that the prefix
> is actually a full directory component of the cwd.

Okay.

> Also, I think it is better to move the 'chdir("..")' after the do loop, 
> so that git won't even chdir up into the ceiling directory.  This 
> actually doesn't matter to me, but I figured that it might be nice for 
> someone.

I'd rather go with the minimal diff, unless there is a good reason to 
change it.

> Finally, just a small thing.  The documentation still says 
> "GIT_CEILING_DIRS".

Okay.

How about this on top (still pretty simple):

---

 Documentation/git.txt          |    2 +-
 path.c                         |   10 ++++++++--
 t/t1504-ceiling-directories.sh |    8 ++++++++
 3 files changed, 17 insertions(+), 3 deletions(-)

diff --git a/Documentation/git.txt b/Documentation/git.txt
index a12d1f8..e4413bf 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -415,7 +415,7 @@ 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'::
+'GIT_CEILING_DIRECTORIES'::
 	If set (to a colon delimited list of absolute directories), Git
 	will refuse to look for the .git/ directory further when hitting
 	one of those directories (otherwise it would traverse the parent
diff --git a/path.c b/path.c
index c0d7364..a097ecc 100644
--- a/path.c
+++ b/path.c
@@ -358,13 +358,18 @@ const char *make_absolute_path(const char *path)
 	return buf;
 }
 
+static int is_separator(char c)
+{
+	return !c || c == '/';
+}
+
 int longest_prefix(const char *path, const char *prefix_list)
 {
 	int max_length = 0, length = 0, i;
 
 	for (i = 0; prefix_list[i]; i++)
 		if (prefix_list[i] == ':') {
-			if (length > max_length)
+			if (length > max_length && is_separator(path[length]))
 				max_length = length;
 			length = 0;
 		}
@@ -374,5 +379,6 @@ int longest_prefix(const char *path, const char *prefix_list)
 			else
 				length = -1;
 		}
-	return max_length > length ? max_length : length;
+	return max_length > length || !is_separator(path[length]) ?
+		max_length : length;
 }
diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
index 1d8ef0b..6c8757d 100644
--- a/t/t1504-ceiling-directories.sh
+++ b/t/t1504-ceiling-directories.sh
@@ -43,4 +43,12 @@ test_expect_success 'with matching ceiling directories' '
 
 '
 
+test_expect_success 'with non-directory prefix' '
+
+	GIT_CEILING_DIRECTORIES="$CWD/sub" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && git rev-parse --git-dir)
+
+'
+
 test_done

Re: [PATCH] Add support for GIT_CEILING_DIRECTORIES

[David Reiss]

This meets my needs.

Junio, if you'd like, I can incorporate your suggestions of normalizing paths
internally and testing more corner cases.  But if you just want to take this
version, I'll be fine.

--David

Johannes Schindelin wrote:
> Hi,
> 
> On Thu, 15 May 2008, David Reiss wrote:
> 
>> longest_prefix is just a textual check.  It doesn't verify that the prefix
>> is actually a full directory component of the cwd.
> 
> Okay.
> 
>> Also, I think it is better to move the 'chdir("..")' after the do loop,
>> so that git won't even chdir up into the ceiling directory.  This
>> actually doesn't matter to me, but I figured that it might be nice for
>> someone.
> 
> I'd rather go with the minimal diff, unless there is a good reason to
> change it.
> 
>> Finally, just a small thing.  The documentation still says
>> "GIT_CEILING_DIRS".
> 
> Okay.
> 
> How about this on top (still pretty simple):
> 
> ---
> 
>  Documentation/git.txt          |    2 +-
>  path.c                         |   10 ++++++++--
>  t/t1504-ceiling-directories.sh |    8 ++++++++
>  3 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index a12d1f8..e4413bf 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -415,7 +415,7 @@ 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'::
> +'GIT_CEILING_DIRECTORIES'::
>         If set (to a colon delimited list of absolute directories), Git
>         will refuse to look for the .git/ directory further when hitting
>         one of those directories (otherwise it would traverse the parent
> diff --git a/path.c b/path.c
> index c0d7364..a097ecc 100644
> --- a/path.c
> +++ b/path.c
> @@ -358,13 +358,18 @@ const char *make_absolute_path(const char *path)
>         return buf;
>  }
> 
> +static int is_separator(char c)
> +{
> +       return !c || c == '/';
> +}
> +
>  int longest_prefix(const char *path, const char *prefix_list)
>  {
>         int max_length = 0, length = 0, i;
> 
>         for (i = 0; prefix_list[i]; i++)
>                 if (prefix_list[i] == ':') {
> -                       if (length > max_length)
> +                       if (length > max_length &&
> is_separator(path[length]))
>                                 max_length = length;
>                         length = 0;
>                 }
> @@ -374,5 +379,6 @@ int longest_prefix(const char *path, const char
> *prefix_list)
>                         else
>                                 length = -1;
>                 }
> -       return max_length > length ? max_length : length;
> +       return max_length > length || !is_separator(path[length]) ?
> +               max_length : length;
>  }
> diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
> index 1d8ef0b..6c8757d 100644
> --- a/t/t1504-ceiling-directories.sh
> +++ b/t/t1504-ceiling-directories.sh
> @@ -43,4 +43,12 @@ test_expect_success 'with matching ceiling directories' '
> 
>  '
> 
> +test_expect_success 'with non-directory prefix' '
> +
> +       GIT_CEILING_DIRECTORIES="$CWD/sub" &&
> +       export GIT_CEILING_DIRECTORIES &&
> +       (cd subdir && git rev-parse --git-dir)
> +
> +'
> +
>  test_done
> 

[SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

In certain setups, trying to access a non-existing .git/ can take quite
some time, for example when the directory is an automount directory.

Allow the user to specify directories where Git would stop looking for
a .git/ directory: GIT_CEILING_DIRECTORIES, if set, is expected to be
a colon delimited list of such barrier directories.

Note: if GIT_CEILING_DIRECTORIES=/a/b and your current working directory
is /a, Git will _not_ stop looking.

Note2: you must not specify the directories with trailing slashes.

Initial implementation by David Reiss.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Thu, 15 May 2008, David Reiss wrote:

	> This meets my needs.
	> 
	> Junio, if you'd like, I can incorporate your suggestions of 
	> normalizing paths internally and testing more corner cases.  But
	> if you just want to take this version, I'll be fine.

	For your pleasure, the combined patch of my work (so that it is 
	easier to hack on normalizing paths, if you still want that --
	you might want to look at get_pathspec() and its history in that 
	case).

 Documentation/git.txt          |    6 ++++
 cache.h                        |    2 +
 path.c                         |   25 ++++++++++++++++++
 setup.c                        |   11 ++++++-
 t/t1504-ceiling-directories.sh |   54 ++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                  |    1 +
 6 files changed, 97 insertions(+), 2 deletions(-)
 create mode 100644 t/t1504-ceiling-directories.sh

diff --git a/Documentation/git.txt b/Documentation/git.txt
index adcd3e0..e4413bf 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -415,6 +415,12 @@ 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_DIRECTORIES'::
+	If set (to a colon delimited list of absolute directories), Git
+	will refuse to look for the .git/ directory further when hitting
+	one of those directories (otherwise it would traverse the parent
+	directories until hitting the root directory).
+
 git Commits
 ~~~~~~~~~~~
 'GIT_AUTHOR_NAME'::
diff --git a/cache.h b/cache.h
index a8638b1..c31b4c7 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_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
@@ -522,6 +523,7 @@ static inline int is_absolute_path(const char *path)
 	return path[0] == '/';
 }
 const char *make_absolute_path(const char *path);
+int longest_prefix(const char *path, const char *prefix_list);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index b7c24a2..a097ecc 100644
--- a/path.c
+++ b/path.c
@@ -357,3 +357,28 @@ const char *make_absolute_path(const char *path)
 
 	return buf;
 }
+
+static int is_separator(char c)
+{
+	return !c || c == '/';
+}
+
+int longest_prefix(const char *path, const char *prefix_list)
+{
+	int max_length = 0, length = 0, i;
+
+	for (i = 0; prefix_list[i]; i++)
+		if (prefix_list[i] == ':') {
+			if (length > max_length && is_separator(path[length]))
+				max_length = length;
+			length = 0;
+		}
+		else if (length >= 0) {
+			if (prefix_list[i] == path[length])
+				length++;
+			else
+				length = -1;
+		}
+	return max_length > length || !is_separator(path[length]) ?
+		max_length : length;
+}
diff --git a/setup.c b/setup.c
index 9e9a2b1..cece3e4 100644
--- a/setup.c
+++ b/setup.c
@@ -365,10 +365,13 @@ 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 *ceiling_directories =
+		getenv(CEILING_DIRECTORIES_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv;
 	const char *gitfile_dir;
 	int len, offset;
+	int min_offset = 0;
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -422,6 +425,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
 
+	if (ceiling_directories)
+		min_offset = longest_prefix(cwd, ceiling_directories);
+
 	/*
 	 * Test in the following order (relative to the cwd):
 	 * - .git (file containing "gitdir: <path>")
@@ -453,7 +459,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		}
 		chdir("..");
 		do {
-			if (!offset) {
+			if (offset <= min_offset) {
 				if (nongit_ok) {
 					if (chdir(cwd))
 						die("Cannot come back to cwd");
@@ -462,7 +468,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				}
 				die("Not a git repository");
 			}
-		} while (cwd[--offset] != '/');
+		} while (offset > min_offset &&
+				--offset >=0 && cwd[offset] != '/');
 	}
 
 	inside_git_dir = 0;
diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
new file mode 100644
index 0000000..6c8757d
--- /dev/null
+++ b/t/t1504-ceiling-directories.sh
@@ -0,0 +1,54 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='test limiting with GIT_CEILING_DIRECTORIES'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	CWD="$(pwd -P)" &&
+	mkdir subdir
+
+'
+
+test_expect_success 'without GIT_CEILING_DIRECTORIES' '
+
+	test .git = "$(git rev-parse --git-dir)" &&
+	(cd subdir && git rev-parse --git-dir) &&
+	echo "$CWD" &&
+	test "$CWD/.git" = "$(cd subdir && git rev-parse --git-dir)"
+
+'
+
+test_expect_success 'with non-matching ceiling directory' '
+
+	test "$(GIT_CEILING_DIRECTORIES="$CWD/X" \
+		git rev-parse --git-dir)" = .git
+
+'
+
+test_expect_success 'with matching ceiling directories' '
+
+	GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	git rev-parse --git-dir &&
+	GIT_CEILING_DIRECTORIES="$CWD/subdir:$CWD/X" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	git rev-parse --git-dir
+
+'
+
+test_expect_success 'with non-directory prefix' '
+
+	GIT_CEILING_DIRECTORIES="$CWD/sub" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && git rev-parse --git-dir)
+
+'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5002fb0..c3a3167 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_DIRECTORIES
 unset SHA1_FILE_DIRECTORIES
 unset SHA1_FILE_DIRECTORY
 GIT_MERGE_VERBOSITY=5
-- 
1.5.5.1.425.g5f464.dirty

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Sixt]

Johannes Schindelin schrieb:
> +'GIT_CEILING_DIRECTORIES'::
> +	If set (to a colon delimited list of absolute directories), Git
> +	will refuse to look for the .git/ directory further when hitting
> +	one of those directories (otherwise it would traverse the parent
> +	directories until hitting the root directory).

Hmm.

Looking at the current implementation, this should be written as:

	If set to a colon delimited list of absolute directories,
	and the current directory is in or below one of them, then
	these are the top-most directories in which Git will look for
	a .git/ directory (otherwise it would traverse the parent
	directories until hitting the root directory).

But from David's initial commit message:

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 implementation it will still look for a non-existing /home/.git, and,
hence, should take a long time to complete.

David, does this really meet your needs?

-- Hannes

Re: [PATCH v3] Add support for GIT_CEILING_DIRS

[Johannes Sixt]

Johannes Schindelin schrieb:
> On Thu, 15 May 2008, Junio C Hamano wrote:
> 
>> David Reiss <dreiss@facebook.com> writes:
>> How well would this colon separated list work with msys folks?
> 
> Not well at all.  At least for the moment, I think setting this variable 
> would fail (since it would be rewritten into a Windows-PATH-style string).  
> Though honestly, I have no idea what getcwd() does on MinGW.

We would have to use ';' instead of ':' in longest_prefix(). Then it will
work automatically. Except that it does not ignore case in path names and
drive letters. Hence, the user must set this environment variable to the
precise path name(s).

> Steffen is working on that rewriting stuff, so maybe it will be a 
> non-issue, eventually.

This will not matter. When we change the code to use is_path_separator()
instead of hard-coded c == ':', then it will work automatically. We do
that already for GIT_ALTERNATE_OBJECT_DIRECTORIES.

-- Hannes

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

Hi,

On Fri, 16 May 2008, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > +'GIT_CEILING_DIRECTORIES'::
> > +	If set (to a colon delimited list of absolute directories), Git
> > +	will refuse to look for the .git/ directory further when hitting
> > +	one of those directories (otherwise it would traverse the parent
> > +	directories until hitting the root directory).
> 
> Hmm.
> 
> Looking at the current implementation, this should be written as:
> 
> 	If set to a colon delimited list of absolute directories,
> 	and the current directory is in or below one of them, then
> 	these are the top-most directories in which Git will look for
> 	a .git/ directory (otherwise it would traverse the parent
> 	directories until hitting the root directory).

According to the test case

        GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
        export GIT_CEILING_DIRECTORIES &&
        (cd subdir && test_must_fail git rev-parse --git-dir)

this is not the case.  If you have something like /bla/subdir and you are 
in /bla/subdir, it will not look for .git/.

Ciao,
Dscho "who leaves it as an exercise to the reader to see why the code 
does what it does"

Re: [PATCH v3] Add support for GIT_CEILING_DIRS

[Johannes Schindelin]

Hi,

On Fri, 16 May 2008, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
> > On Thu, 15 May 2008, Junio C Hamano wrote:
> > 
> >> David Reiss <dreiss@facebook.com> writes:
> >> How well would this colon separated list work with msys folks?
> > 
> > Not well at all.  At least for the moment, I think setting this variable 
> > would fail (since it would be rewritten into a Windows-PATH-style string).  
> > Though honestly, I have no idea what getcwd() does on MinGW.
> 
> We would have to use ';' instead of ':' in longest_prefix(). Then it will
> work automatically. Except that it does not ignore case in path names and
> drive letters. Hence, the user must set this environment variable to the
> precise path name(s).

Yes, good point.

> > Steffen is working on that rewriting stuff, so maybe it will be a 
> > non-issue, eventually.
> 
> This will not matter. When we change the code to use is_path_separator()
> instead of hard-coded c == ':', then it will work automatically. We do
> that already for GIT_ALTERNATE_OBJECT_DIRECTORIES.

You are right, of course.

Ciao,
Dscho

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Sixt]

Johannes Schindelin schrieb:
> Hi,
> 
> On Fri, 16 May 2008, Johannes Sixt wrote:
> 
>> Johannes Schindelin schrieb:
>>> +'GIT_CEILING_DIRECTORIES'::
>>> +	If set (to a colon delimited list of absolute directories), Git
>>> +	will refuse to look for the .git/ directory further when hitting
>>> +	one of those directories (otherwise it would traverse the parent
>>> +	directories until hitting the root directory).
>> Hmm.
>>
>> Looking at the current implementation, this should be written as:
>>
>> 	If set to a colon delimited list of absolute directories,
>> 	and the current directory is in or below one of them, then
>> 	these are the top-most directories in which Git will look for
>> 	a .git/ directory (otherwise it would traverse the parent
>> 	directories until hitting the root directory).
> 
> According to the test case
> 
>         GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
>         export GIT_CEILING_DIRECTORIES &&
>         (cd subdir && test_must_fail git rev-parse --git-dir)
> 
> this is not the case.  If you have something like /bla/subdir and you are 
> in /bla/subdir, it will not look for .git/.

No, the test just shows that it does not *find* a subdir/.git/, but it
does not show that it doesn't even *look* for it.

Let's first clarify the semantics of GIT_CEILING_DIRECTORIES before we
start fixing something: Can a directory that is named in
GIT_CEILING_DIRECTORIES be a git repository or not?

David's original motivation was that it cannot; in your implementation it can.

-- Hannes

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[David Reiss]

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.

--David


Johannes Sixt wrote:
> Johannes Schindelin schrieb:
>> Hi,
>>
>> On Fri, 16 May 2008, Johannes Sixt wrote:
>>
>>> Johannes Schindelin schrieb:
>>>> +'GIT_CEILING_DIRECTORIES'::
>>>> +   If set (to a colon delimited list of absolute directories), Git
>>>> +   will refuse to look for the .git/ directory further when hitting
>>>> +   one of those directories (otherwise it would traverse the parent
>>>> +   directories until hitting the root directory).
>>> Hmm.
>>>
>>> Looking at the current implementation, this should be written as:
>>>
>>>      If set to a colon delimited list of absolute directories,
>>>      and the current directory is in or below one of them, then
>>>      these are the top-most directories in which Git will look for
>>>      a .git/ directory (otherwise it would traverse the parent
>>>      directories until hitting the root directory).
>>
>> According to the test case
>>
>>         GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
>>         export GIT_CEILING_DIRECTORIES &&
>>         (cd subdir && test_must_fail git rev-parse --git-dir)
>>
>> this is not the case.  If you have something like /bla/subdir and you are
>> in /bla/subdir, it will not look for .git/.
> 
> No, the test just shows that it does not *find* a subdir/.git/, but it
> does not show that it doesn't even *look* for it.
> 
> Let's first clarify the semantics of GIT_CEILING_DIRECTORIES before we
> start fixing something: Can a directory that is named in
> GIT_CEILING_DIRECTORIES be a git repository or not?
> 
> David's original motivation was that it cannot; in your implementation
> it can.
> 
> -- Hannes
> 

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

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");
diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
index 6c8757d..edc00be 100644
--- a/t/t1504-ceiling-directories.sh
+++ b/t/t1504-ceiling-directories.sh
@@ -30,6 +30,15 @@ test_expect_success 'with non-matching ceiling directory' '
 
 '
 
+test_expect_success 'with matching ceiling directory' '
+
+	GIT_CEILING_DIRECTORIES="$CWD" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	test_must_fail git rev-parse --git-dir
+
+'
+
 test_expect_success 'with matching ceiling directories' '
 
 	GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
-- snap --

I will post the squashed patch as a response to this mail.

Ciao,
Dscho

[2nd SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

In certain setups, trying to access a non-existing .git/ can take quite
some time, for example when the directory is an automount directory.

Allow the user to specify directories where Git would stop looking for
a .git/ directory: GIT_CEILING_DIRECTORIES, if set, is expected to be
a colon delimited list of such barrier directories.

Note: if GIT_CEILING_DIRECTORIES=/a/b and your current working directory
is /a, Git will _not_ stop looking.

Note2: you must not specify the directories with trailing slashes.

Initial implementation by David Reiss.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 Documentation/git.txt          |    6 ++++
 cache.h                        |    2 +
 path.c                         |   25 ++++++++++++++++
 setup.c                        |   14 +++++++-
 t/t1504-ceiling-directories.sh |   63 ++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                  |    1 +
 6 files changed, 109 insertions(+), 2 deletions(-)
 create mode 100644 t/t1504-ceiling-directories.sh

diff --git a/Documentation/git.txt b/Documentation/git.txt
index adcd3e0..e4413bf 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -415,6 +415,12 @@ 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_DIRECTORIES'::
+	If set (to a colon delimited list of absolute directories), Git
+	will refuse to look for the .git/ directory further when hitting
+	one of those directories (otherwise it would traverse the parent
+	directories until hitting the root directory).
+
 git Commits
 ~~~~~~~~~~~
 'GIT_AUTHOR_NAME'::
diff --git a/cache.h b/cache.h
index a8638b1..c31b4c7 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_DIRECTORIES_ENVIRONMENT "GIT_CEILING_DIRECTORIES"
 #define GITATTRIBUTES_FILE ".gitattributes"
 #define INFOATTRIBUTES_FILE "info/attributes"
 #define ATTRIBUTE_MACRO_PREFIX "[attr]"
@@ -522,6 +523,7 @@ static inline int is_absolute_path(const char *path)
 	return path[0] == '/';
 }
 const char *make_absolute_path(const char *path);
+int longest_prefix(const char *path, const char *prefix_list);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */
 extern int sha1_object_info(const unsigned char *, unsigned long *);
diff --git a/path.c b/path.c
index b7c24a2..a097ecc 100644
--- a/path.c
+++ b/path.c
@@ -357,3 +357,28 @@ const char *make_absolute_path(const char *path)
 
 	return buf;
 }
+
+static int is_separator(char c)
+{
+	return !c || c == '/';
+}
+
+int longest_prefix(const char *path, const char *prefix_list)
+{
+	int max_length = 0, length = 0, i;
+
+	for (i = 0; prefix_list[i]; i++)
+		if (prefix_list[i] == ':') {
+			if (length > max_length && is_separator(path[length]))
+				max_length = length;
+			length = 0;
+		}
+		else if (length >= 0) {
+			if (prefix_list[i] == path[length])
+				length++;
+			else
+				length = -1;
+		}
+	return max_length > length || !is_separator(path[length]) ?
+		max_length : length;
+}
diff --git a/setup.c b/setup.c
index 9e9a2b1..2f7a17a 100644
--- a/setup.c
+++ b/setup.c
@@ -365,10 +365,13 @@ 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 *ceiling_directories =
+		getenv(CEILING_DIRECTORIES_ENVIRONMENT);
 	static char cwd[PATH_MAX+1];
 	const char *gitdirenv;
 	const char *gitfile_dir;
 	int len, offset;
+	int min_offset = 0;
 
 	/*
 	 * Let's assume that we are in a git repository.
@@ -422,6 +425,9 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	if (!getcwd(cwd, sizeof(cwd)-1))
 		die("Unable to read current working directory");
 
+	if (ceiling_directories)
+		min_offset = longest_prefix(cwd, ceiling_directories);
+
 	/*
 	 * Test in the following order (relative to the cwd):
 	 * - .git (file containing "gitdir: <path>")
@@ -435,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))
@@ -453,7 +461,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 		}
 		chdir("..");
 		do {
-			if (!offset) {
+			if (offset <= min_offset) {
+non_git:
 				if (nongit_ok) {
 					if (chdir(cwd))
 						die("Cannot come back to cwd");
@@ -462,7 +471,8 @@ const char *setup_git_directory_gently(int *nongit_ok)
 				}
 				die("Not a git repository");
 			}
-		} while (cwd[--offset] != '/');
+		} while (offset > min_offset &&
+				--offset >=0 && cwd[offset] != '/');
 	}
 
 	inside_git_dir = 0;
diff --git a/t/t1504-ceiling-directories.sh b/t/t1504-ceiling-directories.sh
new file mode 100644
index 0000000..edc00be
--- /dev/null
+++ b/t/t1504-ceiling-directories.sh
@@ -0,0 +1,63 @@
+#!/bin/sh
+#
+# Copyright (c) 2007 Johannes E. Schindelin
+#
+
+test_description='test limiting with GIT_CEILING_DIRECTORIES'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	CWD="$(pwd -P)" &&
+	mkdir subdir
+
+'
+
+test_expect_success 'without GIT_CEILING_DIRECTORIES' '
+
+	test .git = "$(git rev-parse --git-dir)" &&
+	(cd subdir && git rev-parse --git-dir) &&
+	echo "$CWD" &&
+	test "$CWD/.git" = "$(cd subdir && git rev-parse --git-dir)"
+
+'
+
+test_expect_success 'with non-matching ceiling directory' '
+
+	test "$(GIT_CEILING_DIRECTORIES="$CWD/X" \
+		git rev-parse --git-dir)" = .git
+
+'
+
+test_expect_success 'with matching ceiling directory' '
+
+	GIT_CEILING_DIRECTORIES="$CWD" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	test_must_fail git rev-parse --git-dir
+
+'
+
+test_expect_success 'with matching ceiling directories' '
+
+	GIT_CEILING_DIRECTORIES="$CWD/X:$CWD/subdir" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	git rev-parse --git-dir &&
+	GIT_CEILING_DIRECTORIES="$CWD/subdir:$CWD/X" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && test_must_fail git rev-parse --git-dir) &&
+	git rev-parse --git-dir
+
+'
+
+test_expect_success 'with non-directory prefix' '
+
+	GIT_CEILING_DIRECTORIES="$CWD/sub" &&
+	export GIT_CEILING_DIRECTORIES &&
+	(cd subdir && git rev-parse --git-dir)
+
+'
+
+test_done
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 5002fb0..c3a3167 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_DIRECTORIES
 unset SHA1_FILE_DIRECTORIES
 unset SHA1_FILE_DIRECTORY
 GIT_MERGE_VERBOSITY=5
-- 
1.5.5.1.490.g1644c11

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Sixt]

Johannes Schindelin schrieb:
> 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

Re: [SQUASHED PATCH] Add support for GIT_CEILING_DIRECTORIES

[Johannes Schindelin]

Hi,

On Mon, 19 May 2008, Johannes Sixt wrote:

> Johannes Schindelin schrieb:
>
> > 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 do not share the notion that "goto" = BAD.

> [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.]

I am not in support of this feature at all, since I do not need it.

However, I saw that David needs it, but I did not agree with the way he 
implemented his patch.  So I tried to show how it is possible to do it in 
a way that looks simpler to me.

Now the ball is back in David's field: he can just take my patch, mangle 
it until it does what he wants, and resubmit.  That said, if the result 
offends my eye again, I will complain again.

Ciao,
Dscho