{"thread":{"id":"31961","subject":"[PATCH v4 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","startedAt":"2012-10-28T16:16:19Z","lastAt":"2012-11-06T07:34:28Z","messageCount":11,"participants":["Michael Haggerty","Ramsay Jones"],"isPatch":true,"patchVersion":4,"patchTotal":8},"messages":[{"id":"202049","messageId":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":null,"subject":"[PATCH v4 0/8] Fix GIT_CEILING_DIRECTORIES that contain symlinks","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:19Z","receivedAt":"2012-10-28T16:16:19Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"v4 of the series; see the cover letter for v3 [1] for more editorial\ncomments.\n\nChanges since v3:\n\n* For \"test-path-utils longest_ancestor_length\", normalize all of the\n  paths using normalize_path_copy() to counteract the path mangling\n  carried out by bash on Windows.  (Thanks to Johannes Sixt for his\n  helpful advice.)\n\n* Rebased onto a more recent master.\n\n[1] http://thread.gmane.org/gmane.comp.version-control.git/208102\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  setup_git_directory_gently_1(): resolve symlinks in ceiling paths\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                                     |  34 ++++++++-\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                           |  51 +++++++++++++-\n test-string-list.c                          |  20 ------\n 11 files changed, 202 insertions(+), 164 deletions(-)\n\n-- \n1.8.0\n"},{"id":"202050","messageId":"1351440987-26636-2-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 1/8] Introduce new static function real_path_internal()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:20Z","receivedAt":"2012-10-28T16:16:20Z","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.8.0\n"},{"id":"202051","messageId":"1351440987-26636-3-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 2/8] real_path_internal(): add comment explaining use of cwd","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:21Z","receivedAt":"2012-10-28T16:16:21Z","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.8.0\n"},{"id":"202052","messageId":"1351440987-26636-4-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 3/8] Introduce new function real_path_if_valid()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:22Z","receivedAt":"2012-10-28T16:16:22Z","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.8.0\n"},{"id":"202053","messageId":"1351440987-26636-5-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 4/8] longest_ancestor_length(): use string_list_split()","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:23Z","receivedAt":"2012-10-28T16:16:23Z","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.8.0\n"},{"id":"202054","messageId":"1351440987-26636-6-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 5/8] longest_ancestor_length(): take a string_list argument for prefixes","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:24Z","receivedAt":"2012-10-28T16:16:24Z","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 colon-separated string.  This will make\nit easier for the caller to alter the entries before calling\nlongest_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.8.0\n"},{"id":"202055","messageId":"1351440987-26636-7-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:25Z","receivedAt":"2012-10-28T16:16:25Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"Move the responsibility for normalizing prefixes from\nlongest_ancestor_length() to its callers. Use slightly different\nnormalizations at the two callers:\n\nIn setup_git_directory_gently_1(), use the old normalization, which\nignores paths that are not usable.  In the next commit we will change\nthis caller to also resolve symlinks in the paths from\nGIT_CEILING_DIRECTORIES as part of the normalization.\n\nIn \"test-path-utils longest_ancestor_length\", use the old\nnormalization, but die() if any paths are unusable.  Also change t0060\nto only pass normalized paths to the test program (no empty entries or\nnon-absolute paths, strip trailing slashes from the paths, and remove\ntests that thereby become redundant).\n\nThe point of this change is to reduce the scope of the ancestor_length\ntests in t0060 from testing normalization+longest_prefix to testing\nonly mostly longest_prefix.  This is necessary because when\nsetup_git_directory_gently_1() starts resolving symlinks as part of\nits normalization, it will not be reasonable to do the same in the\ntest suite, because that would make the test results depend on the\ncontents of the root directory of the filesystem on which the test is\nrun.  HOWEVER: under Windows, bash mangles arguments that look like\nabsolute POSIX paths into DOS paths.  So we have to retain the level\nof normalization done by normalize_path_copy() to convert the\nbash-mangled DOS paths (which contain backslashes) into paths that use\nforward slashes.\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 test-path-utils.c     | 45 ++++++++++++++++++++++++++++++++++++++++++++-\n 4 files changed, 91 insertions(+), 44 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 \\\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex acb0560..0092cbf 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -1,6 +1,33 @@\n #include \"cache.h\"\n #include \"string-list.h\"\n \n+/*\n+ * A \"string_list_each_func_t\" function that normalizes an entry from\n+ * GIT_CEILING_DIRECTORIES.  If the path is unusable for some reason,\n+ * die with an explanation.\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)\n+\t\tdie(\"Empty path is not supported\");\n+\tif (len > PATH_MAX)\n+\t\tdie(\"Path \\\"%s\\\" is too long\", ceil);\n+\tif (!is_absolute_path(ceil))\n+\t\tdie(\"Path \\\"%s\\\" is not absolute\", ceil);\n+\tif (normalize_path_copy(buf, ceil) < 0)\n+\t\tdie(\"Path \\\"%s\\\" could not be normalized\", ceil);\n+\tlen = strlen(buf);\n+\tif (len > 1 && buf[len-1] == '/')\n+\t\tdie(\"Normalized path \\\"%s\\\" ended with slash\", buf);\n+\tfree(item->string);\n+\titem->string = xstrdup(buf);\n+\treturn 1;\n+}\n+\n int main(int argc, char **argv)\n {\n \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n@@ -33,10 +60,26 @@ int main(int argc, char **argv)\n \tif (argc == 4 && !strcmp(argv[1], \"longest_ancestor_length\")) {\n \t\tint len;\n \t\tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n+\t\tchar *path = xstrdup(argv[2]);\n \n+\t\t/*\n+\t\t * We have to normalize the arguments because under\n+\t\t * Windows, bash mangles arguments that look like\n+\t\t * absolute POSIX paths or colon-separate lists of\n+\t\t * absolute POSIX paths into DOS paths (e.g.,\n+\t\t * \"/foo:/foo/bar\" might be converted to\n+\t\t * \"D:\\Src\\msysgit\\foo;D:\\Src\\msysgit\\foo\\bar\"),\n+\t\t * whereas longest_ancestor_length() requires paths\n+\t\t * that use forward slashes.\n+\t\t */\n+\t\tif (normalize_path_copy(path, path))\n+\t\t\tdie(\"Path \\\"%s\\\" could not be normalized\", argv[2]);\n \t\tstring_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);\n-\t\tlen = longest_ancestor_length(argv[2], &ceiling_dirs);\n+\t\tfilter_string_list(&ceiling_dirs, 0,\n+\t\t\t\t   normalize_ceiling_entry, NULL);\n+\t\tlen = longest_ancestor_length(path, &ceiling_dirs);\n \t\tstring_list_clear(&ceiling_dirs, 0);\n+\t\tfree(path);\n \t\tprintf(\"%d\\n\", len);\n \t\treturn 0;\n \t}\n-- \n1.8.0\n"},{"id":"202056","messageId":"1351440987-26636-8-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 7/8] setup_git_directory_gently_1(): resolve symlinks in ceiling paths","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:26Z","receivedAt":"2012-10-28T16:16:26Z","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().  (Also rename\nnormalize_ceiling_entry() to canonicalize_ceiling_entry() to reflect\nthe change.)\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\nPlease note that test t0060 is *not* changed analogously, because that\nwould make the test suite results dependent on the contents of the\nlocal root directory.  However, real_path() is already tested\nindependently, and the \"ancestor\" tests cover the non-normalization\naspects of longest_ancestor_length(), so coverage remains sufficient.\n\nSigned-off-by: Michael Haggerty <mhagger@alum.mit.edu>\n---\n setup.c | 26 +++++++++++++-------------\n 1 file changed, 13 insertions(+), 13 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex df97ad3..f108c4b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -622,24 +622,23 @@ 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+static int canonicalize_ceiling_entry(struct string_list_item *item,\n+\t\t\t\t      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@@ -681,7 +680,8 @@ 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\tfilter_string_list(&ceiling_dirs, 0,\n+\t\t\t\t   canonicalize_ceiling_entry, NULL);\n \t\tceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);\n \t\tstring_list_clear(&ceiling_dirs, 0);\n \t}\n-- \n1.8.0\n"},{"id":"202057","messageId":"1351440987-26636-9-git-send-email-mhagger@alum.mit.edu","threadId":"31961","inReplyTo":"1351440987-26636-1-git-send-email-mhagger@alum.mit.edu","subject":"[PATCH v4 8/8] string_list_longest_prefix(): remove function","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-10-28T16:16:27Z","receivedAt":"2012-10-28T16:16:27Z","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.8.0\n"},{"id":"202218","messageId":"50901B31.7000402@ramsay1.demon.co.uk","threadId":"31961","inReplyTo":"1351440987-26636-7-git-send-email-mhagger@alum.mit.edu","subject":"Re: [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsay1.demon.co.uk","sentAt":"2012-10-30T18:23:45Z","receivedAt":"2012-10-30T18:23:45Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"Michael Haggerty wrote:\n> Move the responsibility for normalizing prefixes from\n> longest_ancestor_length() to its callers. Use slightly different\n> normalizations at the two callers:\n> \n> In setup_git_directory_gently_1(), use the old normalization, which\n> ignores paths that are not usable.  In the next commit we will change\n> this caller to also resolve symlinks in the paths from\n> GIT_CEILING_DIRECTORIES as part of the normalization.\n> \n> In \"test-path-utils longest_ancestor_length\", use the old\n> normalization, but die() if any paths are unusable.  Also change t0060\n> to only pass normalized paths to the test program (no empty entries or\n> non-absolute paths, strip trailing slashes from the paths, and remove\n> tests that thereby become redundant).\n> \n> The point of this change is to reduce the scope of the ancestor_length\n> tests in t0060 from testing normalization+longest_prefix to testing\n> only mostly longest_prefix.  This is necessary because when\n> setup_git_directory_gently_1() starts resolving symlinks as part of\n> its normalization, it will not be reasonable to do the same in the\n> test suite, because that would make the test results depend on the\n> contents of the root directory of the filesystem on which the test is\n> run.  HOWEVER: under Windows, bash mangles arguments that look like\n> absolute POSIX paths into DOS paths.\n\nJust to be clear, this is true for the MinGW port to Windows, but *not*\nthe cygwin port.\n:-P\n\n>                                      So we have to retain the level\n> of normalization done by normalize_path_copy() to convert the\n> bash-mangled DOS paths (which contain backslashes) into paths that use\n> forward slashes.\n> \n> Signed-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>  test-path-utils.c     | 45 ++++++++++++++++++++++++++++++++++++++++++++-\n>  4 files changed, 91 insertions(+), 44 deletions(-)\n> \n\n[snip]\n\n> diff --git a/test-path-utils.c b/test-path-utils.c\n> index acb0560..0092cbf 100644\n> --- a/test-path-utils.c\n> +++ b/test-path-utils.c\n> @@ -1,6 +1,33 @@\n>  #include \"cache.h\"\n>  #include \"string-list.h\"\n>  \n> +/*\n> + * A \"string_list_each_func_t\" function that normalizes an entry from\n> + * GIT_CEILING_DIRECTORIES.  If the path is unusable for some reason,\n> + * die with an explanation.\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)\n> +\t\tdie(\"Empty path is not supported\");\n> +\tif (len > PATH_MAX)\n> +\t\tdie(\"Path \\\"%s\\\" is too long\", ceil);\n> +\tif (!is_absolute_path(ceil))\n> +\t\tdie(\"Path \\\"%s\\\" is not absolute\", ceil);\n> +\tif (normalize_path_copy(buf, ceil) < 0)\n> +\t\tdie(\"Path \\\"%s\\\" could not be normalized\", ceil);\n> +\tlen = strlen(buf);\n> +\tif (len > 1 && buf[len-1] == '/')\n> +\t\tdie(\"Normalized path \\\"%s\\\" ended with slash\", buf);\n> +\tfree(item->string);\n> +\titem->string = xstrdup(buf);\n> +\treturn 1;\n> +}\n> +\n>  int main(int argc, char **argv)\n>  {\n>  \tif (argc == 3 && !strcmp(argv[1], \"normalize_path_copy\")) {\n> @@ -33,10 +60,26 @@ int main(int argc, char **argv)\n>  \tif (argc == 4 && !strcmp(argv[1], \"longest_ancestor_length\")) {\n>  \t\tint len;\n>  \t\tstruct string_list ceiling_dirs = STRING_LIST_INIT_DUP;\n> +\t\tchar *path = xstrdup(argv[2]);\n>  \n> +\t\t/*\n> +\t\t * We have to normalize the arguments because under\n> +\t\t * Windows, bash mangles arguments that look like\n\nditto\n\n> +\t\t * absolute POSIX paths or colon-separate lists of\n> +\t\t * absolute POSIX paths into DOS paths (e.g.,\n> +\t\t * \"/foo:/foo/bar\" might be converted to\n> +\t\t * \"D:\\Src\\msysgit\\foo;D:\\Src\\msysgit\\foo\\bar\"),\n> +\t\t * whereas longest_ancestor_length() requires paths\n> +\t\t * that use forward slashes.\n> +\t\t */\n> +\t\tif (normalize_path_copy(path, path))\n> +\t\t\tdie(\"Path \\\"%s\\\" could not be normalized\", argv[2]);\n>  \t\tstring_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);\n> -\t\tlen = longest_ancestor_length(argv[2], &ceiling_dirs);\n> +\t\tfilter_string_list(&ceiling_dirs, 0,\n> +\t\t\t\t   normalize_ceiling_entry, NULL);\n> +\t\tlen = longest_ancestor_length(path, &ceiling_dirs);\n>  \t\tstring_list_clear(&ceiling_dirs, 0);\n> +\t\tfree(path);\n>  \t\tprintf(\"%d\\n\", len);\n>  \t\treturn 0;\n>  \t}\n\n\nHTH\n\nATB,\nRamsay Jones\n"},{"id":"202581","messageId":"5098BD84.8040109@alum.mit.edu","threadId":"31961","inReplyTo":"50901B31.7000402@ramsay1.demon.co.uk","subject":"Re: [PATCH v4 6/8] longest_ancestor_length(): require prefix list entries to be normalized","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2012-11-06T07:34:28Z","receivedAt":"2012-11-06T07:34:28Z","isPatch":true,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 10/30/2012 07:23 PM, Ramsay Jones wrote:\n> [...]\n> Just to be clear, this is true for the MinGW port to Windows, but *not*\n> the cygwin port.\n>\n> [...]\n> \n> ditto\n\nThanks for clarifying these points.  It seems like this patch series is\nnot going to be usable (because of fears that it might cause performance\ndegradation in some as-yet-unexplained scenarios), but if that situation\nseems to change then I will incorporate your corrections.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"}]}