{"thread":{"id":"41246","subject":"[PATCHv3 2/2] submodule: port init from shell to C","startedAt":"2016-01-22T23:32:14Z","lastAt":"2016-02-27T08:30:49Z","messageCount":10,"participants":["Stefan Beller","Junio C Hamano","Duy Nguyen"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"276592","messageId":"1453505536-14895-1-git-send-email-sbeller@google.com","threadId":"41246","inReplyTo":"xmqqoacdtfh3.fsf@gitster.mtv.corp.google.com","subject":"[PATCHv3 0/2] Port `git submodule init` from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-22T23:32:14Z","receivedAt":"2016-01-22T23:32:14Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This applies on top of sb/submodule-parallel-update, replacing\nsb/submodule-init.\n\nFixes:\n\n * a more faithful conversion by staying on stdout (We switch to stderr later\n   in another series)\n * use the existing find_last_dir_sep instead of reinventing the wheel.\n\nStefan Beller (2):\n  submodule: port resolve_relative_url from shell to C\n  submodule: port init from shell to C\n\n builtin/submodule--helper.c | 321 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh            | 118 +---------------\n t/t0060-path-utils.sh       |  42 ++++++\n 3 files changed, 361 insertions(+), 120 deletions(-)\n\nInterdiff to  origin/sb/submodule-init (committed 2016-01-20 12:55:30)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 4684f16..c9b0c05 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -51,21 +51,12 @@ static int starts_with_dot_dot_slash(const char *str)\n        return str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n }\n \n-static char *last_dir_separator(char *str)\n-{\n-       char *p = str + strlen(str);\n-       while (p-- > str)\n-               if (is_dir_sep(*p))\n-                       return p;\n-       return NULL;\n-}\n-\n /*\n  * Returns 1 if it was the last chop before ':'.\n  */\n static int chop_last_dir(char **remoteurl, int is_relative)\n {\n-       char *rfind = last_dir_separator(*remoteurl);\n+       char *rfind = find_last_dir_sep(*remoteurl);\n        if (rfind) {\n                *rfind = '\\0';\n                return 0;\n@@ -232,7 +223,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n        struct strbuf sb = STRBUF_INIT;\n        char *url = NULL;\n        const char *upd = NULL;\n-       const char *displaypath = relative_path(xgetcwd(), prefix, &sb);;\n+       const char *displaypath = relative_path(xgetcwd(), prefix, &sb);\n \n        /* Only loads from .gitmodules, no overlay with .git/config */\n        gitmodules_config();\n@@ -276,7 +267,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n                        die(_(\"Failed to register url for submodule path '%s'\"),\n                            displaypath);\n                if (!quiet)\n-                       fprintf(stderr, _(\"Submodule '%s' (%s) registered for path '%s'\\n\"),\n+                       printf(_(\"Submodule '%s' (%s) registered for path '%s'\\n\"),\n                                sub->name, url, displaypath);\n                free(url);\n        }\n\n-- \n2.7.0.rc0.45.g0dba895.dirty\n"},{"id":"276593","messageId":"1453505536-14895-2-git-send-email-sbeller@google.com","threadId":"41246","inReplyTo":"1453505536-14895-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 1/2] submodule: port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-22T23:32:15Z","receivedAt":"2016-01-22T23:32:15Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Later on we want to automatically call `git submodule init` from\nother commands, such that the users don't have to initialize the\nsubmodule themselves.  As these other commands are written in C\nalready, we'd need the init functionality in C, too.  The\n`resolve_relative_url` function is a large part of that init\nfunctionality, so start by porting this function to C.\n\nTo create the tests in t0060, the function `resolve_relative_url`\nwas temporarily enhanced to write all inputs and output to disk\nwhen running the test suite. The added tests in this patch are\na small selection thereof.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 206 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh            |  81 +----------------\n t/t0060-path-utils.sh       |  42 +++++++++\n 3 files changed, 251 insertions(+), 78 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 8002187..92d7d32 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -9,6 +9,208 @@\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n+#include \"refs.h\"\n+#include \"connect.h\"\n+\n+static char *get_default_remote(void)\n+{\n+\tchar *dest = NULL, *ret;\n+\tunsigned char sha1[20];\n+\tint flag = 0;\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+\t/* detached HEAD */\n+\tif (!strcmp(refname, \"HEAD\"))\n+\t\treturn xstrdup(\"origin\");\n+\n+\tif (!skip_prefix(refname, \"refs/heads/\", &refname))\n+\t\tdie(_(\"Expecting a full ref name, got %s\"), refname);\n+\n+\tstrbuf_addf(&sb, \"branch.%s.remote\", refname);\n+\tif (git_config_get_string(sb.buf, &dest))\n+\t\tret = xstrdup(\"origin\");\n+\telse\n+\t\tret = xstrdup(dest);\n+\n+\tstrbuf_release(&sb);\n+\treturn ret;\n+}\n+\n+static int starts_with_dot_slash(const char *str)\n+{\n+\treturn str[0] == '.' && is_dir_sep(str[1]);\n+}\n+\n+static int starts_with_dot_dot_slash(const char *str)\n+{\n+\treturn str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n+}\n+\n+/*\n+ * Returns 1 if it was the last chop before ':'.\n+ */\n+static int chop_last_dir(char **remoteurl, int is_relative)\n+{\n+\tchar *rfind = find_last_dir_sep(*remoteurl);\n+\tif (rfind) {\n+\t\t*rfind = '\\0';\n+\t\treturn 0;\n+\t}\n+\n+\trfind = strrchr(*remoteurl, ':');\n+\tif (rfind) {\n+\t\t*rfind = '\\0';\n+\t\treturn 1;\n+\t}\n+\n+\tif (is_relative || !strcmp(\".\", *remoteurl))\n+\t\tdie(_(\"cannot strip one component off url '%s'\"),\n+\t\t\t*remoteurl);\n+\n+\tfree(*remoteurl);\n+\t*remoteurl = xstrdup(\".\");\n+\treturn 0;\n+}\n+\n+/*\n+ * The `url` argument is the URL that navigates to the submodule origin\n+ * repo. When relative, this URL is relative to the superproject origin\n+ * URL repo. The `up_path` argument, if specified, is the relative\n+ * path that navigates from the submodule working tree to the superproject\n+ * working tree. Returns the origin URL of the submodule.\n+ *\n+ * Return either an absolute URL or filesystem path (if the superproject\n+ * origin URL is an absolute URL or filesystem path, respectively) or a\n+ * relative file system path (if the superproject origin URL is a relative\n+ * 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+ * NEEDSWORK: This works incorrectly on the domain and protocol part.\n+ * remote_url      url              outcome          correct\n+ * http://a.com/b  ../c             http://a.com/c   yes\n+ * http://a.com/b  ../../c          http://c         no (domain should be kept)\n+ * http://a.com/b  ../../../c       http:/c          no\n+ * http://a.com/b  ../../../../c    http:c           no\n+ * http://a.com/b  ../../../../../c    .:c           no\n+ */\n+static char *relative_url(const char *remote_url,\n+\t\t\t\tconst char *url,\n+\t\t\t\tconst char *up_path)\n+{\n+\tint is_relative = 0;\n+\tint colonsep = 0;\n+\tchar *out;\n+\tchar *remoteurl = xstrdup(remote_url);\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tsize_t len = strlen(remoteurl);\n+\n+\tif (is_dir_sep(remoteurl[len]))\n+\t\tremoteurl[len] = '\\0';\n+\n+\tif (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n+\t\tis_relative = 0;\n+\telse {\n+\t\tis_relative = 1;\n+\t\t/*\n+\t\t * Prepend a './' to ensure all relative\n+\t\t * remoteurls start with './' or '../'\n+\t\t */\n+\t\tif (!starts_with_dot_slash(remoteurl) &&\n+\t\t    !starts_with_dot_dot_slash(remoteurl)) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n+\t\t\tfree(remoteurl);\n+\t\t\tremoteurl = strbuf_detach(&sb, NULL);\n+\t\t}\n+\t}\n+\t/*\n+\t * When the url starts with '../', remove that and the\n+\t * last directory in remoteurl.\n+\t */\n+\twhile (url) {\n+\t\tif (starts_with_dot_dot_slash(url)) {\n+\t\t\turl += 3;\n+\t\t\tcolonsep |= chop_last_dir(&remoteurl, is_relative);\n+\t\t} else if (starts_with_dot_slash(url))\n+\t\t\turl += 2;\n+\t\telse\n+\t\t\tbreak;\n+\t}\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, colonsep ? \":\" : \"/\", url);\n+\n+\tif (starts_with_dot_slash(sb.buf))\n+\t\tout = xstrdup(sb.buf + 2);\n+\telse\n+\t\tout = xstrdup(sb.buf);\n+\tstrbuf_reset(&sb);\n+\n+\tfree(remoteurl);\n+\tif (!up_path || !is_relative)\n+\t\treturn out;\n+\n+\tstrbuf_addf(&sb, \"%s%s\", up_path, out);\n+\tfree(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+\tchar *remoteurl = NULL;\n+\tchar *remote = get_default_remote();\n+\tconst char *up_path = NULL;\n+\tchar *res;\n+\tconst char *url;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (argc != 2 && argc != 3)\n+\t\tdie(\"resolve-relative-url only accepts one or two arguments\");\n+\n+\turl = argv[1];\n+\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\tfree(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 (argc == 3)\n+\t\tup_path = argv[2];\n+\n+\tres = relative_url(remoteurl, url, up_path);\n+\tputs(res);\n+\tfree(res);\n+\treturn 0;\n+}\n+\n+static int resolve_relative_url_test(int argc, const char **argv, const char *prefix)\n+{\n+\tchar *remoteurl, *res;\n+\tconst char *up_path, *url;\n+\n+\tif (argc != 4)\n+\t\tdie(\"resolve-relative-url-test only accepts three arguments: <up_path> <remoteurl> <url>\");\n+\n+\tup_path = argv[1];\n+\tremoteurl = xstrdup(argv[2]);\n+\turl = argv[3];\n+\n+\tif (!strcmp(up_path, \"(null)\"))\n+\t\tup_path = NULL;\n+\n+\tres = relative_url(remoteurl, url, up_path);\n+\tputs(res);\n+\tfree(res);\n+\treturn 0;\n+}\n \n struct module_list {\n \tconst struct cache_entry **entries;\n@@ -502,7 +704,9 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list},\n \t{\"name\", module_name},\n \t{\"clone\", module_clone},\n-\t{\"update-clone\", update_clone}\n+\t{\"update-clone\", update_clone},\n+\t{\"resolve-relative-url\", resolve_relative_url},\n+\t{\"resolve-relative-url-test\", resolve_relative_url_test},\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 10c5af9..615ef9b 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@@ -1176,9 +1103,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\"\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 627ef85..8a1579c 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -19,6 +19,13 @@ relative_path() {\n \t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$expected'\"\n }\n \n+test_submodule_relative_url() {\n+\ttest_expect_success \"test_submodule_relative_url: $1 $2 $3 => $4\" \"\n+\t\tactual=\\$(git submodule--helper resolve-relative-url-test '$1' '$2' '$3') &&\n+\t\ttest \\\"\\$actual\\\" = '$4'\n+\t\"\n+}\n+\n test_git_path() {\n \ttest_expect_success \"git-path $1 $2 => $3\" \"\n \t\t$1 git rev-parse --git-path $2 >actual &&\n@@ -286,4 +293,39 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config\n test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs\n test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow\n \n+test_submodule_relative_url \"(null)\" \"../foo/bar\" \"../sub/a/b/c\" \"../foo/sub/a/b/c\"\n+test_submodule_relative_url \"../../../\" \"../foo/bar\" \"../sub/a/b/c\" \"../../../../foo/sub/a/b/c\"\n+test_submodule_relative_url \"(null)\" \"../foo/bar\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"../\" \"../foo/bar\" \"../submodule\" \"../../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"../foo/submodule\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"../\" \"../foo/submodule\" \"../submodule\" \"../../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"../foo\" \"../submodule\" \"../submodule\"\n+test_submodule_relative_url \"../\" \"../foo\" \"../submodule\" \"../../submodule\"\n+test_submodule_relative_url \"(null)\" \"./foo/bar\" \"../submodule\" \"foo/submodule\"\n+test_submodule_relative_url \"../\" \"./foo/bar\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"./foo\" \"../submodule\" \"submodule\"\n+test_submodule_relative_url \"../\" \"./foo\" \"../submodule\" \"../submodule\"\n+test_submodule_relative_url \"(null)\" \"//somewhere else/repo\" \"../subrepo\" \"//somewhere else/subrepo\"\n+test_submodule_relative_url \"(null)\" \"/u//trash directory.t7406-submodule-update/subsuper_update_r\" \"../subsubsuper_update_r\" \"/u//trash directory.t7406-submodule-update/subsubsuper_update_r\"\n+test_submodule_relative_url \"(null)\" \"/u//trash directory.t7406-submodule-update/super_update_r2\" \"../subsuper_update_r\" \"/u//trash directory.t7406-submodule-update/subsuper_update_r\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t3600-rm/.\" \"../.\" \"/u/trash directory.t3600-rm/.\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t3600-rm\" \"./.\" \"/u/trash directory.t3600-rm/.\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7400-submodule-basic/addtest\" \"../repo\" \"/u/trash directory.t7400-submodule-basic/repo\"\n+test_submodule_relative_url \"../\" \"/u/trash directory.t7400-submodule-basic/addtest\" \"../repo\" \"/u/trash directory.t7400-submodule-basic/repo\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7400-submodule-basic\" \"./å äö\" \"/u/trash directory.t7400-submodule-basic/å äö\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7403-submodule-sync/.\" \"../submodule\" \"/u/trash directory.t7403-submodule-sync/submodule\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7407-submodule-foreach/submodule\" \"../submodule\" \"/u/trash directory.t7407-submodule-foreach/submodule\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7409-submodule-detached-worktree/home2/../remote\" \"../bundle1\" \"/u/trash directory.t7409-submodule-detached-worktree/home2/../bundle1\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7613-merge-submodule/submodule_update_repo\" \"./.\" \"/u/trash directory.t7613-merge-submodule/submodule_update_repo/.\"\n+test_submodule_relative_url \"(null)\" \"file:///tmp/repo\" \"../subrepo\" \"file:///tmp/subrepo\"\n+test_submodule_relative_url \"(null)\" \"foo/bar\" \"../submodule\" \"foo/submodule\"\n+test_submodule_relative_url \"../\" \"foo/bar\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"foo\" \"../submodule\" \"submodule\"\n+test_submodule_relative_url \"../\" \"foo\" \"../submodule\" \"../submodule\"\n+test_submodule_relative_url \"(null)\" \"helper:://hostname/repo\" \"../subrepo\" \"helper:://hostname/subrepo\"\n+test_submodule_relative_url \"(null)\" \"ssh://hostname/repo\" \"../subrepo\" \"ssh://hostname/subrepo\"\n+test_submodule_relative_url \"(null)\" \"ssh://hostname:22/repo\" \"../subrepo\" \"ssh://hostname:22/subrepo\"\n+test_submodule_relative_url \"(null)\" \"user@host:path/to/repo\" \"../subrepo\" \"user@host:path/to/subrepo\"\n+test_submodule_relative_url \"(null)\" \"user@host:repo\" \"../subrepo\" \"user@host:subrepo\"\n+\n test_done\n-- \n2.7.0.rc0.45.g0dba895.dirty\n"},{"id":"276591","messageId":"1453505536-14895-3-git-send-email-sbeller@google.com","threadId":"41246","inReplyTo":"1453505536-14895-1-git-send-email-sbeller@google.com","subject":"[PATCHv3 2/2] submodule: port init from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-22T23:32:16Z","receivedAt":"2016-01-22T23:32:16Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"By having the `init` functionality in C, we can reference it easier\nfrom other parts in the code.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 115 ++++++++++++++++++++++++++++++++++++++++++--\n git-submodule.sh            |  39 +--------------\n 2 files changed, 111 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 92d7d32..c9b0c05 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -212,6 +212,115 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr\n \treturn 0;\n }\n \n+static int git_submodule_config(const char *var, const char *value, void *cb)\n+{\n+\treturn parse_submodule_config_option(var, value);\n+}\n+\n+static void init_submodule(const char *path, const char *prefix, int quiet)\n+{\n+\tconst struct submodule *sub;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tchar *url = NULL;\n+\tconst char *upd = NULL;\n+\tconst char *displaypath = relative_path(xgetcwd(), prefix, &sb);\n+\n+\t/* Only loads from .gitmodules, no overlay with .git/config */\n+\tgitmodules_config();\n+\n+\tsub = submodule_from_path(null_sha1, path);\n+\n+\t/*\n+\t * Copy url setting when it is not set yet.\n+\t * To look up the url in .git/config, we must not fall back to\n+\t * .gitmodules, so look it up directly.\n+\t */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tif (git_config_get_string(sb.buf, &url)) {\n+\t\turl = xstrdup(sub->url);\n+\n+\t\tif (!url)\n+\t\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n+\t\t\t\tdisplaypath);\n+\n+\t\t/* Possibly a url relative to parent */\n+\t\tif (starts_with_dot_dot_slash(url) ||\n+\t\t    starts_with_dot_slash(url)) {\n+\t\t\tchar *remoteurl;\n+\t\t\tchar *remote = get_default_remote();\n+\t\t\tstruct strbuf remotesb = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n+\t\t\tfree(remote);\n+\n+\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n+\t\t\t\t/*\n+\t\t\t\t * The repository is its own\n+\t\t\t\t * authoritative upstream\n+\t\t\t\t */\n+\t\t\t\tremoteurl = xgetcwd();\n+\t\t\turl = relative_url(remoteurl, url, NULL);\n+\t\t\tstrbuf_release(&remotesb);\n+\t\t}\n+\n+\t\tif (git_config_set(sb.buf, url))\n+\t\t\tdie(_(\"Failed to register url for submodule path '%s'\"),\n+\t\t\t    displaypath);\n+\t\tif (!quiet)\n+\t\t\tprintf(_(\"Submodule '%s' (%s) registered for path '%s'\\n\"),\n+\t\t\t\tsub->name, url, displaypath);\n+\t\tfree(url);\n+\t}\n+\n+\t/* Copy \"update\" setting when it is not set yet */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.update\", sub->name);\n+\tif (git_config_get_string_const(sb.buf, &upd) && sub->update) {\n+\t\tupd = sub->update;\n+\t\tif (strcmp(sub->update, \"checkout\") &&\n+\t\t    strcmp(sub->update, \"rebase\") &&\n+\t\t    strcmp(sub->update, \"merge\") &&\n+\t\t    strcmp(sub->update, \"none\")) {\n+\t\t\tfprintf(stderr, _(\"warning: unknown update mode '%s' suggested for submodule '%s'\\n\"),\n+\t\t\t\tupd, sub->name);\n+\t\t\tupd = \"none\";\n+\t\t}\n+\t\tif (git_config_set(sb.buf, upd))\n+\t\t\tdie(_(\"Failed to register update mode for submodule path '%s'\"), displaypath);\n+\t}\n+\tstrbuf_release(&sb);\n+}\n+\n+static int module_init(int argc, const char **argv, const char *prefix)\n+{\n+\tint quiet = 0;\n+\tint i;\n+\n+\tstruct option module_init_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\t\t   N_(\"path\"),\n+\t\t\t   N_(\"alternative anchor for relative paths\")),\n+\t\tOPT__QUIET(&quiet, \"Suppress output for initialzing a submodule\"),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper init [<path>]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_init_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (argc == 0)\n+\t\tdie(_(\"Pass at least one submodule\"));\n+\n+\tfor (i = 0; i < argc; i++)\n+\t\tinit_submodule(argv[i], prefix, quiet);\n+\n+\treturn 0;\n+}\n+\n struct module_list {\n \tconst struct cache_entry **entries;\n \tint alloc, nr;\n@@ -457,11 +566,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static int git_submodule_config(const char *var, const char *value, void *cb)\n-{\n-\treturn parse_submodule_config_option(var, value);\n-}\n-\n struct submodule_update_clone {\n \t/* states */\n \tint count;\n@@ -707,6 +811,7 @@ static struct cmd_struct commands[] = {\n \t{\"update-clone\", update_clone},\n \t{\"resolve-relative-url\", resolve_relative_url},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test},\n+\t{\"init\", module_init}\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 615ef9b..6fce0dc 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -398,45 +398,8 @@ cmd_init()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\n-\t\tdisplaypath=$(relative_path \"$sm_path\")\n-\n-\t\t# Copy url setting when it is not set yet\n-\t\tif test -z \"$(git config \"submodule.$name.url\")\"\n-\t\tthen\n-\t\t\turl=$(git config -f .gitmodules submodule.\"$name\".url)\n-\t\t\ttest -z \"$url\" &&\n-\t\t\tdie \"$(eval_gettext \"No url found for submodule path '\\$displaypath' in .gitmodules\")\"\n-\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=$(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-\t\t\tdie \"$(eval_gettext \"Failed to register url for submodule path '\\$displaypath'\")\"\n \n-\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) registered for path '\\$displaypath'\")\"\n-\t\tfi\n-\n-\t\t# Copy \"update\" setting when it is not set yet\n-\t\tif upd=\"$(git config -f .gitmodules submodule.\"$name\".update)\" &&\n-\t\t   test -n \"$upd\" &&\n-\t\t   test -z \"$(git config submodule.\"$name\".update)\"\n-\t\tthen\n-\t\t\tcase \"$upd\" in\n-\t\t\tcheckout | rebase | merge | none)\n-\t\t\t\t;; # known modes of updating\n-\t\t\t*)\n-\t\t\t\techo >&2 \"warning: unknown update mode '$upd' suggested for submodule '$name'\"\n-\t\t\t\tupd=none\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\tgit config submodule.\"$name\".update \"$upd\" ||\n-\t\t\tdie \"$(eval_gettext \"Failed to register update mode for submodule path '\\$displaypath'\")\"\n-\t\tfi\n+\t\tgit submodule--helper init ${GIT_QUIET:+--quiet} \"$sm_path\" || exit\n \tdone\n }\n \n-- \n2.7.0.rc0.45.g0dba895.dirty\n"},{"id":"276744","messageId":"xmqqtwm1e0ps.fsf@gitster.mtv.corp.google.com","threadId":"41246","inReplyTo":"1453505536-14895-1-git-send-email-sbeller@google.com","subject":"Re: [PATCHv3 0/2] Port `git submodule init` from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-25T22:46:55Z","receivedAt":"2016-01-25T22:46:55Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> This applies on top of sb/submodule-parallel-update, replacing\n> sb/submodule-init.\n>\n> Fixes:\n>\n>  * a more faithful conversion by staying on stdout (We switch to stderr later\n>    in another series)\n>  * use the existing find_last_dir_sep instead of reinventing the wheel.\n>\n> Stefan Beller (2):\n>   submodule: port resolve_relative_url from shell to C\n>   submodule: port init from shell to C\n\nThanks, will replace.\n"},{"id":"276949","messageId":"1453948237-9860-1-git-send-email-sbeller@google.com","threadId":"41246","inReplyTo":"xmqqtwm1e0ps.fsf@gitster.mtv.corp.google.com","subject":"[PATCHv4 0/2] Port `git submodule init` from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-28T02:30:35Z","receivedAt":"2016-01-28T02:30:35Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Hi,\n\nI looked at this patch series again and fixed all memory leaks as found by\ncoverity scan.\n\nThis applies on top of sb/submodule-parallel-update\n\nThanks,\nStefan\n\nInterdiff to v3:\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex c9b0c05..dd8b2a5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -35,7 +35,7 @@ static char *get_default_remote(void)\n \tif (git_config_get_string(sb.buf, &dest))\n \t\tret = xstrdup(\"origin\");\n \telse\n-\t\tret = xstrdup(dest);\n+\t\tret = dest;\n \n \tstrbuf_release(&sb);\n \treturn ret;\n@@ -188,6 +188,7 @@ static int resolve_relative_url(int argc, const char **argv, const char *prefix)\n \tres = relative_url(remoteurl, url, up_path);\n \tputs(res);\n \tfree(res);\n+\tfree(remoteurl);\n \treturn 0;\n }\n \n@@ -209,6 +210,7 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr\n \tres = relative_url(remoteurl, url, up_path);\n \tputs(res);\n \tfree(res);\n+\tfree(remoteurl);\n \treturn 0;\n }\n \n@@ -222,8 +224,9 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \tconst struct submodule *sub;\n \tstruct strbuf sb = STRBUF_INIT;\n \tchar *url = NULL;\n-\tconst char *upd = NULL;\n-\tconst char *displaypath = relative_path(xgetcwd(), prefix, &sb);\n+\tchar *upd = NULL;\n+\tchar *cwd = xgetcwd();\n+\tconst char *displaypath = relative_path(cwd, prefix, &sb);\n \n \t/* Only loads from .gitmodules, no overlay with .git/config */\n \tgitmodules_config();\n@@ -261,6 +264,7 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t\t\t\tremoteurl = xgetcwd();\n \t\t\turl = relative_url(remoteurl, url, NULL);\n \t\t\tstrbuf_release(&remotesb);\n+\t\t\tfree(remoteurl);\n \t\t}\n \n \t\tif (git_config_set(sb.buf, url))\n@@ -269,26 +273,27 @@ static void init_submodule(const char *path, const char *prefix, int quiet)\n \t\tif (!quiet)\n \t\t\tprintf(_(\"Submodule '%s' (%s) registered for path '%s'\\n\"),\n \t\t\t\tsub->name, url, displaypath);\n-\t\tfree(url);\n \t}\n \n \t/* Copy \"update\" setting when it is not set yet */\n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"submodule.%s.update\", sub->name);\n-\tif (git_config_get_string_const(sb.buf, &upd) && sub->update) {\n-\t\tupd = sub->update;\n+\tif (git_config_get_string(sb.buf, &upd) && sub->update) {\n+\t\tupd = xstrdup(sub->update);\n \t\tif (strcmp(sub->update, \"checkout\") &&\n \t\t    strcmp(sub->update, \"rebase\") &&\n \t\t    strcmp(sub->update, \"merge\") &&\n \t\t    strcmp(sub->update, \"none\")) {\n \t\t\tfprintf(stderr, _(\"warning: unknown update mode '%s' suggested for submodule '%s'\\n\"),\n \t\t\t\tupd, sub->name);\n-\t\t\tupd = \"none\";\n+\t\t\tupd = xstrdup(\"none\");\n \t\t}\n \t\tif (git_config_set(sb.buf, upd))\n \t\t\tdie(_(\"Failed to register update mode for submodule path '%s'\"), displaypath);\n \t}\n \tstrbuf_release(&sb);\n+\tfree(cwd);\n+\tfree(upd);\n+\tfree(url);\n }\n \n static int module_init(int argc, const char **argv, const char *prefix)\n\n\nStefan Beller (2):\n  submodule: port resolve_relative_url from shell to C\n  submodule: port init from shell to C\n\n builtin/submodule--helper.c | 326 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh            | 118 +---------------\n t/t0060-path-utils.sh       |  42 ++++++\n 3 files changed, 366 insertions(+), 120 deletions(-)\n\n-- \n2.7.0.rc0.42.ge5f5e2d\n"},{"id":"276950","messageId":"1453948237-9860-2-git-send-email-sbeller@google.com","threadId":"41246","inReplyTo":"1453948237-9860-1-git-send-email-sbeller@google.com","subject":"[PATCHv4 1/2] submodule: port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-28T02:30:36Z","receivedAt":"2016-01-28T02:30:36Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Later on we want to automatically call `git submodule init` from\nother commands, such that the users don't have to initialize the\nsubmodule themselves.  As these other commands are written in C\nalready, we'd need the init functionality in C, too.  The\n`resolve_relative_url` function is a large part of that init\nfunctionality, so start by porting this function to C.\n\nTo create the tests in t0060, the function `resolve_relative_url`\nwas temporarily enhanced to write all inputs and output to disk\nwhen running the test suite. The added tests in this patch are\na small selection thereof.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 208 +++++++++++++++++++++++++++++++++++++++++++-\n git-submodule.sh            |  81 +----------------\n t/t0060-path-utils.sh       |  42 +++++++++\n 3 files changed, 253 insertions(+), 78 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 8002187..13583d9 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -9,6 +9,210 @@\n #include \"submodule-config.h\"\n #include \"string-list.h\"\n #include \"run-command.h\"\n+#include \"remote.h\"\n+#include \"refs.h\"\n+#include \"connect.h\"\n+\n+static char *get_default_remote(void)\n+{\n+\tchar *dest = NULL, *ret;\n+\tunsigned char sha1[20];\n+\tint flag = 0;\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+\t/* detached HEAD */\n+\tif (!strcmp(refname, \"HEAD\"))\n+\t\treturn xstrdup(\"origin\");\n+\n+\tif (!skip_prefix(refname, \"refs/heads/\", &refname))\n+\t\tdie(_(\"Expecting a full ref name, got %s\"), refname);\n+\n+\tstrbuf_addf(&sb, \"branch.%s.remote\", refname);\n+\tif (git_config_get_string(sb.buf, &dest))\n+\t\tret = xstrdup(\"origin\");\n+\telse\n+\t\tret = dest;\n+\n+\tstrbuf_release(&sb);\n+\treturn ret;\n+}\n+\n+static int starts_with_dot_slash(const char *str)\n+{\n+\treturn str[0] == '.' && is_dir_sep(str[1]);\n+}\n+\n+static int starts_with_dot_dot_slash(const char *str)\n+{\n+\treturn str[0] == '.' && str[1] == '.' && is_dir_sep(str[2]);\n+}\n+\n+/*\n+ * Returns 1 if it was the last chop before ':'.\n+ */\n+static int chop_last_dir(char **remoteurl, int is_relative)\n+{\n+\tchar *rfind = find_last_dir_sep(*remoteurl);\n+\tif (rfind) {\n+\t\t*rfind = '\\0';\n+\t\treturn 0;\n+\t}\n+\n+\trfind = strrchr(*remoteurl, ':');\n+\tif (rfind) {\n+\t\t*rfind = '\\0';\n+\t\treturn 1;\n+\t}\n+\n+\tif (is_relative || !strcmp(\".\", *remoteurl))\n+\t\tdie(_(\"cannot strip one component off url '%s'\"),\n+\t\t\t*remoteurl);\n+\n+\tfree(*remoteurl);\n+\t*remoteurl = xstrdup(\".\");\n+\treturn 0;\n+}\n+\n+/*\n+ * The `url` argument is the URL that navigates to the submodule origin\n+ * repo. When relative, this URL is relative to the superproject origin\n+ * URL repo. The `up_path` argument, if specified, is the relative\n+ * path that navigates from the submodule working tree to the superproject\n+ * working tree. Returns the origin URL of the submodule.\n+ *\n+ * Return either an absolute URL or filesystem path (if the superproject\n+ * origin URL is an absolute URL or filesystem path, respectively) or a\n+ * relative file system path (if the superproject origin URL is a relative\n+ * 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+ * NEEDSWORK: This works incorrectly on the domain and protocol part.\n+ * remote_url      url              outcome          correct\n+ * http://a.com/b  ../c             http://a.com/c   yes\n+ * http://a.com/b  ../../c          http://c         no (domain should be kept)\n+ * http://a.com/b  ../../../c       http:/c          no\n+ * http://a.com/b  ../../../../c    http:c           no\n+ * http://a.com/b  ../../../../../c    .:c           no\n+ */\n+static char *relative_url(const char *remote_url,\n+\t\t\t\tconst char *url,\n+\t\t\t\tconst char *up_path)\n+{\n+\tint is_relative = 0;\n+\tint colonsep = 0;\n+\tchar *out;\n+\tchar *remoteurl = xstrdup(remote_url);\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tsize_t len = strlen(remoteurl);\n+\n+\tif (is_dir_sep(remoteurl[len]))\n+\t\tremoteurl[len] = '\\0';\n+\n+\tif (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n+\t\tis_relative = 0;\n+\telse {\n+\t\tis_relative = 1;\n+\t\t/*\n+\t\t * Prepend a './' to ensure all relative\n+\t\t * remoteurls start with './' or '../'\n+\t\t */\n+\t\tif (!starts_with_dot_slash(remoteurl) &&\n+\t\t    !starts_with_dot_dot_slash(remoteurl)) {\n+\t\t\tstrbuf_reset(&sb);\n+\t\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n+\t\t\tfree(remoteurl);\n+\t\t\tremoteurl = strbuf_detach(&sb, NULL);\n+\t\t}\n+\t}\n+\t/*\n+\t * When the url starts with '../', remove that and the\n+\t * last directory in remoteurl.\n+\t */\n+\twhile (url) {\n+\t\tif (starts_with_dot_dot_slash(url)) {\n+\t\t\turl += 3;\n+\t\t\tcolonsep |= chop_last_dir(&remoteurl, is_relative);\n+\t\t} else if (starts_with_dot_slash(url))\n+\t\t\turl += 2;\n+\t\telse\n+\t\t\tbreak;\n+\t}\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, colonsep ? \":\" : \"/\", url);\n+\n+\tif (starts_with_dot_slash(sb.buf))\n+\t\tout = xstrdup(sb.buf + 2);\n+\telse\n+\t\tout = xstrdup(sb.buf);\n+\tstrbuf_reset(&sb);\n+\n+\tfree(remoteurl);\n+\tif (!up_path || !is_relative)\n+\t\treturn out;\n+\n+\tstrbuf_addf(&sb, \"%s%s\", up_path, out);\n+\tfree(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+\tchar *remoteurl = NULL;\n+\tchar *remote = get_default_remote();\n+\tconst char *up_path = NULL;\n+\tchar *res;\n+\tconst char *url;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\n+\tif (argc != 2 && argc != 3)\n+\t\tdie(\"resolve-relative-url only accepts one or two arguments\");\n+\n+\turl = argv[1];\n+\tstrbuf_addf(&sb, \"remote.%s.url\", remote);\n+\tfree(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 (argc == 3)\n+\t\tup_path = argv[2];\n+\n+\tres = relative_url(remoteurl, url, up_path);\n+\tputs(res);\n+\tfree(res);\n+\tfree(remoteurl);\n+\treturn 0;\n+}\n+\n+static int resolve_relative_url_test(int argc, const char **argv, const char *prefix)\n+{\n+\tchar *remoteurl, *res;\n+\tconst char *up_path, *url;\n+\n+\tif (argc != 4)\n+\t\tdie(\"resolve-relative-url-test only accepts three arguments: <up_path> <remoteurl> <url>\");\n+\n+\tup_path = argv[1];\n+\tremoteurl = xstrdup(argv[2]);\n+\turl = argv[3];\n+\n+\tif (!strcmp(up_path, \"(null)\"))\n+\t\tup_path = NULL;\n+\n+\tres = relative_url(remoteurl, url, up_path);\n+\tputs(res);\n+\tfree(res);\n+\tfree(remoteurl);\n+\treturn 0;\n+}\n \n struct module_list {\n \tconst struct cache_entry **entries;\n@@ -502,7 +706,9 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list},\n \t{\"name\", module_name},\n \t{\"clone\", module_clone},\n-\t{\"update-clone\", update_clone}\n+\t{\"update-clone\", update_clone},\n+\t{\"resolve-relative-url\", resolve_relative_url},\n+\t{\"resolve-relative-url-test\", resolve_relative_url_test},\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 10c5af9..615ef9b 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@@ -1176,9 +1103,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\"\ndiff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh\nindex 627ef85..8a1579c 100755\n--- a/t/t0060-path-utils.sh\n+++ b/t/t0060-path-utils.sh\n@@ -19,6 +19,13 @@ relative_path() {\n \t\"test \\\"\\$(test-path-utils relative_path '$1' '$2')\\\" = '$expected'\"\n }\n \n+test_submodule_relative_url() {\n+\ttest_expect_success \"test_submodule_relative_url: $1 $2 $3 => $4\" \"\n+\t\tactual=\\$(git submodule--helper resolve-relative-url-test '$1' '$2' '$3') &&\n+\t\ttest \\\"\\$actual\\\" = '$4'\n+\t\"\n+}\n+\n test_git_path() {\n \ttest_expect_success \"git-path $1 $2 => $3\" \"\n \t\t$1 git rev-parse --git-path $2 >actual &&\n@@ -286,4 +293,39 @@ test_git_path GIT_COMMON_DIR=bar config                   bar/config\n test_git_path GIT_COMMON_DIR=bar packed-refs              bar/packed-refs\n test_git_path GIT_COMMON_DIR=bar shallow                  bar/shallow\n \n+test_submodule_relative_url \"(null)\" \"../foo/bar\" \"../sub/a/b/c\" \"../foo/sub/a/b/c\"\n+test_submodule_relative_url \"../../../\" \"../foo/bar\" \"../sub/a/b/c\" \"../../../../foo/sub/a/b/c\"\n+test_submodule_relative_url \"(null)\" \"../foo/bar\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"../\" \"../foo/bar\" \"../submodule\" \"../../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"../foo/submodule\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"../\" \"../foo/submodule\" \"../submodule\" \"../../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"../foo\" \"../submodule\" \"../submodule\"\n+test_submodule_relative_url \"../\" \"../foo\" \"../submodule\" \"../../submodule\"\n+test_submodule_relative_url \"(null)\" \"./foo/bar\" \"../submodule\" \"foo/submodule\"\n+test_submodule_relative_url \"../\" \"./foo/bar\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"./foo\" \"../submodule\" \"submodule\"\n+test_submodule_relative_url \"../\" \"./foo\" \"../submodule\" \"../submodule\"\n+test_submodule_relative_url \"(null)\" \"//somewhere else/repo\" \"../subrepo\" \"//somewhere else/subrepo\"\n+test_submodule_relative_url \"(null)\" \"/u//trash directory.t7406-submodule-update/subsuper_update_r\" \"../subsubsuper_update_r\" \"/u//trash directory.t7406-submodule-update/subsubsuper_update_r\"\n+test_submodule_relative_url \"(null)\" \"/u//trash directory.t7406-submodule-update/super_update_r2\" \"../subsuper_update_r\" \"/u//trash directory.t7406-submodule-update/subsuper_update_r\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t3600-rm/.\" \"../.\" \"/u/trash directory.t3600-rm/.\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t3600-rm\" \"./.\" \"/u/trash directory.t3600-rm/.\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7400-submodule-basic/addtest\" \"../repo\" \"/u/trash directory.t7400-submodule-basic/repo\"\n+test_submodule_relative_url \"../\" \"/u/trash directory.t7400-submodule-basic/addtest\" \"../repo\" \"/u/trash directory.t7400-submodule-basic/repo\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7400-submodule-basic\" \"./å äö\" \"/u/trash directory.t7400-submodule-basic/å äö\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7403-submodule-sync/.\" \"../submodule\" \"/u/trash directory.t7403-submodule-sync/submodule\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7407-submodule-foreach/submodule\" \"../submodule\" \"/u/trash directory.t7407-submodule-foreach/submodule\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7409-submodule-detached-worktree/home2/../remote\" \"../bundle1\" \"/u/trash directory.t7409-submodule-detached-worktree/home2/../bundle1\"\n+test_submodule_relative_url \"(null)\" \"/u/trash directory.t7613-merge-submodule/submodule_update_repo\" \"./.\" \"/u/trash directory.t7613-merge-submodule/submodule_update_repo/.\"\n+test_submodule_relative_url \"(null)\" \"file:///tmp/repo\" \"../subrepo\" \"file:///tmp/subrepo\"\n+test_submodule_relative_url \"(null)\" \"foo/bar\" \"../submodule\" \"foo/submodule\"\n+test_submodule_relative_url \"../\" \"foo/bar\" \"../submodule\" \"../foo/submodule\"\n+test_submodule_relative_url \"(null)\" \"foo\" \"../submodule\" \"submodule\"\n+test_submodule_relative_url \"../\" \"foo\" \"../submodule\" \"../submodule\"\n+test_submodule_relative_url \"(null)\" \"helper:://hostname/repo\" \"../subrepo\" \"helper:://hostname/subrepo\"\n+test_submodule_relative_url \"(null)\" \"ssh://hostname/repo\" \"../subrepo\" \"ssh://hostname/subrepo\"\n+test_submodule_relative_url \"(null)\" \"ssh://hostname:22/repo\" \"../subrepo\" \"ssh://hostname:22/subrepo\"\n+test_submodule_relative_url \"(null)\" \"user@host:path/to/repo\" \"../subrepo\" \"user@host:path/to/subrepo\"\n+test_submodule_relative_url \"(null)\" \"user@host:repo\" \"../subrepo\" \"user@host:subrepo\"\n+\n test_done\n-- \n2.7.0.rc0.42.ge5f5e2d\n"},{"id":"276951","messageId":"1453948237-9860-3-git-send-email-sbeller@google.com","threadId":"41246","inReplyTo":"1453948237-9860-1-git-send-email-sbeller@google.com","subject":"[PATCHv4 2/2] submodule: port init from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-28T02:30:37Z","receivedAt":"2016-01-28T02:30:37Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"By having the `init` functionality in C, we can reference it easier\nfrom other parts in the code.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 119 ++++++++++++++++++++++++++++++++++++++++++--\n git-submodule.sh            |  39 +--------------\n 2 files changed, 115 insertions(+), 43 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 13583d9..689c354 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -214,6 +214,119 @@ static int resolve_relative_url_test(int argc, const char **argv, const char *pr\n \treturn 0;\n }\n \n+static int git_submodule_config(const char *var, const char *value, void *cb)\n+{\n+\treturn parse_submodule_config_option(var, value);\n+}\n+\n+static void init_submodule(const char *path, const char *prefix, int quiet)\n+{\n+\tconst struct submodule *sub;\n+\tstruct strbuf sb = STRBUF_INIT;\n+\tchar *url = NULL;\n+\tchar *upd = NULL;\n+\tchar *cwd = xgetcwd();\n+\tconst char *displaypath = relative_path(cwd, prefix, &sb);\n+\n+\t/* Only loads from .gitmodules, no overlay with .git/config */\n+\tgitmodules_config();\n+\n+\tsub = submodule_from_path(null_sha1, path);\n+\n+\t/*\n+\t * Copy url setting when it is not set yet.\n+\t * To look up the url in .git/config, we must not fall back to\n+\t * .gitmodules, so look it up directly.\n+\t */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.url\", sub->name);\n+\tif (git_config_get_string(sb.buf, &url)) {\n+\t\turl = xstrdup(sub->url);\n+\n+\t\tif (!url)\n+\t\t\tdie(_(\"No url found for submodule path '%s' in .gitmodules\"),\n+\t\t\t\tdisplaypath);\n+\n+\t\t/* Possibly a url relative to parent */\n+\t\tif (starts_with_dot_dot_slash(url) ||\n+\t\t    starts_with_dot_slash(url)) {\n+\t\t\tchar *remoteurl;\n+\t\t\tchar *remote = get_default_remote();\n+\t\t\tstruct strbuf remotesb = STRBUF_INIT;\n+\t\t\tstrbuf_addf(&remotesb, \"remote.%s.url\", remote);\n+\t\t\tfree(remote);\n+\n+\t\t\tif (git_config_get_string(remotesb.buf, &remoteurl))\n+\t\t\t\t/*\n+\t\t\t\t * The repository is its own\n+\t\t\t\t * authoritative upstream\n+\t\t\t\t */\n+\t\t\t\tremoteurl = xgetcwd();\n+\t\t\turl = relative_url(remoteurl, url, NULL);\n+\t\t\tstrbuf_release(&remotesb);\n+\t\t\tfree(remoteurl);\n+\t\t}\n+\n+\t\tif (git_config_set(sb.buf, url))\n+\t\t\tdie(_(\"Failed to register url for submodule path '%s'\"),\n+\t\t\t    displaypath);\n+\t\tif (!quiet)\n+\t\t\tprintf(_(\"Submodule '%s' (%s) registered for path '%s'\\n\"),\n+\t\t\t\tsub->name, url, displaypath);\n+\t}\n+\n+\t/* Copy \"update\" setting when it is not set yet */\n+\tstrbuf_reset(&sb);\n+\tstrbuf_addf(&sb, \"submodule.%s.update\", sub->name);\n+\tif (git_config_get_string(sb.buf, &upd) && sub->update) {\n+\t\tupd = xstrdup(sub->update);\n+\t\tif (strcmp(sub->update, \"checkout\") &&\n+\t\t    strcmp(sub->update, \"rebase\") &&\n+\t\t    strcmp(sub->update, \"merge\") &&\n+\t\t    strcmp(sub->update, \"none\")) {\n+\t\t\tfprintf(stderr, _(\"warning: unknown update mode '%s' suggested for submodule '%s'\\n\"),\n+\t\t\t\tupd, sub->name);\n+\t\t\tupd = xstrdup(\"none\");\n+\t\t}\n+\t\tif (git_config_set(sb.buf, upd))\n+\t\t\tdie(_(\"Failed to register update mode for submodule path '%s'\"), displaypath);\n+\t}\n+\tstrbuf_release(&sb);\n+\tfree(cwd);\n+\tfree(upd);\n+\tfree(url);\n+}\n+\n+static int module_init(int argc, const char **argv, const char *prefix)\n+{\n+\tint quiet = 0;\n+\tint i;\n+\n+\tstruct option module_init_options[] = {\n+\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\t\t   N_(\"path\"),\n+\t\t\t   N_(\"alternative anchor for relative paths\")),\n+\t\tOPT__QUIET(&quiet, \"Suppress output for initialzing a submodule\"),\n+\t\tOPT_END()\n+\t};\n+\n+\tconst char *const git_submodule_helper_usage[] = {\n+\t\tN_(\"git submodule--helper init [<path>]\"),\n+\t\tNULL\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, module_init_options,\n+\t\t\t     git_submodule_helper_usage, 0);\n+\n+\tif (argc == 0)\n+\t\tdie(_(\"Pass at least one submodule\"));\n+\n+\tfor (i = 0; i < argc; i++)\n+\t\tinit_submodule(argv[i], prefix, quiet);\n+\n+\treturn 0;\n+}\n+\n struct module_list {\n \tconst struct cache_entry **entries;\n \tint alloc, nr;\n@@ -459,11 +572,6 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static int git_submodule_config(const char *var, const char *value, void *cb)\n-{\n-\treturn parse_submodule_config_option(var, value);\n-}\n-\n struct submodule_update_clone {\n \t/* states */\n \tint count;\n@@ -709,6 +817,7 @@ static struct cmd_struct commands[] = {\n \t{\"update-clone\", update_clone},\n \t{\"resolve-relative-url\", resolve_relative_url},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test},\n+\t{\"init\", module_init}\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 615ef9b..6fce0dc 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -398,45 +398,8 @@ cmd_init()\n \twhile read mode sha1 stage sm_path\n \tdo\n \t\tdie_if_unmatched \"$mode\"\n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\n-\t\tdisplaypath=$(relative_path \"$sm_path\")\n-\n-\t\t# Copy url setting when it is not set yet\n-\t\tif test -z \"$(git config \"submodule.$name.url\")\"\n-\t\tthen\n-\t\t\turl=$(git config -f .gitmodules submodule.\"$name\".url)\n-\t\t\ttest -z \"$url\" &&\n-\t\t\tdie \"$(eval_gettext \"No url found for submodule path '\\$displaypath' in .gitmodules\")\"\n-\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=$(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-\t\t\tdie \"$(eval_gettext \"Failed to register url for submodule path '\\$displaypath'\")\"\n \n-\t\t\tsay \"$(eval_gettext \"Submodule '\\$name' (\\$url) registered for path '\\$displaypath'\")\"\n-\t\tfi\n-\n-\t\t# Copy \"update\" setting when it is not set yet\n-\t\tif upd=\"$(git config -f .gitmodules submodule.\"$name\".update)\" &&\n-\t\t   test -n \"$upd\" &&\n-\t\t   test -z \"$(git config submodule.\"$name\".update)\"\n-\t\tthen\n-\t\t\tcase \"$upd\" in\n-\t\t\tcheckout | rebase | merge | none)\n-\t\t\t\t;; # known modes of updating\n-\t\t\t*)\n-\t\t\t\techo >&2 \"warning: unknown update mode '$upd' suggested for submodule '$name'\"\n-\t\t\t\tupd=none\n-\t\t\t\t;;\n-\t\t\tesac\n-\t\t\tgit config submodule.\"$name\".update \"$upd\" ||\n-\t\t\tdie \"$(eval_gettext \"Failed to register update mode for submodule path '\\$displaypath'\")\"\n-\t\tfi\n+\t\tgit submodule--helper init ${GIT_QUIET:+--quiet} \"$sm_path\" || exit\n \tdone\n }\n \n-- \n2.7.0.rc0.42.ge5f5e2d\n"},{"id":"277013","messageId":"xmqqzivpl5uo.fsf@gitster.mtv.corp.google.com","threadId":"41246","inReplyTo":"1453948237-9860-2-git-send-email-sbeller@google.com","subject":"Re: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-01-28T22:03:11Z","receivedAt":"2016-01-28T22:03:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> +static char *relative_url(const char *remote_url,\n> +\t\t\t\tconst char *url,\n> +\t\t\t\tconst char *up_path)\n> +{\n> +\tint is_relative = 0;\n> +\tint colonsep = 0;\n> +\tchar *out;\n> +\tchar *remoteurl = xstrdup(remote_url);\n> +\tstruct strbuf sb = STRBUF_INIT;\n> +\tsize_t len = strlen(remoteurl);\n> +\n> +\tif (is_dir_sep(remoteurl[len]))\n> +\t\tremoteurl[len] = '\\0';\n> +\n> +\tif (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n> +\t\tis_relative = 0;\n> +\telse {\n> +\t\tis_relative = 1;\n> +\t\t/*\n> +\t\t * Prepend a './' to ensure all relative\n> +\t\t * remoteurls start with './' or '../'\n> +\t\t */\n> +\t\tif (!starts_with_dot_slash(remoteurl) &&\n> +\t\t    !starts_with_dot_dot_slash(remoteurl)) {\n> +\t\t\tstrbuf_reset(&sb);\n> +\t\t\tstrbuf_addf(&sb, \"./%s\", remoteurl);\n> +\t\t\tfree(remoteurl);\n> +\t\t\tremoteurl = strbuf_detach(&sb, NULL);\n> +\t\t}\n> +\t}\n> +\t/*\n> +\t * When the url starts with '../', remove that and the\n> +\t * last directory in remoteurl.\n> +\t */\n> +\twhile (url) {\n> +\t\tif (starts_with_dot_dot_slash(url)) {\n> +\t\t\turl += 3;\n> +\t\t\tcolonsep |= chop_last_dir(&remoteurl, is_relative);\n> +\t\t} else if (starts_with_dot_slash(url))\n> +\t\t\turl += 2;\n> +\t\telse\n> +\t\t\tbreak;\n> +\t}\n> +\tstrbuf_reset(&sb);\n> +\tstrbuf_addf(&sb, \"%s%s%s\", remoteurl, colonsep ? \":\" : \"/\", url);\n> +\n> +\tif (starts_with_dot_slash(sb.buf))\n> +\t\tout = xstrdup(sb.buf + 2);\n> +\telse\n> +\t\tout = xstrdup(sb.buf);\n> +\tstrbuf_reset(&sb);\n> +\n> +\tfree(remoteurl);\n\nThis is a rather strange place to put this free(), as you are done\nwith it a bit earlier, but it's OK.  I briefly wondered if the code\nbecomes easier to follow with fewer free(remoteurl) if this local\nvariable is made into a strbuf, but I didn't seriously think it\nthrough.\n\nOtherwise looking good.\n\nThanks.\n"},{"id":"277015","messageId":"CAGZ79kYbYEgW+O-wxp0LeT6W5XweV8e0jWzYDRS6vunnOJBKng@mail.gmail.com","threadId":"41246","inReplyTo":"xmqqzivpl5uo.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCHv4 1/2] submodule: port resolve_relative_url from shell to C","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-01-28T22:11:06Z","receivedAt":"2016-01-28T22:11:06Z","isPatch":false,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Jan 28, 2016 at 2:03 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Stefan Beller <sbeller@google.com> writes:\n>\n>> +static char *relative_url(const char *remote_url,\n>> +                             const char *url,\n>> +                             const char *up_path)\n>> +{\n>> +     int is_relative = 0;\n>> +     int colonsep = 0;\n>> +     char *out;\n>> +     char *remoteurl = xstrdup(remote_url);\n>> +     struct strbuf sb = STRBUF_INIT;\n>> +     size_t len = strlen(remoteurl);\n>> +\n>> +     if (is_dir_sep(remoteurl[len]))\n>> +             remoteurl[len] = '\\0';\n>> +\n>> +     if (!url_is_local_not_ssh(remoteurl) || is_absolute_path(remoteurl))\n>> +             is_relative = 0;\n>> +     else {\n>> +             is_relative = 1;\n>> +             /*\n>> +              * Prepend a './' to ensure all relative\n>> +              * remoteurls start with './' or '../'\n>> +              */\n>> +             if (!starts_with_dot_slash(remoteurl) &&\n>> +                 !starts_with_dot_dot_slash(remoteurl)) {\n>> +                     strbuf_reset(&sb);\n>> +                     strbuf_addf(&sb, \"./%s\", remoteurl);\n>> +                     free(remoteurl);\n>> +                     remoteurl = strbuf_detach(&sb, NULL);\n>> +             }\n>> +     }\n>> +     /*\n>> +      * When the url starts with '../', remove that and the\n>> +      * last directory in remoteurl.\n>> +      */\n>> +     while (url) {\n>> +             if (starts_with_dot_dot_slash(url)) {\n>> +                     url += 3;\n>> +                     colonsep |= chop_last_dir(&remoteurl, is_relative);\n>> +             } else if (starts_with_dot_slash(url))\n>> +                     url += 2;\n>> +             else\n>> +                     break;\n>> +     }\n>> +     strbuf_reset(&sb);\n>> +     strbuf_addf(&sb, \"%s%s%s\", remoteurl, colonsep ? \":\" : \"/\", url);\n>> +\n>> +     if (starts_with_dot_slash(sb.buf))\n>> +             out = xstrdup(sb.buf + 2);\n>> +     else\n>> +             out = xstrdup(sb.buf);\n>> +     strbuf_reset(&sb);\n>> +\n>> +     free(remoteurl);\n>\n> This is a rather strange place to put this free(), as you are done\n> with it a bit earlier, but it's OK.  I briefly wondered if the code\n> becomes easier to follow with fewer free(remoteurl) if this local\n> variable is made into a strbuf, but I didn't seriously think it\n> through.\n\nRight. As I did not touch that particular free with the resend, I wondered\nhow it came there, too. And I think I had it at the end of the function and\nthen realized the return just after the current position would leak it, so\nI moved it minimally up. If I'll resend again, I'll move it up to where\nit was last used.\n\n>\n> Otherwise looking good.\n>\n> Thanks.\n\nThanks,\nStefan\n"},{"id":"279632","messageId":"CACsJy8C7t9LEVaEqS9GRQ3oYmXwRp33=qOo-+4R4urHco25bAA@mail.gmail.com","threadId":"41246","inReplyTo":"1453948237-9860-3-git-send-email-sbeller@google.com","subject":"Re: [PATCHv4 2/2] submodule: port init from shell to C","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2016-02-27T08:30:49Z","receivedAt":"2016-02-27T08:30:49Z","isPatch":false,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Thu, Jan 28, 2016 at 9:30 AM, Stefan Beller <sbeller@google.com> wrote:\n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 13583d9..689c354 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> +static int module_init(int argc, const char **argv, const char *prefix)\n> +{\n> +       int quiet = 0;\n> +       int i;\n> +\n> +       struct option module_init_options[] = {\n> +               OPT_STRING(0, \"prefix\", &prefix,\n> +                          N_(\"path\"),\n> +                          N_(\"alternative anchor for relative paths\")),\n> +               OPT__QUIET(&quiet, \"Suppress output for initialzing a submodule\"),\n\nI think we need N_() around this \"Suppress...\" string. By the way,\ntypo in \"initializing\"\n-- \nDuy\n"}]}