Anders Kaseorg [off-list ref] writes:
On 10/29/2012 01:10 AM, Michael Haggerty wrote:
quoted
How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a
slowdown?
Sorry to bring up this old thread again, but I just realized why 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.
Thanks for a report.
Assuming that this says "yes":
D=/afs/athena.mit.edu/user/a/n/andersk/my/dir
cd "$D"
test "$(/bin/pwd)" = "$D" && echo yes
iow, AFS mounting does not have funny symbolic link issues that make
the logical and physical PWD to be different like some automounter
implementation used to have, perhaps we can introduce a way for the
user to say "The element of this CEILING list do not have any
alias due to funny symlinks" to solve this. Perhaps existing of an
empty element in the list would do? E.g.
GIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk
or something like that. And in such a case, we do not run realpath
on the elements on the list before comparing them with what we get
from getcwd(3).
Of course, you could condionally unset the environment while
offline, but that feels like an ugly hack.
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.
After this change was released, Anders Kaseorg [off-list ref]
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.
So provide the following mechanism to turn off symlink resolution in
GIT_CEILING_DIRECTORIES entries: if that environment variable contains
an empty entry (e.g., GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or
GIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy), then do not resolve
symlinks in paths that follow the empty entry.
---
This is a possible implementation (untested!) of Junio's suggestion,
with the slight twist that the empty entry can appear anywhere in
GIT_CEILING_DIRECTORIES and only turns off symlink expansion for
subsequent entries. (The original suggestion would be similarly easy
to implement if it is preferred.)
Unfortunately I am swamped with other work right now so I don't have
time to test the code and might not be able to respond promptly to
feedback.
Another alternative (not implemented here) would be to support a
second environment variable with an ugly name like
GIT_CEILING_DIRECTORIES_NO_SYMLINKS which, when set, tells Git not to
resolve symlinks in GIT_CEILING_DIRECTORIES. Hopefully the variable
name itself would warn the user that symlinks are an issue.
The ugliness of the situation unfortunately seems to preclude a
non-ugly solution of one form or another.
Documentation/git.txt | 18 ++++++++++++------
setup.c | 32 ++++++++++++++++++++++----------
2 files changed, 34 insertions(+), 16 deletions(-)
diff --git a/Documentation/git.txt b/Documentation/git.txt
index da0115f..35c0517 100644
--- a/Documentation/git.txt
+++ b/Documentation/git.txt
@@ -674,12 +674,18 @@ 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 are read to resolve any symlinks that
+ might be present. 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 f108c4b..1b12017 100644
--- 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);
}--
1.8.1.1
On Tue, 19 Feb 2013, Junio C Hamano wrote:
Assuming that this says "yes":
D=/afs/athena.mit.edu/user/a/n/andersk/my/dir
cd "$D"
test "$(/bin/pwd)" = "$D" && echo yes
Correct.
Perhaps existing of an empty element in the list would do? E.g.
GIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk
or something like that. And in such a case, we do not run realpath on
the elements on the list before comparing them with what we get from
getcwd(3).
That seems reasonable, and has the advantage of backwards compatibility
with versions before 1.8.1.2, I guess.
Anders