{"thread":{"id":"40963","subject":"[PATCH] submodule: Port resolve_relative_url from shell to C","startedAt":"2015-12-10T01:07:34Z","lastAt":"2016-01-11T20:17:34Z","messageCount":11,"participants":["Stefan Beller","Johannes Sixt","Torsten Bögershausen","Eric Sunshine","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"274223","messageId":"1449709654-30189-1-git-send-email-sbeller@google.com","threadId":"40963","inReplyTo":null,"subject":"[PATCH] submodule: Port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-10T01:07:34Z","receivedAt":"2015-12-10T01:07:34Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This reimplements the helper function `resolve_relative_url` in shell\nin C. This functionality is needed in C for introducing the groups\nfeature later on. When using groups, the user should not need to run\n`git submodule init`, but it should be implicit at all appropriate places,\nwhich are all in C code. As the we would not just call out to `git\nsubmodule init`, but do a more fine grained structure there, we actually\nneed all the init functionality in C before attempting the groups\nfeature. To get the init functionality in C, rewriting the\nresolve_relative_url subfunction is a major step.\n\nThis also improves the performance:\n(Best out of 3) time ./t7400-submodule-basic.sh\nBefore:\nreal\t0m9.575s\nuser\t0m2.683s\nsys\t0m6.773s\nAfter:\nreal\t0m9.293s\nuser\t0m2.691s\nsys\t0m6.549s\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n\n This applies on origin/master, and I'd carry as its own feature branch\n as I am nowhere near done with the groups feature after reading Jens feedback.\n (It took me a while to identify this as a next best step.)\n \n Thanks,\n Stefan\n\n builtin/submodule--helper.c | 120 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  81 ++----------------------------\n 2 files changed, 124 insertions(+), 77 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff..f48b5b5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -9,6 +9,125 @@\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n+#include \"refs.h\"\n+\n+static const char *get_default_remote(void)\n+{\n+\tchar *dest = NULL;\n+\tunsigned char sha1[20];\n+\tint flag;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, &flag);\n+\n+\tif (!refname)\n+\t\tdie(\"No such ref: HEAD\");\n+\n+\trefname = shorten_unambiguous_ref(refname, 0);\n+\tstrbuf_addf(&sb, \"branch.%s.remote\", refname);\n+\tif (git_config_get_string(sb.buf, &dest))\n+\t\treturn \"origin\";\n+\telse\n+\t\treturn xstrdup(dest);\n+}\n+\n+/*\n+ * The function takes at most 2 arguments. The first argument is the\n+ * URL that navigates to the submodule origin repo. When relative, this URL\n+ * is relative to the superproject origin URL repo. The second up_path\n+ * argument, if specified, is the relative path that navigates\n+ * from the submodule working tree to the superproject working tree.\n+ *\n+ * The output of the function is the origin URL of the submodule.\n+ *\n+ * The output will either be an absolute URL or filesystem path (if the\n+ * superproject origin URL is an absolute URL or filesystem path,\n+ * respectively) or a relative file system path (if the superproject\n+ * origin URL is a relative file system path).\n+ *\n+ * When the output is a relative file system path, the path is either\n+ * relative to the submodule working tree, if up_path is specified, or to\n+ * the superproject working tree otherwise.\n+ */\n+static const char *relative_url(const char *url, const char *up_path)\n+{\n+\tint is_relative = 0;\n+\tsize_t len;\n+\tchar *remoteurl = NULL;\n+\tchar *sep = \"/\";\n+\tconst char *out;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *remote = get_default_remote();\n+\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\n+\tif (git_config_get_string(sb.buf, &remoteurl))\n+\t\t/* the repository is its own authoritative upstream */\n+\t\tremoteurl = xgetcwd();\n+\n+\tif (strip_suffix(remoteurl, \"/\", &len))\n+\t\tremoteurl[len] = '\\0';\n+\n+\tif (strchr(remoteurl, ':') || skip_prefix(remoteurl, \"/\", &out))\n+\t\tis_relative = 0;\n+\telse if (skip_prefix(remoteurl, \"./\", &out) ||\n+\t\t    skip_prefix(remoteurl, \"../\", &out))\n+\t\tis_relative = 1;\n+\telse {\n+\t\tis_relative = 1;\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n+\t\tremoteurl = strbuf_detach(&sb, NULL);\n+\t}\n+\n+\twhile (url) {\n+\t\tif (skip_prefix(url, \"../\", &out)) {\n+\t\t\tchar *rfind;\n+\t\t\turl = out;\n+\n+\t\t\trfind = strrchr(remoteurl, '/');\n+\t\t\tif (rfind)\n+\t\t\t\t*rfind = '\\0';\n+\t\t\telse {\n+\t\t\t\trfind = strrchr(remoteurl, ':');\n+\t\t\t\tif (rfind) {\n+\t\t\t\t\t*rfind = '\\0';\n+\t\t\t\t\tsep = \":\";\n+\t\t\t\t} else {\n+\t\t\t\t\tif (is_relative || !strcmp(\".\", remoteurl))\n+\t\t\t\t\t\tdie(N_(\"cannot strip one component off url '%s'\"), remoteurl);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tremoteurl = \".\";\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} else if (skip_prefix(url, \"./\", &out))\n+\t\t\turl = out;\n+\t\telse\n+\t\t\tbreak;\n+\t}\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n+\n+\tif (!skip_prefix(sb.buf, \"./\", &out))\n+\t\tout = sb.buf;\n+\tout = xstrdup(out);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s\", is_relative && up_path ? up_path : \"\", out);\n+\n+\tfree((char*)out);\n+\treturn strbuf_detach(&sb, NULL);\n+}\n+\n+static int resolve_relative_url(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc == 2)\n+\t\tprintf(\"%s\\n\", relative_url(argv[1], NULL));\n+\telse if (argc == 3)\n+\t\tprintf(\"%s\\n\", relative_url(argv[1], argv[2]));\n+\telse\n+\t\tdie(\"BUG: resolve_relative_url only accepts one or two arguments\");\n+\treturn 0;\n+}\n \n struct module_list {\n \tconst struct cache_entry **entries;\n@@ -264,6 +383,7 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list},\n \t{\"name\", module_name},\n \t{\"clone\", module_clone},\n+\t{\"resolve_relative_url\", resolve_relative_url},\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..6a7a3e4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -46,79 +46,6 @@ prefix=\n custom_name=\n depth=\n \n-# The function takes at most 2 arguments. The first argument is the\n-# URL that navigates to the submodule origin repo. When relative, this URL\n-# is relative to the superproject origin URL repo. The second up_path\n-# argument, if specified, is the relative path that navigates\n-# from the submodule working tree to the superproject working tree.\n-#\n-# The output of the function is the origin URL of the submodule.\n-#\n-# The output will either be an absolute URL or filesystem path (if the\n-# superproject origin URL is an absolute URL or filesystem path,\n-# respectively) or a relative file system path (if the superproject\n-# origin URL is a relative file system path).\n-#\n-# When the output is a relative file system path, the path is either\n-# relative to the submodule working tree, if up_path is specified, or to\n-# the superproject working tree otherwise.\n-resolve_relative_url ()\n-{\n-\tremote=$(get_default_remote)\n-\tremoteurl=$(git config \"remote.$remote.url\") ||\n-\t\tremoteurl=$(pwd) # the repository is its own authoritative upstream\n-\turl=\"$1\"\n-\tremoteurl=${remoteurl%/}\n-\tsep=/\n-\tup_path=\"$2\"\n-\n-\tcase \"$remoteurl\" in\n-\t*:*|/*)\n-\t\tis_relative=\n-\t\t;;\n-\t./*|../*)\n-\t\tis_relative=t\n-\t\t;;\n-\t*)\n-\t\tis_relative=t\n-\t\tremoteurl=\"./$remoteurl\"\n-\t\t;;\n-\tesac\n-\n-\twhile test -n \"$url\"\n-\tdo\n-\t\tcase \"$url\" in\n-\t\t../*)\n-\t\t\turl=\"${url#../}\"\n-\t\t\tcase \"$remoteurl\" in\n-\t\t\t*/*)\n-\t\t\t\tremoteurl=\"${remoteurl%/*}\"\n-\t\t\t\t;;\n-\t\t\t*:*)\n-\t\t\t\tremoteurl=\"${remoteurl%:*}\"\n-\t\t\t\tsep=:\n-\t\t\t\t;;\n-\t\t\t*)\n-\t\t\t\tif test -z \"$is_relative\" || test \".\" = \"$remoteurl\"\n-\t\t\t\tthen\n-\t\t\t\t\tdie \"$(eval_gettext \"cannot strip one component off url '\\$remoteurl'\")\"\n-\t\t\t\telse\n-\t\t\t\t\tremoteurl=.\n-\t\t\t\tfi\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\t;;\n-\t\t./*)\n-\t\t\turl=\"${url#./}\"\n-\t\t\t;;\n-\t\t*)\n-\t\t\tbreak;;\n-\t\tesac\n-\tdone\n-\tremoteurl=\"$remoteurl$sep${url%/}\"\n-\techo \"${is_relative:+${up_path}}${remoteurl#./}\"\n-}\n-\n # Resolve a path to be relative to another path.  This is intended for\n # converting submodule paths when git-submodule is run in a subdirectory\n # and only handles paths where the directory separator is '/'.\n@@ -281,7 +208,7 @@ cmd_add()\n \t\tdie \"$(gettext \"Relative path can only be used from the toplevel of the working tree\")\"\n \n \t\t# dereference source url relative to parent's url\n-\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n+\t\trealrepo=$(git submodule--helper resolve_relative_url \"$repo\") || exit\n \t\t;;\n \t*:*|/*)\n \t\t# absolute url\n@@ -485,7 +412,7 @@ cmd_init()\n \t\t\t# Possibly a url relative to parent\n \t\t\tcase \"$url\" in\n \t\t\t./*|../*)\n-\t\t\t\turl=$(resolve_relative_url \"$url\") || exit\n+\t\t\t\turl=$(git submodule--helper resolve_relative_url \"$url\") || exit\n \t\t\t\t;;\n \t\t\tesac\n \t\t\tgit config submodule.\"$name\".url \"$url\" ||\n@@ -1190,9 +1117,9 @@ cmd_sync()\n \t\t\t# guarantee a trailing /\n \t\t\tup_path=${up_path%/}/ &&\n \t\t\t# path from submodule work tree to submodule origin repo\n-\t\t\tsub_origin_url=$(resolve_relative_url \"$url\" \"$up_path\") &&\n+\t\t\tsub_origin_url=$(git submodule--helper resolve_relative_url \"$url\" \"$up_path\") &&\n \t\t\t# path from superproject work tree to submodule origin repo\n-\t\t\tsuper_config_url=$(resolve_relative_url \"$url\") || exit\n+\t\t\tsuper_config_url=$(git submodule--helper resolve_relative_url \"$url\") || exit\n \t\t\t;;\n \t\t*)\n \t\t\tsub_origin_url=\"$url\"\n-- \n2.6.3.470.g4b82c23.dirty\n"},{"id":"274227","messageId":"5669202C.2090100@kdbg.org","threadId":"40963","inReplyTo":"1449709654-30189-1-git-send-email-sbeller@google.com","subject":"Re: [PATCH] submodule: Port resolve_relative_url from shell to C","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-12-10T06:48:12Z","receivedAt":"2015-12-10T06:48:12Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 10.12.2015 um 02:07 schrieb Stefan Beller:\n> This reimplements the helper function `resolve_relative_url` in shell\n> in C. This functionality is needed in C for introducing the groups\n> feature later on. When using groups, the user should not need to run\n> `git submodule init`, but it should be implicit at all appropriate places,\n> which are all in C code. As the we would not just call out to `git\n> submodule init`, but do a more fine grained structure there, we actually\n> need all the init functionality in C before attempting the groups\n> feature. To get the init functionality in C, rewriting the\n> resolve_relative_url subfunction is a major step.\n\nI see lots of '/', but no is_dir_sep() in the C version. Did you \nconsider that local URLs can use a backslash as path separator on \nWindows? In the shell version, this did not matter because bash converts \nthe backslashes to forward slashes for us. But when rewritten in C, this \ndoes not happen.\n\nValid URLs are\n\n   D:\\foo\\bar.git\n   \\\\server\\share\\foo\\bar\n   ..\\..\\foo\\bar\n\nand all of them with some or all backslashes replaced by forward slashes.\n\nSee also connect.c:url_is_local_not_ssh, which ensures that the first \nexample above is considered a local path with a drive letter, not a \nremote ssh path.\n\n>\n> This also improves the performance:\n> (Best out of 3) time ./t7400-submodule-basic.sh\n> Before:\n> real\t0m9.575s\n> user\t0m2.683s\n> sys\t0m6.773s\n> After:\n> real\t0m9.293s\n> user\t0m2.691s\n> sys\t0m6.549s\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>\n>   This applies on origin/master, and I'd carry as its own feature branch\n>   as I am nowhere near done with the groups feature after reading Jens feedback.\n>   (It took me a while to identify this as a next best step.)\n>\n>   Thanks,\n>   Stefan\n>\n>   builtin/submodule--helper.c | 120 ++++++++++++++++++++++++++++++++++++++++++++\n>   git-submodule.sh            |  81 ++----------------------------\n>   2 files changed, 124 insertions(+), 77 deletions(-)\n>\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index f4c3eff..f48b5b5 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -9,6 +9,125 @@\n>   #include \"submodule-config.h\"\n>   #include \"string-list.h\"\n>   #include \"run-command.h\"\n> +#include \"remote.h\"\n> +#include \"refs.h\"\n> +\n> +static const char *get_default_remote(void)\n> +{\n> +\tchar *dest = NULL;\n> +\tunsigned char sha1[20];\n> +\tint flag;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, &flag);\n> +\n> +\tif (!refname)\n> +\t\tdie(\"No such ref: HEAD\");\n> +\n> +\trefname = shorten_unambiguous_ref(refname, 0);\n> +\tstrbuf_addf(&sb, \"branch.%s.remote\", refname);\n> +\tif (git_config_get_string(sb.buf, &dest))\n> +\t\treturn \"origin\";\n> +\telse\n> +\t\treturn xstrdup(dest);\n> +}\n> +\n> +/*\n> + * The function takes at most 2 arguments. The first argument is the\n> + * URL that navigates to the submodule origin repo. When relative, this URL\n> + * is relative to the superproject origin URL repo. The second up_path\n> + * argument, if specified, is the relative path that navigates\n> + * from the submodule working tree to the superproject working tree.\n> + *\n> + * The output of the function is the origin URL of the submodule.\n> + *\n> + * The output will either be an absolute URL or filesystem path (if the\n> + * superproject origin URL is an absolute URL or filesystem path,\n> + * respectively) or a relative file system path (if the superproject\n> + * origin URL is a relative file system path).\n> + *\n> + * When the output is a relative file system path, the path is either\n> + * relative to the submodule working tree, if up_path is specified, or to\n> + * the superproject working tree otherwise.\n> + */\n> +static const char *relative_url(const char *url, const char *up_path)\n> +{\n> +\tint is_relative = 0;\n> +\tsize_t len;\n> +\tchar *remoteurl = NULL;\n> +\tchar *sep = \"/\";\n> +\tconst char *out;\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tconst char *remote = get_default_remote();\n> +\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n> +\n> +\tif (git_config_get_string(sb.buf, &remoteurl))\n> +\t\t/* the repository is its own authoritative upstream */\n> +\t\tremoteurl = xgetcwd();\n> +\n> +\tif (strip_suffix(remoteurl, \"/\", &len))\n> +\t\tremoteurl[len] = '\\0';\n> +\n> +\tif (strchr(remoteurl, ':') || skip_prefix(remoteurl, \"/\", &out))\n> +\t\tis_relative = 0;\n> +\telse if (skip_prefix(remoteurl, \"./\", &out) ||\n> +\t\t    skip_prefix(remoteurl, \"../\", &out))\n> +\t\tis_relative = 1;\n> +\telse {\n> +\t\tis_relative = 1;\n> +\t\tstrbuf_reset(&sb);\n> +\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n> +\t\tremoteurl = strbuf_detach(&sb, NULL);\n> +\t}\n> +\n> +\twhile (url) {\n> +\t\tif (skip_prefix(url, \"../\", &out)) {\n> +\t\t\tchar *rfind;\n> +\t\t\turl = out;\n> +\n> +\t\t\trfind = strrchr(remoteurl, '/');\n> +\t\t\tif (rfind)\n> +\t\t\t\t*rfind = '\\0';\n> +\t\t\telse {\n> +\t\t\t\trfind = strrchr(remoteurl, ':');\n> +\t\t\t\tif (rfind) {\n> +\t\t\t\t\t*rfind = '\\0';\n> +\t\t\t\t\tsep = \":\";\n> +\t\t\t\t} else {\n> +\t\t\t\t\tif (is_relative || !strcmp(\".\", remoteurl))\n> +\t\t\t\t\t\tdie(N_(\"cannot strip one component off url '%s'\"), remoteurl);\n> +\t\t\t\t\telse\n> +\t\t\t\t\t\tremoteurl = \".\";\n> +\t\t\t\t}\n> +\t\t\t}\n> +\t\t} else if (skip_prefix(url, \"./\", &out))\n> +\t\t\turl = out;\n> +\t\telse\n> +\t\t\tbreak;\n> +\t}\n> +\tstrbuf_reset(&sb);\n> +\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n> +\n> +\tif (!skip_prefix(sb.buf, \"./\", &out))\n> +\t\tout = sb.buf;\n> +\tout = xstrdup(out);\n> +\n> +\tstrbuf_reset(&sb);\n> +\tstrbuf_addf(&sb, \"%s%s\", is_relative && up_path ? up_path : \"\", out);\n> +\n> +\tfree((char*)out);\n> +\treturn strbuf_detach(&sb, NULL);\n> +}\n> +\n> +static int resolve_relative_url(int argc, const char **argv, const char *prefix)\n> +{\n> +\tif (argc == 2)\n> +\t\tprintf(\"%s\\n\", relative_url(argv[1], NULL));\n> +\telse if (argc == 3)\n> +\t\tprintf(\"%s\\n\", relative_url(argv[1], argv[2]));\n> +\telse\n> +\t\tdie(\"BUG: resolve_relative_url only accepts one or two arguments\");\n> +\treturn 0;\n> +}\n>\n>   struct module_list {\n>   \tconst struct cache_entry **entries;\n> @@ -264,6 +383,7 @@ static struct cmd_struct commands[] = {\n>   \t{\"list\", module_list},\n>   \t{\"name\", module_name},\n>   \t{\"clone\", module_clone},\n> +\t{\"resolve_relative_url\", resolve_relative_url},\n>   };\n>\n>   int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 9bc5c5f..6a7a3e4 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -46,79 +46,6 @@ prefix=\n>   custom_name=\n>   depth=\n>\n> -# The function takes at most 2 arguments. The first argument is the\n> -# URL that navigates to the submodule origin repo. When relative, this URL\n> -# is relative to the superproject origin URL repo. The second up_path\n> -# argument, if specified, is the relative path that navigates\n> -# from the submodule working tree to the superproject working tree.\n> -#\n> -# The output of the function is the origin URL of the submodule.\n> -#\n> -# The output will either be an absolute URL or filesystem path (if the\n> -# superproject origin URL is an absolute URL or filesystem path,\n> -# respectively) or a relative file system path (if the superproject\n> -# origin URL is a relative file system path).\n> -#\n> -# When the output is a relative file system path, the path is either\n> -# relative to the submodule working tree, if up_path is specified, or to\n> -# the superproject working tree otherwise.\n> -resolve_relative_url ()\n> -{\n> -\tremote=$(get_default_remote)\n> -\tremoteurl=$(git config \"remote.$remote.url\") ||\n> -\t\tremoteurl=$(pwd) # the repository is its own authoritative upstream\n> -\turl=\"$1\"\n> -\tremoteurl=${remoteurl%/}\n> -\tsep=/\n> -\tup_path=\"$2\"\n> -\n> -\tcase \"$remoteurl\" in\n> -\t*:*|/*)\n> -\t\tis_relative=\n> -\t\t;;\n> -\t./*|../*)\n> -\t\tis_relative=t\n> -\t\t;;\n> -\t*)\n> -\t\tis_relative=t\n> -\t\tremoteurl=\"./$remoteurl\"\n> -\t\t;;\n> -\tesac\n> -\n> -\twhile test -n \"$url\"\n> -\tdo\n> -\t\tcase \"$url\" in\n> -\t\t../*)\n> -\t\t\turl=\"${url#../}\"\n> -\t\t\tcase \"$remoteurl\" in\n> -\t\t\t*/*)\n> -\t\t\t\tremoteurl=\"${remoteurl%/*}\"\n> -\t\t\t\t;;\n> -\t\t\t*:*)\n> -\t\t\t\tremoteurl=\"${remoteurl%:*}\"\n> -\t\t\t\tsep=:\n> -\t\t\t\t;;\n> -\t\t\t*)\n> -\t\t\t\tif test -z \"$is_relative\" || test \".\" = \"$remoteurl\"\n> -\t\t\t\tthen\n> -\t\t\t\t\tdie \"$(eval_gettext \"cannot strip one component off url '\\$remoteurl'\")\"\n> -\t\t\t\telse\n> -\t\t\t\t\tremoteurl=.\n> -\t\t\t\tfi\n> -\t\t\t\t;;\n> -\t\t\tesac\n> -\t\t\t;;\n> -\t\t./*)\n> -\t\t\turl=\"${url#./}\"\n> -\t\t\t;;\n> -\t\t*)\n> -\t\t\tbreak;;\n> -\t\tesac\n> -\tdone\n> -\tremoteurl=\"$remoteurl$sep${url%/}\"\n> -\techo \"${is_relative:+${up_path}}${remoteurl#./}\"\n> -}\n> -\n>   # Resolve a path to be relative to another path.  This is intended for\n>   # converting submodule paths when git-submodule is run in a subdirectory\n>   # and only handles paths where the directory separator is '/'.\n> @@ -281,7 +208,7 @@ cmd_add()\n>   \t\tdie \"$(gettext \"Relative path can only be used from the toplevel of the working tree\")\"\n>\n>   \t\t# dereference source url relative to parent's url\n> -\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n> +\t\trealrepo=$(git submodule--helper resolve_relative_url \"$repo\") || exit\n>   \t\t;;\n>   \t*:*|/*)\n>   \t\t# absolute url\n> @@ -485,7 +412,7 @@ cmd_init()\n>   \t\t\t# Possibly a url relative to parent\n>   \t\t\tcase \"$url\" in\n>   \t\t\t./*|../*)\n> -\t\t\t\turl=$(resolve_relative_url \"$url\") || exit\n> +\t\t\t\turl=$(git submodule--helper resolve_relative_url \"$url\") || exit\n>   \t\t\t\t;;\n>   \t\t\tesac\n>   \t\t\tgit config submodule.\"$name\".url \"$url\" ||\n> @@ -1190,9 +1117,9 @@ cmd_sync()\n>   \t\t\t# guarantee a trailing /\n>   \t\t\tup_path=${up_path%/}/ &&\n>   \t\t\t# path from submodule work tree to submodule origin repo\n> -\t\t\tsub_origin_url=$(resolve_relative_url \"$url\" \"$up_path\") &&\n> +\t\t\tsub_origin_url=$(git submodule--helper resolve_relative_url \"$url\" \"$up_path\") &&\n>   \t\t\t# path from superproject work tree to submodule origin repo\n> -\t\t\tsuper_config_url=$(resolve_relative_url \"$url\") || exit\n> +\t\t\tsuper_config_url=$(git submodule--helper resolve_relative_url \"$url\") || exit\n>   \t\t\t;;\n>   \t\t*)\n>   \t\t\tsub_origin_url=\"$url\"\n>\n"},{"id":"274631","messageId":"CAGZ79kb07Sv7LzDZUCxFk1Cmxxk2cga5sN-pT_R1e9_4wSJirA@mail.gmail.com","threadId":"40963","inReplyTo":"5669202C.2090100@kdbg.org","subject":"Re: [PATCH] submodule: Port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-16T22:36:46Z","receivedAt":"2015-12-16T22:36:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Wed, Dec 9, 2015 at 10:48 PM, Johannes Sixt <j6t@kdbg.org> wrote:\n> Am 10.12.2015 um 02:07 schrieb Stefan Beller:\n>>\n>> This reimplements the helper function `resolve_relative_url` in shell\n>> in C. This functionality is needed in C for introducing the groups\n>> feature later on. When using groups, the user should not need to run\n>> `git submodule init`, but it should be implicit at all appropriate places,\n>> which are all in C code. As the we would not just call out to `git\n>> submodule init`, but do a more fine grained structure there, we actually\n>> need all the init functionality in C before attempting the groups\n>> feature. To get the init functionality in C, rewriting the\n>> resolve_relative_url subfunction is a major step.\n>\n>\n> I see lots of '/', but no is_dir_sep() in the C version. Did you consider\n> that local URLs can use a backslash as path separator on Windows? In the\n> shell version, this did not matter because bash converts the backslashes to\n> forward slashes for us. But when rewritten in C, this does not happen.\n\nI see. That's a pity. :(\n\n>\n> Valid URLs are\n>\n>   D:\\foo\\bar.git\n>   \\\\server\\share\\foo\\bar\n>   ..\\..\\foo\\bar\n\nI am staring at the code in desperation as backslashes\nin Linux are valid for file names, i.e.:\n\"/tmp/testfile\\\" is a valid filename\n\nNow look at the code where the first slash occurs, it's\n\n    if (strip_suffix(remoteurl, \"/\", &len))\n        remoteurl[len] = '\\0';\n\nThe intention is to strip off the last character if it is a directory separator.\nSo in the unix world we want to keep \"/tmp/testfile\\\" as it is,\nwhereas in Windows\nwe want to chop off the backslash (because there is no file with a\nbackslash allowed,\nit is the directory separator?)\n\nSo what I think I am going to do for next round is something like\n\n    static int has_same_dir_prefix(const char *str, const char **out)\n    {\n    #ifdef GIT_WINDOWS_NATIVE\n        return skip_prefix(str, \"./\", out)\n           || skip_prefix(str, \".\\\\\", out);\n    #else\n        return skip_prefix(str, \"./\", out);\n    #endif\n    }\n\n    static int has_upper_dir_prefix(const char *str, const char **out)\n    {\n    #ifdef GIT_WINDOWS_NATIVE\n        return skip_prefix(str, \"../\", out)\n           || skip_prefix(str, \"..\\\\\", out);\n    #else\n        return skip_prefix(str, \"../\", out);\n    #endif\n    }\n\nin the submodule helper function, or alternatively in the wrapper.c ?\nand then rely on these functions being accurate.\n\nWould that approach make sense?\n\nThanks,\nStefan\n\n\n>\n> and all of them with some or all backslashes replaced by forward slashes.\n>\n> See also connect.c:url_is_local_not_ssh, which ensures that the first\n> example above is considered a local path with a drive letter, not a remote\n> ssh path.\n>\n>\n>>\n>> This also improves the performance:\n>> (Best out of 3) time ./t7400-submodule-basic.sh\n>> Before:\n>> real    0m9.575s\n>> user    0m2.683s\n>> sys     0m6.773s\n>> After:\n>> real    0m9.293s\n>> user    0m2.691s\n>> sys     0m6.549s\n>>\n>> Signed-off-by: Stefan Beller <sbeller@google.com>\n>> ---\n>>\n>>   This applies on origin/master, and I'd carry as its own feature branch\n>>   as I am nowhere near done with the groups feature after reading Jens\n>> feedback.\n>>   (It took me a while to identify this as a next best step.)\n>>\n>>   Thanks,\n>>   Stefan\n>>\n>>   builtin/submodule--helper.c | 120\n>> ++++++++++++++++++++++++++++++++++++++++++++\n>>   git-submodule.sh            |  81 ++----------------------------\n>>   2 files changed, 124 insertions(+), 77 deletions(-)\n>>\n>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n>> index f4c3eff..f48b5b5 100644\n>> --- a/builtin/submodule--helper.c\n>> +++ b/builtin/submodule--helper.c\n>> @@ -9,6 +9,125 @@\n>>   #include \"submodule-config.h\"\n>>   #include \"string-list.h\"\n>>   #include \"run-command.h\"\n>> +#include \"remote.h\"\n>> +#include \"refs.h\"\n>> +\n>> +static const char *get_default_remote(void)\n>> +{\n>> +       char *dest = NULL;\n>> +       unsigned char sha1[20];\n>> +       int flag;\n>> +       struct strbuf sb = STRBUF_INIT;\n>> +       const char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, &flag);\n>> +\n>> +       if (!refname)\n>> +               die(\"No such ref: HEAD\");\n>> +\n>> +       refname = shorten_unambiguous_ref(refname, 0);\n>> +       strbuf_addf(&sb, \"branch.%s.remote\", refname);\n>> +       if (git_config_get_string(sb.buf, &dest))\n>> +               return \"origin\";\n>> +       else\n>> +               return xstrdup(dest);\n>> +}\n>> +\n>> +/*\n>> + * The function takes at most 2 arguments. The first argument is the\n>> + * URL that navigates to the submodule origin repo. When relative, this\n>> URL\n>> + * is relative to the superproject origin URL repo. The second up_path\n>> + * argument, if specified, is the relative path that navigates\n>> + * from the submodule working tree to the superproject working tree.\n>> + *\n>> + * The output of the function is the origin URL of the submodule.\n>> + *\n>> + * The output will either be an absolute URL or filesystem path (if the\n>> + * superproject origin URL is an absolute URL or filesystem path,\n>> + * respectively) or a relative file system path (if the superproject\n>> + * origin URL is a relative file system path).\n>> + *\n>> + * When the output is a relative file system path, the path is either\n>> + * relative to the submodule working tree, if up_path is specified, or to\n>> + * the superproject working tree otherwise.\n>> + */\n>> +static const char *relative_url(const char *url, const char *up_path)\n>> +{\n>> +       int is_relative = 0;\n>> +       size_t len;\n>> +       char *remoteurl = NULL;\n>> +       char *sep = \"/\";\n>> +       const char *out;\n>> +       struct strbuf sb = STRBUF_INIT;\n>> +       const char *remote = get_default_remote();\n>> +       strbuf_addf(&sb, \"remote.%s.url\", remote);\n>> +\n>> +       if (git_config_get_string(sb.buf, &remoteurl))\n>> +               /* the repository is its own authoritative upstream */\n>> +               remoteurl = xgetcwd();\n>> +\n>> +       if (strip_suffix(remoteurl, \"/\", &len))\n>> +               remoteurl[len] = '\\0';\n>> +\n>> +       if (strchr(remoteurl, ':') || skip_prefix(remoteurl, \"/\", &out))\n>> +               is_relative = 0;\n>> +       else if (skip_prefix(remoteurl, \"./\", &out) ||\n>> +                   skip_prefix(remoteurl, \"../\", &out))\n>> +               is_relative = 1;\n>> +       else {\n>> +               is_relative = 1;\n>> +               strbuf_reset(&sb);\n>> +               strbuf_addf(&sb, \"./%s\", remoteurl);\n>> +               remoteurl = strbuf_detach(&sb, NULL);\n>> +       }\n>> +\n>> +       while (url) {\n>> +               if (skip_prefix(url, \"../\", &out)) {\n>> +                       char *rfind;\n>> +                       url = out;\n>> +\n>> +                       rfind = strrchr(remoteurl, '/');\n>> +                       if (rfind)\n>> +                               *rfind = '\\0';\n>> +                       else {\n>> +                               rfind = strrchr(remoteurl, ':');\n>> +                               if (rfind) {\n>> +                                       *rfind = '\\0';\n>> +                                       sep = \":\";\n>> +                               } else {\n>> +                                       if (is_relative || !strcmp(\".\",\n>> remoteurl))\n>> +                                               die(N_(\"cannot strip one\n>> component off url '%s'\"), remoteurl);\n>> +                                       else\n>> +                                               remoteurl = \".\";\n>> +                               }\n>> +                       }\n>> +               } else if (skip_prefix(url, \"./\", &out))\n>> +                       url = out;\n>> +               else\n>> +                       break;\n>> +       }\n>> +       strbuf_reset(&sb);\n>> +       strbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n>> +\n>> +       if (!skip_prefix(sb.buf, \"./\", &out))\n>> +               out = sb.buf;\n>> +       out = xstrdup(out);\n>> +\n>> +       strbuf_reset(&sb);\n>> +       strbuf_addf(&sb, \"%s%s\", is_relative && up_path ? up_path : \"\",\n>> out);\n>> +\n>> +       free((char*)out);\n>> +       return strbuf_detach(&sb, NULL);\n>> +}\n>> +\n>> +static int resolve_relative_url(int argc, const char **argv, const char\n>> *prefix)\n>> +{\n>> +       if (argc == 2)\n>> +               printf(\"%s\\n\", relative_url(argv[1], NULL));\n>> +       else if (argc == 3)\n>> +               printf(\"%s\\n\", relative_url(argv[1], argv[2]));\n>> +       else\n>> +               die(\"BUG: resolve_relative_url only accepts one or two\n>> arguments\");\n>> +       return 0;\n>> +}\n>>\n>>   struct module_list {\n>>         const struct cache_entry **entries;\n>> @@ -264,6 +383,7 @@ static struct cmd_struct commands[] = {\n>>         {\"list\", module_list},\n>>         {\"name\", module_name},\n>>         {\"clone\", module_clone},\n>> +       {\"resolve_relative_url\", resolve_relative_url},\n>>   };\n>>\n>>   int cmd_submodule__helper(int argc, const char **argv, const char\n>> *prefix)\n>> diff --git a/git-submodule.sh b/git-submodule.sh\n>> index 9bc5c5f..6a7a3e4 100755\n>> --- a/git-submodule.sh\n>> +++ b/git-submodule.sh\n>> @@ -46,79 +46,6 @@ prefix=\n>>   custom_name=\n>>   depth=\n>>\n>> -# The function takes at most 2 arguments. The first argument is the\n>> -# URL that navigates to the submodule origin repo. When relative, this\n>> URL\n>> -# is relative to the superproject origin URL repo. The second up_path\n>> -# argument, if specified, is the relative path that navigates\n>> -# from the submodule working tree to the superproject working tree.\n>> -#\n>> -# The output of the function is the origin URL of the submodule.\n>> -#\n>> -# The output will either be an absolute URL or filesystem path (if the\n>> -# superproject origin URL is an absolute URL or filesystem path,\n>> -# respectively) or a relative file system path (if the superproject\n>> -# origin URL is a relative file system path).\n>> -#\n>> -# When the output is a relative file system path, the path is either\n>> -# relative to the submodule working tree, if up_path is specified, or to\n>> -# the superproject working tree otherwise.\n>> -resolve_relative_url ()\n>> -{\n>> -       remote=$(get_default_remote)\n>> -       remoteurl=$(git config \"remote.$remote.url\") ||\n>> -               remoteurl=$(pwd) # the repository is its own authoritative\n>> upstream\n>> -       url=\"$1\"\n>> -       remoteurl=${remoteurl%/}\n>> -       sep=/\n>> -       up_path=\"$2\"\n>> -\n>> -       case \"$remoteurl\" in\n>> -       *:*|/*)\n>> -               is_relative=\n>> -               ;;\n>> -       ./*|../*)\n>> -               is_relative=t\n>> -               ;;\n>> -       *)\n>> -               is_relative=t\n>> -               remoteurl=\"./$remoteurl\"\n>> -               ;;\n>> -       esac\n>> -\n>> -       while test -n \"$url\"\n>> -       do\n>> -               case \"$url\" in\n>> -               ../*)\n>> -                       url=\"${url#../}\"\n>> -                       case \"$remoteurl\" in\n>> -                       */*)\n>> -                               remoteurl=\"${remoteurl%/*}\"\n>> -                               ;;\n>> -                       *:*)\n>> -                               remoteurl=\"${remoteurl%:*}\"\n>> -                               sep=:\n>> -                               ;;\n>> -                       *)\n>> -                               if test -z \"$is_relative\" || test \".\" =\n>> \"$remoteurl\"\n>> -                               then\n>> -                                       die \"$(eval_gettext \"cannot strip\n>> one component off url '\\$remoteurl'\")\"\n>> -                               else\n>> -                                       remoteurl=.\n>> -                               fi\n>> -                               ;;\n>> -                       esac\n>> -                       ;;\n>> -               ./*)\n>> -                       url=\"${url#./}\"\n>> -                       ;;\n>> -               *)\n>> -                       break;;\n>> -               esac\n>> -       done\n>> -       remoteurl=\"$remoteurl$sep${url%/}\"\n>> -       echo \"${is_relative:+${up_path}}${remoteurl#./}\"\n>> -}\n>> -\n>>   # Resolve a path to be relative to another path.  This is intended for\n>>   # converting submodule paths when git-submodule is run in a subdirectory\n>>   # and only handles paths where the directory separator is '/'.\n>> @@ -281,7 +208,7 @@ cmd_add()\n>>                 die \"$(gettext \"Relative path can only be used from the\n>> toplevel of the working tree\")\"\n>>\n>>                 # dereference source url relative to parent's url\n>> -               realrepo=$(resolve_relative_url \"$repo\") || exit\n>> +               realrepo=$(git submodule--helper resolve_relative_url\n>> \"$repo\") || exit\n>>                 ;;\n>>         *:*|/*)\n>>                 # absolute url\n>> @@ -485,7 +412,7 @@ cmd_init()\n>>                         # Possibly a url relative to parent\n>>                         case \"$url\" in\n>>                         ./*|../*)\n>> -                               url=$(resolve_relative_url \"$url\") || exit\n>> +                               url=$(git submodule--helper\n>> resolve_relative_url \"$url\") || exit\n>>                                 ;;\n>>                         esac\n>>                         git config submodule.\"$name\".url \"$url\" ||\n>> @@ -1190,9 +1117,9 @@ cmd_sync()\n>>                         # guarantee a trailing /\n>>                         up_path=${up_path%/}/ &&\n>>                         # path from submodule work tree to submodule\n>> origin repo\n>> -                       sub_origin_url=$(resolve_relative_url \"$url\"\n>> \"$up_path\") &&\n>> +                       sub_origin_url=$(git submodule--helper\n>> resolve_relative_url \"$url\" \"$up_path\") &&\n>>                         # path from superproject work tree to submodule\n>> origin repo\n>> -                       super_config_url=$(resolve_relative_url \"$url\") ||\n>> exit\n>> +                       super_config_url=$(git submodule--helper\n>> resolve_relative_url \"$url\") || exit\n>>                         ;;\n>>                 *)\n>>                         sub_origin_url=\"$url\"\n>>\n>\n"},{"id":"274641","messageId":"1450311999-3992-1-git-send-email-sbeller@google.com","threadId":"40963","inReplyTo":"1449709654-30189-1-git-send-email-sbeller@google.com","subject":"[PATCHv2] Porting resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-17T00:26:38Z","receivedAt":"2015-12-17T00:26:38Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"A new version of the patch, which spells out more its intent and\nmay actually work in Windows.\n\nAny comment welcome,\nThanks,\nStefan\n\nStefan Beller (1):\n  submodule: Port resolve_relative_url from shell to C\n\n builtin/submodule--helper.c | 151 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  81 ++----------------------\n 2 files changed, 155 insertions(+), 77 deletions(-)\n\ninterdiff to previous version:\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f48b5b5..b925bed 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -31,6 +31,36 @@ static const char *get_default_remote(void)\n \t\treturn xstrdup(dest);\n }\n \n+static int has_same_dir_prefix(const char *str, const char **out)\n+{\n+#ifdef GIT_WINDOWS_NATIVE\n+\treturn skip_prefix(str, \"./\", out)\n+\t\t|| skip_prefix(str, \".\\\\\", out);\n+#else\n+\treturn skip_prefix(str, \"./\", out);\n+#endif\n+}\n+\n+static int has_upper_dir_prefix(const char *str, const char **out)\n+{\n+#ifdef GIT_WINDOWS_NATIVE\n+\treturn skip_prefix(str, \"../\", out)\n+\t\t|| skip_prefix(str, \"..\\\\\", out);\n+#else\n+\treturn skip_prefix(str, \"../\", out);\n+#endif\n+}\n+\n+static char *last_dir_separator(const char *str)\n+{\n+#ifdef GIT_WINDOWS_NATIVE\n+\treturn strrchr(str, \"/\")\n+\t\t|| strrchr(str, \"\\\\\");\n+#else\n+\treturn strrchr(str, '/');\n+#endif\n+}\n+\n /*\n  * The function takes at most 2 arguments. The first argument is the\n  * URL that navigates to the submodule origin repo. When relative, this URL\n@@ -64,13 +94,14 @@ static const char *relative_url(const char *url, const char *up_path)\n \t\t/* the repository is its own authoritative upstream */\n \t\tremoteurl = xgetcwd();\n \n-\tif (strip_suffix(remoteurl, \"/\", &len))\n+\tlen = strlen(remoteurl);\n+\tif (is_dir_sep(remoteurl[len]))\n \t\tremoteurl[len] = '\\0';\n \n-\tif (strchr(remoteurl, ':') || skip_prefix(remoteurl, \"/\", &out))\n+\tif (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))\n \t\tis_relative = 0;\n-\telse if (skip_prefix(remoteurl, \"./\", &out) ||\n-\t\t    skip_prefix(remoteurl, \"../\", &out))\n+\telse if (has_same_dir_prefix(remoteurl, &out) ||\n+\t\t    has_upper_dir_prefix(remoteurl, &out))\n \t\tis_relative = 1;\n \telse {\n \t\tis_relative = 1;\n@@ -80,11 +111,11 @@ static const char *relative_url(const char *url, const char *up_path)\n \t}\n \n \twhile (url) {\n-\t\tif (skip_prefix(url, \"../\", &out)) {\n+\t\tif (has_upper_dir_prefix(url, &out)) {\n \t\t\tchar *rfind;\n \t\t\turl = out;\n \n-\t\t\trfind = strrchr(remoteurl, '/');\n+\t\t\trfind = last_dir_separator(remoteurl);\n \t\t\tif (rfind)\n \t\t\t\t*rfind = '\\0';\n \t\t\telse {\n@@ -99,7 +130,7 @@ static const char *relative_url(const char *url, const char *up_path)\n \t\t\t\t\t\tremoteurl = \".\";\n \t\t\t\t}\n \t\t\t}\n-\t\t} else if (skip_prefix(url, \"./\", &out))\n+\t\t} else if (has_same_dir_prefix(url, &out))\n \t\t\turl = out;\n \t\telse\n \t\t\tbreak;\n@@ -107,7 +138,7 @@ static const char *relative_url(const char *url, const char *up_path)\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n \n-\tif (!skip_prefix(sb.buf, \"./\", &out))\n+\tif (!has_same_dir_prefix(sb.buf, &out))\n \t\tout = sb.buf;\n \tout = xstrdup(out);\n \n\n-- \n2.7.0.rc1.2.gfc39790.dirty\n"},{"id":"274642","messageId":"1450311999-3992-2-git-send-email-sbeller@google.com","threadId":"40963","inReplyTo":"1450311999-3992-1-git-send-email-sbeller@google.com","subject":"[PATCHv2] submodule: Port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2015-12-17T00:26:39Z","receivedAt":"2015-12-17T00:26:39Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This reimplements the helper function `resolve_relative_url` in shell\nin C. This functionality is needed in C for introducing the groups\nfeature later on. When using groups, the user should not need to run\n`git submodule init`, but it should be implicit at all appropriate places,\nwhich are all in C code. As the we would not just call out to `git\nsubmodule init`, but do a more fine grained structure there, we actually\nneed all the init functionality in C before attempting the groups\nfeature. To get the init functionality in C, rewriting the\nresolve_relative_url subfunction is a major step.\n\nThis also improves the performance:\n(Best out of 3) time ./t7400-submodule-basic.sh\nBefore:\nreal\t0m9.575s\nuser\t0m2.683s\nsys\t0m6.773s\nAfter:\nreal\t0m9.293s\nuser\t0m2.691s\nsys\t0m6.549s\n\nThat's about 3%.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 151 ++++++++++++++++++++++++++++++++++++++++++++\n git-submodule.sh            |  81 ++----------------------\n 2 files changed, 155 insertions(+), 77 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex f4c3eff..b925bed 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -9,6 +9,156 @@\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n+#include \"refs.h\"\n+\n+static const char *get_default_remote(void)\n+{\n+\tchar *dest = NULL;\n+\tunsigned char sha1[20];\n+\tint flag;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, &flag);\n+\n+\tif (!refname)\n+\t\tdie(\"No such ref: HEAD\");\n+\n+\trefname = shorten_unambiguous_ref(refname, 0);\n+\tstrbuf_addf(&sb, \"branch.%s.remote\", refname);\n+\tif (git_config_get_string(sb.buf, &dest))\n+\t\treturn \"origin\";\n+\telse\n+\t\treturn xstrdup(dest);\n+}\n+\n+static int has_same_dir_prefix(const char *str, const char **out)\n+{\n+#ifdef GIT_WINDOWS_NATIVE\n+\treturn skip_prefix(str, \"./\", out)\n+\t\t|| skip_prefix(str, \".\\\\\", out);\n+#else\n+\treturn skip_prefix(str, \"./\", out);\n+#endif\n+}\n+\n+static int has_upper_dir_prefix(const char *str, const char **out)\n+{\n+#ifdef GIT_WINDOWS_NATIVE\n+\treturn skip_prefix(str, \"../\", out)\n+\t\t|| skip_prefix(str, \"..\\\\\", out);\n+#else\n+\treturn skip_prefix(str, \"../\", out);\n+#endif\n+}\n+\n+static char *last_dir_separator(const char *str)\n+{\n+#ifdef GIT_WINDOWS_NATIVE\n+\treturn strrchr(str, \"/\")\n+\t\t|| strrchr(str, \"\\\\\");\n+#else\n+\treturn strrchr(str, '/');\n+#endif\n+}\n+\n+/*\n+ * The function takes at most 2 arguments. The first argument is the\n+ * URL that navigates to the submodule origin repo. When relative, this URL\n+ * is relative to the superproject origin URL repo. The second up_path\n+ * argument, if specified, is the relative path that navigates\n+ * from the submodule working tree to the superproject working tree.\n+ *\n+ * The output of the function is the origin URL of the submodule.\n+ *\n+ * The output will either be an absolute URL or filesystem path (if the\n+ * superproject origin URL is an absolute URL or filesystem path,\n+ * respectively) or a relative file system path (if the superproject\n+ * origin URL is a relative file system path).\n+ *\n+ * When the output is a relative file system path, the path is either\n+ * relative to the submodule working tree, if up_path is specified, or to\n+ * the superproject working tree otherwise.\n+ */\n+static const char *relative_url(const char *url, const char *up_path)\n+{\n+\tint is_relative = 0;\n+\tsize_t len;\n+\tchar *remoteurl = NULL;\n+\tchar *sep = \"/\";\n+\tconst char *out;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tconst char *remote = get_default_remote();\n+\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\n+\tif (git_config_get_string(sb.buf, &remoteurl))\n+\t\t/* the repository is its own authoritative upstream */\n+\t\tremoteurl = xgetcwd();\n+\n+\tlen = strlen(remoteurl);\n+\tif (is_dir_sep(remoteurl[len]))\n+\t\tremoteurl[len] = '\\0';\n+\n+\tif (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))\n+\t\tis_relative = 0;\n+\telse if (has_same_dir_prefix(remoteurl, &out) ||\n+\t\t    has_upper_dir_prefix(remoteurl, &out))\n+\t\tis_relative = 1;\n+\telse {\n+\t\tis_relative = 1;\n+\t\tstrbuf_reset(&sb);\n+\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n+\t\tremoteurl = strbuf_detach(&sb, NULL);\n+\t}\n+\n+\twhile (url) {\n+\t\tif (has_upper_dir_prefix(url, &out)) {\n+\t\t\tchar *rfind;\n+\t\t\turl = out;\n+\n+\t\t\trfind = last_dir_separator(remoteurl);\n+\t\t\tif (rfind)\n+\t\t\t\t*rfind = '\\0';\n+\t\t\telse {\n+\t\t\t\trfind = strrchr(remoteurl, ':');\n+\t\t\t\tif (rfind) {\n+\t\t\t\t\t*rfind = '\\0';\n+\t\t\t\t\tsep = \":\";\n+\t\t\t\t} else {\n+\t\t\t\t\tif (is_relative || !strcmp(\".\", remoteurl))\n+\t\t\t\t\t\tdie(N_(\"cannot strip one component off url '%s'\"), remoteurl);\n+\t\t\t\t\telse\n+\t\t\t\t\t\tremoteurl = \".\";\n+\t\t\t\t}\n+\t\t\t}\n+\t\t} else if (has_same_dir_prefix(url, &out))\n+\t\t\turl = out;\n+\t\telse\n+\t\t\tbreak;\n+\t}\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n+\n+\tif (!has_same_dir_prefix(sb.buf, &out))\n+\t\tout = sb.buf;\n+\tout = xstrdup(out);\n+\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s\", is_relative && up_path ? up_path : \"\", out);\n+\n+\tfree((char*)out);\n+\treturn strbuf_detach(&sb, NULL);\n+}\n+\n+static int resolve_relative_url(int argc, const char **argv, const char *prefix)\n+{\n+\tif (argc == 2)\n+\t\tprintf(\"%s\\n\", relative_url(argv[1], NULL));\n+\telse if (argc == 3)\n+\t\tprintf(\"%s\\n\", relative_url(argv[1], argv[2]));\n+\telse\n+\t\tdie(\"BUG: resolve_relative_url only accepts one or two arguments\");\n+\treturn 0;\n+}\n \n struct module_list {\n \tconst struct cache_entry **entries;\n@@ -264,6 +414,7 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list},\n \t{\"name\", module_name},\n \t{\"clone\", module_clone},\n+\t{\"resolve_relative_url\", resolve_relative_url},\n };\n \n int cmd_submodule__helper(int argc, const char **argv, const char *prefix)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 9bc5c5f..6a7a3e4 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -46,79 +46,6 @@ prefix=\n custom_name=\n depth=\n \n-# The function takes at most 2 arguments. The first argument is the\n-# URL that navigates to the submodule origin repo. When relative, this URL\n-# is relative to the superproject origin URL repo. The second up_path\n-# argument, if specified, is the relative path that navigates\n-# from the submodule working tree to the superproject working tree.\n-#\n-# The output of the function is the origin URL of the submodule.\n-#\n-# The output will either be an absolute URL or filesystem path (if the\n-# superproject origin URL is an absolute URL or filesystem path,\n-# respectively) or a relative file system path (if the superproject\n-# origin URL is a relative file system path).\n-#\n-# When the output is a relative file system path, the path is either\n-# relative to the submodule working tree, if up_path is specified, or to\n-# the superproject working tree otherwise.\n-resolve_relative_url ()\n-{\n-\tremote=$(get_default_remote)\n-\tremoteurl=$(git config \"remote.$remote.url\") ||\n-\t\tremoteurl=$(pwd) # the repository is its own authoritative upstream\n-\turl=\"$1\"\n-\tremoteurl=${remoteurl%/}\n-\tsep=/\n-\tup_path=\"$2\"\n-\n-\tcase \"$remoteurl\" in\n-\t*:*|/*)\n-\t\tis_relative=\n-\t\t;;\n-\t./*|../*)\n-\t\tis_relative=t\n-\t\t;;\n-\t*)\n-\t\tis_relative=t\n-\t\tremoteurl=\"./$remoteurl\"\n-\t\t;;\n-\tesac\n-\n-\twhile test -n \"$url\"\n-\tdo\n-\t\tcase \"$url\" in\n-\t\t../*)\n-\t\t\turl=\"${url#../}\"\n-\t\t\tcase \"$remoteurl\" in\n-\t\t\t*/*)\n-\t\t\t\tremoteurl=\"${remoteurl%/*}\"\n-\t\t\t\t;;\n-\t\t\t*:*)\n-\t\t\t\tremoteurl=\"${remoteurl%:*}\"\n-\t\t\t\tsep=:\n-\t\t\t\t;;\n-\t\t\t*)\n-\t\t\t\tif test -z \"$is_relative\" || test \".\" = \"$remoteurl\"\n-\t\t\t\tthen\n-\t\t\t\t\tdie \"$(eval_gettext \"cannot strip one component off url '\\$remoteurl'\")\"\n-\t\t\t\telse\n-\t\t\t\t\tremoteurl=.\n-\t\t\t\tfi\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\t;;\n-\t\t./*)\n-\t\t\turl=\"${url#./}\"\n-\t\t\t;;\n-\t\t*)\n-\t\t\tbreak;;\n-\t\tesac\n-\tdone\n-\tremoteurl=\"$remoteurl$sep${url%/}\"\n-\techo \"${is_relative:+${up_path}}${remoteurl#./}\"\n-}\n-\n # Resolve a path to be relative to another path.  This is intended for\n # converting submodule paths when git-submodule is run in a subdirectory\n # and only handles paths where the directory separator is '/'.\n@@ -281,7 +208,7 @@ cmd_add()\n \t\tdie \"$(gettext \"Relative path can only be used from the toplevel of the working tree\")\"\n \n \t\t# dereference source url relative to parent's url\n-\t\trealrepo=$(resolve_relative_url \"$repo\") || exit\n+\t\trealrepo=$(git submodule--helper resolve_relative_url \"$repo\") || exit\n \t\t;;\n \t*:*|/*)\n \t\t# absolute url\n@@ -485,7 +412,7 @@ cmd_init()\n \t\t\t# Possibly a url relative to parent\n \t\t\tcase \"$url\" in\n \t\t\t./*|../*)\n-\t\t\t\turl=$(resolve_relative_url \"$url\") || exit\n+\t\t\t\turl=$(git submodule--helper resolve_relative_url \"$url\") || exit\n \t\t\t\t;;\n \t\t\tesac\n \t\t\tgit config submodule.\"$name\".url \"$url\" ||\n@@ -1190,9 +1117,9 @@ cmd_sync()\n \t\t\t# guarantee a trailing /\n \t\t\tup_path=${up_path%/}/ &&\n \t\t\t# path from submodule work tree to submodule origin repo\n-\t\t\tsub_origin_url=$(resolve_relative_url \"$url\" \"$up_path\") &&\n+\t\t\tsub_origin_url=$(git submodule--helper resolve_relative_url \"$url\" \"$up_path\") &&\n \t\t\t# path from superproject work tree to submodule origin repo\n-\t\t\tsuper_config_url=$(resolve_relative_url \"$url\") || exit\n+\t\t\tsuper_config_url=$(git submodule--helper resolve_relative_url \"$url\") || exit\n \t\t\t;;\n \t\t*)\n \t\t\tsub_origin_url=\"$url\"\n-- \n2.7.0.rc1.2.gfc39790.dirty\n"},{"id":"274652","messageId":"56726886.6060305@web.de","threadId":"40963","inReplyTo":"1450311999-3992-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv2] Porting resolve_relative_url from shell to C","fromName":"Torsten Bögershausen","fromEmail":"tboegi@web.de","sentAt":"2015-12-17T07:47:18Z","receivedAt":"2015-12-17T07:47:18Z","isPatch":false,"sender":{"key":"tboegi@web.de","avatar":"https://avatars.githubusercontent.com/u/7138363?v=4"},"body":"On 17.12.15 01:26, Stefan Beller wrote:\n> A new version of the patch, which spells out more its intent and\n> may actually work in Windows.\n> \n> Any comment welcome,\n> Thanks,\n> Stefan\n> \n> Stefan Beller (1):\n>   submodule: Port resolve_relative_url from shell to C\n> \n>  builtin/submodule--helper.c | 151 ++++++++++++++++++++++++++++++++++++++++++++\n>  git-submodule.sh            |  81 ++----------------------\n>  2 files changed, 155 insertions(+), 77 deletions(-)\n> \n> interdiff to previous version:\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index f48b5b5..b925bed 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -31,6 +31,36 @@ static const char *get_default_remote(void)\n>  \t\treturn xstrdup(dest);\n>  }\n>  \n> +static int has_same_dir_prefix(const char *str, const char **out)\n> +{\n> +#ifdef GIT_WINDOWS_NATIVE\nShould that be\nif defined(GIT_WINDOWS_NATIVE) || defined(__CYGWIN__)\n"},{"id":"274654","messageId":"CAPig+cT2ia-2LdJDTwkFUrFL84PfHvWXzQGJYzp9Os6=9aDxnw@mail.gmail.com","threadId":"40963","inReplyTo":"1450311999-3992-2-git-send-email-sbeller@google.com","subject":"Re: [PATCHv2] submodule: Port resolve_relative_url from shell to C","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2015-12-17T08:17:49Z","receivedAt":"2015-12-17T08:17:49Z","isPatch":false,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"A rather superficial review...\n\nOn Wed, Dec 16, 2015 at 7:26 PM, Stefan Beller <sbeller@google.com> wrote:\n> This reimplements the helper function `resolve_relative_url` in shell\n\ns/This reimplements/Reimplement/\n\n> in C. This functionality is needed in C for introducing the groups\n> feature later on. When using groups, the user should not need to run\n> `git submodule init`, but it should be implicit at all appropriate places,\n> which are all in C code. As the we would not just call out to `git\n\nI guess you mean \"As then we...\" or something?\n\n> submodule init`, but do a more fine grained structure there, we actually\n> need all the init functionality in C before attempting the groups\n> feature. To get the init functionality in C, rewriting the\n> resolve_relative_url subfunction is a major step.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> @@ -9,6 +9,156 @@\n> +static const char *get_default_remote(void)\n> +{\n> +       char *dest = NULL;\n> +       unsigned char sha1[20];\n> +       int flag;\n> +       struct strbuf sb = STRBUF_INIT;\n> +       const char *refname = resolve_ref_unsafe(\"HEAD\", 0, sha1, &flag);\n> +\n> +       if (!refname)\n> +               die(\"No such ref: HEAD\");\n> +\n> +       refname = shorten_unambiguous_ref(refname, 0);\n> +       strbuf_addf(&sb, \"branch.%s.remote\", refname);\n> +       if (git_config_get_string(sb.buf, &dest))\n> +               return \"origin\";\n> +       else\n> +               return xstrdup(dest);\n\nLeaking the strbuf at both returns.\n\nAnd, leaking the strdup'd dest (in the caller), but I suppose that's\nintentional?\n\n> +}\n> +\n> +static int has_same_dir_prefix(const char *str, const char **out)\n> +{\n> +#ifdef GIT_WINDOWS_NATIVE\n> +       return skip_prefix(str, \"./\", out)\n> +               || skip_prefix(str, \".\\\\\", out);\n> +#else\n> +       return skip_prefix(str, \"./\", out);\n> +#endif\n> +}\n> +\n> +static int has_upper_dir_prefix(const char *str, const char **out)\n> +{\n> +#ifdef GIT_WINDOWS_NATIVE\n> +       return skip_prefix(str, \"../\", out)\n> +               || skip_prefix(str, \"..\\\\\", out);\n> +#else\n> +       return skip_prefix(str, \"../\", out);\n> +#endif\n> +}\n> +\n> +static char *last_dir_separator(const char *str)\n> +{\n> +#ifdef GIT_WINDOWS_NATIVE\n> +       return strrchr(str, \"/\")\n> +               || strrchr(str, \"\\\\\");\n> +#else\n> +       return strrchr(str, '/');\n> +#endif\n> +}\n\nCleaner would be to move the #if's outside the functions:\n\n    #ifdef GIT_WINDOWS_NATIVE\n\n    /* Windows implementations */\n    static int has_same_dir_prefix(...) {...}\n    static int has_upper_dir_prefix(...) {...}\n    static char *last_dir_separator(...) {...}\n\n    #else\n\n    /* POSIX implementations */\n    static int has_same_dir_prefix(...) {...}\n    static int has_upper_dir_prefix(...) {...}\n    static char *last_dir_separator(...) {...}\n\n    #endif\n\n> +static const char *relative_url(const char *url, const char *up_path)\n> +{\n> +       int is_relative = 0;\n> +       size_t len;\n> +       char *remoteurl = NULL;\n> +       char *sep = \"/\";\n\n'sep' only ever holds a single character, so why not declare it 'char'\nrather than 'char *'? (And, adjust the format string of strbuf_addf(),\nof course.)\n\n> +       const char *out;\n> +       [...]\n> +       while (url) {\n> +               if (has_upper_dir_prefix(url, &out)) {\n> +                       [...]\n> +                       if (rfind)\n> +                               *rfind = '\\0';\n> +                       else {\n> +                               rfind = strrchr(remoteurl, ':');\n> +                               if (rfind) {\n> +                                       *rfind = '\\0';\n> +                                       sep = \":\";\n> +                               } else {\n> +                                       [...]\n> +                               }\n> +                       }\n> +               } else if (has_same_dir_prefix(url, &out))\n> +                       url = out;\n> +               else\n> +                       break;\n> +       }\n> +       strbuf_reset(&sb);\n> +       strbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n> +\n> +       if (!has_same_dir_prefix(sb.buf, &out))\n> +               out = sb.buf;\n> +       out = xstrdup(out);\n> +\n> +       strbuf_reset(&sb);\n> +       strbuf_addf(&sb, \"%s%s\", is_relative && up_path ? up_path : \"\", out);\n> +\n> +       free((char*)out);\n\nWhy declare 'out' const if you know you will be freeing it?\n\n> +       return strbuf_detach(&sb, NULL);\n> +}\n> +\n> +static int resolve_relative_url(int argc, const char **argv, const char *prefix)\n> +{\n> +       if (argc == 2)\n> +               printf(\"%s\\n\", relative_url(argv[1], NULL));\n> +       else if (argc == 3)\n> +               printf(\"%s\\n\", relative_url(argv[1], argv[2]));\n> +       else\n> +               die(\"BUG: resolve_relative_url only accepts one or two arguments\");\n> +       return 0;\n> +}\n>\n>  struct module_list {\n>         const struct cache_entry **entries;\n> @@ -264,6 +414,7 @@ static struct cmd_struct commands[] = {\n>         {\"list\", module_list},\n>         {\"name\", module_name},\n>         {\"clone\", module_clone},\n> +       {\"resolve_relative_url\", resolve_relative_url},\n\nCan we please use hyphens rather than underscores, and name this\n\"resolve-relative-url\" instead? Quoting from a review[1] of an earlier\nversion of git-submodule--helper:\n\n    ... these subcommands would be better spelled with a hyphen than\n    an underscore. If I recall correctly, the arguments for using\n    underscore were (1) a less noisy diff, (2) these are internal\n    commands nobody will be typing anyhow. However, (1) the diff\n    noise will be the same with hyphens, and (2) people will want to\n    test these commands manually anyhow, and its easier to type\n    hyphens and easier to remember them since the precedent for\n    hyphens in command-names is already well established.\n\n    Also, the reason that the original shell code used underscores\n    was because hyphens are not valid characters in shell function\n    names, but that's an implementation detail which shouldn't be\n    allowed to bleed over to user-facing interface design (and these\n    subcommands are user-facing).\n\n[1]: http://article.gmane.org/gmane.comp.version-control.git/276947\n\n>  };\n"},{"id":"274681","messageId":"5673052F.7050000@kdbg.org","threadId":"40963","inReplyTo":"1450311999-3992-2-git-send-email-sbeller@google.com","subject":"Re: [PATCHv2] submodule: Port resolve_relative_url from shell to C","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-12-17T18:55:43Z","receivedAt":"2015-12-17T18:55:43Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.12.2015 um 01:26 schrieb Stefan Beller:\n> This reimplements the helper function `resolve_relative_url` in shell\n> in C. This functionality is needed in C for introducing the groups\n> feature later on. When using groups, the user should not need to run\n> `git submodule init`, but it should be implicit at all appropriate places,\n> which are all in C code. As the we would not just call out to `git\n> submodule init`, but do a more fine grained structure there, we actually\n> need all the init functionality in C before attempting the groups\n> feature. To get the init functionality in C, rewriting the\n> resolve_relative_url subfunction is a major step.\n>\n> This also improves the performance:\n> (Best out of 3) time ./t7400-submodule-basic.sh\n> Before:\n> real\t0m9.575s\n> user\t0m2.683s\n> sys\t0m6.773s\n> After:\n> real\t0m9.293s\n> user\t0m2.691s\n> sys\t0m6.549s\n>\n> That's about 3%.\n\nI appreciate this effort as it should help us on Windows. Although the\nnumbers (and my own timings) suggest that this is only a small step\nforward. That's not surprising as the patch removes only two forks.\n\nAs to the implementation, find a patch below that removes the ifdefs\nand a few other suggestions. It is a mechanical conversion without\nunderstanding what relative_url() does. I have the gut feeling that the\ntwo strbuf_addf towards the end of the function can be contracted and\nthe temporarily allocate copy in 'out' can be removed.\n\nIf there were a few examples in the comment above the function, it\nwould be much simpler to understand.\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex b925bed..8ec0975 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -11,6 +11,7 @@\n #include \"run-command.h\"\n #include \"remote.h\"\n #include \"refs.h\"\n+#include \"connect.h\"\n \n static const char *get_default_remote(void)\n {\n@@ -31,34 +32,23 @@ static const char *get_default_remote(void)\n \t\treturn xstrdup(dest);\n }\n \n-static int has_same_dir_prefix(const char *str, const char **out)\n+static int starts_with_dot_slash(const char *str)\n {\n-#ifdef GIT_WINDOWS_NATIVE\n-\treturn skip_prefix(str, \"./\", out)\n-\t\t|| skip_prefix(str, \".\\\\\", out);\n-#else\n-\treturn skip_prefix(str, \"./\", out);\n-#endif\n+\treturn str[0] == '.' && is_dir_sep(str[1]);\n }\n \n-static int has_upper_dir_prefix(const char *str, const char **out)\n+static int starts_with_dot_dot_slash(const char *str)\n {\n-#ifdef GIT_WINDOWS_NATIVE\n-\treturn skip_prefix(str, \"../\", out)\n-\t\t|| skip_prefix(str, \"..\\\\\", out);\n-#else\n-\treturn skip_prefix(str, \"../\", out);\n-#endif\n+\treturn str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n }\n \n-static char *last_dir_separator(const char *str)\n+static char *last_dir_separator(char *str)\n {\n-#ifdef GIT_WINDOWS_NATIVE\n-\treturn strrchr(str, \"/\")\n-\t\t|| strrchr(str, \"\\\\\");\n-#else\n-\treturn strrchr(str, '/');\n-#endif\n+\tchar* p = str + strlen(str);\n+\twhile (p-- != str)\n+\t\tif (is_dir_sep(*p))\n+\t\t\treturn p;\n+\treturn NULL;\n }\n \n /*\n@@ -85,9 +75,10 @@ static const char *relative_url(const char *url, const char *up_path)\n \tsize_t len;\n \tchar *remoteurl = NULL;\n \tchar *sep = \"/\";\n-\tconst char *out;\n+\tchar *out;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *remote = get_default_remote();\n+\n \tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n \n \tif (git_config_get_string(sb.buf, &remoteurl))\n@@ -98,22 +89,23 @@ static const char *relative_url(const char *url, const char *up_path)\n \tif (is_dir_sep(remoteurl[len]))\n \t\tremoteurl[len] = '\\0';\n \n-\tif (strchr(remoteurl, ':') || is_dir_sep(*remoteurl))\n+\tif (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n \t\tis_relative = 0;\n-\telse if (has_same_dir_prefix(remoteurl, &out) ||\n-\t\t    has_upper_dir_prefix(remoteurl, &out))\n+\telse if (starts_with_dot_slash(remoteurl) ||\n+\t\t    starts_with_dot_dot_slash(remoteurl))\n \t\tis_relative = 1;\n \telse {\n \t\tis_relative = 1;\n \t\tstrbuf_reset(&sb);\n \t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n+\t\tfree(remoteurl);\n \t\tremoteurl = strbuf_detach(&sb, NULL);\n \t}\n \n \twhile (url) {\n-\t\tif (has_upper_dir_prefix(url, &out)) {\n+\t\tif (starts_with_dot_dot_slash(url)) {\n \t\t\tchar *rfind;\n-\t\t\turl = out;\n+\t\t\turl += 3;\n \n \t\t\trfind = last_dir_separator(remoteurl);\n \t\t\tif (rfind)\n@@ -130,22 +122,23 @@ static const char *relative_url(const char *url, const char *up_path)\n \t\t\t\t\t\tremoteurl = \".\";\n \t\t\t\t}\n \t\t\t}\n-\t\t} else if (has_same_dir_prefix(url, &out))\n-\t\t\turl = out;\n-\t\telse\n+\t\t} else if (starts_with_dot_slash(url)) {\n+\t\t\turl += 2;\n+\t\t} else\n \t\t\tbreak;\n \t}\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, sep, url);\n \n-\tif (!has_same_dir_prefix(sb.buf, &out))\n-\t\tout = sb.buf;\n-\tout = xstrdup(out);\n+\tif (starts_with_dot_slash(sb.buf))\n+\t\tout = strdup(sb.buf + 2);\n+\telse\n+\t\tout = xstrdup(sb.buf);\n \n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s%s\", is_relative && up_path ? up_path : \"\", out);\n \n-\tfree((char*)out);\n+\tfree(out);\n \treturn strbuf_detach(&sb, NULL);\n }\n \n"},{"id":"274682","messageId":"56730A4C.4080102@kdbg.org","threadId":"40963","inReplyTo":"5673052F.7050000@kdbg.org","subject":"Re: [PATCHv2] submodule: Port resolve_relative_url from shell to C","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2015-12-17T19:17:32Z","receivedAt":"2015-12-17T19:17:32Z","isPatch":false,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 17.12.2015 um 19:55 schrieb Johannes Sixt:\n> As to the implementation, find a patch below that removes the ifdefs\n> and a few other suggestions.\n\nThe word \"offers\" is missing in this last half-sentence ;)\n"},{"id":"274711","messageId":"20151218032708.GA8530@sigill.intra.peff.net","threadId":"40963","inReplyTo":"5673052F.7050000@kdbg.org","subject":"Re: [PATCHv2] submodule: Port resolve_relative_url from shell to C","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2015-12-18T03:27:08Z","receivedAt":"2015-12-18T03:27:08Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Dec 17, 2015 at 07:55:43PM +0100, Johannes Sixt wrote:\n\n> -static int has_same_dir_prefix(const char *str, const char **out)\n> +static int starts_with_dot_slash(const char *str)\n>  {\n> -#ifdef GIT_WINDOWS_NATIVE\n> -\treturn skip_prefix(str, \"./\", out)\n> -\t\t|| skip_prefix(str, \".\\\\\", out);\n> -#else\n> -\treturn skip_prefix(str, \"./\", out);\n> -#endif\n> +\treturn str[0] == '.' && is_dir_sep(str[1]);\n>  }\n>  \n> -static int has_upper_dir_prefix(const char *str, const char **out)\n> +static int starts_with_dot_dot_slash(const char *str)\n>  {\n> -#ifdef GIT_WINDOWS_NATIVE\n> -\treturn skip_prefix(str, \"../\", out)\n> -\t\t|| skip_prefix(str, \"..\\\\\", out);\n> -#else\n> -\treturn skip_prefix(str, \"../\", out);\n> -#endif\n> +\treturn str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n>  }\n\nAs the set of prefixes you are looking is probably bounded, it may not\nbe worth generalizing this. But I wondered if something like:\n\n  /*\n   * Like skip_prefix, but consider any \"/\" in the prefix as a\n   * directory separator for the platform.\n   */\n  int skip_prefix_fs(const char *str, const char *prefix, const char **out)\n  {\n\twhile (1) {\n\t\tif (!*prefix) {\n\t\t\t*out = str;\n\t\t\treturn 1;\n\t\t} else if (*prefix == '/') {\n\t\t\tif (!is_dir_sep(*str))\n\t\t\t\treturn 0;\n\t\t} else {\n\t\t\tif (*str != *prefix)\n\t\t\t\treturn 0;\n\t\t}\n\t\tstr++;\n\t\tprefix++;\n\t}\n  }\n\n  ...\n  /* works on all platforms! */\n  if (skip_prefix_fs(foo, \"./\", &out))\n\t...\n\nwould be helpful. I don't know if there are other opportunities in the\ncode base that could make use of this. If it's just these two sites,\nit's probably not worth it.\n\n-Peff\n"},{"id":"275702","messageId":"CAGZ79kb_XOReZZ8E+7QL8dAz6k7PR10Rg-kMgDczRQ6gPpp7Fw@mail.gmail.com","threadId":"40963","inReplyTo":"5673052F.7050000@kdbg.org","subject":"Re: [PATCHv2] submodule: Port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-11T20:17:34Z","receivedAt":"2016-01-11T20:17:34Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I forgot to send out the draft last year.\n\nOn Thu, Dec 17, 2015 at 10:55 AM, Johannes Sixt <j6t@kdbg.org> wrote:\n>> That's about 3%.\n>\n> I appreciate this effort as it should help us on Windows. Although the\n> numbers (and my own timings) suggest that this is only a small step\n> forward. That's not surprising as the patch removes only two forks.\n\nok. Probably the timings are not as important here anyways.\n\n>\n> As to the implementation, find a patch below that removes the ifdefs\n> and a few other suggestions. It is a mechanical conversion without\n> understanding what relative_url() does. I have the gut feeling that the\n> two strbuf_addf towards the end of the function can be contracted and\n> the temporarily allocate copy in 'out' can be removed.\n\nI have the gut feeling, too. But I could not quite write down the solution.\n\n>\n> If there were a few examples in the comment above the function, it\n> would be much simpler to understand.\n\nI agree. So rewording the comment in the next reroll.\n"}]}