Original design of relative_path() is simple, just strip the prefix
(*base) from the absolute path (*abs). In most cases, we need a real
relative path, such as: ../foo, ../../bar. That's why there is another
reimplementation (path_relative()) in quote.c.

Borrowed some codes from path_relative() in quote.c to refactor
relative_path() in path.c, so that it could return real relative path,
and user can reuse this function without reimplement his/her own.
The function path_relative() in quote.c will be substituted, and I
would use the new relative_path() function when implement the
interactive git-clean later.

Different results for relative_path() before and after this refactor:

    abs path  base path  relative (original)  relative (refactor)
    ========  =========  ===================  ===================
    /a/b/c/   /a/b       c/                   c/
    /a/b//c/  //a///b/   c/                   c/
    /a/b      /a/b       .                    ./
    /a/b/     /a/b       .                    ./
    /a        /a/b/      /a                   ../
    /         /a/b/      /                    ../../
    /a/c      /a/b/      /a/c                 ../c
    /x/y      /a/b/      /x/y                 ../../x/y
    /a/b      (empty)    /a/b                 /a/b
    /a/b      (null)     /a/b                 /a/b

    a/b/c/    a/b/       c/                   c/
    a/b/c/    a/b        c/                   c/
    a/b//c    a//b       c                    c
    a/b/      a/b/       .                    ./
    a/b/      a/b        .                    ./
    a         a/b        a                    ../
    x/y       a/b/       x/y                  ../../x/y
    a/c       a/b        a/c                  ../c
    a/b       (empty)    a/b                  a/b
    a/b       (null)     a/b                  a/b

    (empty)   (null)     (empty)              ./
    (empty)   (empty)    (empty)              ./
    (empty)   /a/b       (empty)              ./
    (null)    (null)     (null)               ./
    (null)    (empty)    (null)               ./
    (null)    /a/b       (segfault)           ./

You may notice that return value "." has been changed to "./".
It is because:

 * Function quote_path_relative() in quote.c will show the relative
   path as "./" if abs(in) and base(prefix) are the same.

 * Function relative_path() is called only once (in setup.c), and
   it will be OK for the return value as "./" instead of ".".

Signed-off-by: Jiang Xin <worldhello....@gmail.com>
Signed-off-by: Junio C Hamano <gits...@pobox.com>
---
 cache.h               |   2 +-
 path.c                | 116 +++++++++++++++++++++++++++++++++++++-------------
 setup.c               |   5 ++-
 t/t0060-path-utils.sh |  39 ++++++++---------
 test-path-utils.c     |   4 +-
 5 files changed, 113 insertions(+), 53 deletions(-)

diff --git a/cache.h b/cache.h
index dd0fb..c2886 100644
--- a/cache.h
+++ b/cache.h
@@ -758,7 +758,7 @@ int is_directory(const char *);
 const char *real_path(const char *path);
 const char *real_path_if_valid(const char *path);
 const char *absolute_path(const char *path);
-const char *relative_path(const char *abs, const char *base);
+const char *relative_path(const char *in, const char *prefix, struct strbuf 
*sb);
 int normalize_path_copy(char *dst, const char *src);
 int longest_ancestor_length(const char *path, struct string_list *prefixes);
 char *strip_path_suffix(const char *path, const char *suffix);
diff --git a/path.c b/path.c
index 04ff..fbe52c9b 100644
--- a/path.c
+++ b/path.c
@@ -441,42 +441,100 @@ int adjust_shared_perm(const char *path)
        return 0;
 }
 
-const char *relative_path(const char *abs, const char *base)
+/*
+ * Give path as relative to prefix.
+ *
+ * The strbuf may or may not be used, so do not assume it contains the
+ * returned path.
+ */
+const char *relative_path(const char *in, const char *prefix,
+                         struct strbuf *sb)
 {
-       static char buf[PATH_MAX + 1];
-       int i = 0, j = 0;
-
-       if (!base || !base[0])
-               return abs;
-       while (base[i]) {
-               if (is_dir_sep(base[i])) {
-                       if (!is_dir_sep(abs[j]))
-                               return abs;
-                       while (is_dir_sep(base[i]))
+       int in_len = in ? strlen(in) : 0;
+       int prefix_len = prefix ? strlen(prefix) : 0;
+       int in_off = 0;
+       int prefix_off = 0;
+       int i = 0,j = 0;
+
+       if (!in_len)
+               return "./";
+       else if (!prefix_len)
+               return in;
+
+       while (i < prefix_len && j < in_len && prefix[i] == in[j]) {
+               if (is_dir_sep(prefix[i])) {
+                       while (is_dir_sep(prefix[i]))
                                i++;
-                       while (is_dir_sep(abs[j]))
+                       while (is_dir_sep(in[j]))
                                j++;
+                       prefix_off = i;
+                       in_off = j;
+               } else {
+                       i++;
+                       j++;
+               }
+       }
+
+       if (
+           /* "prefix" seems like prefix of "in" */
+           i >= prefix_len &&
+           /*
+            * but "/foo" is not a prefix of "/foobar"
+            * (i.e. prefix not end with '/')
+            */
+           prefix_off < prefix_len) {
+               if (j >= in_len) {
+                       /* in="/a/b", prefix="/a/b" */
+                       in_off = in_len;
+               } else if (is_dir_sep(in[j])) {
+                       /* in="/a/b/c", prefix="/a/b" */
+                       while (is_dir_sep(in[j]))
+                               j++;
+                       in_off = j;
+               } else {
+                       /* in="/a/bbb/c", prefix="/a/b" */
+                       i = prefix_off;
+               }
+       } else if (
+                  /* "in" is short than "prefix" */
+                  j >= in_len &&
+                  /* "in" not end with '/' */
+                  in_off < in_len) {
+               if (is_dir_sep(prefix[i])) {
+                       /* in="/a/b", prefix="/a/b/c/" */
+                       while (is_dir_sep(prefix[i]))
+                               i++;
+                       in_off = in_len;
+               }
+       }
+       in += in_off;
+       in_len -= in_off;
+
+       if (i >= prefix_len) {
+               if (!in_len)
+                       return "./";
+               else
+                       return in;
+       }
+
+       strbuf_reset(sb);
+       strbuf_grow(sb, in_len);
+
+       while (i < prefix_len) {
+               if (is_dir_sep(prefix[i])) {
+                       strbuf_addstr(sb, "../");
+                       while (is_dir_sep(prefix[i]))
+                               i++;
                        continue;
-               } else if (abs[j] != base[i]) {
-                       return abs;
                }
                i++;
-               j++;
        }
-       if (
-           /* "/foo" is a prefix of "/foo" */
-           abs[j] &&
-           /* "/foo" is not a prefix of "/foobar" */
-           !is_dir_sep(base[i-1]) && !is_dir_sep(abs[j])
-          )
-               return abs;
-       while (is_dir_sep(abs[j]))
-               j++;
-       if (!abs[j])
-               strcpy(buf, ".");
-       else
-               strcpy(buf, abs + j);
-       return buf;
+       if (!is_dir_sep(prefix[prefix_len - 1]))
+               strbuf_addstr(sb, "../");
+
+       strbuf_addstr(sb, in);
+
+       return sb->buf;
 }
 
 /*
diff --git a/setup.c b/setup.c
index 94c1e..0d9ea 100644
--- a/setup.c
+++ b/setup.c
@@ -360,6 +360,7 @@ int is_inside_work_tree(void)
 
 void setup_work_tree(void)
 {
+       struct strbuf sb = STRBUF_INIT;
        const char *work_tree, *git_dir;
        static int initialized = 0;
 
@@ -379,8 +380,10 @@ void setup_work_tree(void)
        if (getenv(GIT_WORK_TREE_ENVIRONMENT))
                setenv(GIT_WORK_TREE_ENVIRONMENT, ".", 1);
 
-       set_git_dir(relative_path(git_dir, work_tree));
+       set_git_dir(relative_path(git_dir, work_tree, &sb));
        initialized = 1;
+
+       strbuf_release(&sb);
 }
 
 static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 72e89..76c779 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -191,33 +191,30 @@ test_expect_success SYMLINKS 'real path works on 
symlinks' '
 relative_path /a/b/c/  /a/b/           c/
 relative_path /a/b/c/  /a/b            c/
 relative_path /a//b//c/        //a/b//         c/      POSIX
-relative_path /a/b     /a/b            .
-relative_path /a/b/    /a/b            .
-relative_path /a       /a/b            /a      POSIX
-relative_path /                /a/b/           /       POSIX
-relative_path /a/c     /a/b/           /a/c    POSIX
-relative_path /a/c     /a/b            /a/c    POSIX
-relative_path /x/y     /a/b/           /x/y    POSIX
+relative_path /a/b     /a/b            ./
+relative_path /a/b/    /a/b            ./
+relative_path /a       /a/b            ../
+relative_path /                /a/b/           ../../
+relative_path /a/c     /a/b/           ../c
+relative_path /a/c     /a/b            ../c
+relative_path /x/y     /a/b/           ../../x/y
 relative_path /a/b     "<empty>"       /a/b    POSIX
 relative_path /a/b     "<null>"        /a/b    POSIX
 relative_path a/b/c/   a/b/            c/
 relative_path a/b/c/   a/b             c/
 relative_path a/b//c   a//b            c
-relative_path a/b/     a/b/            .
-relative_path a/b/     a/b             .
-relative_path a                a/b             a       # TODO: should be: ..
-relative_path x/y      a/b             x/y     # TODO: should be: ../../x/y
-relative_path a/c      a/b             a/c     # TODO: should be: ../c
+relative_path a/b/     a/b/            ./
+relative_path a/b/     a/b             ./
+relative_path a                a/b             ../
+relative_path x/y      a/b             ../../x/y
+relative_path a/c      a/b             ../c
 relative_path a/b      "<empty>"       a/b
 relative_path a/b      "<null>"        a/b
-relative_path "<empty>"        /a/b            "(empty)"
-relative_path "<empty>"        "<empty>"       "(empty)"
-relative_path "<empty>"        "<null>"        "(empty)"
-relative_path "<null>" "<empty>"       "(null)"
-relative_path "<null>" "<null>"        "(null)"
-
-test_expect_failure 'relative path: <null> /a/b => segfault' '
-       test-path-utils relative_path "<null>" "/a/b"
-'
+relative_path "<empty>"        /a/b            ./
+relative_path "<empty>"        "<empty>"       ./
+relative_path "<empty>"        "<null>"        ./
+relative_path "<null>" "<empty>"       ./
+relative_path "<null>" "<null>"        ./
+relative_path "<null>" /a/b            ./
 
 test_done
diff --git a/test-path-utils.c b/test-path-utils.c
index 8a6d2..1bf473 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -117,14 +117,16 @@ int main(int argc, char **argv)
        }
 
        if (argc == 4 && !strcmp(argv[1], "relative_path")) {
+               struct strbuf sb = STRBUF_INIT;
                const char *in, *prefix, *rel;
                normalize_argv_string(&in, argv[2]);
                normalize_argv_string(&prefix, argv[3]);
-               rel = relative_path(in, prefix);
+               rel = relative_path(in, prefix, &sb);
                if (!rel)
                        puts("(null)");
                else
                        puts(strlen(rel) > 0 ? rel : "(empty)");
+               strbuf_release(&sb);
                return 0;
        }
 
-- 
1.8.3.1.756.g2e9b71f

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to