{"thread":{"id":"49142","subject":"[RFC PATCH 0/7] Unset the submodule URL in the superproject when no longer needed","startedAt":"2018-08-16T02:31:13Z","lastAt":"2018-08-21T21:39:34Z","messageCount":14,"participants":["Stefan Beller","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"355788","messageId":"20180816023100.161626-1-sbeller@google.com","threadId":"49142","inReplyTo":null,"subject":"[RFC PATCH 0/7] Unset the submodule URL in the superproject when no longer needed","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:53Z","receivedAt":"2018-08-16T02:31:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This is available as\n\n  git fetch //github.com/stefanbeller/git unsetsubmoduleurl\n  \nand was hinted at in\nhttps://public-inbox.org/git/CAGZ79kYfoK9hfXM2-VMAZLPpqBOFQYKtyYuYJb8twzz6Oz5ymQ@mail.gmail.com/\n\n  Originally we have had the url in the config, (a) that we can change\n  the URLs after the \"git submodule init\" and \"git submodule update\"\n  step that actually clones the submodule if not present and much more\n  importantly (b) to know which submodule \"was initialized/active\".\n  \n  Now that we have the submodule.active or even\n  submodule.<name>.active flags, we do not need (b) any more.\n  So the URL turns into a useless piece of cruft that just is unneeded\n  and might confuse the user.\n\nOpinions?\n\nThanks,\nStefan\n\nStefan Beller (7):\n  t7410: update to new style\n  builtin/submodule--helper: remove stray new line\n  submodule: is_submodule_active to differentiate between new and old\n    mode\n  submodule sync: omit setting submodule URL in config if possible\n  submodule--helper: factor out allocation of callback cookie\n  submodule--helper, update_clone: store index to update_clone instead\n    of ce\n  builtin/submodule--helper: unset submodule url if possible\n\n builtin/submodule--helper.c      | 82 ++++++++++++++++++--------\n submodule.c                      |  5 +-\n submodule.h                      |  6 ++\n t/t5526-fetch-submodules.sh      |  2 +-\n t/t7406-submodule-update.sh      |  8 +++\n t/t7410-submodule-checkout-to.sh | 99 +++++++++++++++++++-------------\n 6 files changed, 131 insertions(+), 71 deletions(-)\n\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355789","messageId":"20180816023100.161626-2-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 1/7] t7410: update to new style","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:54Z","receivedAt":"2018-08-16T02:31:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"While at it fix a typo (s/independed/independent) and\nmake sure git is not in a chain of pipes.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n t/t7410-submodule-checkout-to.sh | 99 +++++++++++++++++++-------------\n 1 file changed, 58 insertions(+), 41 deletions(-)\n\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex 1acef32647a..f1b492ebc46 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -6,55 +6,72 @@ test_description='Combination of submodules and multiple workdirs'\n \n base_path=$(pwd -P)\n \n-test_expect_success 'setup: make origin' \\\n-    'mkdir -p origin/sub && ( cd origin/sub && git init &&\n-\techo file1 >file1 &&\n-\tgit add file1 &&\n-\tgit commit -m file1 ) &&\n-    mkdir -p origin/main && ( cd origin/main && git init &&\n-\tgit submodule add ../sub &&\n-\tgit commit -m \"add sub\" ) &&\n-    ( cd origin/sub &&\n-\techo file1updated >file1 &&\n-\tgit add file1 &&\n-\tgit commit -m \"file1 updated\" ) &&\n-    ( cd origin/main/sub && git pull ) &&\n-    ( cd origin/main &&\n-\tgit add sub &&\n-\tgit commit -m \"sub updated\" )'\n-\n-test_expect_success 'setup: clone' \\\n-    'mkdir clone && ( cd clone &&\n-\tgit clone --recursive \"$base_path/origin/main\")'\n+test_expect_success 'setup: make origin'  '\n+\tmkdir -p origin/sub &&\n+\t(\n+\t\tcd origin/sub && git init &&\n+\t\techo file1 >file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m file1\n+\t) &&\n+\tmkdir -p origin/main &&\n+\t(\n+\t\tcd origin/main && git init &&\n+\t\tgit submodule add ../sub &&\n+\t\tgit commit -m \"add sub\"\n+\t) &&\n+\t(\n+\t\tcd origin/sub &&\n+\t\techo file1updated >file1 &&\n+\t\tgit add file1 &&\n+\t\tgit commit -m \"file1 updated\"\n+\t) &&\n+\tgit -C origin/main/sub pull &&\n+\t(\n+\t\tcd origin/main &&\n+\t\tgit add sub &&\n+\t\tgit commit -m \"sub updated\"\n+\t)\n+'\n+\n+test_expect_success 'setup: clone' '\n+\tmkdir clone &&\n+\tgit -C clone clone --recursive \"$base_path/origin/main\"\n+'\n \n rev1_hash_main=$(git --git-dir=origin/main/.git show --pretty=format:%h -q \"HEAD~1\")\n rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q \"HEAD~1\")\n \n-test_expect_success 'checkout main' \\\n-    'mkdir default_checkout &&\n-    (cd clone/main &&\n-\tgit worktree add \"$base_path/default_checkout/main\" \"$rev1_hash_main\")'\n+test_expect_success 'checkout main' '\n+\tmkdir default_checkout &&\n+\tgit -C clone/main worktree add \"$base_path/default_checkout/main\" \"$rev1_hash_main\"\n+'\n \n-test_expect_failure 'can see submodule diffs just after checkout' \\\n-    '(cd default_checkout/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+test_expect_failure 'can see submodule diffs just after checkout' '\n+\tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n+\tgrep \"file1 updated\" out\n+'\n \n-test_expect_success 'checkout main and initialize independed clones' \\\n-    'mkdir fully_cloned_submodule &&\n-    (cd clone/main &&\n-\tgit worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\") &&\n-    (cd fully_cloned_submodule/main && git submodule update)'\n+test_expect_success 'checkout main and initialize independent clones' '\n+\tmkdir fully_cloned_submodule &&\n+\tgit -C clone/main worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\" &&\n+\tgit -C fully_cloned_submodule/main submodule update\n+'\n \n-test_expect_success 'can see submodule diffs after independed cloning' \\\n-    '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+test_expect_success 'can see submodule diffs after independent cloning' '\n+\tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n+\tgrep \"file1 updated\" out\n+'\n \n-test_expect_success 'checkout sub manually' \\\n-    'mkdir linked_submodule &&\n-    (cd clone/main &&\n-\tgit worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n-    (cd clone/main/sub &&\n-\tgit worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n+test_expect_success 'checkout sub manually' '\n+\tmkdir linked_submodule &&\n+\tgit -C clone/main worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\" &&\n+\tgit -C clone/main/sub worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\"\n+'\n \n-test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n-    '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n+test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n+\tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n+\tgrep \"file1 updated\" out\n+'\n \n test_done\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355790","messageId":"20180816023100.161626-3-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 2/7] builtin/submodule--helper: remove stray new line","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:55Z","receivedAt":"2018-08-16T02:31:18Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 1 -\n 1 file changed, 1 deletion(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 5c9d1fb496d..2f20bd4abdc 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1024,7 +1024,6 @@ static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data\n {\n \tstruct sync_cb *info = cb_data;\n \tsync_submodule(list_item->name, info->prefix, info->flags);\n-\n }\n \n static int module_sync(int argc, const char **argv, const char *prefix)\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355791","messageId":"20180816023100.161626-4-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 3/7] submodule: is_submodule_active to differentiate between new and old mode","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:56Z","receivedAt":"2018-08-16T02:31:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The change a086f921a72 (submodule: decouple url and submodule interest,\n2017-03-17) enables us to do more than originally thought.\nAs the url setting was used both to actually set the url where to\nobtain the submodule from, as well as used as a boolean flag later\nto see if it was active, we would need to keep the url around.\n\nNow that submodules can be activated using the submodule.[<name>.]active\nsetting, we could remove the url if the submodule is activated via that\nsetting.\n\nIn preparation to do so, pave the way by providing an easy way to see\nif a submodule is considered active via the new .active setting or via\nthe old .url setting.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n submodule.c | 5 +----\n submodule.h | 6 ++++++\n 2 files changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/submodule.c b/submodule.c\nindex 6e14547e9e0..d56350ed094 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -221,9 +221,6 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n \treturn 0;\n }\n \n-/*\n- * Determine if a submodule has been initialized at a given 'path'\n- */\n int is_submodule_active(struct repository *repo, const char *path)\n {\n \tint ret = 0;\n@@ -267,7 +264,7 @@ int is_submodule_active(struct repository *repo, const char *path)\n \n \t/* fallback to checking if the URL is set */\n \tkey = xstrfmt(\"submodule.%s.url\", module->name);\n-\tret = !repo_config_get_string(repo, key, &value);\n+\tret = !repo_config_get_string(repo, key, &value) ? 2 : 0;\n \n \tfree(value);\n \tfree(key);\ndiff --git a/submodule.h b/submodule.h\nindex 4644683e6cb..bfc070e4629 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -45,6 +45,12 @@ extern int git_default_submodule_config(const char *var, const char *value, void\n struct option;\n int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n \t\t\t\t\t\t     const char *arg, int unset);\n+/*\n+ * Determine if a submodule has been initialized at a given 'path'.\n+ * Returns 1 if it is considered active via the submodule.[<name>.]active\n+ * setting, or return 2 if it is active via the older submodule.url setting.\n+ */\n+#define SUBMODULE_ACTIVE_VIA_URL 2\n extern int is_submodule_active(struct repository *repo, const char *path);\n /*\n  * Determine if a submodule has been populated at a given 'path' by checking if\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355792","messageId":"20180816023100.161626-5-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 4/7] submodule sync: omit setting submodule URL in config if possible","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:57Z","receivedAt":"2018-08-16T02:31:24Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"We do not need to update the submodule url in the superprojects config\nif the url is not used to keep the submodule active.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 14 +++++++++-----\n 1 file changed, 9 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 2f20bd4abdc..639d0bb20a1 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -922,8 +922,10 @@ static void sync_submodule(const char *path, const char *prefix,\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tchar *sub_config_path = NULL;\n+\tint active, r;\n \n-\tif (!is_submodule_active(the_repository, path))\n+\tactive = is_submodule_active(the_repository, path);\n+\tif (!active)\n \t\treturn;\n \n \tsub = submodule_from_path(the_repository, &null_oid, path);\n@@ -983,13 +985,15 @@ static void sync_submodule(const char *path, const char *prefix,\n \tstrbuf_strip_suffix(&sb, \"\\n\");\n \tremote_key = xstrfmt(\"remote.%s.url\", sb.buf);\n \n+\tif (active == SUBMODULE_ACTIVE_VIA_URL)\n+\t\tFREE_AND_NULL(sub_origin_url);\n \tstrbuf_reset(&sb);\n \tsubmodule_to_gitdir(&sb, path);\n \tstrbuf_addstr(&sb, \"/config\");\n-\n-\tif (git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url))\n-\t\tdie(_(\"failed to update remote for submodule '%s'\"),\n-\t\t      path);\n+\tif ((r = git_config_set_in_file_gently(sb.buf, remote_key, sub_origin_url)))\n+\t\tif (sub_origin_url || r != CONFIG_NOTHING_SET)\n+\t\t\tdie(_(\"failed to update remote for submodule '%s'\"),\n+\t\t\t      path);\n \n \tif (flags & OPT_RECURSIVE) {\n \t\tstruct child_process cpr = CHILD_PROCESS_INIT;\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355793","messageId":"20180816023100.161626-6-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 5/7] submodule--helper: factor out allocation of callback cookie","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:58Z","receivedAt":"2018-08-16T02:31:26Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 16 +++++++++-------\n 1 file changed, 9 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 639d0bb20a1..1c9a12781fd 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1688,6 +1688,13 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \treturn needs_cloning;\n }\n \n+static void *update_clone_alloc_cb(int i)\n+{\n+\tint *p = xmalloc(sizeof(*p));\n+\t*p = i;\n+\treturn p;\n+}\n+\n static int update_clone_get_next_task(struct child_process *child,\n \t\t\t\t      struct strbuf *err,\n \t\t\t\t      void *suc_cb,\n@@ -1700,9 +1707,7 @@ static int update_clone_get_next_task(struct child_process *child,\n \tfor (; suc->current < suc->list.nr; suc->current++) {\n \t\tce = suc->list.entries[suc->current];\n \t\tif (prepare_to_clone_next_submodule(ce, child, suc, err)) {\n-\t\t\tint *p = xmalloc(sizeof(*p));\n-\t\t\t*p = suc->current;\n-\t\t\t*idx_task_cb = p;\n+\t\t\t*idx_task_cb = update_clone_alloc_cb(suc->current);\n \t\t\tsuc->current++;\n \t\t\treturn 1;\n \t\t}\n@@ -1715,7 +1720,6 @@ static int update_clone_get_next_task(struct child_process *child,\n \t */\n \tindex = suc->current - suc->list.nr;\n \tif (index < suc->failed_clones_nr) {\n-\t\tint *p;\n \t\tce = suc->failed_clones[index];\n \t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err)) {\n \t\t\tsuc->current ++;\n@@ -1724,9 +1728,7 @@ static int update_clone_get_next_task(struct child_process *child,\n \t\t\t\t\t   \"any more?\\n\");\n \t\t\treturn 0;\n \t\t}\n-\t\tp = xmalloc(sizeof(*p));\n-\t\t*p = suc->current;\n-\t\t*idx_task_cb = p;\n+\t\t*idx_task_cb = update_clone_alloc_cb(suc->current);\n \t\tsuc->current ++;\n \t\treturn 1;\n \t}\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355794","messageId":"20180816023100.161626-7-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 6/7] submodule--helper, update_clone: store index to update_clone instead of ce","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:30:59Z","receivedAt":"2018-08-16T02:31:29Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In update_submodules, we use the run_processes_parallel(get_task, finished)\nAPI, which allows to pass around a task specific callback cookie from the\nget_next function to the finish function. That finish function in turn may\nalter generic callback cookie to have the next call of get_task come up\nwith another new task.\n\nUp to now we passed around the index into a list of cache entries,\nwhich was stored in the generic callback cookie which is a struct\nsubmodule_update_clone.\n\nChange this to an index into 'update_clone' array, which is the potential\noutput of this helper. This will allow for a future change to make\nuse of the data the update_clone_data struct.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 31 +++++++++++++++++++------------\n 1 file changed, 19 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 1c9a12781fd..36de64902ec 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1511,8 +1511,10 @@ static int module_update_module_mode(int argc, const char **argv, const char *pr\n \n struct update_clone_data {\n \tconst struct submodule *sub;\n+\tconst struct cache_entry *ce;\n \tstruct object_id oid;\n \tunsigned just_cloned;\n+\tunsigned retried;\n };\n \n struct submodule_update_clone {\n@@ -1541,8 +1543,8 @@ struct submodule_update_clone {\n \t/* If we want to stop as fast as possible and return an error */\n \tunsigned quickstop : 1;\n \n-\t/* failed clones to be retried again */\n-\tconst struct cache_entry **failed_clones;\n+\t/* failed clones to be retried again, indexes into update_clone */\n+\tint *failed_clones;\n \tint failed_clones_nr, failed_clones_alloc;\n \n \tint max_jobs;\n@@ -1649,6 +1651,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \toidcpy(&suc->update_clone[suc->update_clone_nr].oid, &ce->oid);\n \tsuc->update_clone[suc->update_clone_nr].just_cloned = needs_cloning;\n \tsuc->update_clone[suc->update_clone_nr].sub = sub;\n+\tsuc->update_clone[suc->update_clone_nr].retried = 0;\n+\tsuc->update_clone[suc->update_clone_nr].ce = ce;\n \tsuc->update_clone_nr++;\n \n \tif (!needs_cloning)\n@@ -1707,7 +1711,8 @@ static int update_clone_get_next_task(struct child_process *child,\n \tfor (; suc->current < suc->list.nr; suc->current++) {\n \t\tce = suc->list.entries[suc->current];\n \t\tif (prepare_to_clone_next_submodule(ce, child, suc, err)) {\n-\t\t\t*idx_task_cb = update_clone_alloc_cb(suc->current);\n+\t\t\t*idx_task_cb = update_clone_alloc_cb(\n+\t\t\t\tsuc->update_clone_nr - 1);\n \t\t\tsuc->current++;\n \t\t\treturn 1;\n \t\t}\n@@ -1720,7 +1725,9 @@ static int update_clone_get_next_task(struct child_process *child,\n \t */\n \tindex = suc->current - suc->list.nr;\n \tif (index < suc->failed_clones_nr) {\n-\t\tce = suc->failed_clones[index];\n+\t\tint ucd_index = suc->failed_clones[index];\n+\t\tstruct update_clone_data *ucd = &suc->update_clone[ucd_index];\n+\t\tce = ucd->ce;\n \t\tif (!prepare_to_clone_next_submodule(ce, child, suc, err)) {\n \t\t\tsuc->current ++;\n \t\t\tstrbuf_addstr(err, \"BUG: submodule considered for \"\n@@ -1728,7 +1735,7 @@ static int update_clone_get_next_task(struct child_process *child,\n \t\t\t\t\t   \"any more?\\n\");\n \t\t\treturn 0;\n \t\t}\n-\t\t*idx_task_cb = update_clone_alloc_cb(suc->current);\n+\t\t*idx_task_cb = update_clone_alloc_cb(ucd_index);\n \t\tsuc->current ++;\n \t\treturn 1;\n \t}\n@@ -1750,31 +1757,31 @@ static int update_clone_task_finished(int result,\n \t\t\t\t      void *suc_cb,\n \t\t\t\t      void *idx_task_cb)\n {\n-\tconst struct cache_entry *ce;\n \tstruct submodule_update_clone *suc = suc_cb;\n+\tstruct update_clone_data *ucd;\n \n \tint *idxP = idx_task_cb;\n \tint idx = *idxP;\n+\tucd = &suc->update_clone[idx];\n \tfree(idxP);\n \n \tif (!result)\n \t\treturn 0;\n \n-\tif (idx < suc->list.nr) {\n-\t\tce  = suc->list.entries[idx];\n+\tif (!ucd->retried) {\n+\t\tucd->retried = 1;\n \t\tstrbuf_addf(err, _(\"Failed to clone '%s'. Retry scheduled\"),\n-\t\t\t    ce->name);\n+\t\t\t    ucd->ce->name);\n \t\tstrbuf_addch(err, '\\n');\n \t\tALLOC_GROW(suc->failed_clones,\n \t\t\t   suc->failed_clones_nr + 1,\n \t\t\t   suc->failed_clones_alloc);\n-\t\tsuc->failed_clones[suc->failed_clones_nr++] = ce;\n+\t\tsuc->failed_clones[suc->failed_clones_nr++] = idx;\n \t\treturn 0;\n \t} else {\n \t\tidx -= suc->list.nr;\n-\t\tce  = suc->failed_clones[idx];\n \t\tstrbuf_addf(err, _(\"Failed to clone '%s' a second time, aborting\"),\n-\t\t\t    ce->name);\n+\t\t\t    ucd->ce->name);\n \t\tstrbuf_addch(err, '\\n');\n \t\tsuc->quickstop = 1;\n \t\treturn 1;\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355795","messageId":"20180816023100.161626-8-sbeller@google.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"[PATCH 7/7] builtin/submodule--helper: unset submodule url if possible","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T02:31:00Z","receivedAt":"2018-08-16T02:31:31Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Signed-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c      | 24 ++++++++++++++++++++++--\n t/t5526-fetch-submodules.sh      |  2 +-\n t/t7406-submodule-update.sh      |  8 ++++++++\n t/t7410-submodule-checkout-to.sh |  2 +-\n 4 files changed, 32 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 36de64902ec..3aa385bce5c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1515,6 +1515,7 @@ struct update_clone_data {\n \tstruct object_id oid;\n \tunsigned just_cloned;\n \tunsigned retried;\n+\tunsigned cleanup_url;\n };\n \n struct submodule_update_clone {\n@@ -1590,7 +1591,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tstruct strbuf displaypath_sb = STRBUF_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *displaypath = NULL;\n-\tint needs_cloning = 0;\n+\tint needs_cloning = 0, active;\n \n \tif (ce_stage(ce)) {\n \t\tif (suc->recursive_prefix)\n@@ -1632,7 +1633,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t}\n \n \t/* Check if the submodule has been initialized. */\n-\tif (!is_submodule_active(the_repository, ce->name)) {\n+\tactive = is_submodule_active(the_repository, ce->name);\n+\tif (!active) {\n \t\tnext_submodule_warn_missing(suc, out, displaypath);\n \t\tgoto cleanup;\n \t}\n@@ -1653,6 +1655,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tsuc->update_clone[suc->update_clone_nr].sub = sub;\n \tsuc->update_clone[suc->update_clone_nr].retried = 0;\n \tsuc->update_clone[suc->update_clone_nr].ce = ce;\n+\tsuc->update_clone[suc->update_clone_nr].cleanup_url =\n+\t\t(active != SUBMODULE_ACTIVE_VIA_URL);\n \tsuc->update_clone_nr++;\n \n \tif (!needs_cloning)\n@@ -1801,6 +1805,22 @@ static int git_update_clone_config(const char *var, const char *value,\n \n static void update_submodule(struct update_clone_data *ucd)\n {\n+\tif (ucd->cleanup_url) {\n+\t\tstruct strbuf cfg = STRBUF_INIT;\n+\t\tstruct strbuf submodule_url = STRBUF_INIT;\n+\t\tint r;\n+\n+\t\tstrbuf_addf(&submodule_url, \"submodule.%s.url\", ucd->sub->name);\n+\t\tstrbuf_repo_git_path(&cfg, the_repository, \"config\");\n+\n+\t\tr = git_config_set_in_file_gently(cfg.buf, submodule_url.buf, NULL);\n+\t\tif (r && r != CONFIG_NOTHING_SET)\n+\t\t\tdie(_(\"failed to remove '%s'\"), submodule_url.buf);\n+\n+\t\tstrbuf_release(&cfg);\n+\t\tstrbuf_release(&submodule_url);\n+\t}\n+\n \tfprintf(stdout, \"dummy %s %d\\t%s\\n\",\n \t\toid_to_hex(&ucd->oid),\n \t\tucd->just_cloned,\ndiff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh\nindex 0f730d77815..cd1bd131b59 100755\n--- a/t/t5526-fetch-submodules.sh\n+++ b/t/t5526-fetch-submodules.sh\n@@ -508,7 +508,7 @@ test_expect_success \"'fetch.recurseSubmodules=on-demand' works also without .git\n \t\tgit config -f .gitmodules submodule.fake.path fake &&\n \t\tgit config -f .gitmodules submodule.fake.url fakeurl &&\n \t\tgit add .gitmodules &&\n-\t\tgit config --unset submodule.submodule.url &&\n+\t\ttest_might_fail git config --unset submodule.submodule.url &&\n \t\tgit fetch >../actual.out 2>../actual.err &&\n \t\t# cleanup\n \t\tgit config --unset fetch.recurseSubmodules &&\ndiff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh\nindex f604ef7a729..f581fea28e0 100755\n--- a/t/t7406-submodule-update.sh\n+++ b/t/t7406-submodule-update.sh\n@@ -84,6 +84,14 @@ test_expect_success 'submodule update detaching the HEAD ' '\n \t)\n '\n \n+test_expect_success 'active submodule leaves no URL config in superproject' '\n+\t# relies on previous test\n+\t(\n+\t\tcd super &&\n+\t\ttest_must_fail git config -f .git/config submodule.submodule.url\n+\t)\n+'\n+\n test_expect_success 'submodule update from subdirectory' '\n \t(cd super/submodule &&\n \t git reset --hard HEAD~1\ndiff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\nindex f1b492ebc46..683e957934b 100755\n--- a/t/t7410-submodule-checkout-to.sh\n+++ b/t/t7410-submodule-checkout-to.sh\n@@ -55,7 +55,7 @@ test_expect_failure 'can see submodule diffs just after checkout' '\n test_expect_success 'checkout main and initialize independent clones' '\n \tmkdir fully_cloned_submodule &&\n \tgit -C clone/main worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\" &&\n-\tgit -C fully_cloned_submodule/main submodule update\n+\tgit -C fully_cloned_submodule/main submodule update --init\n '\n \n test_expect_success 'can see submodule diffs after independent cloning' '\n-- \n2.18.0.265.g16de1b435c9.dirty\n\n"},{"id":"355839","messageId":"xmqqd0uigxby.fsf@gitster-ct.c.googlers.com","threadId":"49142","inReplyTo":"20180816023100.161626-1-sbeller@google.com","subject":"Re: [RFC PATCH 0/7] Unset the submodule URL in the superproject when no longer needed","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-16T15:12:01Z","receivedAt":"2018-08-16T15:12:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>   Originally we have had the url in the config, (a) that we can change\n>   the URLs after the \"git submodule init\" and \"git submodule update\"\n>   step that actually clones the submodule if not present and much more\n>   importantly (b) to know which submodule \"was initialized/active\".\n>   \n>   Now that we have the submodule.active or even\n>   submodule.<name>.active flags, we do not need (b) any more.\n\nUp to that point the description is sane.\n\n>   So the URL turns into a useless piece of cruft that just is unneeded\n>   and might confuse the user.\n>\n> Opinions?\n\nYou spelled out why you do not need for (b) but not for (a) and\nworse it is is unclear if you never need it for (a) or under what\ncondition you need it for (a).  So there isn't enough information to\nform an opinion in the above.  Sorry--readers need to go to the real\npatches.\n\n"},{"id":"355842","messageId":"CAGZ79kbe_acinQSK5is=i1FufSwGjtqMZPL77nTTuT93ifhi0Q@mail.gmail.com","threadId":"49142","inReplyTo":"xmqqd0uigxby.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH 0/7] Unset the submodule URL in the superproject when no longer needed","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-16T15:45:35Z","receivedAt":"2018-08-16T15:45:50Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 16, 2018 at 8:12 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Stefan Beller <sbeller@google.com> writes:\n>\n> >   Originally we have had the url in the config, (a) that we can change\n> >   the URLs after the \"git submodule init\" and \"git submodule update\"\n> >   step that actually clones the submodule if not present and much more\n> >   importantly (b) to know which submodule \"was initialized/active\".\n> >\n> >   Now that we have the submodule.active or even\n> >   submodule.<name>.active flags, we do not need (b) any more.\n>\n> Up to that point the description is sane.\n>\n> >   So the URL turns into a useless piece of cruft that just is unneeded\n> >   and might confuse the user.\n> >\n> > Opinions?\n>\n> You spelled out why you do not need for (b) but not for (a) and\n> worse it is is unclear if you never need it for (a) or under what\n> condition you need it for (a).  So there isn't enough information to\n> form an opinion in the above.  Sorry--readers need to go to the real\n> patches.\n\nRegarding (a): Once the submodule is cloned, you either need\nto change the remote in the submodule or you can use \"git submodule sync\"\nwhich can bypass the superproject config, too (that copies the URL from\n.gitmodules to the submodules config/remote)\n\nI don't think (a) is needed after the clone of a submodule is done.\n"},{"id":"355854","messageId":"xmqqa7pmfdhi.fsf@gitster-ct.c.googlers.com","threadId":"49142","inReplyTo":"20180816023100.161626-2-sbeller@google.com","subject":"Re: [PATCH 1/7] t7410: update to new style","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-16T17:06:01Z","receivedAt":"2018-08-16T17:06:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> While at it fix a typo (s/independed/independent) and\n> make sure git is not in a chain of pipes.\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  t/t7410-submodule-checkout-to.sh | 99 +++++++++++++++++++-------------\n>  1 file changed, 58 insertions(+), 41 deletions(-)\n\nMakes sense and the result does read easier.\n\n> diff --git a/t/t7410-submodule-checkout-to.sh b/t/t7410-submodule-checkout-to.sh\n> index 1acef32647a..f1b492ebc46 100755\n> --- a/t/t7410-submodule-checkout-to.sh\n> +++ b/t/t7410-submodule-checkout-to.sh\n> @@ -6,55 +6,72 @@ test_description='Combination of submodules and multiple workdirs'\n>  \n>  base_path=$(pwd -P)\n>  \n> -test_expect_success 'setup: make origin' \\\n> -    'mkdir -p origin/sub && ( cd origin/sub && git init &&\n> -\techo file1 >file1 &&\n> -\tgit add file1 &&\n> -\tgit commit -m file1 ) &&\n> -    mkdir -p origin/main && ( cd origin/main && git init &&\n> -\tgit submodule add ../sub &&\n> -\tgit commit -m \"add sub\" ) &&\n> -    ( cd origin/sub &&\n> -\techo file1updated >file1 &&\n> -\tgit add file1 &&\n> -\tgit commit -m \"file1 updated\" ) &&\n> -    ( cd origin/main/sub && git pull ) &&\n> -    ( cd origin/main &&\n> -\tgit add sub &&\n> -\tgit commit -m \"sub updated\" )'\n> -\n> -test_expect_success 'setup: clone' \\\n> -    'mkdir clone && ( cd clone &&\n> -\tgit clone --recursive \"$base_path/origin/main\")'\n> +test_expect_success 'setup: make origin'  '\n> +\tmkdir -p origin/sub &&\n> +\t(\n> +\t\tcd origin/sub && git init &&\n> +\t\techo file1 >file1 &&\n> +\t\tgit add file1 &&\n> +\t\tgit commit -m file1\n> +\t) &&\n> +\tmkdir -p origin/main &&\n> +\t(\n> +\t\tcd origin/main && git init &&\n> +\t\tgit submodule add ../sub &&\n> +\t\tgit commit -m \"add sub\"\n> +\t) &&\n> +\t(\n> +\t\tcd origin/sub &&\n> +\t\techo file1updated >file1 &&\n> +\t\tgit add file1 &&\n> +\t\tgit commit -m \"file1 updated\"\n> +\t) &&\n> +\tgit -C origin/main/sub pull &&\n> +\t(\n> +\t\tcd origin/main &&\n> +\t\tgit add sub &&\n> +\t\tgit commit -m \"sub updated\"\n> +\t)\n> +'\n> +\n> +test_expect_success 'setup: clone' '\n> +\tmkdir clone &&\n> +\tgit -C clone clone --recursive \"$base_path/origin/main\"\n> +'\n>  \n>  rev1_hash_main=$(git --git-dir=origin/main/.git show --pretty=format:%h -q \"HEAD~1\")\n>  rev1_hash_sub=$(git --git-dir=origin/sub/.git show --pretty=format:%h -q \"HEAD~1\")\n>  \n> -test_expect_success 'checkout main' \\\n> -    'mkdir default_checkout &&\n> -    (cd clone/main &&\n> -\tgit worktree add \"$base_path/default_checkout/main\" \"$rev1_hash_main\")'\n> +test_expect_success 'checkout main' '\n> +\tmkdir default_checkout &&\n> +\tgit -C clone/main worktree add \"$base_path/default_checkout/main\" \"$rev1_hash_main\"\n> +'\n>  \n> -test_expect_failure 'can see submodule diffs just after checkout' \\\n> -    '(cd default_checkout/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n> +test_expect_failure 'can see submodule diffs just after checkout' '\n> +\tgit -C default_checkout/main diff --submodule master\"^!\" >out &&\n> +\tgrep \"file1 updated\" out\n> +'\n>  \n> -test_expect_success 'checkout main and initialize independed clones' \\\n> -    'mkdir fully_cloned_submodule &&\n> -    (cd clone/main &&\n> -\tgit worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\") &&\n> -    (cd fully_cloned_submodule/main && git submodule update)'\n> +test_expect_success 'checkout main and initialize independent clones' '\n> +\tmkdir fully_cloned_submodule &&\n> +\tgit -C clone/main worktree add \"$base_path/fully_cloned_submodule/main\" \"$rev1_hash_main\" &&\n> +\tgit -C fully_cloned_submodule/main submodule update\n> +'\n>  \n> -test_expect_success 'can see submodule diffs after independed cloning' \\\n> -    '(cd fully_cloned_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n> +test_expect_success 'can see submodule diffs after independent cloning' '\n> +\tgit -C fully_cloned_submodule/main diff --submodule master\"^!\" >out &&\n> +\tgrep \"file1 updated\" out\n> +'\n>  \n> -test_expect_success 'checkout sub manually' \\\n> -    'mkdir linked_submodule &&\n> -    (cd clone/main &&\n> -\tgit worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\") &&\n> -    (cd clone/main/sub &&\n> -\tgit worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\")'\n> +test_expect_success 'checkout sub manually' '\n> +\tmkdir linked_submodule &&\n> +\tgit -C clone/main worktree add \"$base_path/linked_submodule/main\" \"$rev1_hash_main\" &&\n> +\tgit -C clone/main/sub worktree add \"$base_path/linked_submodule/main/sub\" \"$rev1_hash_sub\"\n> +'\n>  \n> -test_expect_success 'can see submodule diffs after manual checkout of linked submodule' \\\n> -    '(cd linked_submodule/main && git diff --submodule master\"^!\" | grep \"file1 updated\")'\n> +test_expect_success 'can see submodule diffs after manual checkout of linked submodule' '\n> +\tgit -C linked_submodule/main diff --submodule master\"^!\" >out &&\n> +\tgrep \"file1 updated\" out\n> +'\n>  \n>  test_done\n"},{"id":"355858","messageId":"xmqq600afc1e.fsf@gitster-ct.c.googlers.com","threadId":"49142","inReplyTo":"20180816023100.161626-4-sbeller@google.com","subject":"Re: [PATCH 3/7] submodule: is_submodule_active to differentiate between new and old mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-16T17:37:17Z","receivedAt":"2018-08-16T17:37:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n> The change a086f921a72 (submodule: decouple url and submodule interest,\n> 2017-03-17) enables us to do more than originally thought.\n> As the url setting was used both to actually set the url where to\n> obtain the submodule from, as well as used as a boolean flag later\n> to see if it was active, we would need to keep the url around.\n>\n> Now that submodules can be activated using the submodule.[<name>.]active\n> setting, we could remove the url if the submodule is activated via that\n> setting.\n\n... as the cloned submodule repository has $GIT_DIR/config which\nknows its own origin, we do not need to record URL in superproject's\n$GIT_DIR/config.  Back before we started using a directory under\n$GIT_DIR/modules/ as a more permanent location to store submodule,\nand point at it by a gitdir file in $path/.git to allow removal of a\nsubmodule from the working tree of the superproject without having\nto reclone when resurrecting the same submodule, it may have been\nuseful to keep custom URL somewhere in the superproject, but that no\nlonger is the case.\n\n> In preparation to do so, pave the way by providing an easy way to see\n> if a submodule is considered active via the new .active setting or via\n> the old .url setting.\n\nMakes sense.\n\n>\n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  submodule.c | 5 +----\n>  submodule.h | 6 ++++++\n>  2 files changed, 7 insertions(+), 4 deletions(-)\n>\n> diff --git a/submodule.c b/submodule.c\n> index 6e14547e9e0..d56350ed094 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -221,9 +221,6 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n>  \treturn 0;\n>  }\n>  \n> -/*\n> - * Determine if a submodule has been initialized at a given 'path'\n> - */\n>  int is_submodule_active(struct repository *repo, const char *path)\n>  {\n>  \tint ret = 0;\n> @@ -267,7 +264,7 @@ int is_submodule_active(struct repository *repo, const char *path)\n>  \n>  \t/* fallback to checking if the URL is set */\n>  \tkey = xstrfmt(\"submodule.%s.url\", module->name);\n> -\tret = !repo_config_get_string(repo, key, &value);\n> +\tret = !repo_config_get_string(repo, key, &value) ? 2 : 0;\n>\n>  \tfree(value);\n>  \tfree(key);\n> diff --git a/submodule.h b/submodule.h\n> index 4644683e6cb..bfc070e4629 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -45,6 +45,12 @@ extern int git_default_submodule_config(const char *var, const char *value, void\n>  struct option;\n>  int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n>  \t\t\t\t\t\t     const char *arg, int unset);\n> +/*\n> + * Determine if a submodule has been initialized at a given 'path'.\n> + * Returns 1 if it is considered active via the submodule.[<name>.]active\n> + * setting, or return 2 if it is active via the older submodule.url setting.\n> + */\n> +#define SUBMODULE_ACTIVE_VIA_URL 2\n>  extern int is_submodule_active(struct repository *repo, const char *path);\n>  /*\n>   * Determine if a submodule has been populated at a given 'path' by checking if\n\nThis change assumes that all the existing return sites in the\nis_submodule_active() function signals true with 1 (or at least some\nvalue that is different from 2).\n\nBut the part that uses submodule.active as pathspec list calls\nmatch_pathspec() and uses its return value to signal if the module\nis active.  match_pathspec() in turn uses dir.c::do_match_pathspec()\nwhich signals _how_ the set of pathspec list elements matched to the\ngiven name by returning various forms of true, like MATCHED_FNMATCH,\netc.\n\nSo I think this patch is insufficient, and needs to at least change\nthe \"submodule.active\" codepath to return !!ret; otherwise, a caller\nthat now expects 0 (not active), 1 (active but can lose URL) and 2\n(active and the presence of URL makes it so) will be confused when\none of the MATCHED_* constants from dir.h comes back.\n\n\n\n"},{"id":"356133","messageId":"CAGZ79kb-_7PTqe_9i4=FQQE7EmQsKN8+iB4snLetPD+r2UgpBQ@mail.gmail.com","threadId":"49142","inReplyTo":"xmqq600afc1e.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 3/7] submodule: is_submodule_active to differentiate between new and old mode","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-20T19:50:48Z","receivedAt":"2018-08-20T19:51:02Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Thu, Aug 16, 2018 at 10:37 AM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Stefan Beller <sbeller@google.com> writes:\n>\n> > The change a086f921a72 (submodule: decouple url and submodule interest,\n> > 2017-03-17) enables us to do more than originally thought.\n> > As the url setting was used both to actually set the url where to\n> > obtain the submodule from, as well as used as a boolean flag later\n> > to see if it was active, we would need to keep the url around.\n> >\n> > Now that submodules can be activated using the submodule.[<name>.]active\n> > setting, we could remove the url if the submodule is activated via that\n> > setting.\n>\n> ... as the cloned submodule repository has $GIT_DIR/config which\n> knows its own origin, we do not need to record URL in superproject's\n> $GIT_DIR/config.  Back before we started using a directory under\n> $GIT_DIR/modules/ as a more permanent location to store submodule,\n> and point at it by a gitdir file in $path/.git to allow removal of a\n> submodule from the working tree of the superproject without having\n> to reclone when resurrecting the same submodule, it may have been\n> useful to keep custom URL somewhere in the superproject, but that no\n> longer is the case.\n>\n> > In preparation to do so, pave the way by providing an easy way to see\n> > if a submodule is considered active via the new .active setting or via\n> > the old .url setting.\n>\n> Makes sense.\n>\n> >\n> > Signed-off-by: Stefan Beller <sbeller@google.com>\n> > ---\n> >  submodule.c | 5 +----\n> >  submodule.h | 6 ++++++\n> >  2 files changed, 7 insertions(+), 4 deletions(-)\n> >\n> > diff --git a/submodule.c b/submodule.c\n> > index 6e14547e9e0..d56350ed094 100644\n> > --- a/submodule.c\n> > +++ b/submodule.c\n> > @@ -221,9 +221,6 @@ int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n> >       return 0;\n> >  }\n> >\n> > -/*\n> > - * Determine if a submodule has been initialized at a given 'path'\n> > - */\n> >  int is_submodule_active(struct repository *repo, const char *path)\n> >  {\n> >       int ret = 0;\n> > @@ -267,7 +264,7 @@ int is_submodule_active(struct repository *repo, const char *path)\n> >\n> >       /* fallback to checking if the URL is set */\n> >       key = xstrfmt(\"submodule.%s.url\", module->name);\n> > -     ret = !repo_config_get_string(repo, key, &value);\n> > +     ret = !repo_config_get_string(repo, key, &value) ? 2 : 0;\n> >\n> >       free(value);\n> >       free(key);\n> > diff --git a/submodule.h b/submodule.h\n> > index 4644683e6cb..bfc070e4629 100644\n> > --- a/submodule.h\n> > +++ b/submodule.h\n> > @@ -45,6 +45,12 @@ extern int git_default_submodule_config(const char *var, const char *value, void\n> >  struct option;\n> >  int option_parse_recurse_submodules_worktree_updater(const struct option *opt,\n> >                                                    const char *arg, int unset);\n> > +/*\n> > + * Determine if a submodule has been initialized at a given 'path'.\n> > + * Returns 1 if it is considered active via the submodule.[<name>.]active\n> > + * setting, or return 2 if it is active via the older submodule.url setting.\n> > + */\n> > +#define SUBMODULE_ACTIVE_VIA_URL 2\n> >  extern int is_submodule_active(struct repository *repo, const char *path);\n> >  /*\n> >   * Determine if a submodule has been populated at a given 'path' by checking if\n>\n> This change assumes that all the existing return sites in the\n> is_submodule_active() function signals true with 1 (or at least some\n> value that is different from 2).\n\nYes.\n\n> So I think this patch is insufficient, and needs to at least change\n> the \"submodule.active\" codepath to return !!ret; otherwise, a caller\n> that now expects 0 (not active), 1 (active but can lose URL) and 2\n> (active and the presence of URL makes it so) will be confused when\n> one of the MATCHED_* constants from dir.h comes back.\n\nYes.\n\nI'll resend when appropriately.\n\nThanks,\nStefan\n"},{"id":"356222","messageId":"xmqqpnyb7627.fsf@gitster-ct.c.googlers.com","threadId":"49142","inReplyTo":"CAGZ79kb-_7PTqe_9i4=FQQE7EmQsKN8+iB4snLetPD+r2UgpBQ@mail.gmail.com","subject":"Re: [PATCH 3/7] submodule: is_submodule_active to differentiate between new and old mode","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-21T21:39:28Z","receivedAt":"2018-08-21T21:39:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Beller <sbeller@google.com> writes:\n\n>> So I think this patch is insufficient, and needs to at least change\n>> the \"submodule.active\" codepath to return !!ret; otherwise, a caller\n>> that now expects 0 (not active), 1 (active but can lose URL) and 2\n>> (active and the presence of URL makes it so) will be confused when\n>> one of the MATCHED_* constants from dir.h comes back.\n>\n> Yes.\n>\n> I'll resend when appropriately.\n\nThanks.\n"}]}