{"thread":{"id":"31880","subject":"[PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","startedAt":"2012-10-21T05:57:29Z","lastAt":"2013-02-22T07:23:08Z","messageCount":26,"participants":["Michael Haggerty","Junio C Hamano","Johannes Sixt","David Aguilar","Lars Damerow","Anders Kaseorg"],"isPatch":true,"patchVersion":3,"patchTotal":8},"messages":[{"id":"201608","messageId":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":null,"subject":"[PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:29Z","receivedAt":"2012-10-21T05:57:29Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"v3 of the series, reworked WRT v2:\n\n* Change longest_ancestor_length() to take a string_list for its\n  prefixes argument (instead of a PATH_SEP-separated string).\n\n* Move the responsibility for canonicalizing prefixes from\n  longest_ancestor_length() to its caller in\n  setup_git_directory_gently_1().  This keeps\n  longest_ancestor_length() testable, though the test inputs now have\n  to be canonicalized.\n\n* Remove function string_list_longest_prefix(), which was mainly\n  intended to be used in the implementation of\n  longest_ancestor_length() but is now unneeded.\n\nThanks to Junio for his comments.\n\nI would like to explicitly point out a possible objection to this\nwhole enterprise.  GIT_CEILING_DIRECTORIES is used to prevent git from\nmucking around in certain directories, to avoid expensive accesses\n(for example of remote directories).  The original motivation for the\nfeature was a user whose home directory /home/$USER was automounted.\nWhen git was invoked outside of a git tree, it would crawl up the\nfilesystem tree, eventually reaching /home, and then try to access the\npaths\n\n    /home\n    /home/.git\n    /home/.git/objects\n    /home/objects\n\nThe last three accesses would be very expensive because the system\nwould attempt to automount the entries listed.\n\nThis patch series has the side effect that all of the directories\nlisted in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to\nresolve any symlinks that are present in their paths.  It is\nadmittedly odd that a feature intended to avoid accessing expensive\ndirectories would now *intentionally* access directories near the\nexpensive ones.  In the above scenario this shouldn't be a problem,\nbecause /home would be the directory listed in\nGIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be\nexpensive.  But there might be other scenarios for which this patch\nseries causes a performance regression.\n\nAnother point: After this change, it would be trivially possible to\nsupport non-absolute paths in GIT_CEILING_DIRECTORIES; just remove the\ncall to is_absolute_path() from normalize_ceiling_entry().  This might\nbe convenient for the test suite.\n\nMichael Haggerty (8):\n  Introduce new static function real_path_internal()\n  real_path_internal(): add comment explaining use of cwd\n  Introduce new function real_path_if_valid()\n  longest_ancestor_length(): use string_list_split()\n  longest_ancestor_length(): take a string_list argument for prefixes\n  longest_ancestor_length(): require prefix list entries to be\n    normalized\n  normalize_ceiling_entry(): resolve symlinks\n  string_list_longest_prefix(): remove function\n\n Documentation/technical/api-string-list.txt |   8 ---\n abspath.c                                   | 105 ++++++++++++++++++++++------\n cache.h                                     |   3 +-\n path.c                                      |  46 ++++++------\n setup.c                                     |  32 ++++++++-\n string-list.c                               |  20 ------\n string-list.h                               |   8 ---\n t/t0060-path-utils.sh                       |  41 ++++-------\n t/t0063-string-list.sh                      |  30 --------\n test-path-utils.c                           |   8 ++-\n test-string-list.c                          |  20 ------\n 11 files changed, 157 insertions(+), 164 deletions(-)\n\n-- \n1.7.11.3\n"},{"id":"201610","messageId":"1350799057-13846-2-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 1/8] Introduce new static function real_path_internal()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:30Z","receivedAt":"2012-10-21T05:57:30Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"It accepts a new parameter, die_on_error.  If die_on_error is false,\nit simply cleans up after itself and returns NULL rather than dying.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n abspath.c | 93 ++++++++++++++++++++++++++++++++++++++++++++++++---------------\n 1 file changed, 72 insertions(+), 21 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 05f2d79..a7ab8e9 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -15,15 +15,26 @@ int is_directory(const char *path)\n #define MAXDEPTH 5\n \n /*\n- * Use this to get the real path, i.e. resolve links. If you want an\n- * absolute path but don't mind links, use absolute_path.\n+ * Return the real path (i.e., absolute path, with symlinks resolved\n+ * and extra slashes removed) equivalent to the specified path.  (If\n+ * you want an absolute path but don't mind links, use\n+ * absolute_path().)  The return value is a pointer to a static\n+ * buffer.\n+ *\n+ * The input and all intermediate paths must be shorter than MAX_PATH.\n+ * The directory part of path (i.e., everything up to the last\n+ * dir_sep) must denote a valid, existing directory, but the last\n+ * component need not exist.  If die_on_error is set, then die with an\n+ * informative error message if there is a problem.  Otherwise, return\n+ * NULL on errors (without generating any output).\n  *\n  * If path is our buffer, then return path, as it's already what the\n  * user wants.\n  */\n-const char *real_path(const char *path)\n+static const char *real_path_internal(const char *path, int die_on_error)\n {\n \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n+\tchar *retval = NULL;\n \tchar cwd[1024] = \"\";\n \tint buf_index = 1;\n \n@@ -35,11 +46,19 @@ const char *real_path(const char *path)\n \tif (path == buf || path == next_buf)\n \t\treturn path;\n \n-\tif (!*path)\n-\t\tdie(\"The empty string is not a valid path\");\n+\tif (!*path) {\n+\t\tif (die_on_error)\n+\t\t\tdie(\"The empty string is not a valid path\");\n+\t\telse\n+\t\t\tgoto error_out;\n+\t}\n \n-\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n-\t\tdie (\"Too long path: %.*s\", 60, path);\n+\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) {\n+\t\tif (die_on_error)\n+\t\t\tdie(\"Too long path: %.*s\", 60, path);\n+\t\telse\n+\t\t\tgoto error_out;\n+\t}\n \n \twhile (depth--) {\n \t\tif (!is_directory(buf)) {\n@@ -54,20 +73,36 @@ const char *real_path(const char *path)\n \t\t}\n \n \t\tif (*buf) {\n-\t\t\tif (!*cwd && !getcwd(cwd, sizeof(cwd)))\n-\t\t\t\tdie_errno (\"Could not get current working directory\");\n+\t\t\tif (!*cwd && !getcwd(cwd, sizeof(cwd))) {\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie_errno(\"Could not get current working directory\");\n+\t\t\t\telse\n+\t\t\t\t\tgoto error_out;\n+\t\t\t}\n \n-\t\t\tif (chdir(buf))\n-\t\t\t\tdie_errno (\"Could not switch to '%s'\", buf);\n+\t\t\tif (chdir(buf)) {\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie_errno(\"Could not switch to '%s'\", buf);\n+\t\t\t\telse\n+\t\t\t\t\tgoto error_out;\n+\t\t\t}\n+\t\t}\n+\t\tif (!getcwd(buf, PATH_MAX)) {\n+\t\t\tif (die_on_error)\n+\t\t\t\tdie_errno(\"Could not get current working directory\");\n+\t\t\telse\n+\t\t\t\tgoto error_out;\n \t\t}\n-\t\tif (!getcwd(buf, PATH_MAX))\n-\t\t\tdie_errno (\"Could not get current working directory\");\n \n \t\tif (last_elem) {\n \t\t\tsize_t len = strlen(buf);\n-\t\t\tif (len + strlen(last_elem) + 2 > PATH_MAX)\n-\t\t\t\tdie (\"Too long path name: '%s/%s'\",\n-\t\t\t\t\t\tbuf, last_elem);\n+\t\t\tif (len + strlen(last_elem) + 2 > PATH_MAX) {\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie(\"Too long path name: '%s/%s'\",\n+\t\t\t\t\t    buf, last_elem);\n+\t\t\t\telse\n+\t\t\t\t\tgoto error_out;\n+\t\t\t}\n \t\t\tif (len && !is_dir_sep(buf[len-1]))\n \t\t\t\tbuf[len++] = '/';\n \t\t\tstrcpy(buf + len, last_elem);\n@@ -77,10 +112,18 @@ const char *real_path(const char *path)\n \n \t\tif (!lstat(buf, &st) && S_ISLNK(st.st_mode)) {\n \t\t\tssize_t len = readlink(buf, next_buf, PATH_MAX);\n-\t\t\tif (len < 0)\n-\t\t\t\tdie_errno (\"Invalid symlink '%s'\", buf);\n-\t\t\tif (PATH_MAX <= len)\n-\t\t\t\tdie(\"symbolic link too long: %s\", buf);\n+\t\t\tif (len < 0) {\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie_errno(\"Invalid symlink '%s'\", buf);\n+\t\t\t\telse\n+\t\t\t\t\tgoto error_out;\n+\t\t\t}\n+\t\t\tif (PATH_MAX <= len) {\n+\t\t\t\tif (die_on_error)\n+\t\t\t\t\tdie(\"symbolic link too long: %s\", buf);\n+\t\t\t\telse\n+\t\t\t\t\tgoto error_out;\n+\t\t\t}\n \t\t\tnext_buf[len] = '\\0';\n \t\t\tbuf = next_buf;\n \t\t\tbuf_index = 1 - buf_index;\n@@ -89,10 +132,18 @@ const char *real_path(const char *path)\n \t\t\tbreak;\n \t}\n \n+\tretval = buf;\n+error_out:\n+\tfree(last_elem);\n \tif (*cwd && chdir(cwd))\n \t\tdie_errno (\"Could not change back to '%s'\", cwd);\n \n-\treturn buf;\n+\treturn retval;\n+}\n+\n+const char *real_path(const char *path)\n+{\n+\treturn real_path_internal(path, 1);\n }\n \n static const char *get_pwd_cwd(void)\n-- \n1.7.11.3\n"},{"id":"201609","messageId":"1350799057-13846-3-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 2/8] real_path_internal(): add comment explaining use of cwd","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:31Z","receivedAt":"2012-10-21T05:57:31Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n abspath.c | 7 +++++++\n 1 file changed, 7 insertions(+)\n\ndiff --git a/abspath.c b/abspath.c\nindex a7ab8e9..f8a526f 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -35,7 +35,14 @@ static const char *real_path_internal(const char *path, int die_on_error)\n {\n \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n \tchar *retval = NULL;\n+\n+\t/*\n+\t * If we have to temporarily chdir(), store the original CWD\n+\t * here so that we can chdir() back to it at the end of the\n+\t * function:\n+\t */\n \tchar cwd[1024] = \"\";\n+\n \tint buf_index = 1;\n \n \tint depth = MAXDEPTH;\n-- \n1.7.11.3\n"},{"id":"201614","messageId":"1350799057-13846-4-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 3/8] Introduce new function real_path_if_valid()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:32Z","receivedAt":"2012-10-21T05:57:32Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"The function is like real_path(), except that it returns NULL on error\ninstead of dying.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n abspath.c | 5 +++++\n cache.h   | 1 +\n 2 files changed, 6 insertions(+)\n\ndiff --git a/abspath.c b/abspath.c\nindex f8a526f..40cdc46 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -153,6 +153,11 @@ const char *real_path(const char *path)\n \treturn real_path_internal(path, 1);\n }\n \n+const char *real_path_if_valid(const char *path)\n+{\n+\treturn real_path_internal(path, 0);\n+}\n+\n static const char *get_pwd_cwd(void)\n {\n \tstatic char cwd[PATH_MAX + 1];\ndiff --git a/cache.h b/cache.h\nindex a58df84..b0d75bc 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -714,6 +714,7 @@ static inline int is_absolute_path(const char *path)\n }\n int is_directory(const char *);\n const char *real_path(const char *path);\n+const char *real_path_if_valid(const char *path);\n const char *absolute_path(const char *path);\n const char *relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n-- \n1.7.11.3\n"},{"id":"201616","messageId":"1350799057-13846-5-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 4/8] longest_ancestor_length(): use string_list_split()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:33Z","receivedAt":"2012-10-21T05:57:33Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n path.c | 18 +++++++++++-------\n 1 file changed, 11 insertions(+), 7 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex cbbdf7d..f455e8e 100644\n--- a/path.c\n+++ b/path.c\n@@ -12,6 +12,7 @@\n  */\n #include \"cache.h\"\n #include \"strbuf.h\"\n+#include \"string-list.h\"\n \n static char bad_path[] = \"/bad-path/\";\n \n@@ -582,20 +583,22 @@ int normalize_path_copy(char *dst, const char *src)\n  */\n int longest_ancestor_length(const char *path, const char *prefix_list)\n {\n+\tstruct string_list prefixes = STRING_LIST_INIT_DUP;\n \tchar buf[PATH_MAX+1];\n-\tconst char *ceil, *colon;\n-\tint len, max_len = -1;\n+\tint i, max_len = -1;\n \n \tif (prefix_list == NULL || !strcmp(path, \"/\"))\n \t\treturn -1;\n \n-\tfor (colon = ceil = prefix_list; *colon; ceil = colon+1) {\n-\t\tfor (colon = ceil; *colon && *colon != PATH_SEP; colon++);\n-\t\tlen = colon - ceil;\n+\tstring_list_split(&prefixes, prefix_list, PATH_SEP, -1);\n+\n+\tfor (i = 0; i < prefixes.nr; i++) {\n+\t\tconst char *ceil = prefixes.items[i].string;\n+\t\tint len = strlen(ceil);\n+\n \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n \t\t\tcontinue;\n-\t\tstrlcpy(buf, ceil, len+1);\n-\t\tif (normalize_path_copy(buf, buf) < 0)\n+\t\tif (normalize_path_copy(buf, ceil) < 0)\n \t\t\tcontinue;\n \t\tlen = strlen(buf);\n \t\tif (len > 0 && buf[len-1] == '/')\n@@ -608,6 +611,7 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \t\t}\n \t}\n \n+\tstring_list_clear(&prefixes, 0);\n \treturn max_len;\n }\n \n-- \n1.7.11.3\n"},{"id":"201617","messageId":"1350799057-13846-6-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 5/8] longest_ancestor_length(): take a string_list argument for prefixes","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:34Z","receivedAt":"2012-10-21T05:57:34Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Change longest_ancestor_length() to take the prefixes argument as a\nstring_list rather than as a PATH_SEP-separated string.  This will\nmake it easier to change the caller to alter the entries before\ncalling longest_ancestor_length().\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n cache.h           |  2 +-\n path.c            | 22 +++++++++-------------\n setup.c           | 11 +++++++++--\n test-path-utils.c |  8 +++++++-\n 4 files changed, 26 insertions(+), 17 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex b0d75bc..8103385 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -718,7 +718,7 @@ const char *real_path_if_valid(const char *path);\n const char *absolute_path(const char *path);\n const char *relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n-int longest_ancestor_length(const char *path, const char *prefix_list);\n+int longest_ancestor_length(const char *path, struct string_list *prefixes);\n char *strip_path_suffix(const char *path, const char *suffix);\n int daemon_avoid_alias(const char *path);\n int offset_1st_component(const char *path);\ndiff --git a/path.c b/path.c\nindex f455e8e..b80d2e6 100644\n--- a/path.c\n+++ b/path.c\n@@ -570,30 +570,27 @@ int normalize_path_copy(char *dst, const char *src)\n \n /*\n  * path = Canonical absolute path\n- * prefix_list = Colon-separated list of absolute paths\n+ * prefixes = string_list containing absolute paths\n  *\n- * Determines, for each path in prefix_list, whether the \"prefix\" really\n+ * Determines, for each path in prefixes, whether the \"prefix\" really\n  * is an ancestor directory of path.  Returns the length of the longest\n  * ancestor directory, excluding any trailing slashes, or -1 if no prefix\n- * is an ancestor.  (Note that this means 0 is returned if prefix_list is\n- * \"/\".) \"/foo\" is not considered an ancestor of \"/foobar\".  Directories\n+ * is an ancestor.  (Note that this means 0 is returned if prefixes is\n+ * [\"/\"].) \"/foo\" is not considered an ancestor of \"/foobar\".  Directories\n  * are not considered to be their own ancestors.  path must be in a\n  * canonical form: empty components, or \".\" or \"..\" components are not\n- * allowed.  prefix_list may be null, which is like \"\".\n+ * allowed.  Empty strings in prefixes are ignored.\n  */\n-int longest_ancestor_length(const char *path, const char *prefix_list)\n+int longest_ancestor_length(const char *path, struct string_list *prefixes)\n {\n-\tstruct string_list prefixes = STRING_LIST_INIT_DUP;\n \tchar buf[PATH_MAX+1];\n \tint i, max_len = -1;\n \n-\tif (prefix_list == NULL || !strcmp(path, \"/\"))\n+\tif (!strcmp(path, \"/\"))\n \t\treturn -1;\n \n-\tstring_list_split(&prefixes, prefix_list, PATH_SEP, -1);\n-\n-\tfor (i = 0; i < prefixes.nr; i++) {\n-\t\tconst char *ceil = prefixes.items[i].string;\n+\tfor (i = 0; i < prefixes->nr; i++) {\n+\t\tconst char *ceil = prefixes->items[i].string;\n \t\tint len = strlen(ceil);\n \n \t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n@@ -611,7 +608,6 @@ int longest_ancestor_length(const char *path, const char *prefix_list)\n \t\t}\n \t}\n \n-\tstring_list_clear(&prefixes, 0);\n \treturn max_len;\n }\n \ndiff --git a/setup.c b/setup.c\nindex 3a1b2fd..b4cd356 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -1,5 +1,6 @@\n #include \"cache.h\"\n #include \"dir.h\"\n+#include \"string-list.h\"\n \n static int inside_git_dir = -1;\n static int inside_work_tree = -1;\n@@ -627,10 +628,11 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n static const char *setup_git_directory_gently_1(int *nongit_ok)\n {\n \tconst char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);\n+\tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n \tstatic char cwd[PATH_MAX+1];\n \tconst char *gitdirenv, *ret;\n \tchar *gitfile;\n-\tint len, offset, offset_parent, ceil_offset;\n+\tint len, offset, offset_parent, ceil_offset = -1;\n \tdev_t current_device = 0;\n \tint one_filesystem = 1;\n \n@@ -655,7 +657,12 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \tif (gitdirenv)\n \t\treturn setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);\n \n-\tceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);\n+\tif (env_ceiling_dirs) {\n+\t\tstring_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);\n+\t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n+\t\tstring_list_clear(&ceiling_dirs, 0);\n+\t}\n+\n \tif (ceil_offset < 0 && has_dos_drive_prefix(cwd))\n \t\tceil_offset = 1;\n \ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex 3bc20e9..acb0560 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -1,4 +1,5 @@\n #include \"cache.h\"\n+#include \"string-list.h\"\n \n int main(int argc, char **argv)\n {\n@@ -30,7 +31,12 @@ int main(int argc, char **argv)\n \t}\n \n \tif (argc == 4 && !strcmp(argv[1], \"longest_ancestor_length\")) {\n-\t\tint len = longest_ancestor_length(argv[2], argv[3]);\n+\t\tint len;\n+\t\tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n+\n+\t\tstring_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);\n+\t\tlen = longest_ancestor_length(argv[2], &ceiling_dirs);\n+\t\tstring_list_clear(&ceiling_dirs, 0);\n \t\tprintf(\"%d\\n\", len);\n \t\treturn 0;\n \t}\n-- \n1.7.11.3\n"},{"id":"201612","messageId":"1350799057-13846-7-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 6/8] longest_ancestor_length(): require prefix list entries to be normalized","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:35Z","receivedAt":"2012-10-21T05:57:35Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Move the responsibility for normalizing the prefixes passed to\nlongest_ancestor_length() to its caller.  In t0060, only test\nlongest_ancestor_lengths using normalized paths: remove empty entries\nand non-absolute paths, strip trailing slashes from the paths, and\nremove tests that thereby become redundant.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n path.c                | 26 +++++++++++---------------\n setup.c               | 23 +++++++++++++++++++++++\n t/t0060-path-utils.sh | 41 +++++++++++++----------------------------\n 3 files changed, 47 insertions(+), 43 deletions(-)\n\ndiff --git a/path.c b/path.c\nindex b80d2e6..d3d3f8b 100644\n--- a/path.c\n+++ b/path.c\n@@ -570,20 +570,20 @@ int normalize_path_copy(char *dst, const char *src)\n \n /*\n  * path = Canonical absolute path\n- * prefixes = string_list containing absolute paths\n+ * prefixes = string_list containing normalized, absolute paths without\n+ * trailing slashes (except for the root directory, which is denoted by \"/\").\n  *\n- * Determines, for each path in prefixes, whether the \"prefix\" really\n+ * Determines, for each path in prefixes, whether the \"prefix\"\n  * is an ancestor directory of path.  Returns the length of the longest\n  * ancestor directory, excluding any trailing slashes, or -1 if no prefix\n  * is an ancestor.  (Note that this means 0 is returned if prefixes is\n  * [\"/\"].) \"/foo\" is not considered an ancestor of \"/foobar\".  Directories\n  * are not considered to be their own ancestors.  path must be in a\n  * canonical form: empty components, or \".\" or \"..\" components are not\n- * allowed.  Empty strings in prefixes are ignored.\n+ * allowed.\n  */\n int longest_ancestor_length(const char *path, struct string_list *prefixes)\n {\n-\tchar buf[PATH_MAX+1];\n \tint i, max_len = -1;\n \n \tif (!strcmp(path, \"/\"))\n@@ -593,19 +593,15 @@ int longest_ancestor_length(const char *path, struct string_list *prefixes)\n \t\tconst char *ceil = prefixes->items[i].string;\n \t\tint len = strlen(ceil);\n \n-\t\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n-\t\t\tcontinue;\n-\t\tif (normalize_path_copy(buf, ceil) < 0)\n-\t\t\tcontinue;\n-\t\tlen = strlen(buf);\n-\t\tif (len > 0 && buf[len-1] == '/')\n-\t\t\tbuf[--len] = '\\0';\n+\t\tif (len == 1 && ceil[0] == '/')\n+\t\t\tlen = 0; /* root matches anything, with length 0 */\n+\t\telse if (!strncmp(path, ceil, len) && path[len] == '/')\n+\t\t\t; /* match of length len */\n+\t\telse\n+\t\t\tcontinue; /* no match */\n \n-\t\tif (!strncmp(path, buf, len) &&\n-\t\t    path[len] == '/' &&\n-\t\t    len > max_len) {\n+\t\tif (len > max_len)\n \t\t\tmax_len = len;\n-\t\t}\n \t}\n \n \treturn max_len;\ndiff --git a/setup.c b/setup.c\nindex b4cd356..df97ad3 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -622,6 +622,28 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n }\n \n /*\n+ * A \"string_list_each_func_t\" function that normalizes an entry from\n+ * GIT_CEILING_DIRECTORIES or discards it if unusable.\n+ */\n+static int normalize_ceiling_entry(struct string_list_item *item, void *unused)\n+{\n+\tconst char *ceil = item->string;\n+\tint len = strlen(ceil);\n+\tchar buf[PATH_MAX+1];\n+\n+\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n+\t\treturn 0;\n+\tif (normalize_path_copy(buf, ceil) < 0)\n+\t\treturn 0;\n+\tlen = strlen(buf);\n+\tif (len > 1 && buf[len-1] == '/')\n+\t\tbuf[--len] = '\\0';\n+\tfree(item->string);\n+\titem->string = xstrdup(buf);\n+\treturn 1;\n+}\n+\n+/*\n  * We cannot decide in this function whether we are in the work tree or\n  * not, since the config can only be read _after_ this function was called.\n  */\n@@ -659,6 +681,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \n \tif (env_ceiling_dirs) {\n \t\tstring_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);\n+\t\tfilter_string_list(&ceiling_dirs, 0, normalize_ceiling_entry, NULL);\n \t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n \t\tstring_list_clear(&ceiling_dirs, 0);\n \t}\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 4ef2345..09a42a4 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -93,47 +93,32 @@ norm_path /d1/s1//../s2/../../d2 /d2 POSIX\n norm_path /d1/.../d2 /d1/.../d2 POSIX\n norm_path /d1/..././../d2 /d1/d2 POSIX\n \n-ancestor / \"\" -1\n ancestor / / -1\n-ancestor /foo \"\" -1\n-ancestor /foo : -1\n-ancestor /foo ::. -1\n-ancestor /foo ::..:: -1\n ancestor /foo / 0\n ancestor /foo /fo -1\n ancestor /foo /foo -1\n-ancestor /foo /foo/ -1\n ancestor /foo /bar -1\n-ancestor /foo /bar/ -1\n ancestor /foo /foo/bar -1\n-ancestor /foo /foo:/bar/ -1\n-ancestor /foo /foo/:/bar/ -1\n-ancestor /foo /foo::/bar/ -1\n-ancestor /foo /:/foo:/bar/ 0\n-ancestor /foo /foo:/:/bar/ 0\n-ancestor /foo /:/bar/:/foo 0\n-ancestor /foo/bar \"\" -1\n+ancestor /foo /foo:/bar -1\n+ancestor /foo /:/foo:/bar 0\n+ancestor /foo /foo:/:/bar 0\n+ancestor /foo /:/bar:/foo 0\n ancestor /foo/bar / 0\n ancestor /foo/bar /fo -1\n-ancestor /foo/bar foo -1\n ancestor /foo/bar /foo 4\n-ancestor /foo/bar /foo/ 4\n ancestor /foo/bar /foo/ba -1\n ancestor /foo/bar /:/fo 0\n ancestor /foo/bar /foo:/foo/ba 4\n ancestor /foo/bar /bar -1\n-ancestor /foo/bar /bar/ -1\n-ancestor /foo/bar /fo: -1\n-ancestor /foo/bar :/fo -1\n-ancestor /foo/bar /foo:/bar/ 4\n-ancestor /foo/bar /:/foo:/bar/ 4\n-ancestor /foo/bar /foo:/:/bar/ 4\n-ancestor /foo/bar /:/bar/:/fo 0\n-ancestor /foo/bar /:/bar/ 0\n-ancestor /foo/bar .:/foo/. 4\n-ancestor /foo/bar .:/foo/.:.: 4\n-ancestor /foo/bar /foo/./:.:/bar 4\n-ancestor /foo/bar .:/bar -1\n+ancestor /foo/bar /fo -1\n+ancestor /foo/bar /foo:/bar 4\n+ancestor /foo/bar /:/foo:/bar 4\n+ancestor /foo/bar /foo:/:/bar 4\n+ancestor /foo/bar /:/bar:/fo 0\n+ancestor /foo/bar /:/bar 0\n+ancestor /foo/bar /foo 4\n+ancestor /foo/bar /foo:/bar 4\n+ancestor /foo/bar /bar -1\n \n test_expect_success 'strip_path_suffix' '\n \ttest c:/msysgit = $(test-path-utils strip_path_suffix \\\n-- \n1.7.11.3\n"},{"id":"201615","messageId":"1350799057-13846-8-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 7/8] normalize_ceiling_entry(): resolve symlinks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:36Z","receivedAt":"2012-10-21T05:57:36Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"longest_ancestor_length() relies on a textual comparison of directory\nparts to find the part of path that overlaps with one of the paths in\nprefix_list.  But this doesn't work if any of the prefixes involves a\nsymbolic link, because the directories will look different even though\nthey might logically refer to the same directory.  So canonicalize the\npaths listed in GIT_CEILING_DIRECTORIES using real_path_if_valid()\nbefore passing them to longest_ancestor_length().\n\npath is already in canonical form, so doesn't need to be canonicalized\nagain.\n\nThis fixes some problems with using GIT_CEILING_DIRECTORIES that\ncontains paths involving symlinks, including t4035 if run with --root\nset to a path involving symlinks.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n setup.c | 20 +++++++++-----------\n 1 file changed, 9 insertions(+), 11 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex df97ad3..2eb5b75 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -622,24 +622,22 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n }\n \n /*\n- * A \"string_list_each_func_t\" function that normalizes an entry from\n- * GIT_CEILING_DIRECTORIES or discards it if unusable.\n+ * A \"string_list_each_func_t\" function that canonicalizes an entry\n+ * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or\n+ * discards it if unusable.\n  */\n static int normalize_ceiling_entry(struct string_list_item *item, void *unused)\n {\n-\tconst char *ceil = item->string;\n-\tint len = strlen(ceil);\n-\tchar buf[PATH_MAX+1];\n+\tchar *ceil = item->string;\n+\tconst char *real_path;\n \n-\tif (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))\n+\tif (!*ceil || !is_absolute_path(ceil))\n \t\treturn 0;\n-\tif (normalize_path_copy(buf, ceil) < 0)\n+\treal_path = real_path_if_valid(ceil);\n+\tif (!real_path)\n \t\treturn 0;\n-\tlen = strlen(buf);\n-\tif (len > 1 && buf[len-1] == '/')\n-\t\tbuf[--len] = '\\0';\n \tfree(item->string);\n-\titem->string = xstrdup(buf);\n+\titem->string = xstrdup(real_path);\n \treturn 1;\n }\n \n-- \n1.7.11.3\n"},{"id":"201613","messageId":"1350799057-13846-9-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v3 8/8] string_list_longest_prefix(): remove function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-21T05:57:37Z","receivedAt":"2012-10-21T05:57:37Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"This function was added in f103f95b11d087f07c0c48bf784cd9197e18f203 in\nthe erroneous expectation that it would be used in the\nreimplementation of longest_ancestor_length().  But it turned out to\nbe easier to use a function specialized for comparing path prefixes\n(i.e., one that knows about slashes and root paths) than to prepare\nthe paths in such a way that a generic string prefix comparison\nfunction can be used.  So delete string_list_longest_prefix() and its\ndocumentation and test cases.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n Documentation/technical/api-string-list.txt |  8 --------\n string-list.c                               | 20 -------------------\n string-list.h                               |  8 --------\n t/t0063-string-list.sh                      | 30 -----------------------------\n test-string-list.c                          | 20 -------------------\n 5 files changed, 86 deletions(-)\n\ndiff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt\nindex 94d7a2b..618400d 100644\n--- a/Documentation/technical/api-string-list.txt\n+++ b/Documentation/technical/api-string-list.txt\n@@ -75,14 +75,6 @@ Functions\n \tto be deleted.  Preserve the order of the items that are\n \tretained.\n \n-`string_list_longest_prefix`::\n-\n-\tReturn the longest string within a string_list that is a\n-\tprefix (in the sense of prefixcmp()) of the specified string,\n-\tor NULL if no such prefix exists.  This function does not\n-\trequire the string_list to be sorted (it does a linear\n-\tsearch).\n-\n `print_string_list`::\n \n \tDump a string_list to stdout, useful mainly for debugging purposes. It\ndiff --git a/string-list.c b/string-list.c\nindex c54b816..decfa74 100644\n--- a/string-list.c\n+++ b/string-list.c\n@@ -136,26 +136,6 @@ void filter_string_list(struct string_list *list, int free_util,\n \tlist->nr = dst;\n }\n \n-char *string_list_longest_prefix(const struct string_list *prefixes,\n-\t\t\t\t const char *string)\n-{\n-\tint i, max_len = -1;\n-\tchar *retval = NULL;\n-\n-\tfor (i = 0; i < prefixes->nr; i++) {\n-\t\tchar *prefix = prefixes->items[i].string;\n-\t\tif (!prefixcmp(string, prefix)) {\n-\t\t\tint len = strlen(prefix);\n-\t\t\tif (len > max_len) {\n-\t\t\t\tretval = prefix;\n-\t\t\t\tmax_len = len;\n-\t\t\t}\n-\t\t}\n-\t}\n-\n-\treturn retval;\n-}\n-\n void string_list_clear(struct string_list *list, int free_util)\n {\n \tif (list->items) {\ndiff --git a/string-list.h b/string-list.h\nindex 5efd07b..3a6a6dc 100644\n--- a/string-list.h\n+++ b/string-list.h\n@@ -38,14 +38,6 @@ int for_each_string_list(struct string_list *list,\n void filter_string_list(struct string_list *list, int free_util,\n \t\t\tstring_list_each_func_t want, void *cb_data);\n \n-/*\n- * Return the longest string in prefixes that is a prefix (in the\n- * sense of prefixcmp()) of string, or NULL if no such prefix exists.\n- * This function does not require the string_list to be sorted (it\n- * does a linear search).\n- */\n-char *string_list_longest_prefix(const struct string_list *prefixes, const char *string);\n-\n \n /* Use these functions only on sorted lists: */\n int string_list_has_string(const struct string_list *list, const char *string);\ndiff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh\nindex 41c8826..dbfc05e 100755\n--- a/t/t0063-string-list.sh\n+++ b/t/t0063-string-list.sh\n@@ -17,14 +17,6 @@ test_split () {\n \t\"\n }\n \n-test_longest_prefix () {\n-\ttest \"$(test-string-list longest_prefix \"$1\" \"$2\")\" = \"$3\"\n-}\n-\n-test_no_longest_prefix () {\n-\ttest_must_fail test-string-list longest_prefix \"$1\" \"$2\"\n-}\n-\n test_split \"foo:bar:baz\" \":\" \"-1\" <<EOF\n 3\n [0]: \"foo\"\n@@ -96,26 +88,4 @@ test_expect_success \"test remove_duplicates\" '\n \ttest a:b:c = \"$(test-string-list remove_duplicates a:a:a:b:b:b:c:c:c)\"\n '\n \n-test_expect_success \"test longest_prefix\" '\n-\ttest_no_longest_prefix - '' &&\n-\ttest_no_longest_prefix - x &&\n-\ttest_longest_prefix \"\" x \"\" &&\n-\ttest_longest_prefix x x x &&\n-\ttest_longest_prefix \"\" foo \"\" &&\n-\ttest_longest_prefix : foo \"\" &&\n-\ttest_longest_prefix f foo f &&\n-\ttest_longest_prefix foo foobar foo &&\n-\ttest_longest_prefix foo foo foo &&\n-\ttest_no_longest_prefix bar foo &&\n-\ttest_no_longest_prefix bar:bar foo &&\n-\ttest_no_longest_prefix foobar foo &&\n-\ttest_longest_prefix foo:bar foo foo &&\n-\ttest_longest_prefix foo:bar bar bar &&\n-\ttest_longest_prefix foo::bar foo foo &&\n-\ttest_longest_prefix foo:foobar foo foo &&\n-\ttest_longest_prefix foobar:foo foo foo &&\n-\ttest_longest_prefix foo: bar \"\" &&\n-\ttest_longest_prefix :foo bar \"\"\n-'\n-\n test_done\ndiff --git a/test-string-list.c b/test-string-list.c\nindex 4693295..00ce6c9 100644\n--- a/test-string-list.c\n+++ b/test-string-list.c\n@@ -97,26 +97,6 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n-\tif (argc == 4 && !strcmp(argv[1], \"longest_prefix\")) {\n-\t\t/* arguments: <colon-separated-prefixes>|- <string> */\n-\t\tstruct string_list prefixes = STRING_LIST_INIT_DUP;\n-\t\tint retval;\n-\t\tconst char *prefix_string = argv[2];\n-\t\tconst char *string = argv[3];\n-\t\tconst char *match;\n-\n-\t\tparse_string_list(&prefixes, prefix_string);\n-\t\tmatch = string_list_longest_prefix(&prefixes, string);\n-\t\tif (match) {\n-\t\t\tprintf(\"%s\\n\", match);\n-\t\t\tretval = 0;\n-\t\t}\n-\t\telse\n-\t\t\tretval = 1;\n-\t\tstring_list_clear(&prefixes, 0);\n-\t\treturn retval;\n-\t}\n-\n \tfprintf(stderr, \"%s: unknown function name: %s\\n\", argv[0],\n \t\targv[1] ? argv[1] : \"(there was none)\");\n \treturn 1;\n-- \n1.7.11.3\n"},{"id":"201620","messageId":"7v7gqkgvxe.fsf@alter.siamese.dyndns.org","threadId":"31880","inReplyTo":"1350799057-13846-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-21T06:51:09Z","receivedAt":"2012-10-21T06:51:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> This patch series has the side effect that all of the directories\n> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to\n> resolve any symlinks that are present in their paths.  It is\n> admittedly odd that a feature intended to avoid accessing expensive\n> directories would now *intentionally* access directories near the\n> expensive ones.  In the above scenario this shouldn't be a problem,\n> because /home would be the directory listed in\n> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be\n> expensive.\n\nInteresting observation.  In the last sentence, \"accessing /home\"\ndoes not exactly mean accessing /home, but accessing / to learn\nabout \"home\" in it, no?\n\n> But there might be other scenarios for which this patch\n> series causes a performance regression.\n\nYeah, after merging this to 'next', we should ask people who care\nabout CEILING to test it sufficiently.\n\nThanks for rerolling.\n"},{"id":"201683","messageId":"5085033E.2070607@alum.mit.edu","threadId":"31880","inReplyTo":"7v7gqkgvxe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-22T08:26:38Z","receivedAt":"2012-10-22T08:26:38Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/21/2012 08:51 AM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> This patch series has the side effect that all of the directories\n>> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to\n>> resolve any symlinks that are present in their paths.  It is\n>> admittedly odd that a feature intended to avoid accessing expensive\n>> directories would now *intentionally* access directories near the\n>> expensive ones.  In the above scenario this shouldn't be a problem,\n>> because /home would be the directory listed in\n>> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be\n>> expensive.\n> \n> Interesting observation.  In the last sentence, \"accessing /home\"\n> does not exactly mean accessing /home, but accessing / to learn\n> about \"home\" in it, no?\n\nThis is the extra overhead on my system for using\nGIT_CEILING_DIRECTORIES=/home:\n\nstat(\"/home\", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0\ngetcwd(\"/home/mhagger\", 1024)           = 14\nchdir(\"/home\")                          = 0\ngetcwd(\"/home\", 4096)                   = 6\nlstat(\"/home\", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0\nchdir(\"/home/mhagger\")                  = 0\n\nIf I use GIT_CEILING_DIRECTORIES=/dev/shm, which is a symlink to\n/run/shm on my system, the overhead is comparable:\n\nstat(\"/dev/shm\", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=200, ...}) = 0\ngetcwd(\"/home/mhagger\", 1024)           = 14\nchdir(\"/dev/shm\")                       = 0\ngetcwd(\"/run/shm\", 4096)                = 9\nlstat(\"/run/shm\", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=200, ...}) = 0\nchdir(\"/home/mhagger\")                  = 0\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"201707","messageId":"5085A6E2.3060807@kdbg.org","threadId":"31880","inReplyTo":"1350799057-13846-7-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v3 6/8] longest_ancestor_length(): require prefix list entries to be normalized","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2012-10-22T20:04:50Z","receivedAt":"2012-10-22T20:04:50Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 21.10.2012 07:57, schrieb Michael Haggerty:\n> Move the responsibility for normalizing the prefixes passed to\n> longest_ancestor_length() to its caller.  In t0060, only test\n> longest_ancestor_lengths using normalized paths: remove empty entries\n> and non-absolute paths, strip trailing slashes from the paths, and\n> remove tests that thereby become redundant.\n\nt0060 does not pass on Windows with this change. Bash's path mangling\nstrikes again, but the real reason for the failure is that\ntest-path-util does not normalize its input before it calls into\nlongest_ancestor_length(). It is not sufficient to reduce the test cases\nto normalized ones, because due to path mangling on Windows they are\nturned into unnormalized paths (with backslashes instead of forward\nslashes).\n\nI suggest to export the new normalize_ceiling_entry() and use it from\ntest-path-util. Then don't change or remove existing tests.\n\n-- Hannes\n"},{"id":"202078","messageId":"CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com","threadId":"31880","inReplyTo":"7v7gqkgvxe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-10-29T00:15:29Z","receivedAt":"2012-10-29T00:15:29Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>\n>> This patch series has the side effect that all of the directories\n>> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to\n>> resolve any symlinks that are present in their paths.  It is\n>> admittedly odd that a feature intended to avoid accessing expensive\n>> directories would now *intentionally* access directories near the\n>> expensive ones.  In the above scenario this shouldn't be a problem,\n>> because /home would be the directory listed in\n>> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be\n>> expensive.\n>\n> Interesting observation.  In the last sentence, \"accessing /home\"\n> does not exactly mean accessing /home, but accessing / to learn\n> about \"home\" in it, no?\n>\n>> But there might be other scenarios for which this patch\n>> series causes a performance regression.\n>\n> Yeah, after merging this to 'next', we should ask people who care\n> about CEILING to test it sufficiently.\n>\n> Thanks for rerolling.\n\nGIT_CEILING_DIRECTORIES was always about trying to avoid\nhitting them at all; they can be (busy) NFS volumes there.\n\nHere's the description from the 1.6.0 release notes:\n\n* A new environment variable GIT_CEILING_DIRECTORIES can be used to stop\n  the discovery process of the toplevel of working tree; this may be useful\n  when you are working in a slow network disk and are outside any working tree,\n  as bash-completion and \"git help\" may still need to run in these places.\n\nIn 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added\nGIT_ONE_FILESYSTEM to fix a related issue.\nDo you guys have GIT_CEILING_DIRECTORIES set too?\n\nWe use GIT_CEILING_DIRECTORIES and I'm pretty sure\nwe don't want every git command hitting them, so this would\nbe a regression when seen from the POV of our current usage\nof this variable, which would be a bummer.\n\nIs there another way to accomplish this without the performance hit?\nMaybe something that can be solved with configuration?\n\nI'd be happy to lend a hand if you guys have some ideas\non how we can help keep it fast.  Thoughts?\n\nOriginal patches for those just joining us:\nhttp://thread.gmane.org/gmane.comp.version-control.git/208102\n-- \nDavid\n"},{"id":"202079","messageId":"4c48983e-83a1-4a8f-8dee-114039779a47@email.android.com","threadId":"31880","inReplyTo":"CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-10-29T01:42:55Z","receivedAt":"2012-10-29T01:42:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\n\nDavid Aguilar <davvid@gmail.com> wrote:\n\n>Is there another way to accomplish this without the performance hit?\n\nPerhaps not canonicalize elements on the CEILING list ourselves? If we make it a user error to put symlinked alias in the variable, and document it clearly, wouldn't it suffice?\n"},{"id":"202081","messageId":"508E0FAC.5050600@alum.mit.edu","threadId":"31880","inReplyTo":"CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-29T05:10:04Z","receivedAt":"2012-10-29T05:10:04Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/29/2012 01:15 AM, David Aguilar wrote:\n> On Sat, Oct 20, 2012 at 11:51 PM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>>\n>>> This patch series has the side effect that all of the directories\n>>> listed in GIT_CEILING_DIRECTORIES are accessed *unconditionally* to\n>>> resolve any symlinks that are present in their paths.  It is\n>>> admittedly odd that a feature intended to avoid accessing expensive\n>>> directories would now *intentionally* access directories near the\n>>> expensive ones.  In the above scenario this shouldn't be a problem,\n>>> because /home would be the directory listed in\n>>> GIT_CEILING_DIRECTORIES, and accessing /home itself shouldn't be\n>>> expensive.\n>>\n>> Interesting observation.  In the last sentence, \"accessing /home\"\n>> does not exactly mean accessing /home, but accessing / to learn\n>> about \"home\" in it, no?\n>>\n>>> But there might be other scenarios for which this patch\n>>> series causes a performance regression.\n>>\n>> Yeah, after merging this to 'next', we should ask people who care\n>> about CEILING to test it sufficiently.\n>>\n>> Thanks for rerolling.\n> \n> GIT_CEILING_DIRECTORIES was always about trying to avoid\n> hitting them at all; they can be (busy) NFS volumes there.\n> \n> Here's the description from the 1.6.0 release notes:\n> \n> * A new environment variable GIT_CEILING_DIRECTORIES can be used to stop\n>   the discovery process of the toplevel of working tree; this may be useful\n>   when you are working in a slow network disk and are outside any working tree,\n>   as bash-completion and \"git help\" may still need to run in these places.\n> \n> In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added\n> GIT_ONE_FILESYSTEM to fix a related issue.\n> Do you guys have GIT_CEILING_DIRECTORIES set too?\n> \n> We use GIT_CEILING_DIRECTORIES and I'm pretty sure\n> we don't want every git command hitting them, so this would\n> be a regression when seen from the POV of our current usage\n> of this variable, which would be a bummer.\n\nI would certainly withdraw the patch series if it causes a performance hit.\n\nThe log message of the original commit (0454dd93bf) described the\nfollowing scenario: a /home partition under which user home directories\nare automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid\nhitting /home/.git, /home/.git/objects, and /home/objects (which would\nattempt to automount those directories).  I believe that this scenario\nwould not be slowed down by my patches.\n\nHow do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a\nslowdown?\n\n> Is there another way to accomplish this without the performance hit?\n> Maybe something that can be solved with configuration?\n\nWithout doing the symlink expansion there is no way for git to detect\nthat GIT_CEILING_DIRECTORIES contains symlinks and is therefore\nineffective.  So the user has no warning about the misconfiguration\n(except that git runs slowly).\n\nOn 10/29/2012 02:42 AM, Junio C Hamano wrote:\n> Perhaps not canonicalize elements on the CEILING list ourselves? If\n> we make it a user error to put symlinked alias in the variable, and\n> document it clearly, wouldn't it suffice?\n\nThere may be no other choice.  (That, and fix the test suite in another\nway to tolerate a $PWD that involves symlinks.)\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"202085","messageId":"20121029053400.GA19771@pixar.com","threadId":"31880","inReplyTo":"CAJDDKr4ki+NjSeuZpAU6bM=YAQ_3mdHCtawstdCqe9Ewvp=arQ@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Lars Damerow","fromEmail":"lars@pixar.com","sentAt":"2012-10-29T05:34:00Z","receivedAt":"2012-10-29T05:34:00Z","isPatch":true,"sender":{"key":"lars@pixar.com","avatar":"https://avatars.githubusercontent.com/u/318366?v=4"},"body":">From David Aguilar <davvid@gmail.com>, Sun, Oct 28, 2012 at 05:15:29PM -0700:\n> \n> In 8030e44215fe8f34edd57d711a35f2f0f97a0423 Lars added\n> GIT_ONE_FILESYSTEM to fix a related issue.\n> Do you guys have GIT_CEILING_DIRECTORIES set too?\n\nOnce GIT_DISCOVERY_ACROSS_FILESYSTEM (the eventual name for GIT_ONE_FILESYSTEM) was accepted, we stopped setting GIT_CEILING_DIRECTORIES in our environment.\n\nEven so, the way our filesystems work, stat() or lstat() calls on the paths that we used to have in GIT_CEILING_DIRECTORIES wouldn't have caused a problem.\n\nI don't personally have an objection to patches that explicitly access the GIT_CEILING_DIRECTORIES paths, but it seems like other folks' configurations could be harmed by them.\n\n-lars\n\n--\nlars r. damerow :: button pusher :: pixar animation studios\n"},{"id":"202979","messageId":"7v4nkuiuhe.fsf@alter.siamese.dyndns.org","threadId":"31880","inReplyTo":"508E0FAC.5050600@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-11-12T17:47:25Z","receivedAt":"2012-11-12T17:47:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> The log message of the original commit (0454dd93bf) described the\n> following scenario: a /home partition under which user home directories\n> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid\n> hitting /home/.git, /home/.git/objects, and /home/objects (which would\n> attempt to automount those directories).  I believe that this scenario\n> would not be slowed down by my patches.\n>\n> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a\n> slowdown?\n\nYeah, I was also wondering about that.\n\nDavid?\n\n>> Is there another way to accomplish this without the performance hit?\n>> Maybe something that can be solved with configuration?\n>\n> Without doing the symlink expansion there is no way for git to detect\n> that GIT_CEILING_DIRECTORIES contains symlinks and is therefore\n> ineffective.  So the user has no warning about the misconfiguration\n> (except that git runs slowly).\n>\n> On 10/29/2012 02:42 AM, Junio C Hamano wrote:\n>> Perhaps not canonicalize elements on the CEILING list ourselves? If\n>> we make it a user error to put symlinked alias in the variable, and\n>> document it clearly, wouldn't it suffice?\n>\n> There may be no other choice.  (That, and fix the test suite in another\n> way to tolerate a $PWD that involves symlinks.)\n"},{"id":"203147","messageId":"CAJDDKr5F5EcXaTuPWgE5MZJQ=Of6MwW+RmRhhXOLyfQzanjEwQ@mail.gmail.com","threadId":"31880","inReplyTo":"7v4nkuiuhe.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"David Aguilar","fromEmail":"davvid@gmail.com","sentAt":"2012-11-13T20:50:18Z","receivedAt":"2012-11-13T20:50:18Z","isPatch":true,"sender":{"key":"davvid@gmail.com","avatar":"https://avatars.githubusercontent.com/u/13196?v=4"},"body":"On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano <gitster@pobox.com> wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>\n>> The log message of the original commit (0454dd93bf) described the\n>> following scenario: a /home partition under which user home directories\n>> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid\n>> hitting /home/.git, /home/.git/objects, and /home/objects (which would\n>> attempt to automount those directories).  I believe that this scenario\n>> would not be slowed down by my patches.\n>>\n>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a\n>> slowdown?\n>\n> Yeah, I was also wondering about that.\n>\n> David?\n\nI double-checked our configuration and all the parent directories\nof those listed in GIT_CEILING_DIRECTORIES are local,\nso our particular configuration would not have a performance hit.\n\nWe do have multiple directories listed there.  Some of them share\na parent directory.  I'm assuming the implementation is simple and\ndoes not try and avoid repeating the check when the parent dir is\nthe same across multiple entries.\n\nIn any case, it won't be a problem in practice based on my\nreading of the current code.\n\n\n\n>>> Is there another way to accomplish this without the performance hit?\n>>> Maybe something that can be solved with configuration?\n>>\n>> Without doing the symlink expansion there is no way for git to detect\n>> that GIT_CEILING_DIRECTORIES contains symlinks and is therefore\n>> ineffective.  So the user has no warning about the misconfiguration\n>> (except that git runs slowly).\n>>\n>> On 10/29/2012 02:42 AM, Junio C Hamano wrote:\n>>> Perhaps not canonicalize elements on the CEILING list ourselves? If\n>>> we make it a user error to put symlinked alias in the variable, and\n>>> document it clearly, wouldn't it suffice?\n>>\n>> There may be no other choice.  (That, and fix the test suite in another\n>> way to tolerate a $PWD that involves symlinks.)\n-- \nDavid\n"},{"id":"203276","messageId":"50A4A541.6090201@alum.mit.edu","threadId":"31880","inReplyTo":"CAJDDKr5F5EcXaTuPWgE5MZJQ=Of6MwW+RmRhhXOLyfQzanjEwQ@mail.gmail.com","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-11-15T08:18:09Z","receivedAt":"2012-11-15T08:18:09Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/13/2012 09:50 PM, David Aguilar wrote:\n> On Mon, Nov 12, 2012 at 9:47 AM, Junio C Hamano <gitster@pobox.com> wrote:\n>> Michael Haggerty <mhagger@alum.mit.edu> writes:\n>>\n>>> The log message of the original commit (0454dd93bf) described the\n>>> following scenario: a /home partition under which user home directories\n>>> are automounted, and setting GIT_CEILING_DIRECTORIES=/home to avoid\n>>> hitting /home/.git, /home/.git/objects, and /home/objects (which would\n>>> attempt to automount those directories).  I believe that this scenario\n>>> would not be slowed down by my patches.\n>>>\n>>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a\n>>> slowdown?\n>>\n>> Yeah, I was also wondering about that.\n>>\n>> David?\n> \n> I double-checked our configuration and all the parent directories\n> of those listed in GIT_CEILING_DIRECTORIES are local,\n> so our particular configuration would not have a performance hit.\n> \n> We do have multiple directories listed there.  Some of them share\n> a parent directory.  I'm assuming the implementation is simple and\n> does not try and avoid repeating the check when the parent dir is\n> the same across multiple entries.\n> \n> In any case, it won't be a problem in practice based on my\n> reading of the current code.\n\nOK, so we're back to the following status: some people (including me)\nare nervous that this change could cause a performance regression,\nthough it seems that the most sensible ways of using the\nGIT_CEILING_DIRECTORIES feature would not be affected.\n\nIn favor: Currently, if a directory containing a symlink is added to\nGIT_CEILING_DIRECTORIES, then GIT_CEILING_DIRECTORIES will not work, git\nhas no way of recognizing that there is a problem, and the only symptom\nobservable by the user is that the hoped-for performance improvement\nfrom using GIT_CEILING_DIRECTORIES will not materialize (or will\ndisappear after a filesystem reorg) [1].\n\nAgainst: The change will cause a performance regression if a\nslow-to-stat directory is listed in GIT_CEILING_DIRECTORIES.  The\nslowdown will occur whenever git is run outside of a true git-managed\nproject, most nastily in the case of using __git_ps1 in a shell prompt.\n\nI don't have a preference either way about whether these patches should\nbe merged.\n\nMichael\n\n[1] It is also conceivable that GIT_CEILING_DIRECTORIES is being used to\n*hide* an enclosing git project rather than to inform git that there are\nno enclosing projects, in which case the enclosing project would *not*\nbe hidden.  This is in fact the mechanism by which the problem causes\nfailures in our test suite.  But I don't expect that this is a common\nreal-world scenario, and anyway such a failure would be obvious to the\nuser and quickly fixed.\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"209886","messageId":"51246B31.6000605@mit.edu","threadId":"31880","inReplyTo":"508E0FAC.5050600@alum.mit.edu","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2013-02-20T06:20:33Z","receivedAt":"2013-02-20T06:20:33Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On 10/29/2012 01:10 AM, Michael Haggerty wrote:\n> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a\n> slowdown?\n\nSorry to bring up this old thread again, but I just realized why my \ncomputer has been acting so slow when I’m not connected to the network. \n  I put various network filesystem paths in $GIT_CEILING_DIRECTORIES, \nsuch as /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its \nparents /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and \n/afs/athena.mit.edu/user/a/n which all live in different AFS volumes). \nNow when I’m not connected to the network, every invocation of Git, \nincluding the __git_ps1 in my shell prompt, waits for AFS to timeout.\n\nObviously I’m going to stop using $GIT_CEILING_DIRECTORIES now that I \nknow what the problem is, but I figured you might want to know why this \nfeature is now useless for me.\n\nAnders\n"},{"id":"209888","messageId":"7vmwuzzd76.fsf@alter.siamese.dyndns.org","threadId":"31880","inReplyTo":"51246B31.6000605@mit.edu","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-20T06:55:41Z","receivedAt":"2013-02-20T06:55:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Anders Kaseorg <andersk@MIT.EDU> writes:\n\n> On 10/29/2012 01:10 AM, Michael Haggerty wrote:\n>> How do you use GIT_CEILING_DIRECTORIES that the proposed changes cause a\n>> slowdown?\n>\n> Sorry to bring up this old thread again, but I just realized why my\n> computer has been acting so slow when I’m not connected to the\n> network. I put various network filesystem paths in\n> $GIT_CEILING_DIRECTORIES, such as /afs/athena.mit.edu/user/a/n/andersk\n> (to avoid hitting its parents /afs/athena.mit.edu,\n> /afs/athena.mit.edu/user/a, and /afs/athena.mit.edu/user/a/n which all\n> live in different AFS volumes). Now when I’m not connected to the\n> network, every invocation of Git, including the __git_ps1 in my shell\n> prompt, waits for AFS to timeout.\n\nThanks for a report.\n\nAssuming that this says \"yes\":\n\n\tD=/afs/athena.mit.edu/user/a/n/andersk/my/dir\n        cd \"$D\"\n        test \"$(/bin/pwd)\" = \"$D\" && echo yes\n\niow, AFS mounting does not have funny symbolic link issues that make\nthe logical and physical PWD to be different like some automounter\nimplementation used to have, perhaps we can introduce a way for the\nuser to say \"The element of this CEILING list do not have any\nalias due to funny symlinks\" to solve this.  Perhaps existing of an\nempty element in the list would do?  E.g.\n\n\tGIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk\n\nor something like that.  And in such a case, we do not run realpath\non the elements on the list before comparing them with what we get\nfrom getcwd(3).\n\nOf course, you could condionally unset the environment while\noffline, but that feels like an ugly hack.\n"},{"id":"209902","messageId":"1361351364-15479-1-git-send-email-mhagger@alum.mit.edu","threadId":"31880","inReplyTo":"7vmwuzzd76.fsf@alter.siamese.dyndns.org","subject":"[RFC] Provide a mechanism to turn off symlink resolution in ceiling paths","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-20T09:09:24Z","receivedAt":"2013-02-20T09:09:24Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks in\nceiling paths' changed the setup code to resolve symlinks in the\nentries in GIT_CEILING_DIRECTORIES.  Because those entries are\ncompared textually to the symlink-resolved current directory, an entry\nin GIT_CEILING_DIRECTORIES that contained a symlink would have no\neffect.  It was known that this could cause performance problems if\nthe symlink resolution *itself* touched slow filesystems, but it was\nthought that such use cases would be unlikely.\n\nAfter this change was released, Anders Kaseorg <andersk@mit.edu>\nreported:\n\n> [...] my computer has been acting so slow when I’m not connected to\n> the network.  I put various network filesystem paths in\n> $GIT_CEILING_DIRECTORIES, such as\n> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents\n> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and\n> /afs/athena.mit.edu/user/a/n which all live in different AFS\n> volumes).  Now when I’m not connected to the network, every\n> invocation of Git, including the __git_ps1 in my shell prompt, waits\n> for AFS to timeout.\n\nSo provide the following mechanism to turn off symlink resolution in\nGIT_CEILING_DIRECTORIES entries: if that environment variable contains\nan empty entry (e.g., GIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or\nGIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy), then do not resolve\nsymlinks in paths that follow the empty entry.\n\n---\n\nThis is a possible implementation (untested!) of Junio's suggestion,\nwith the slight twist that the empty entry can appear anywhere in\nGIT_CEILING_DIRECTORIES and only turns off symlink expansion for\nsubsequent entries.  (The original suggestion would be similarly easy\nto implement if it is preferred.)\n\nUnfortunately I am swamped with other work right now so I don't have\ntime to test the code and might not be able to respond promptly to\nfeedback.\n\nAnother alternative (not implemented here) would be to support a\nsecond environment variable with an ugly name like\nGIT_CEILING_DIRECTORIES_NO_SYMLINKS which, when set, tells Git not to\nresolve symlinks in GIT_CEILING_DIRECTORIES.  Hopefully the variable\nname itself would warn the user that symlinks are an issue.\n\nThe ugliness of the situation unfortunately seems to preclude a\nnon-ugly solution of one form or another.\n\n Documentation/git.txt | 18 ++++++++++++------\n setup.c               | 32 ++++++++++++++++++++++----------\n 2 files changed, 34 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex da0115f..35c0517 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -674,12 +674,18 @@ git so take care if using Cogito etc.\n \tThe '--namespace' command-line option also sets this value.\n \n 'GIT_CEILING_DIRECTORIES'::\n-\tThis should be a colon-separated list of absolute paths.\n-\tIf set, it is a list of directories that git should not chdir\n-\tup into while looking for a repository directory.\n-\tIt will not exclude the current working directory or\n-\ta GIT_DIR set on the command line or in the environment.\n-\t(Useful for excluding slow-loading network directories.)\n+\tThis should be a colon-separated list of absolute paths.  If\n+\tset, it is a list of directories that git should not chdir up\n+\tinto while looking for a repository directory (useful for\n+\texcluding slow-loading network directories).  It will not\n+\texclude the current working directory or a GIT_DIR set on the\n+\tcommand line or in the environment.  Normally, Git has to read\n+\tthe entries in this list are read to resolve any symlinks that\n+\tmight be present.  However, if even this access is slow, you\n+\tcan add an empty entry to the list to tell Git that the\n+\tsubsequent entries are not symlinks and needn't be resolved;\n+\te.g.,\n+\t'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.\n \n 'GIT_DISCOVERY_ACROSS_FILESYSTEM'::\n \tWhen run in a directory that does not have \".git\" repository\ndiff --git a/setup.c b/setup.c\nindex f108c4b..1b12017 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n /*\n  * A \"string_list_each_func_t\" function that canonicalizes an entry\n  * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or\n- * discards it if unusable.\n+ * discards it if unusable.  The presence of an empty entry in\n+ * GIT_CEILING_DIRECTORIES turns off canonicalization for all\n+ * subsequent entries.\n  */\n static int canonicalize_ceiling_entry(struct string_list_item *item,\n-\t\t\t\t      void *unused)\n+\t\t\t\t      void *cb_data)\n {\n+\tint *empty_entry_found = cb_data;\n \tchar *ceil = item->string;\n-\tconst char *real_path;\n \n-\tif (!*ceil || !is_absolute_path(ceil))\n+\tif (!*ceil) {\n+\t\t*empty_entry_found = 1;\n \t\treturn 0;\n-\treal_path = real_path_if_valid(ceil);\n-\tif (!real_path)\n+\t} else if (!is_absolute_path(ceil)) {\n \t\treturn 0;\n-\tfree(item->string);\n-\titem->string = xstrdup(real_path);\n-\treturn 1;\n+\t} else if (*empty_entry_found) {\n+\t\t/* Keep entry but do not canonicalize it */\n+\t\treturn 1;\n+\t} else {\n+\t\tconst char *real_path = real_path_if_valid(ceil);\n+\t\tif (!real_path)\n+\t\t\treturn 0;\n+\t\tfree(item->string);\n+\t\titem->string = xstrdup(real_path);\n+\t\treturn 1;\n+\t}\n }\n \n /*\n@@ -679,9 +689,11 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \t\treturn setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);\n \n \tif (env_ceiling_dirs) {\n+\t\tint empty_entry_found = 0;\n+\n \t\tstring_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);\n \t\tfilter_string_list(&ceiling_dirs, 0,\n-\t\t\t\t   canonicalize_ceiling_entry, NULL);\n+\t\t\t\t   canonicalize_ceiling_entry, &empty_entry_found);\n \t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n \t\tstring_list_clear(&ceiling_dirs, 0);\n \t}\n-- \n1.8.1.1\n"},{"id":"209904","messageId":"alpine.DEB.2.00.1302200427220.3204@dr-wily.mit.edu","threadId":"31880","inReplyTo":"7vmwuzzd76.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH v3 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Anders Kaseorg","fromEmail":"andersk@mit.edu","sentAt":"2013-02-20T09:39:38Z","receivedAt":"2013-02-20T09:39:38Z","isPatch":true,"sender":{"key":"andersk@mit.edu","avatar":"https://avatars.githubusercontent.com/u/26471?v=4"},"body":"On Tue, 19 Feb 2013, Junio C Hamano wrote:\n> Assuming that this says \"yes\":\n> \n> \tD=/afs/athena.mit.edu/user/a/n/andersk/my/dir\n>         cd \"$D\"\n>         test \"$(/bin/pwd)\" = \"$D\" && echo yes\n\nCorrect.\n\n> Perhaps existing of an empty element in the list would do?  E.g.\n> \n> \tGIT_CEILING_DIRECTORIES=:/afs/athena.mit.edu/users/a/n/andesk\n> \n> or something like that.  And in such a case, we do not run realpath on \n> the elements on the list before comparing them with what we get from \n> getcwd(3).\n\nThat seems reasonable, and has the advantage of backwards compatibility \nwith versions before 1.8.1.2, I guess.\n\nAnders\n"},{"id":"209915","messageId":"7vsj4qyjbj.fsf@alter.siamese.dyndns.org","threadId":"31880","inReplyTo":"1361351364-15479-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-20T17:41:04Z","receivedAt":"2013-02-20T17:41:04Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> This is a possible implementation (untested!) of Junio's suggestion,\n> with the slight twist that the empty entry can appear anywhere in\n> GIT_CEILING_DIRECTORIES and only turns off symlink expansion for\n> subsequent entries.\n\nSounds like a good way to go, I think.\n\nOriginally I thought that checking with the elements literally and\nstop when we have a match, and then falling back to call realpath()\non them to compare, might be a solution, but I do not think it is.\nI haven't thought things through to convince myself that is the best\napproach to require the users to explicitly tell us to omit calls to\nrealpath().  Perhaps it is the best we could do, but there might be\neven better ways somebody can think up.\n"},{"id":"210000","messageId":"7vk3q1th1l.fsf@alter.siamese.dyndns.org","threadId":"31880","inReplyTo":"1361351364-15479-1-git-send-email-mhagger@alum.mit.edu","subject":"Re: [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-02-21T22:53:42Z","receivedAt":"2013-02-21T22:53:42Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty <mhagger@alum.mit.edu> writes:\n\n> Unfortunately I am swamped with other work right now so I don't have\n> time to test the code and might not be able to respond promptly to\n> feedback.\n\nA note like the above is a good way to give a cue to others so that\nwe can work together to pick up, tie the loose ends and move us\ncloser to the goal, and is very much appreciated.\n\nI think the patch makes sense; I expanded on the part that has\nAnders's report in the log message and added a trivial test.\n\nTesting and eyeballing by others would help very much.  We'd\nobviously need our sign-off as well ;-)\n\nThanks.\n\n-- >8 --\nFrom: Michael Haggerty <mhagger@alum.mit.edu>\nDate: Wed, 20 Feb 2013 10:09:24 +0100\nSubject: [PATCH] Provide a mechanism to turn off symlink resolution in ceiling paths\n\nCommit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks\nin ceiling paths' changed the setup code to resolve symlinks in the\nentries in GIT_CEILING_DIRECTORIES.  Because those entries are\ncompared textually to the symlink-resolved current directory, an\nentry in GIT_CEILING_DIRECTORIES that contained a symlink would have\nno effect.  It was known that this could cause performance problems\nif the symlink resolution *itself* touched slow filesystems, but it\nwas thought that such use cases would be unlikely.  The intention of\nthe earlier change was to deal with a case when the user has this:\n\n\tGIT_CEILING_DIRECTORIES=/home/gitster\n\nbut in reality, /home/gitster is a symbolic link to somewhere else,\ne.g. /net/machine/home4/gitster. A textual comparison between the\nspecified value /home/gitster and the location getcwd(3) returns\nwould not help us, but readlink(\"/home/gitster\") would still be\nfast.\n\nAfter this change was released, Anders Kaseorg <andersk@mit.edu>\nreported:\n\n> [...] my computer has been acting so slow when I’m not connected to\n> the network.  I put various network filesystem paths in\n> $GIT_CEILING_DIRECTORIES, such as\n> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents\n> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and\n> /afs/athena.mit.edu/user/a/n which all live in different AFS\n> volumes).  Now when I’m not connected to the network, every\n> invocation of Git, including the __git_ps1 in my shell prompt, waits\n> for AFS to timeout.\n\nTo allow users to work this around, give them a mechanism to turn\noff symlink resolution in GIT_CEILING_DIRECTORIES entries.  All the\nentries that follow an empty entry will not be checked for symbolic\nlinks and used literally in comparison.  E.g. with these:\n\n\tGIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or\n\tGIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy\n\nwe will not readlink(\"/xyzzy\"), and with the former, we will not\nreadlink(\"/foo/bar\"), either.\n---\n Documentation/git.txt   | 19 +++++++++++++------\n setup.c                 | 32 ++++++++++++++++++++++----------\n t/t1504-ceiling-dirs.sh | 17 +++++++++++++++++\n 3 files changed, 52 insertions(+), 16 deletions(-)\n\ndiff --git a/Documentation/git.txt b/Documentation/git.txt\nindex 6710cb0..5c03616 100644\n--- a/Documentation/git.txt\n+++ b/Documentation/git.txt\n@@ -653,12 +653,19 @@ git so take care if using Cogito etc.\n \tThe '--namespace' command-line option also sets this value.\n \n 'GIT_CEILING_DIRECTORIES'::\n-\tThis should be a colon-separated list of absolute paths.\n-\tIf set, it is a list of directories that git should not chdir\n-\tup into while looking for a repository directory.\n-\tIt will not exclude the current working directory or\n-\ta GIT_DIR set on the command line or in the environment.\n-\t(Useful for excluding slow-loading network directories.)\n+\tThis should be a colon-separated list of absolute paths.  If\n+\tset, it is a list of directories that git should not chdir up\n+\tinto while looking for a repository directory (useful for\n+\texcluding slow-loading network directories).  It will not\n+\texclude the current working directory or a GIT_DIR set on the\n+\tcommand line or in the environment.  Normally, Git has to read\n+\tthe entries in this list are read to resolve any symlinks that\n+\tmight be present in order to compare them with the current\n+\tdirectory.  However, if even this access is slow, you\n+\tcan add an empty entry to the list to tell Git that the\n+\tsubsequent entries are not symlinks and needn't be resolved;\n+\te.g.,\n+\t'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.\n \n 'GIT_DISCOVERY_ACROSS_FILESYSTEM'::\n \tWhen run in a directory that does not have \".git\" repository\ndiff --git a/setup.c b/setup.c\nindex f108c4b..1b12017 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n /*\n  * A \"string_list_each_func_t\" function that canonicalizes an entry\n  * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or\n- * discards it if unusable.\n+ * discards it if unusable.  The presence of an empty entry in\n+ * GIT_CEILING_DIRECTORIES turns off canonicalization for all\n+ * subsequent entries.\n  */\n static int canonicalize_ceiling_entry(struct string_list_item *item,\n-\t\t\t\t      void *unused)\n+\t\t\t\t      void *cb_data)\n {\n+\tint *empty_entry_found = cb_data;\n \tchar *ceil = item->string;\n-\tconst char *real_path;\n \n-\tif (!*ceil || !is_absolute_path(ceil))\n+\tif (!*ceil) {\n+\t\t*empty_entry_found = 1;\n \t\treturn 0;\n-\treal_path = real_path_if_valid(ceil);\n-\tif (!real_path)\n+\t} else if (!is_absolute_path(ceil)) {\n \t\treturn 0;\n-\tfree(item->string);\n-\titem->string = xstrdup(real_path);\n-\treturn 1;\n+\t} else if (*empty_entry_found) {\n+\t\t/* Keep entry but do not canonicalize it */\n+\t\treturn 1;\n+\t} else {\n+\t\tconst char *real_path = real_path_if_valid(ceil);\n+\t\tif (!real_path)\n+\t\t\treturn 0;\n+\t\tfree(item->string);\n+\t\titem->string = xstrdup(real_path);\n+\t\treturn 1;\n+\t}\n }\n \n /*\n@@ -679,9 +689,11 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n \t\treturn setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);\n \n \tif (env_ceiling_dirs) {\n+\t\tint empty_entry_found = 0;\n+\n \t\tstring_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);\n \t\tfilter_string_list(&ceiling_dirs, 0,\n-\t\t\t\t   canonicalize_ceiling_entry, NULL);\n+\t\t\t\t   canonicalize_ceiling_entry, &empty_entry_found);\n \t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n \t\tstring_list_clear(&ceiling_dirs, 0);\n \t}\ndiff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\nindex cce87a5..3d51615 100755\n--- a/t/t1504-ceiling-dirs.sh\n+++ b/t/t1504-ceiling-dirs.sh\n@@ -44,6 +44,10 @@ test_prefix ceil_at_sub \"\"\n GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub/\"\n test_prefix ceil_at_sub_slash \"\"\n \n+if test_have_prereq SYMLINKS\n+then\n+\tln -s sub top\n+fi\n \n mkdir -p sub/dir || exit 1\n cd sub/dir || exit 1\n@@ -68,6 +72,19 @@ test_fail subdir_ceil_at_sub\n GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub/\"\n test_fail subdir_ceil_at_sub_slash\n \n+if test_have_prereq SYMLINKS\n+then\n+\tGIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/top\"\n+\ttest_fail subdir_ceil_at_top\n+\tGIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/top/\"\n+\ttest_fail subdir_ceil_at_top_slash\n+\n+\tGIT_CEILING_DIRECTORIES=\":$TRASH_ROOT/top\"\n+\ttest_prefix subdir_ceil_at_top_no_resolve \"sub/dir/\"\n+\tGIT_CEILING_DIRECTORIES=\":$TRASH_ROOT/top/\"\n+\ttest_prefix subdir_ceil_at_top_slash_no_resolve \"sub/dir/\"\n+fi\n+\n GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub/dir\"\n test_prefix subdir_ceil_at_subdir \"sub/dir/\"\n \n-- \n1.8.2.rc0.152.g52dbac4\n"},{"id":"210017","messageId":"51271CDC.4000001@alum.mit.edu","threadId":"31880","inReplyTo":"7vk3q1th1l.fsf@alter.siamese.dyndns.org","subject":"Re: [RFC] Provide a mechanism to turn off symlink resolution in ceiling paths","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2013-02-22T07:23:08Z","receivedAt":"2013-02-22T07:23:08Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 02/21/2013 11:53 PM, Junio C Hamano wrote:\n> Michael Haggerty <mhagger@alum.mit.edu> writes:\n> \n>> Unfortunately I am swamped with other work right now so I don't have\n>> time to test the code and might not be able to respond promptly to\n>> feedback.\n> \n> A note like the above is a good way to give a cue to others so that\n> we can work together to pick up, tie the loose ends and move us\n> closer to the goal, and is very much appreciated.\n> \n> I think the patch makes sense; I expanded on the part that has\n> Anders's report in the log message and added a trivial test.\n> \n> Testing and eyeballing by others would help very much.  We'd\n> obviously need our sign-off as well ;-)\n\nThanks for following up on this.  Your tests look OK by eyeball and they\nrun successfully here whether the testing --root is under a symlink or\nnot.  I did notice some minor niggles in the text (including one in my\noriginal submission); see below.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n\nMichael\n\n> -- >8 --\n> From: Michael Haggerty <mhagger@alum.mit.edu>\n> Date: Wed, 20 Feb 2013 10:09:24 +0100\n> Subject: [PATCH] Provide a mechanism to turn off symlink resolution in ceiling paths\n> \n> Commit 1b77d83cab 'setup_git_directory_gently_1(): resolve symlinks\n> in ceiling paths' changed the setup code to resolve symlinks in the\n> entries in GIT_CEILING_DIRECTORIES.  Because those entries are\n> compared textually to the symlink-resolved current directory, an\n> entry in GIT_CEILING_DIRECTORIES that contained a symlink would have\n> no effect.  It was known that this could cause performance problems\n> if the symlink resolution *itself* touched slow filesystems, but it\n> was thought that such use cases would be unlikely.  The intention of\n> the earlier change was to deal with a case when the user has this:\n> \n> \tGIT_CEILING_DIRECTORIES=/home/gitster\n> \n> but in reality, /home/gitster is a symbolic link to somewhere else,\n> e.g. /net/machine/home4/gitster. A textual comparison between the\n> specified value /home/gitster and the location getcwd(3) returns\n> would not help us, but readlink(\"/home/gitster\") would still be\n> fast.\n> \n> After this change was released, Anders Kaseorg <andersk@mit.edu>\n> reported:\n> \n>> [...] my computer has been acting so slow when I’m not connected to\n>> the network.  I put various network filesystem paths in\n>> $GIT_CEILING_DIRECTORIES, such as\n>> /afs/athena.mit.edu/user/a/n/andersk (to avoid hitting its parents\n>> /afs/athena.mit.edu, /afs/athena.mit.edu/user/a, and\n>> /afs/athena.mit.edu/user/a/n which all live in different AFS\n>> volumes).  Now when I’m not connected to the network, every\n>> invocation of Git, including the __git_ps1 in my shell prompt, waits\n>> for AFS to timeout.\n> \n> To allow users to work this around, give them a mechanism to turn\n\ns/this around/around this problem/\n\n> off symlink resolution in GIT_CEILING_DIRECTORIES entries.  All the\n> entries that follow an empty entry will not be checked for symbolic\n> links and used literally in comparison.  E.g. with these:\n\nMake it clear that \"not\" doesn't apply to both sides of the \"and\", since\nthe operator precedence in English is undocumented:\n\ns/and/but rather will be/\n\n> \n> \tGIT_CEILING_DIRECTORIES=:/foo/bar:/xyzzy or\n> \tGIT_CEILING_DIRECTORIES=/foo/bar::/xyzzy\n> \n> we will not readlink(\"/xyzzy\"), and with the former, we will not\n> readlink(\"/foo/bar\"), either.\n> ---\n>  Documentation/git.txt   | 19 +++++++++++++------\n>  setup.c                 | 32 ++++++++++++++++++++++----------\n>  t/t1504-ceiling-dirs.sh | 17 +++++++++++++++++\n>  3 files changed, 52 insertions(+), 16 deletions(-)\n> \n> diff --git a/Documentation/git.txt b/Documentation/git.txt\n> index 6710cb0..5c03616 100644\n> --- a/Documentation/git.txt\n> +++ b/Documentation/git.txt\n> @@ -653,12 +653,19 @@ git so take care if using Cogito etc.\n>  \tThe '--namespace' command-line option also sets this value.\n>  \n>  'GIT_CEILING_DIRECTORIES'::\n> -\tThis should be a colon-separated list of absolute paths.\n> -\tIf set, it is a list of directories that git should not chdir\n> -\tup into while looking for a repository directory.\n> -\tIt will not exclude the current working directory or\n> -\ta GIT_DIR set on the command line or in the environment.\n> -\t(Useful for excluding slow-loading network directories.)\n> +\tThis should be a colon-separated list of absolute paths.  If\n> +\tset, it is a list of directories that git should not chdir up\n> +\tinto while looking for a repository directory (useful for\n> +\texcluding slow-loading network directories).  It will not\n> +\texclude the current working directory or a GIT_DIR set on the\n> +\tcommand line or in the environment.  Normally, Git has to read\n> +\tthe entries in this list are read to resolve any symlinks that\n\n\"read\" is duplicated:\n\ns/are read to//\n\n> +\tmight be present in order to compare them with the current\n> +\tdirectory.  However, if even this access is slow, you\n> +\tcan add an empty entry to the list to tell Git that the\n> +\tsubsequent entries are not symlinks and needn't be resolved;\n> +\te.g.,\n> +\t'GIT_CEILING_DIRECTORIES=/maybe/symlink::/very/slow/non/symlink'.\n>  \n>  'GIT_DISCOVERY_ACROSS_FILESYSTEM'::\n>  \tWhen run in a directory that does not have \".git\" repository\n> diff --git a/setup.c b/setup.c\n> index f108c4b..1b12017 100644\n> --- a/setup.c\n> +++ b/setup.c\n> @@ -624,22 +624,32 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n>  /*\n>   * A \"string_list_each_func_t\" function that canonicalizes an entry\n>   * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or\n> - * discards it if unusable.\n> + * discards it if unusable.  The presence of an empty entry in\n> + * GIT_CEILING_DIRECTORIES turns off canonicalization for all\n> + * subsequent entries.\n>   */\n>  static int canonicalize_ceiling_entry(struct string_list_item *item,\n> -\t\t\t\t      void *unused)\n> +\t\t\t\t      void *cb_data)\n>  {\n> +\tint *empty_entry_found = cb_data;\n>  \tchar *ceil = item->string;\n> -\tconst char *real_path;\n>  \n> -\tif (!*ceil || !is_absolute_path(ceil))\n> +\tif (!*ceil) {\n> +\t\t*empty_entry_found = 1;\n>  \t\treturn 0;\n> -\treal_path = real_path_if_valid(ceil);\n> -\tif (!real_path)\n> +\t} else if (!is_absolute_path(ceil)) {\n>  \t\treturn 0;\n> -\tfree(item->string);\n> -\titem->string = xstrdup(real_path);\n> -\treturn 1;\n> +\t} else if (*empty_entry_found) {\n> +\t\t/* Keep entry but do not canonicalize it */\n> +\t\treturn 1;\n> +\t} else {\n> +\t\tconst char *real_path = real_path_if_valid(ceil);\n> +\t\tif (!real_path)\n> +\t\t\treturn 0;\n> +\t\tfree(item->string);\n> +\t\titem->string = xstrdup(real_path);\n> +\t\treturn 1;\n> +\t}\n>  }\n>  \n>  /*\n> @@ -679,9 +689,11 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)\n>  \t\treturn setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok);\n>  \n>  \tif (env_ceiling_dirs) {\n> +\t\tint empty_entry_found = 0;\n> +\n>  \t\tstring_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);\n>  \t\tfilter_string_list(&ceiling_dirs, 0,\n> -\t\t\t\t   canonicalize_ceiling_entry, NULL);\n> +\t\t\t\t   canonicalize_ceiling_entry, &empty_entry_found);\n>  \t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n>  \t\tstring_list_clear(&ceiling_dirs, 0);\n>  \t}\n> diff --git a/t/t1504-ceiling-dirs.sh b/t/t1504-ceiling-dirs.sh\n> index cce87a5..3d51615 100755\n> --- a/t/t1504-ceiling-dirs.sh\n> +++ b/t/t1504-ceiling-dirs.sh\n> @@ -44,6 +44,10 @@ test_prefix ceil_at_sub \"\"\n>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub/\"\n>  test_prefix ceil_at_sub_slash \"\"\n>  \n> +if test_have_prereq SYMLINKS\n> +then\n> +\tln -s sub top\n> +fi\n>  \n>  mkdir -p sub/dir || exit 1\n>  cd sub/dir || exit 1\n> @@ -68,6 +72,19 @@ test_fail subdir_ceil_at_sub\n>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub/\"\n>  test_fail subdir_ceil_at_sub_slash\n>  \n> +if test_have_prereq SYMLINKS\n> +then\n> +\tGIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/top\"\n> +\ttest_fail subdir_ceil_at_top\n> +\tGIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/top/\"\n> +\ttest_fail subdir_ceil_at_top_slash\n> +\n> +\tGIT_CEILING_DIRECTORIES=\":$TRASH_ROOT/top\"\n> +\ttest_prefix subdir_ceil_at_top_no_resolve \"sub/dir/\"\n> +\tGIT_CEILING_DIRECTORIES=\":$TRASH_ROOT/top/\"\n> +\ttest_prefix subdir_ceil_at_top_slash_no_resolve \"sub/dir/\"\n> +fi\n> +\n>  GIT_CEILING_DIRECTORIES=\"$TRASH_ROOT/sub/dir\"\n>  test_prefix subdir_ceil_at_subdir \"sub/dir/\"\n>  \n> \n\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}