Provide a mechanism to turn off symlink resolution in ceiling paths

Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks
in ceiling paths' changed the setup code to resolve symlinks in the
entries in GIT_CEILING_DIRECTORIES.  Because those entries are
compared textually to the symlink-resolved current directory, an
entry in GIT_CEILING_DIRECTORIES that contained a symlink would have
no effect.  It was known that this could cause performance problems
if the symlink resolution *itself* touched slow filesystems, but it
was thought that such use cases would be unlikely.  The intention of
the earlier change was to deal with a case when the user has this:

	GIT_CEILING_DIRECTORIES=/home/gitster

but in reality, /home/gitster is a symbolic link to somewhere else,
e.g. /net/machine/home4/gitster. A textual comparison between the
specified value /home/gitster and the location getcwd(3) returns
would not help us, but readlink("/home/gitster") would still be
fast.

After this change was released, Anders Kaseorg <andersk@mit.edu>
reported:

> [...] my computer has been acting so slow when I’m not connected to
> the network.  I put various network filesystem paths in
> $GIT_CEILING_DIRECTORIES, such as
> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents
> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and
> /afs/athena.mit.edu/user/a/n which all live in different AFS
> volumes).  Now when I’m not connected to the network, every
> invocation of Git, including the __git_ps1 in my shell prompt, waits
> for AFS to timeout.

To allow users to work around this problem, give them a mechanism to
turn off symlink resolution in GIT_CEILING_DIRECTORIES entries.  All
the entries that follow an empty entry will not be checked for symbolic
links and used literally in comparison.  E.g. with these:

	GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or
	GIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy

we will not readlink("/xyzzy") because it comes after an empty entry.

With the former (but not with the latter), "/foo/bar" comes after an
empty entry, and we will not readlink it, either.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/git.txt b/Documentation/git.txt
index 6710cb0a418fef503f0a8419e9b3cd617a15c485..6bc18014cd946e0330f9ed68aceda8ff3e659b94 100644 (file)
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -653,12 +653,19 @@ git so take care if using Cogito etc.
        The '--namespace' command-line option also sets this value.
 
 'GIT_CEILING_DIRECTORIES'::
-       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.)
+       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 (useful for
+       excluding slow-loading network directories).  It will not
+       exclude the current working directory or a GIT_DIR set on the
+       command line or in the environment.  Normally, Git has to read
+       the entries in this list and resolve any symlink that
+       might be present in order to compare them with the current
+       directory.  However, if even this access is slow, you
+       can add an empty entry to the list to tell Git that the
+       subsequent entries are not symlinks and needn't be resolved;
+       e.g.,
+       'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.
 
 'GIT_DISCOVERY_ACROSS_FILESYSTEM'::
        When run in a directory that does not have ".git" repository

diff --git a/setup.c b/setup.c
index f108c4b990c0e4f6a98c1fe34d3031040c84195a..1b120175f2184b782f389d2fc99f3e37710f35d3 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
 /*
  * A "string_list_each_func_t" function that canonicalizes an entry
  * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or
- * discards it if unusable.
+ * discards it if unusable.  The presence of an empty entry in
+ * GIT_CEILING_DIRECTORIES turns off canonicalization for all
+ * subsequent entries.
  */
 static int canonicalize_ceiling_entry(struct string_list_item *item,
-                                     void *unused)
+                                     void *cb_data)
 {
+       int *empty_entry_found = cb_data;
        char *ceil = item->string;
-       const char *real_path;
 
-       if (!*ceil || !is_absolute_path(ceil))
+       if (!*ceil) {
+               *empty_entry_found = 1;
                return 0;
-       real_path = real_path_if_valid(ceil);
-       if (!real_path)
+       } else if (!is_absolute_path(ceil)) {
                return 0;
-       free(item->string);
-       item->string = xstrdup(real_path);
-       return 1;
+       } else if (*empty_entry_found) {
+               /* Keep entry but do not canonicalize it */
+               return 1;
+       } else {
+               const char *real_path = real_path_if_valid(ceil);
+               if (!real_path)
+                       return 0;
+               free(item->string);
+               item->string = xstrdup(real_path);
+               return 1;
+       }
 }
 
 /*
@@ -679,9 +689,11 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
                return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);
 
        if (env_ceiling_dirs) {
+               int empty_entry_found = 0;
+
                string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
                filter_string_list(&ceiling_dirs, 0,
-                                  canonicalize_ceiling_entry, NULL);
+                                  canonicalize_ceiling_entry, &empty_entry_found);
                ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
                string_list_clear(&ceiling_dirs, 0);
        }

diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh
index cce87a5ab523d3a4232e9e11863aa3a6e8ca0211..3d51615e42d53a0c7794533eb27049606981fbf3 100755 (executable)
--- a/t/t1504-ceiling-dirs.sh
+++ b/t/t1504-ceiling-dirs.sh
@@ -44,6 +44,10 @@ test_prefix ceil_at_sub ""
 GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
 test_prefix ceil_at_sub_slash ""
 
+if test_have_prereq SYMLINKS
+then
+       ln -s sub top
+fi
 
 mkdir -p sub/dir || exit 1
 cd sub/dir || exit 1
@@ -68,6 +72,19 @@ test_fail subdir_ceil_at_sub
 GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/"
 test_fail subdir_ceil_at_sub_slash
 
+if test_have_prereq SYMLINKS
+then
+       GIT_CEILING_DIRECTORIES="$TRASH_ROOT/top"
+       test_fail subdir_ceil_at_top
+       GIT_CEILING_DIRECTORIES="$TRASH_ROOT/top/"
+       test_fail subdir_ceil_at_top_slash
+
+       GIT_CEILING_DIRECTORIES=":$TRASH_ROOT/top"
+       test_prefix subdir_ceil_at_top_no_resolve "sub/dir/"
+       GIT_CEILING_DIRECTORIES=":$TRASH_ROOT/top/"
+       test_prefix subdir_ceil_at_top_slash_no_resolve "sub/dir/"
+fi
+
 GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub/dir"
 test_prefix subdir_ceil_at_subdir "sub/dir/"