[PATCH] submodule: Port resolve_relative_url from shell to C

This reimplements the helper function `resolve_relative_url` in shell
in C. This functionality is needed in C for introducing the groups
feature later on. When using groups, the user should not need to run
`git submodule init`, but it should be implicit at all appropriate places,
which are all in C code. As the we would not just call out to `git
submodule init`, but do a more fine grained structure there, we actually
need all the init functionality in C before attempting the groups
feature. To get the init functionality in C, rewriting the
resolve_relative_url subfunction is a major step.

This also improves the performance:
(Best out of 3) time ./t7400-submodule-basic.sh
Before:
real	0m9.575s
user	0m2.683s
sys	0m6.773s
After:
real	0m9.293s
user	0m2.691s
sys	0m6.549s

Signed-off-by: Stefan Beller <redacted>
---

 This applies on origin/master, and I'd carry as its own feature branch
 as I am nowhere near done with the groups feature after reading Jens feedback.
 (It took me a while to identify this as a next best step.)
 
 Thanks,
 Stefan

 builtin/submodule--helper.c | 120 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  81 ++----------------------------
 2 files changed, 124 insertions(+), 77 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..f48b5b5 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c

@@ -9,6 +9,125 @@
 #include "submodule-config.h"
 #include "string-list.h"
 #include "run-command.h"
+#include "remote.h"
+#include "refs.h"
+
+static const char *get_default_remote(void)
+{
+	char *dest = NULL;
+	unsigned char sha1[20];
+	int flag;
+	struct strbuf sb = STRBUF_INIT;
+	const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, &flag);
+
+	if (!refname)
+		die("No such ref: HEAD");
+
+	refname = shorten_unambiguous_ref(refname, 0);
+	strbuf_addf(&sb, "branch.%s.remote", refname);
+	if (git_config_get_string(sb.buf, &dest))
+		return "origin";
+	else
+		return xstrdup(dest);
+}
+
+/*
+ * The function takes at most 2 arguments. The first argument is the
+ * URL that navigates to the submodule origin repo. When relative, this URL
+ * is relative to the superproject origin URL repo. The second up_path
+ * argument, if specified, is the relative path that navigates
+ * from the submodule working tree to the superproject working tree.
+ *
+ * The output of the function is the origin URL of the submodule.
+ *
+ * The output will either be an absolute URL or filesystem path (if the
+ * superproject origin URL is an absolute URL or filesystem path,
+ * respectively) or a relative file system path (if the superproject
+ * origin URL is a relative file system path).
+ *
+ * When the output is a relative file system path, the path is either
+ * relative to the submodule working tree, if up_path is specified, or to
+ * the superproject working tree otherwise.
+ */
+static const char *relative_url(const char *url, const char *up_path)
+{
+	int is_relative = 0;
+	size_t len;
+	char *remoteurl = NULL;
+	char *sep = "/";
+	const char *out;
+	struct strbuf sb = STRBUF_INIT;
+	const char *remote = get_default_remote();
+	strbuf_addf(&sb, "remote.%s.url", remote);
+
+	if (git_config_get_string(sb.buf, &remoteurl))
+		/* the repository is its own authoritative upstream */
+		remoteurl = xgetcwd();
+
+	if (strip_suffix(remoteurl, "/", &len))
+		remoteurl[len] = '\0';
+
+	if (strchr(remoteurl, ':') || skip_prefix(remoteurl, "/", &out))
+		is_relative = 0;
+	else if (skip_prefix(remoteurl, "./", &out) ||
+		    skip_prefix(remoteurl, "../", &out))
+		is_relative = 1;
+	else {
+		is_relative = 1;
+		strbuf_reset(&sb);
+		strbuf_addf(&sb, "./%s", remoteurl);
+		remoteurl = strbuf_detach(&sb, NULL);
+	}
+
+	while (url) {
+		if (skip_prefix(url, "../", &out)) {
+			char *rfind;
+			url = out;
+
+			rfind = strrchr(remoteurl, '/');
+			if (rfind)
+				*rfind = '\0';
+			else {
+				rfind = strrchr(remoteurl, ':');
+				if (rfind) {
+					*rfind = '\0';
+					sep = ":";
+				} else {
+					if (is_relative || !strcmp(".", remoteurl))
+						die(N_("cannot strip one component off url '%s'"), remoteurl);
+					else
+						remoteurl = ".";
+				}
+			}
+		} else if (skip_prefix(url, "./", &out))
+			url = out;
+		else
+			break;
+	}
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s%s%s", remoteurl, sep, url);
+
+	if (!skip_prefix(sb.buf, "./", &out))
+		out = sb.buf;
+	out = xstrdup(out);
+
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s%s", is_relative && up_path ? up_path : "", out);
+
+	free((char*)out);
+	return strbuf_detach(&sb, NULL);
+}
+
+static int resolve_relative_url(int argc, const char **argv, const char *prefix)
+{
+	if (argc == 2)
+		printf("%s\n", relative_url(argv[1], NULL));
+	else if (argc == 3)
+		printf("%s\n", relative_url(argv[1], argv[2]));
+	else
+		die("BUG: resolve_relative_url only accepts one or two arguments");
+	return 0;
+}
 
 struct module_list {
 	const struct cache_entry **entries;

@@ -264,6 +383,7 @@ static struct cmd_struct commands[] = {
 	{"list", module_list},
 	{"name", module_name},
 	{"clone", module_clone},
+	{"resolve_relative_url", resolve_relative_url},
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)

diff --git a/git-submodule.sh b/git-submodule.sh
index 9bc5c5f..6a7a3e4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -46,79 +46,6 @@ prefix=
 custom_name=
 depth=
 
-# The function takes at most 2 arguments. The first argument is the
-# URL that navigates to the submodule origin repo. When relative, this URL
-# is relative to the superproject origin URL repo. The second up_path
-# argument, if specified, is the relative path that navigates
-# from the submodule working tree to the superproject working tree.
-#
-# The output of the function is the origin URL of the submodule.
-#
-# The output will either be an absolute URL or filesystem path (if the
-# superproject origin URL is an absolute URL or filesystem path,
-# respectively) or a relative file system path (if the superproject
-# origin URL is a relative file system path).
-#
-# When the output is a relative file system path, the path is either
-# relative to the submodule working tree, if up_path is specified, or to
-# the superproject working tree otherwise.
-resolve_relative_url ()
-{
-	remote=$(get_default_remote)
-	remoteurl=$(git config "remote.$remote.url") ||
-		remoteurl=$(pwd) # the repository is its own authoritative upstream
-	url="$1"
-	remoteurl=${remoteurl%/}
-	sep=/
-	up_path="$2"
-
-	case "$remoteurl" in
-	*:*|/*)
-		is_relative=
-		;;
-	./*|../*)
-		is_relative=t
-		;;
-	*)
-		is_relative=t
-		remoteurl="./$remoteurl"
-		;;
-	esac
-
-	while test -n "$url"
-	do
-		case "$url" in
-		../*)
-			url="${url#../}"
-			case "$remoteurl" in
-			*/*)
-				remoteurl="${remoteurl%/*}"
-				;;
-			*:*)
-				remoteurl="${remoteurl%:*}"
-				sep=:
-				;;
-			*)
-				if test -z "$is_relative" || test "." = "$remoteurl"
-				then
-					die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")"
-				else
-					remoteurl=.
-				fi
-				;;
-			esac
-			;;
-		./*)
-			url="${url#./}"
-			;;
-		*)
-			break;;
-		esac
-	done
-	remoteurl="$remoteurl$sep${url%/}"
-	echo "${is_relative:+${up_path}}${remoteurl#./}"
-}
-
 # Resolve a path to be relative to another path.  This is intended for
 # converting submodule paths when git-submodule is run in a subdirectory
 # and only handles paths where the directory separator is '/'.

@@ -281,7 +208,7 @@ cmd_add()
 		die "$(gettext "Relative path can only be used from the toplevel of the working tree")"
 
 		# dereference source url relative to parent's url
-		realrepo=$(resolve_relative_url "$repo") || exit
+		realrepo=$(git submodule--helper resolve_relative_url "$repo") || exit
 		;;
 	*:*|/*)
 		# absolute url

@@ -485,7 +412,7 @@ cmd_init()
 			# Possibly a url relative to parent
 			case "$url" in
 			./*|../*)
-				url=$(resolve_relative_url "$url") || exit
+				url=$(git submodule--helper resolve_relative_url "$url") || exit
 				;;
 			esac
 			git config submodule."$name".url "$url" ||

@@ -1190,9 +1117,9 @@ cmd_sync()
 			# guarantee a trailing /
 			up_path=${up_path%/}/ &&
 			# path from submodule work tree to submodule origin repo
-			sub_origin_url=$(resolve_relative_url "$url" "$up_path") &&
+			sub_origin_url=$(git submodule--helper resolve_relative_url "$url" "$up_path") &&
 			# path from superproject work tree to submodule origin repo
-			super_config_url=$(resolve_relative_url "$url") || exit
+			super_config_url=$(git submodule--helper resolve_relative_url "$url") || exit
 			;;
 		*)
 			sub_origin_url="$url"

-- 
2.6.3.470.g4b82c23.dirty

Am 10.12.2015 um 02:07 schrieb Stefan Beller:

This reimplements the helper function `resolve_relative_url` in shell
in C. This functionality is needed in C for introducing the groups
feature later on. When using groups, the user should not need to run
`git submodule init`, but it should be implicit at all appropriate places,
which are all in C code. As the we would not just call out to `git
submodule init`, but do a more fine grained structure there, we actually
need all the init functionality in C before attempting the groups
feature. To get the init functionality in C, rewriting the
resolve_relative_url subfunction is a major step.

I see lots of '/', but no is_dir_sep() in the C version. Did you 
consider that local URLs can use a backslash as path separator on 
Windows? In the shell version, this did not matter because bash converts 
the backslashes to forward slashes for us. But when rewritten in C, this 
does not happen.

Valid URLs are

   D:\foo\bar.git
   \\server\share\foo\bar
   ..\..\foo\bar

and all of them with some or all backslashes replaced by forward slashes.

See also connect.c:url_is_local_not_ssh, which ensures that the first 
example above is considered a local path with a drive letter, not a 
remote ssh path.

On Wed, Dec 9, 2015 at 10:48 PM, Johannes Sixt [off-list ref] wrote:

Am 10.12.2015 um 02:07 schrieb Stefan Beller:

quoted

This reimplements the helper function `resolve_relative_url` in shell
in C. This functionality is needed in C for introducing the groups
feature later on. When using groups, the user should not need to run
`git submodule init`, but it should be implicit at all appropriate places,
which are all in C code. As the we would not just call out to `git
submodule init`, but do a more fine grained structure there, we actually
need all the init functionality in C before attempting the groups
feature. To get the init functionality in C, rewriting the
resolve_relative_url subfunction is a major step.

I see lots of '/', but no is_dir_sep() in the C version. Did you consider
that local URLs can use a backslash as path separator on Windows? In the
shell version, this did not matter because bash converts the backslashes to
forward slashes for us. But when rewritten in C, this does not happen.

I see. That's a pity. :(

Valid URLs are

  D:\foo\bar.git
  \\server\share\foo\bar
  ..\..\foo\bar

I am staring at the code in desperation as backslashes
in Linux are valid for file names, i.e.:
"/tmp/testfile\" is a valid filename

Now look at the code where the first slash occurs, it's

    if (strip_suffix(remoteurl, "/", &len))
        remoteurl[len] = '\0';

The intention is to strip off the last character if it is a directory separator.
So in the unix world we want to keep "/tmp/testfile\" as it is,
whereas in Windows
we want to chop off the backslash (because there is no file with a
backslash allowed,
it is the directory separator?)

So what I think I am going to do for next round is something like

    static int has_same_dir_prefix(const char *str, const char **out)
    {
    #ifdef GIT_WINDOWS_NATIVE
        return skip_prefix(str, "./", out)
           || skip_prefix(str, ".\\", out);
    #else
        return skip_prefix(str, "./", out);
    #endif
    }

    static int has_upper_dir_prefix(const char *str, const char **out)
    {
    #ifdef GIT_WINDOWS_NATIVE
        return skip_prefix(str, "../", out)
           || skip_prefix(str, "..\\", out);
    #else
        return skip_prefix(str, "../", out);
    #endif
    }

in the submodule helper function, or alternatively in the wrapper.c ?
and then rely on these functions being accurate.

Would that approach make sense?

Thanks,
Stefan

and all of them with some or all backslashes replaced by forward slashes.

See also connect.c:url_is_local_not_ssh, which ensures that the first
example above is considered a local path with a drive letter, not a remote
ssh path.

quoted

This also improves the performance:
(Best out of 3) time ./t7400-submodule-basic.sh
Before:
real    0m9.575s
user    0m2.683s
sys     0m6.773s
After:
real    0m9.293s
user    0m2.691s
sys     0m6.549s

Signed-off-by: Stefan Beller <redacted>
---

  This applies on origin/master, and I'd carry as its own feature branch
  as I am nowhere near done with the groups feature after reading Jens
feedback.
  (It took me a while to identify this as a next best step.)

  Thanks,
  Stefan

  builtin/submodule--helper.c | 120
++++++++++++++++++++++++++++++++++++++++++++
  git-submodule.sh            |  81 ++----------------------------
  2 files changed, 124 insertions(+), 77 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..f48b5b5 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c

@@ -9,6 +9,125 @@
  #include "submodule-config.h"
  #include "string-list.h"
  #include "run-command.h"
+#include "remote.h"
+#include "refs.h"
+
+static const char *get_default_remote(void)
+{
+       char *dest = NULL;
+       unsigned char sha1[20];
+       int flag;
+       struct strbuf sb = STRBUF_INIT;
+       const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, &flag);
+
+       if (!refname)
+               die("No such ref: HEAD");
+
+       refname = shorten_unambiguous_ref(refname, 0);
+       strbuf_addf(&sb, "branch.%s.remote", refname);
+       if (git_config_get_string(sb.buf, &dest))
+               return "origin";
+       else
+               return xstrdup(dest);
+}
+
+/*
+ * The function takes at most 2 arguments. The first argument is the
+ * URL that navigates to the submodule origin repo. When relative, this

URL
+ * is relative to the superproject origin URL repo. The second up_path
+ * argument, if specified, is the relative path that navigates
+ * from the submodule working tree to the superproject working tree.
+ *
+ * The output of the function is the origin URL of the submodule.
+ *
+ * The output will either be an absolute URL or filesystem path (if the
+ * superproject origin URL is an absolute URL or filesystem path,
+ * respectively) or a relative file system path (if the superproject
+ * origin URL is a relative file system path).
+ *
+ * When the output is a relative file system path, the path is either
+ * relative to the submodule working tree, if up_path is specified, or to
+ * the superproject working tree otherwise.
+ */
+static const char *relative_url(const char *url, const char *up_path)
+{
+       int is_relative = 0;
+       size_t len;
+       char *remoteurl = NULL;
+       char *sep = "/";
+       const char *out;
+       struct strbuf sb = STRBUF_INIT;
+       const char *remote = get_default_remote();
+       strbuf_addf(&sb, "remote.%s.url", remote);
+
+       if (git_config_get_string(sb.buf, &remoteurl))
+               /* the repository is its own authoritative upstream */
+               remoteurl = xgetcwd();
+
+       if (strip_suffix(remoteurl, "/", &len))
+               remoteurl[len] = '\0';
+
+       if (strchr(remoteurl, ':') || skip_prefix(remoteurl, "/", &out))
+               is_relative = 0;
+       else if (skip_prefix(remoteurl, "./", &out) ||
+                   skip_prefix(remoteurl, "../", &out))
+               is_relative = 1;
+       else {
+               is_relative = 1;
+               strbuf_reset(&sb);
+               strbuf_addf(&sb, "./%s", remoteurl);
+               remoteurl = strbuf_detach(&sb, NULL);
+       }
+
+       while (url) {
+               if (skip_prefix(url, "../", &out)) {
+                       char *rfind;
+                       url = out;
+
+                       rfind = strrchr(remoteurl, '/');
+                       if (rfind)
+                               *rfind = '\0';
+                       else {
+                               rfind = strrchr(remoteurl, ':');
+                               if (rfind) {
+                                       *rfind = '\0';
+                                       sep = ":";
+                               } else {
+                                       if (is_relative || !strcmp(".",
remoteurl))
+                                               die(N_("cannot strip one
component off url '%s'"), remoteurl);
+                                       else
+                                               remoteurl = ".";
+                               }
+                       }
+               } else if (skip_prefix(url, "./", &out))
+                       url = out;
+               else
+                       break;
+       }
+       strbuf_reset(&sb);
+       strbuf_addf(&sb, "%s%s%s", remoteurl, sep, url);
+
+       if (!skip_prefix(sb.buf, "./", &out))
+               out = sb.buf;
+       out = xstrdup(out);
+
+       strbuf_reset(&sb);
+       strbuf_addf(&sb, "%s%s", is_relative && up_path ? up_path : "",
out);
+
+       free((char*)out);
+       return strbuf_detach(&sb, NULL);
+}
+
+static int resolve_relative_url(int argc, const char **argv, const char
*prefix)
+{
+       if (argc == 2)
+               printf("%s\n", relative_url(argv[1], NULL));
+       else if (argc == 3)
+               printf("%s\n", relative_url(argv[1], argv[2]));
+       else
+               die("BUG: resolve_relative_url only accepts one or two
arguments");
+       return 0;
+}

  struct module_list {
        const struct cache_entry **entries;

@@ -264,6 +383,7 @@ static struct cmd_struct commands[] = {
        {"list", module_list},
        {"name", module_name},
        {"clone", module_clone},
+       {"resolve_relative_url", resolve_relative_url},
  };

  int cmd_submodule__helper(int argc, const char **argv, const char

*prefix)

diff --git a/git-submodule.sh b/git-submodule.sh
index 9bc5c5f..6a7a3e4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -46,79 +46,6 @@ prefix=
  custom_name=
  depth=

-# The function takes at most 2 arguments. The first argument is the
-# URL that navigates to the submodule origin repo. When relative, this

URL
-# is relative to the superproject origin URL repo. The second up_path
-# argument, if specified, is the relative path that navigates
-# from the submodule working tree to the superproject working tree.
-#
-# The output of the function is the origin URL of the submodule.
-#
-# The output will either be an absolute URL or filesystem path (if the
-# superproject origin URL is an absolute URL or filesystem path,
-# respectively) or a relative file system path (if the superproject
-# origin URL is a relative file system path).
-#
-# When the output is a relative file system path, the path is either
-# relative to the submodule working tree, if up_path is specified, or to
-# the superproject working tree otherwise.
-resolve_relative_url ()
-{
-       remote=$(get_default_remote)
-       remoteurl=$(git config "remote.$remote.url") ||
-               remoteurl=$(pwd) # the repository is its own authoritative
upstream
-       url="$1"
-       remoteurl=${remoteurl%/}
-       sep=/
-       up_path="$2"
-
-       case "$remoteurl" in
-       *:*|/*)
-               is_relative=
-               ;;
-       ./*|../*)
-               is_relative=t
-               ;;
-       *)
-               is_relative=t
-               remoteurl="./$remoteurl"
-               ;;
-       esac
-
-       while test -n "$url"
-       do
-               case "$url" in
-               ../*)
-                       url="${url#../}"
-                       case "$remoteurl" in
-                       */*)
-                               remoteurl="${remoteurl%/*}"
-                               ;;
-                       *:*)
-                               remoteurl="${remoteurl%:*}"
-                               sep=:
-                               ;;
-                       *)
-                               if test -z "$is_relative" || test "." =
"$remoteurl"
-                               then
-                                       die "$(eval_gettext "cannot strip
one component off url '\$remoteurl'")"
-                               else
-                                       remoteurl=.
-                               fi
-                               ;;
-                       esac
-                       ;;
-               ./*)
-                       url="${url#./}"
-                       ;;
-               *)
-                       break;;
-               esac
-       done
-       remoteurl="$remoteurl$sep${url%/}"
-       echo "${is_relative:+${up_path}}${remoteurl#./}"
-}
-
  # Resolve a path to be relative to another path.  This is intended for
  # converting submodule paths when git-submodule is run in a subdirectory
  # and only handles paths where the directory separator is '/'.

@@ -281,7 +208,7 @@ cmd_add()
                die "$(gettext "Relative path can only be used from the

toplevel of the working tree")"

                # dereference source url relative to parent's url
-               realrepo=$(resolve_relative_url "$repo") || exit
+               realrepo=$(git submodule--helper resolve_relative_url
"$repo") || exit
                ;;
        *:*|/*)
                # absolute url

@@ -485,7 +412,7 @@ cmd_init()
                        # Possibly a url relative to parent
                        case "$url" in
                        ./*|../*)
-                               url=$(resolve_relative_url "$url") || exit
+                               url=$(git submodule--helper

resolve_relative_url "$url") || exit
                                ;;
                        esac
                        git config submodule."$name".url "$url" ||

@@ -1190,9 +1117,9 @@ cmd_sync()
                        # guarantee a trailing /
                        up_path=${up_path%/}/ &&
                        # path from submodule work tree to submodule

origin repo
-                       sub_origin_url=$(resolve_relative_url "$url"
"$up_path") &&
+                       sub_origin_url=$(git submodule--helper
resolve_relative_url "$url" "$up_path") &&
                        # path from superproject work tree to submodule
origin repo
-                       super_config_url=$(resolve_relative_url "$url") ||
exit
+                       super_config_url=$(git submodule--helper
resolve_relative_url "$url") || exit
                        ;;
                *)
                        sub_origin_url="$url"

A new version of the patch, which spells out more its intent and
may actually work in Windows.

Any comment welcome,
Thanks,
Stefan

Stefan Beller (1):
  submodule: Port resolve_relative_url from shell to C

 builtin/submodule--helper.c | 151 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  81 ++----------------------
 2 files changed, 155 insertions(+), 77 deletions(-)

interdiff to previous version:

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f48b5b5..b925bed 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c

@@ -31,6 +31,36 @@ static const char *get_default_remote(void)
 		return xstrdup(dest);
 }
 
+static int has_same_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+	return skip_prefix(str, "./", out)
+		|| skip_prefix(str, ".\\", out);
+#else
+	return skip_prefix(str, "./", out);
+#endif
+}
+
+static int has_upper_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+	return skip_prefix(str, "../", out)
+		|| skip_prefix(str, "..\\", out);
+#else
+	return skip_prefix(str, "../", out);
+#endif
+}
+
+static char *last_dir_separator(const char *str)
+{
+#ifdef GIT_WINDOWS_NATIVE
+	return strrchr(str, "/")
+		|| strrchr(str, "\\");
+#else
+	return strrchr(str, '/');
+#endif
+}
+
 /*
  * The function takes at most 2 arguments. The first argument is the
  * URL that navigates to the submodule origin repo. When relative, this URL

@@ -64,13 +94,14 @@ static const char *relative_url(const char *url, const char *up_path)
 		/* the repository is its own authoritative upstream */
 		remoteurl = xgetcwd();
 
-	if (strip_suffix(remoteurl, "/", &len))
+	len = strlen(remoteurl);
+	if (is_dir_sep(remoteurl[len]))
 		remoteurl[len] = '\0';
 
-	if (strchr(remoteurl, ':') || skip_prefix(remoteurl, "/", &out))
+	if (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))
 		is_relative = 0;
-	else if (skip_prefix(remoteurl, "./", &out) ||
-		    skip_prefix(remoteurl, "../", &out))
+	else if (has_same_dir_prefix(remoteurl, &out) ||
+		    has_upper_dir_prefix(remoteurl, &out))
 		is_relative = 1;
 	else {
 		is_relative = 1;

@@ -80,11 +111,11 @@ static const char *relative_url(const char *url, const char *up_path)
 	}
 
 	while (url) {
-		if (skip_prefix(url, "../", &out)) {
+		if (has_upper_dir_prefix(url, &out)) {
 			char *rfind;
 			url = out;
 
-			rfind = strrchr(remoteurl, '/');
+			rfind = last_dir_separator(remoteurl);
 			if (rfind)
 				*rfind = '\0';
 			else {

@@ -99,7 +130,7 @@ static const char *relative_url(const char *url, const char *up_path)
 						remoteurl = ".";
 				}
 			}
-		} else if (skip_prefix(url, "./", &out))
+		} else if (has_same_dir_prefix(url, &out))
 			url = out;
 		else
 			break;

@@ -107,7 +138,7 @@ static const char *relative_url(const char *url, const char *up_path)
 	strbuf_reset(&sb);
 	strbuf_addf(&sb, "%s%s%s", remoteurl, sep, url);
 
-	if (!skip_prefix(sb.buf, "./", &out))
+	if (!has_same_dir_prefix(sb.buf, &out))
 		out = sb.buf;
 	out = xstrdup(out);
 

-- 
2.7.0.rc1.2.gfc39790.dirty

This reimplements the helper function `resolve_relative_url` in shell
in C. This functionality is needed in C for introducing the groups
feature later on. When using groups, the user should not need to run
`git submodule init`, but it should be implicit at all appropriate places,
which are all in C code. As the we would not just call out to `git
submodule init`, but do a more fine grained structure there, we actually
need all the init functionality in C before attempting the groups
feature. To get the init functionality in C, rewriting the
resolve_relative_url subfunction is a major step.

This also improves the performance:
(Best out of 3) time ./t7400-submodule-basic.sh
Before:
real	0m9.575s
user	0m2.683s
sys	0m6.773s
After:
real	0m9.293s
user	0m2.691s
sys	0m6.549s

That's about 3%.

Signed-off-by: Stefan Beller <redacted>
---
 builtin/submodule--helper.c | 151 ++++++++++++++++++++++++++++++++++++++++++++
 git-submodule.sh            |  81 ++----------------------
 2 files changed, 155 insertions(+), 77 deletions(-)

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index f4c3eff..b925bed 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c

@@ -9,6 +9,156 @@
 #include "submodule-config.h"
 #include "string-list.h"
 #include "run-command.h"
+#include "remote.h"
+#include "refs.h"
+
+static const char *get_default_remote(void)
+{
+	char *dest = NULL;
+	unsigned char sha1[20];
+	int flag;
+	struct strbuf sb = STRBUF_INIT;
+	const char *refname = resolve_ref_unsafe("HEAD", 0, sha1, &flag);
+
+	if (!refname)
+		die("No such ref: HEAD");
+
+	refname = shorten_unambiguous_ref(refname, 0);
+	strbuf_addf(&sb, "branch.%s.remote", refname);
+	if (git_config_get_string(sb.buf, &dest))
+		return "origin";
+	else
+		return xstrdup(dest);
+}
+
+static int has_same_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+	return skip_prefix(str, "./", out)
+		|| skip_prefix(str, ".\\", out);
+#else
+	return skip_prefix(str, "./", out);
+#endif
+}
+
+static int has_upper_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+	return skip_prefix(str, "../", out)
+		|| skip_prefix(str, "..\\", out);
+#else
+	return skip_prefix(str, "../", out);
+#endif
+}
+
+static char *last_dir_separator(const char *str)
+{
+#ifdef GIT_WINDOWS_NATIVE
+	return strrchr(str, "/")
+		|| strrchr(str, "\\");
+#else
+	return strrchr(str, '/');
+#endif
+}
+
+/*
+ * The function takes at most 2 arguments. The first argument is the
+ * URL that navigates to the submodule origin repo. When relative, this URL
+ * is relative to the superproject origin URL repo. The second up_path
+ * argument, if specified, is the relative path that navigates
+ * from the submodule working tree to the superproject working tree.
+ *
+ * The output of the function is the origin URL of the submodule.
+ *
+ * The output will either be an absolute URL or filesystem path (if the
+ * superproject origin URL is an absolute URL or filesystem path,
+ * respectively) or a relative file system path (if the superproject
+ * origin URL is a relative file system path).
+ *
+ * When the output is a relative file system path, the path is either
+ * relative to the submodule working tree, if up_path is specified, or to
+ * the superproject working tree otherwise.
+ */
+static const char *relative_url(const char *url, const char *up_path)
+{
+	int is_relative = 0;
+	size_t len;
+	char *remoteurl = NULL;
+	char *sep = "/";
+	const char *out;
+	struct strbuf sb = STRBUF_INIT;
+	const char *remote = get_default_remote();
+	strbuf_addf(&sb, "remote.%s.url", remote);
+
+	if (git_config_get_string(sb.buf, &remoteurl))
+		/* the repository is its own authoritative upstream */
+		remoteurl = xgetcwd();
+
+	len = strlen(remoteurl);
+	if (is_dir_sep(remoteurl[len]))
+		remoteurl[len] = '\0';
+
+	if (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))
+		is_relative = 0;
+	else if (has_same_dir_prefix(remoteurl, &out) ||
+		    has_upper_dir_prefix(remoteurl, &out))
+		is_relative = 1;
+	else {
+		is_relative = 1;
+		strbuf_reset(&sb);
+		strbuf_addf(&sb, "./%s", remoteurl);
+		remoteurl = strbuf_detach(&sb, NULL);
+	}
+
+	while (url) {
+		if (has_upper_dir_prefix(url, &out)) {
+			char *rfind;
+			url = out;
+
+			rfind = last_dir_separator(remoteurl);
+			if (rfind)
+				*rfind = '\0';
+			else {
+				rfind = strrchr(remoteurl, ':');
+				if (rfind) {
+					*rfind = '\0';
+					sep = ":";
+				} else {
+					if (is_relative || !strcmp(".", remoteurl))
+						die(N_("cannot strip one component off url '%s'"), remoteurl);
+					else
+						remoteurl = ".";
+				}
+			}
+		} else if (has_same_dir_prefix(url, &out))
+			url = out;
+		else
+			break;
+	}
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s%s%s", remoteurl, sep, url);
+
+	if (!has_same_dir_prefix(sb.buf, &out))
+		out = sb.buf;
+	out = xstrdup(out);
+
+	strbuf_reset(&sb);
+	strbuf_addf(&sb, "%s%s", is_relative && up_path ? up_path : "", out);
+
+	free((char*)out);
+	return strbuf_detach(&sb, NULL);
+}
+
+static int resolve_relative_url(int argc, const char **argv, const char *prefix)
+{
+	if (argc == 2)
+		printf("%s\n", relative_url(argv[1], NULL));
+	else if (argc == 3)
+		printf("%s\n", relative_url(argv[1], argv[2]));
+	else
+		die("BUG: resolve_relative_url only accepts one or two arguments");
+	return 0;
+}
 
 struct module_list {
 	const struct cache_entry **entries;

@@ -264,6 +414,7 @@ static struct cmd_struct commands[] = {
 	{"list", module_list},
 	{"name", module_name},
 	{"clone", module_clone},
+	{"resolve_relative_url", resolve_relative_url},
 };
 
 int cmd_submodule__helper(int argc, const char **argv, const char *prefix)

diff --git a/git-submodule.sh b/git-submodule.sh
index 9bc5c5f..6a7a3e4 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh

@@ -46,79 +46,6 @@ prefix=
 custom_name=
 depth=
 
-# The function takes at most 2 arguments. The first argument is the
-# URL that navigates to the submodule origin repo. When relative, this URL
-# is relative to the superproject origin URL repo. The second up_path
-# argument, if specified, is the relative path that navigates
-# from the submodule working tree to the superproject working tree.
-#
-# The output of the function is the origin URL of the submodule.
-#
-# The output will either be an absolute URL or filesystem path (if the
-# superproject origin URL is an absolute URL or filesystem path,
-# respectively) or a relative file system path (if the superproject
-# origin URL is a relative file system path).
-#
-# When the output is a relative file system path, the path is either
-# relative to the submodule working tree, if up_path is specified, or to
-# the superproject working tree otherwise.
-resolve_relative_url ()
-{
-	remote=$(get_default_remote)
-	remoteurl=$(git config "remote.$remote.url") ||
-		remoteurl=$(pwd) # the repository is its own authoritative upstream
-	url="$1"
-	remoteurl=${remoteurl%/}
-	sep=/
-	up_path="$2"
-
-	case "$remoteurl" in
-	*:*|/*)
-		is_relative=
-		;;
-	./*|../*)
-		is_relative=t
-		;;
-	*)
-		is_relative=t
-		remoteurl="./$remoteurl"
-		;;
-	esac
-
-	while test -n "$url"
-	do
-		case "$url" in
-		../*)
-			url="${url#../}"
-			case "$remoteurl" in
-			*/*)
-				remoteurl="${remoteurl%/*}"
-				;;
-			*:*)
-				remoteurl="${remoteurl%:*}"
-				sep=:
-				;;
-			*)
-				if test -z "$is_relative" || test "." = "$remoteurl"
-				then
-					die "$(eval_gettext "cannot strip one component off url '\$remoteurl'")"
-				else
-					remoteurl=.
-				fi
-				;;
-			esac
-			;;
-		./*)
-			url="${url#./}"
-			;;
-		*)
-			break;;
-		esac
-	done
-	remoteurl="$remoteurl$sep${url%/}"
-	echo "${is_relative:+${up_path}}${remoteurl#./}"
-}
-
 # Resolve a path to be relative to another path.  This is intended for
 # converting submodule paths when git-submodule is run in a subdirectory
 # and only handles paths where the directory separator is '/'.

@@ -281,7 +208,7 @@ cmd_add()
 		die "$(gettext "Relative path can only be used from the toplevel of the working tree")"
 
 		# dereference source url relative to parent's url
-		realrepo=$(resolve_relative_url "$repo") || exit
+		realrepo=$(git submodule--helper resolve_relative_url "$repo") || exit
 		;;
 	*:*|/*)
 		# absolute url

@@ -485,7 +412,7 @@ cmd_init()
 			# Possibly a url relative to parent
 			case "$url" in
 			./*|../*)
-				url=$(resolve_relative_url "$url") || exit
+				url=$(git submodule--helper resolve_relative_url "$url") || exit
 				;;
 			esac
 			git config submodule."$name".url "$url" ||

@@ -1190,9 +1117,9 @@ cmd_sync()
 			# guarantee a trailing /
 			up_path=${up_path%/}/ &&
 			# path from submodule work tree to submodule origin repo
-			sub_origin_url=$(resolve_relative_url "$url" "$up_path") &&
+			sub_origin_url=$(git submodule--helper resolve_relative_url "$url" "$up_path") &&
 			# path from superproject work tree to submodule origin repo
-			super_config_url=$(resolve_relative_url "$url") || exit
+			super_config_url=$(git submodule--helper resolve_relative_url "$url") || exit
 			;;
 		*)
 			sub_origin_url="$url"

-- 
2.7.0.rc1.2.gfc39790.dirty

On 17.12.15 01:26, Stefan Beller wrote:

Should that be
if defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)

A rather superficial review...

On Wed, Dec 16, 2015 at 7:26 PM, Stefan Beller [off-list ref] wrote:

This reimplements the helper function `resolve_relative_url` in shell

s/This reimplements/Reimplement/

in C. This functionality is needed in C for introducing the groups
feature later on. When using groups, the user should not need to run
`git submodule init`, but it should be implicit at all appropriate places,
which are all in C code. As the we would not just call out to `git

I guess you mean "As then we..." or something?

Leaking the strbuf at both returns.

And, leaking the strdup'd dest (in the caller), but I suppose that's
intentional?

+}
+
+static int has_same_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+       return skip_prefix(str, "./", out)
+               || skip_prefix(str, ".\\", out);
+#else
+       return skip_prefix(str, "./", out);
+#endif
+}
+
+static int has_upper_dir_prefix(const char *str, const char **out)
+{
+#ifdef GIT_WINDOWS_NATIVE
+       return skip_prefix(str, "../", out)
+               || skip_prefix(str, "..\\", out);
+#else
+       return skip_prefix(str, "../", out);
+#endif
+}
+
+static char *last_dir_separator(const char *str)
+{
+#ifdef GIT_WINDOWS_NATIVE
+       return strrchr(str, "/")
+               || strrchr(str, "\\");
+#else
+       return strrchr(str, '/');
+#endif
+}

Cleaner would be to move the #if's outside the functions:

    #ifdef GIT_WINDOWS_NATIVE

    /* Windows implementations */
    static int has_same_dir_prefix(...) {...}
    static int has_upper_dir_prefix(...) {...}
    static char *last_dir_separator(...) {...}

    #else

    /* POSIX implementations */
    static int has_same_dir_prefix(...) {...}
    static int has_upper_dir_prefix(...) {...}
    static char *last_dir_separator(...) {...}

    #endif

+static const char *relative_url(const char *url, const char *up_path)
+{
+       int is_relative = 0;
+       size_t len;
+       char *remoteurl = NULL;
+       char *sep = "/";

'sep' only ever holds a single character, so why not declare it 'char'
rather than 'char *'? (And, adjust the format string of strbuf_addf(),
of course.)

+       const char *out;
+       [...]
+       while (url) {
+               if (has_upper_dir_prefix(url, &out)) {
+                       [...]
+                       if (rfind)
+                               *rfind = '\0';
+                       else {
+                               rfind = strrchr(remoteurl, ':');
+                               if (rfind) {
+                                       *rfind = '\0';
+                                       sep = ":";
+                               } else {
+                                       [...]
+                               }
+                       }
+               } else if (has_same_dir_prefix(url, &out))
+                       url = out;
+               else
+                       break;
+       }
+       strbuf_reset(&sb);
+       strbuf_addf(&sb, "%s%s%s", remoteurl, sep, url);
+
+       if (!has_same_dir_prefix(sb.buf, &out))
+               out = sb.buf;
+       out = xstrdup(out);
+
+       strbuf_reset(&sb);
+       strbuf_addf(&sb, "%s%s", is_relative && up_path ? up_path : "", out);
+
+       free((char*)out);

Why declare 'out' const if you know you will be freeing it?