longest_ancestor_length(): require prefix list entries to be normalized

Move the responsibility for normalizing prefixes from
longest_ancestor_length() to its callers. Use slightly different
normalizations at the two callers:

In setup_git_directory_gently_1(), use the old normalization, which
ignores paths that are not usable.  In the next commit we will change
this caller to also resolve symlinks in the paths from
GIT_CEILING_DIRECTORIES as part of the normalization.

In "test-path-utils longest_ancestor_length", use the old
normalization, but die() if any paths are unusable.  Also change t0060
to only pass normalized paths to the test program (no empty entries or
non-absolute paths, strip trailing slashes from the paths, and remove
tests that thereby become redundant).

The point of this change is to reduce the scope of the ancestor_length
tests in t0060 from testing normalization+longest_prefix to testing
only mostly longest_prefix.  This is necessary because when
setup_git_directory_gently_1() starts resolving symlinks as part of
its normalization, it will not be reasonable to do the same in the
test suite, because that would make the test results depend on the
contents of the root directory of the filesystem on which the test is
run.  HOWEVER: under Windows, bash mangles arguments that look like
absolute POSIX paths into DOS paths.  So we have to retain the level
of normalization done by normalize_path_copy() to convert the
bash-mangled DOS paths (which contain backslashes) into paths that use
forward slashes.

Signed-off-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Jeff King <peff@peff.net>

diff --git a/path.c b/path.c
index b80d2e6e4c292847439aa5acee621ac2d51f204f..d3d3f8b8ad75b9817df3014296aabc34b6a4eb14 100644 (file)
--- a/path.c
+++ b/path.c
@@ -570,20 +570,20 @@ int normalize_path_copy(char *dst, const char *src)
 
 /*
  * path = Canonical absolute path
- * prefixes = string_list containing absolute paths
+ * prefixes = string_list containing normalized, absolute paths without
+ * trailing slashes (except for the root directory, which is denoted by "/").
  *
- * Determines, for each path in prefixes, whether the "prefix" really
+ * Determines, for each path in prefixes, whether the "prefix"
  * is an ancestor directory of path.  Returns the length of the longest
  * ancestor directory, excluding any trailing slashes, or -1 if no prefix
  * is an ancestor.  (Note that this means 0 is returned if prefixes is
  * ["/"].) "/foo" is not considered an ancestor of "/foobar".  Directories
  * are not considered to be their own ancestors.  path must be in a
  * canonical form: empty components, or "." or ".." components are not
- * allowed.  Empty strings in prefixes are ignored.
+ * allowed.
  */
 int longest_ancestor_length(const char *path, struct string_list *prefixes)
 {
-       char buf[PATH_MAX+1];
        int i, max_len = -1;
 
        if (!strcmp(path, "/"))
@@ -593,19 +593,15 @@ int longest_ancestor_length(const char *path, struct string_list *prefixes)
                const char *ceil = prefixes->items[i].string;
                int len = strlen(ceil);
 
-               if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
-                       continue;
-               if (normalize_path_copy(buf, ceil) < 0)
-                       continue;
-               len = strlen(buf);
-               if (len > 0 && buf[len-1] == '/')
-                       buf[--len] = '\0';
+               if (len == 1 && ceil[0] == '/')
+                       len = 0; /* root matches anything, with length 0 */
+               else if (!strncmp(path, ceil, len) && path[len] == '/')
+                       ; /* match of length len */
+               else
+                       continue; /* no match */
 
-               if (!strncmp(path, buf, len) &&
-                   path[len] == '/' &&
-                   len > max_len) {
+               if (len > max_len)
                        max_len = len;
-               }
        }
 
        return max_len;

diff --git a/setup.c b/setup.c
index b4cd35645618558a43b497294536b6947f7b190f..df97ad397643de60c6e9c4eb67537daa8a7d67bd 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -621,6 +621,28 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_
        return buf.st_dev;
 }
 
+/*
+ * A "string_list_each_func_t" function that normalizes an entry from
+ * GIT_CEILING_DIRECTORIES or discards it if unusable.
+ */
+static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
+{
+       const char *ceil = item->string;
+       int len = strlen(ceil);
+       char buf[PATH_MAX+1];
+
+       if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
+               return 0;
+       if (normalize_path_copy(buf, ceil) < 0)
+               return 0;
+       len = strlen(buf);
+       if (len > 1 && buf[len-1] == '/')
+               buf[--len] = '\0';
+       free(item->string);
+       item->string = xstrdup(buf);
+       return 1;
+}
+
 /*
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
@@ -659,6 +681,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok)
 
        if (env_ceiling_dirs) {
                string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1);
+               filter_string_list(&ceiling_dirs, 0, normalize_ceiling_entry, NULL);
                ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs);
                string_list_clear(&ceiling_dirs, 0);
        }

diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 4ef2345982fe0e0c175ed67edcb20b04c81b9680..09a42a428e1d286c5ab9e1003d49481ef6df3e44 100755 (executable)
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -93,47 +93,32 @@ norm_path /d1/s1//../s2/../../d2 /d2 POSIX
 norm_path /d1/.../d2 /d1/.../d2 POSIX
 norm_path /d1/..././../d2 /d1/d2 POSIX
 
-ancestor / "" -1
 ancestor / / -1
-ancestor /foo "" -1
-ancestor /foo : -1
-ancestor /foo ::. -1
-ancestor /foo ::..:: -1
 ancestor /foo / 0
 ancestor /foo /fo -1
 ancestor /foo /foo -1
-ancestor /foo /foo/ -1
 ancestor /foo /bar -1
-ancestor /foo /bar/ -1
 ancestor /foo /foo/bar -1
-ancestor /foo /foo:/bar/ -1
-ancestor /foo /foo/:/bar/ -1
-ancestor /foo /foo::/bar/ -1
-ancestor /foo /:/foo:/bar/ 0
-ancestor /foo /foo:/:/bar/ 0
-ancestor /foo /:/bar/:/foo 0
-ancestor /foo/bar "" -1
+ancestor /foo /foo:/bar -1
+ancestor /foo /:/foo:/bar 0
+ancestor /foo /foo:/:/bar 0
+ancestor /foo /:/bar:/foo 0
 ancestor /foo/bar / 0
 ancestor /foo/bar /fo -1
-ancestor /foo/bar foo -1
 ancestor /foo/bar /foo 4
-ancestor /foo/bar /foo/ 4
 ancestor /foo/bar /foo/ba -1
 ancestor /foo/bar /:/fo 0
 ancestor /foo/bar /foo:/foo/ba 4
 ancestor /foo/bar /bar -1
-ancestor /foo/bar /bar/ -1
-ancestor /foo/bar /fo: -1
-ancestor /foo/bar :/fo -1
-ancestor /foo/bar /foo:/bar/ 4
-ancestor /foo/bar /:/foo:/bar/ 4
-ancestor /foo/bar /foo:/:/bar/ 4
-ancestor /foo/bar /:/bar/:/fo 0
-ancestor /foo/bar /:/bar/ 0
-ancestor /foo/bar .:/foo/. 4
-ancestor /foo/bar .:/foo/.:.: 4
-ancestor /foo/bar /foo/./:.:/bar 4
-ancestor /foo/bar .:/bar -1
+ancestor /foo/bar /fo -1
+ancestor /foo/bar /foo:/bar 4
+ancestor /foo/bar /:/foo:/bar 4
+ancestor /foo/bar /foo:/:/bar 4
+ancestor /foo/bar /:/bar:/fo 0
+ancestor /foo/bar /:/bar 0
+ancestor /foo/bar /foo 4
+ancestor /foo/bar /foo:/bar 4
+ancestor /foo/bar /bar -1
 
 test_expect_success 'strip_path_suffix' '
        test c:/msysgit = $(test-path-utils strip_path_suffix \

diff --git a/test-path-utils.c b/test-path-utils.c
index acb05600df7fa1ac2a567a5d3b02bb67b78f2aca..0092cbf3540e2fdfd2e882461ba39ba2a833b672 100644 (file)
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -1,6 +1,33 @@
 #include "cache.h"
 #include "string-list.h"
 
+/*
+ * A "string_list_each_func_t" function that normalizes an entry from
+ * GIT_CEILING_DIRECTORIES.  If the path is unusable for some reason,
+ * die with an explanation.
+ */
+static int normalize_ceiling_entry(struct string_list_item *item, void *unused)
+{
+       const char *ceil = item->string;
+       int len = strlen(ceil);
+       char buf[PATH_MAX+1];
+
+       if (len == 0)
+               die("Empty path is not supported");
+       if (len > PATH_MAX)
+               die("Path \"%s\" is too long", ceil);
+       if (!is_absolute_path(ceil))
+               die("Path \"%s\" is not absolute", ceil);
+       if (normalize_path_copy(buf, ceil) < 0)
+               die("Path \"%s\" could not be normalized", ceil);
+       len = strlen(buf);
+       if (len > 1 && buf[len-1] == '/')
+               die("Normalized path \"%s\" ended with slash", buf);
+       free(item->string);
+       item->string = xstrdup(buf);
+       return 1;
+}
+
 int main(int argc, char **argv)
 {
        if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
@@ -33,10 +60,26 @@ int main(int argc, char **argv)
        if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
                int len;
                struct string_list ceiling_dirs = STRING_LIST_INIT_DUP;
+               char *path = xstrdup(argv[2]);
 
+               /*
+                * We have to normalize the arguments because under
+                * Windows, bash mangles arguments that look like
+                * absolute POSIX paths or colon-separate lists of
+                * absolute POSIX paths into DOS paths (e.g.,
+                * "/foo:/foo/bar" might be converted to
+                * "D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar"),
+                * whereas longest_ancestor_length() requires paths
+                * that use forward slashes.
+                */
+               if (normalize_path_copy(path, path))
+                       die("Path \"%s\" could not be normalized", argv[2]);
                string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1);
-               len = longest_ancestor_length(argv[2], &ceiling_dirs);
+               filter_string_list(&ceiling_dirs, 0,
+                                  normalize_ceiling_entry, NULL);
+               len = longest_ancestor_length(path, &ceiling_dirs);
                string_list_clear(&ceiling_dirs, 0);
+               free(path);
                printf("%d\n", len);
                return 0;
        }