{"thread":{"id":"49027","subject":"[PATCH 0/7] Resend of sb/submodule-update-in-c","startedAt":"2018-08-03T22:24:04Z","lastAt":"2018-08-20T19:44:57Z","messageCount":24,"participants":["Stefan Beller","Junio C Hamano","Brandon Williams","Duy Nguyen"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"354445","messageId":"20180803222322.261813-1-sbeller@google.com","threadId":"49027","inReplyTo":null,"subject":"[PATCH 0/7] Resend of sb/submodule-update-in-c","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:15Z","receivedAt":"2018-08-03T22:24:04Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"* Introduce new patch\n  \"submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree\"\n  that resolves the conflict with earlier versions of this series with\n  sb/submodule-core-worktree\n* This series is based on master, which already contains \n  sb/submodule-core-worktree\n  \nThanks,\nStefan\n\nStefan Beller (7):\n  git-submodule.sh: align error reporting for update mode to use path\n  git-submodule.sh: rename unused variables\n  builtin/submodule--helper: factor out submodule updating\n  builtin/submodule--helper: store update_clone information in a struct\n  builtin/submodule--helper: factor out method to update a single\n    submodule\n  submodule--helper: replace connect-gitdir-workingtree by\n    ensure-core-worktree\n  submodule--helper: introduce new update-module-mode helper\n\n builtin/submodule--helper.c | 216 ++++++++++++++++++++++++++----------\n git-submodule.sh            |  29 +----\n 2 files changed, 164 insertions(+), 81 deletions(-)\n\n  ./git-range-diff origin/sb/submodule-update-in-c...HEAD\n      [...]\n  -:  ----------- > 338:  1d89318c48d Fifth batch for 2.19 cycle\n  1:  c1cb423b249 = 339:  3090cbcb46e git-submodule.sh: align error reporting for update mode to use path\n  2:  f188b30a9b9 = 340:  850a16e9085 git-submodule.sh: rename unused variables\n  3:  70d84fa6a09 ! 341:  88af0cdcba6 builtin/submodule--helper: factor out submodule updating\n    @@ -73,10 +73,10 @@\n      \t};\n      \tsuc.prefix = prefix;\n      \n    --\tconfig_from_gitmodules(gitmodules_update_clone_config, &max_jobs);\n    --\tgit_config(gitmodules_update_clone_config, &max_jobs);\n    -+\tconfig_from_gitmodules(gitmodules_update_clone_config, &suc.max_jobs);\n    -+\tgit_config(gitmodules_update_clone_config, &suc.max_jobs);\n    +-\tupdate_clone_config_from_gitmodules(&max_jobs);\n    +-\tgit_config(git_update_clone_config, &max_jobs);\n    ++\tupdate_clone_config_from_gitmodules(&suc.max_jobs);\n    ++\tgit_config(git_update_clone_config, &suc.max_jobs);\n      \n      \targc = parse_options(argc, argv, prefix, module_update_clone_options,\n      \t\t\t     git_submodule_helper_usage, 0);\n  4:  511e8a139c9 = 342:  2fdd479a6d5 builtin/submodule--helper: store update_clone information in a struct\n  5:  65b2a720b90 = 343:  34589e724b3 builtin/submodule--helper: factor out method to update a single submodule\n  -:  ----------- > 344:  ee2bb4f23d8 submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree\n  6:  e5803d07f9b ! 345:  03300626ba7 submodule--helper: introduce new update-module-mode helper\n    @@ -88,15 +88,15 @@\n      \t{\"clone\", module_clone, 0},\n     +\t{\"update-module-mode\", module_update_module_mode, 0},\n      \t{\"update-clone\", update_clone, 0},\n    + \t{\"ensure-core-worktree\", ensure_core_worktree, 0},\n      \t{\"relative-path\", resolve_relative_path, 0},\n    - \t{\"resolve-relative-url\", resolve_relative_url, 0},\n     \n     diff --git a/git-submodule.sh b/git-submodule.sh\n     --- a/git-submodule.sh\n     +++ b/git-submodule.sh\n     @@\n    - \tdo\n    - \t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n    + \n    + \t\tgit submodule--helper ensure-core-worktree \"$sm_path\"\n      \n     -\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n     -\t\tif ! test -z \"$update\"\n"},{"id":"354446","messageId":"20180803222322.261813-3-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 2/7] git-submodule.sh: rename unused variables","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:17Z","receivedAt":"2018-08-03T22:24:09Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The 'mode' variable is not used in cmd_update for its original purpose,\nrename it to 'dummy' as it only serves the purpose to abort quickly\ndocumenting this knowledge.\n\nThe variable 'stage' is also not used any more in cmd_update, so remove it.\n\nThis went unnoticed as first each function used the commonly used\nsubmodule listing, which was converted in 74703a1e4df (submodule: rewrite\n`module_list` shell function in C, 2015-09-02). When cmd_update was\nusing its own function starting in 48308681b07 (git submodule update:\nhave a dedicated helper for cloning, 2016-02-29), its removal was missed.\n\nA later patch in this series also touches the communication between\nthe submodule helper and git-submodule.sh, but let's have this as\na preparatory patch, as it eases the next patch, which stores the\nraw data instead of the line printed for this communication.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 5 ++---\n git-submodule.sh            | 4 ++--\n 2 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a3c4564c6c8..da700c88963 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1573,9 +1573,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tneeds_cloning = !file_exists(sb.buf);\n \n \tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%06o %s %d %d\\t%s\\n\", ce->ce_mode,\n-\t\t\toid_to_hex(&ce->oid), ce_stage(ce),\n-\t\t\tneeds_cloning, ce->name);\n+\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n+\t\t    oid_to_hex(&ce->oid), needs_cloning, ce->name);\n \tstring_list_append(&suc->projectlines, sb.buf);\n \n \tif (!needs_cloning)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 5a58812645d..8caaf274e25 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -531,9 +531,9 @@ cmd_update()\n \t\t\"$@\" || echo \"#unmatched\" $?\n \t} | {\n \terr=\n-\twhile read -r mode sha1 stage just_cloned sm_path\n+\twhile read -r quickabort sha1 just_cloned sm_path\n \tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n+\t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n \n \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n \t\tif ! test -z \"$update\"\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354447","messageId":"20180803222322.261813-2-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 1/7] git-submodule.sh: align error reporting for update mode to use path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:16Z","receivedAt":"2018-08-03T22:24:10Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"All other error messages in cmd_update are reporting the submodule based\non its path, so let's do that for invalid update modes, too.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-submodule.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8b5ad59bdee..5a58812645d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -632,7 +632,7 @@ cmd_update()\n \t\t\t\tmust_die_on_failure=yes\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\tdie \"$(eval_gettext \"Invalid update mode '$update_module' for submodule '$name'\")\"\n+\t\t\t\tdie \"$(eval_gettext \"Invalid update mode '$update_module' for submodule path '$path'\")\"\n \t\t\tesac\n \n \t\t\tif (sanitize_submodule_env; cd \"$sm_path\" && $command \"$sha1\")\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354448","messageId":"20180803222322.261813-4-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 3/7] builtin/submodule--helper: factor out submodule updating","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:18Z","receivedAt":"2018-08-03T22:24:11Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Separate the command line parsing from the actual execution of the command\nwithin the repository. For now there is not a lot of execution as\nmost of it is still in git-submodule.sh.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 59 +++++++++++++++++++++----------------\n 1 file changed, 33 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex da700c88963..32f00ca6f87 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1474,6 +1474,8 @@ struct submodule_update_clone {\n \t/* failed clones to be retried again */\n \tconst struct cache_entry **failed_clones;\n \tint failed_clones_nr, failed_clones_alloc;\n+\n+\tint max_jobs;\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \\\n@@ -1716,11 +1718,36 @@ static int git_update_clone_config(const char *var, const char *value,\n \treturn 0;\n }\n \n+static int update_submodules(struct submodule_update_clone *suc)\n+{\n+\tstruct string_list_item *item;\n+\n+\trun_processes_parallel(suc->max_jobs,\n+\t\t\t       update_clone_get_next_task,\n+\t\t\t       update_clone_start_failure,\n+\t\t\t       update_clone_task_finished,\n+\t\t\t       suc);\n+\n+\t/*\n+\t * We saved the output and put it out all at once now.\n+\t * That means:\n+\t * - the listener does not have to interleave their (checkout)\n+\t *   work with our fetching.  The writes involved in a\n+\t *   checkout involve more straightforward sequential I/O.\n+\t * - the listener can avoid doing any work if fetching failed.\n+\t */\n+\tif (suc->quickstop)\n+\t\treturn 1;\n+\n+\tfor_each_string_list_item(item, &suc->projectlines)\n+\t\tfprintf(stdout, \"%s\", item->string);\n+\n+\treturn 0;\n+}\n+\n static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tconst char *update = NULL;\n-\tint max_jobs = 1;\n-\tstruct string_list_item *item;\n \tstruct pathspec pathspec;\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n \n@@ -1742,7 +1769,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"<depth>\",\n \t\t\t   N_(\"Create a shallow clone truncated to the \"\n \t\t\t      \"specified number of revisions\")),\n-\t\tOPT_INTEGER('j', \"jobs\", &max_jobs,\n+\t\tOPT_INTEGER('j', \"jobs\", &suc.max_jobs,\n \t\t\t    N_(\"parallel jobs\")),\n \t\tOPT_BOOL(0, \"recommend-shallow\", &suc.recommend_shallow,\n \t\t\t    N_(\"whether the initial clone should follow the shallow recommendation\")),\n@@ -1758,8 +1785,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t};\n \tsuc.prefix = prefix;\n \n-\tupdate_clone_config_from_gitmodules(&max_jobs);\n-\tgit_config(git_update_clone_config, &max_jobs);\n+\tupdate_clone_config_from_gitmodules(&suc.max_jobs);\n+\tgit_config(git_update_clone_config, &suc.max_jobs);\n \n \targc = parse_options(argc, argv, prefix, module_update_clone_options,\n \t\t\t     git_submodule_helper_usage, 0);\n@@ -1774,27 +1801,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tif (pathspec.nr)\n \t\tsuc.warn_if_uninitialized = 1;\n \n-\trun_processes_parallel(max_jobs,\n-\t\t\t       update_clone_get_next_task,\n-\t\t\t       update_clone_start_failure,\n-\t\t\t       update_clone_task_finished,\n-\t\t\t       &suc);\n-\n-\t/*\n-\t * We saved the output and put it out all at once now.\n-\t * That means:\n-\t * - the listener does not have to interleave their (checkout)\n-\t *   work with our fetching.  The writes involved in a\n-\t *   checkout involve more straightforward sequential I/O.\n-\t * - the listener can avoid doing any work if fetching failed.\n-\t */\n-\tif (suc.quickstop)\n-\t\treturn 1;\n-\n-\tfor_each_string_list_item(item, &suc.projectlines)\n-\t\tfprintf(stdout, \"%s\", item->string);\n-\n-\treturn 0;\n+\treturn update_submodules(&suc);\n }\n \n static int resolve_relative_path(int argc, const char **argv, const char *prefix)\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354449","messageId":"20180803222322.261813-5-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 4/7] builtin/submodule--helper: store update_clone information in a struct","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:19Z","receivedAt":"2018-08-03T22:24:13Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The information that is printed for update_submodules in\n'submodule--helper update-clone' and consumed by 'git submodule update'\nis stored as a string per submodule. This made sense at the time of\n48308681b07 (git submodule update: have a dedicated helper for cloning,\n2016-02-29), but as we want to migrate the rest of the submodule update\ninto C, we're better off having access to the raw information in a helper\nstruct.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 37 +++++++++++++++++++++++++++----------\n 1 file changed, 27 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 32f00ca6f87..40b94dd622e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1446,6 +1446,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct update_clone_data {\n+\tconst struct submodule *sub;\n+\tstruct object_id oid;\n+\tunsigned just_cloned;\n+};\n+\n struct submodule_update_clone {\n \t/* index into 'list', the list of submodules to look into for cloning */\n \tint current;\n@@ -1465,8 +1471,9 @@ struct submodule_update_clone {\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n \n-\t/* Machine-readable status lines to be consumed by git-submodule.sh */\n-\tstruct string_list projectlines;\n+\t/* to be consumed by git-submodule.sh */\n+\tstruct update_clone_data *update_clone;\n+\tint update_clone_nr; int update_clone_alloc;\n \n \t/* If we want to stop as fast as possible and return an error */\n \tunsigned quickstop : 1;\n@@ -1480,7 +1487,7 @@ struct submodule_update_clone {\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \\\n \tNULL, NULL, NULL, \\\n-\tSTRING_LIST_INIT_DUP, 0, NULL, 0, 0}\n+\tNULL, 0, 0, 0, NULL, 0, 0, 0}\n \n \n static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n@@ -1574,10 +1581,12 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n \tneeds_cloning = !file_exists(sb.buf);\n \n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n-\t\t    oid_to_hex(&ce->oid), needs_cloning, ce->name);\n-\tstring_list_append(&suc->projectlines, sb.buf);\n+\tALLOC_GROW(suc->update_clone, suc->update_clone_nr + 1,\n+\t\t   suc->update_clone_alloc);\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_nr++;\n \n \tif (!needs_cloning)\n \t\tgoto cleanup;\n@@ -1720,7 +1729,8 @@ static int git_update_clone_config(const char *var, const char *value,\n \n static int update_submodules(struct submodule_update_clone *suc)\n {\n-\tstruct string_list_item *item;\n+\tint i;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n \trun_processes_parallel(suc->max_jobs,\n \t\t\t       update_clone_get_next_task,\n@@ -1739,9 +1749,16 @@ static int update_submodules(struct submodule_update_clone *suc)\n \tif (suc->quickstop)\n \t\treturn 1;\n \n-\tfor_each_string_list_item(item, &suc->projectlines)\n-\t\tfprintf(stdout, \"%s\", item->string);\n+\tfor (i = 0; i < suc->update_clone_nr; i++) {\n+\t\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n+\t\t\toid_to_hex(&suc->update_clone[i].oid),\n+\t\t\tsuc->update_clone[i].just_cloned,\n+\t\t\tsuc->update_clone[i].sub->path);\n+\t\tfprintf(stdout, \"%s\", sb.buf);\n+\t\tstrbuf_reset(&sb);\n+\t}\n \n+\tstrbuf_release(&sb);\n \treturn 0;\n }\n \n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354450","messageId":"20180803222322.261813-6-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 5/7] builtin/submodule--helper: factor out method to update a single submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:20Z","receivedAt":"2018-08-03T22:24:16Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we'll find this method handy.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 40b94dd622e..8b1088ab58a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1727,10 +1727,17 @@ static int git_update_clone_config(const char *var, const char *value,\n \treturn 0;\n }\n \n+static void update_submodule(struct update_clone_data *ucd)\n+{\n+\tfprintf(stdout, \"dummy %s %d\\t%s\\n\",\n+\t\toid_to_hex(&ucd->oid),\n+\t\tucd->just_cloned,\n+\t\tucd->sub->path);\n+}\n+\n static int update_submodules(struct submodule_update_clone *suc)\n {\n \tint i;\n-\tstruct strbuf sb = STRBUF_INIT;\n \n \trun_processes_parallel(suc->max_jobs,\n \t\t\t       update_clone_get_next_task,\n@@ -1749,16 +1756,9 @@ static int update_submodules(struct submodule_update_clone *suc)\n \tif (suc->quickstop)\n \t\treturn 1;\n \n-\tfor (i = 0; i < suc->update_clone_nr; i++) {\n-\t\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n-\t\t\toid_to_hex(&suc->update_clone[i].oid),\n-\t\t\tsuc->update_clone[i].just_cloned,\n-\t\t\tsuc->update_clone[i].sub->path);\n-\t\tfprintf(stdout, \"%s\", sb.buf);\n-\t\tstrbuf_reset(&sb);\n-\t}\n+\tfor (i = 0; i < suc->update_clone_nr; i++)\n+\t\tupdate_submodule(&suc->update_clone[i]);\n \n-\tstrbuf_release(&sb);\n \treturn 0;\n }\n \n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354451","messageId":"20180803222322.261813-7-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:21Z","receivedAt":"2018-08-03T22:24:19Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"e98317508c0 (submodule: ensure core.worktree is set after update,\n2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir\nas that ensures both the 'core.worktree' configuration is set as well as\nsetting up correct gitlink file pointing at the git directory.\n\nWe do not need to check for the gitlink in this part of the cmd_update\nin git-submodule.sh, as the initial call to update-clone will have ensured\nthat. So we can reduce the work to only (check and potentially) set the\n'core.worktree' setting.\n\nWhile at it move the check from shell to C as that proves to be useful in\na follow up patch, as we do not need the 'name' in shell now.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\n---\n builtin/submodule--helper.c | 64 +++++++++++++++++++++++--------------\n git-submodule.sh            |  7 ++--\n 2 files changed, 42 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 8b1088ab58a..e7635d5d9ab 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int ensure_core_worktree(int argc, const char **argv, const char *prefix)\n+{\n+\tconst struct submodule *sub;\n+\tconst char *path;\n+\tchar *cw;\n+\tstruct repository subrepo;\n+\n+\tif (argc != 2)\n+\t\tBUG(\"submodule--helper connect-gitdir-workingtree <name> <path>\");\n+\n+\tpath = argv[1];\n+\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n+\tif (!sub)\n+\t\tBUG(\"We could get the submodule handle before?\");\n+\n+\tif (repo_submodule_init(&subrepo, the_repository, path))\n+\t\tdie(_(\"could not get a repository handle for submodule '%s'\"), path);\n+\n+\tif (!repo_config_get_string(&subrepo, \"core.worktree\", &cw)) {\n+\t\tchar *cfg_file, *abs_path;\n+\t\tconst char *rel_path;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tcfg_file = xstrfmt(\"%s/config\", subrepo.gitdir);\n+\n+\t\tabs_path = absolute_pathdup(path);\n+\t\trel_path = relative_path(abs_path, subrepo.gitdir, &sb);\n+\n+\t\tgit_config_set_in_file(cfg_file, \"core.worktree\", rel_path);\n+\n+\t\tfree(cfg_file);\n+\t\tfree(abs_path);\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -2029,29 +2068,6 @@ static int check_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *name, *path;\n-\tchar *sm_gitdir;\n-\n-\tif (argc != 3)\n-\t\tBUG(\"submodule--helper connect-gitdir-workingtree <name> <path>\");\n-\n-\tname = argv[1];\n-\tpath = argv[2];\n-\n-\tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n-\tsm_gitdir = absolute_pathdup(sb.buf);\n-\n-\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n-\n-\tstrbuf_release(&sb);\n-\tfree(sm_gitdir);\n-\n-\treturn 0;\n-}\n-\n #define SUPPORT_SUPER_PREFIX (1<<0)\n \n struct cmd_struct {\n@@ -2065,7 +2081,7 @@ static struct cmd_struct commands[] = {\n \t{\"name\", module_name, 0},\n \t{\"clone\", module_clone, 0},\n \t{\"update-clone\", update_clone, 0},\n-\t{\"connect-gitdir-workingtree\", connect_gitdir_workingtree, 0},\n+\t{\"ensure-core-worktree\", ensure_core_worktree, 0},\n \t{\"relative-path\", resolve_relative_path, 0},\n \t{\"resolve-relative-url\", resolve_relative_url, 0},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8caaf274e25..19d010eac06 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -535,6 +535,8 @@ cmd_update()\n \tdo\n \t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n \n+\t\tgit submodule--helper ensure-core-worktree \"$sm_path\"\n+\n \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n \t\tif ! test -z \"$update\"\n \t\tthen\n@@ -577,11 +579,6 @@ cmd_update()\n \t\t\tdie \"$(eval_gettext \"Unable to find current \\${remote_name}/\\${branch} revision in submodule path '\\$sm_path'\")\"\n \t\tfi\n \n-\t\tif ! $(git config -f \"$(git rev-parse --git-common-dir)/modules/$name/config\" core.worktree) 2>/dev/null\n-\t\tthen\n-\t\t\tgit submodule--helper connect-gitdir-workingtree \"$name\" \"$sm_path\"\n-\t\tfi\n-\n \t\tif test \"$subsha1\" != \"$sha1\" || test -n \"$force\"\n \t\tthen\n \t\t\tsubforce=$force\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354452","messageId":"20180803222322.261813-8-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 7/7] submodule--helper: introduce new update-module-mode helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-03T22:23:22Z","receivedAt":"2018-08-03T22:24:21Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This chews off a bit of the shell part of the update command in\ngit-submodule.sh. When writing the C code, keep in mind that the\nsubmodule--helper part will go away eventually and we want to have\na C function that is able to determine the submodule update strategy,\nit as a nicety, make determine_submodule_update_strategy accessible\nfor arbitrary repositories.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 61 +++++++++++++++++++++++++++++++++++++\n git-submodule.sh            | 16 +---------\n 2 files changed, 62 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex e7635d5d9ab..e72157664f5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1446,6 +1446,66 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static void determine_submodule_update_strategy(struct repository *r,\n+\t\t\t\t\t\tint just_cloned,\n+\t\t\t\t\t\tconst char *path,\n+\t\t\t\t\t\tconst char *update,\n+\t\t\t\t\t\tstruct submodule_update_strategy *out)\n+{\n+\tconst struct submodule *sub = submodule_from_path(r, &null_oid, path);\n+\tchar *key;\n+\tconst char *val;\n+\n+\tkey = xstrfmt(\"submodule.%s.update\", sub->name);\n+\n+\tif (update) {\n+\t\ttrace_printf(\"parsing update\");\n+\t\tif (parse_submodule_update_strategy(update, out) < 0)\n+\t\t\tdie(_(\"Invalid update mode '%s' for submodule path '%s'\"),\n+\t\t\t\tupdate, path);\n+\t} else if (!repo_config_get_string_const(r, key, &val)) {\n+\t\tif (parse_submodule_update_strategy(val, out) < 0)\n+\t\t\tdie(_(\"Invalid update mode '%s' configured for submodule path '%s'\"),\n+\t\t\t\tval, path);\n+\t} else if (sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) {\n+\t\ttrace_printf(\"loaded thing\");\n+\t\tout->type = sub->update_strategy.type;\n+\t\tout->command = sub->update_strategy.command;\n+\t} else\n+\t\tout->type = SM_UPDATE_CHECKOUT;\n+\n+\tif (just_cloned &&\n+\t    (out->type == SM_UPDATE_MERGE ||\n+\t     out->type == SM_UPDATE_REBASE ||\n+\t     out->type == SM_UPDATE_NONE))\n+\t\tout->type = SM_UPDATE_CHECKOUT;\n+\n+\tfree(key);\n+}\n+\n+static int module_update_module_mode(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *path, *update = NULL;\n+\tint just_cloned;\n+\tstruct submodule_update_strategy update_strategy = { .type = SM_UPDATE_CHECKOUT };\n+\n+\tif (argc < 3 || argc > 4)\n+\t\tdie(\"submodule--helper update-module-clone expects <just-cloned> <path> [<update>]\");\n+\n+\tjust_cloned = git_config_int(\"just_cloned\", argv[1]);\n+\tpath = argv[2];\n+\n+\tif (argc == 4)\n+\t\tupdate = argv[3];\n+\n+\tdetermine_submodule_update_strategy(the_repository,\n+\t\t\t\t\t    just_cloned, path, update,\n+\t\t\t\t\t    &update_strategy);\n+\tfputs(submodule_strategy_to_string(&update_strategy), stdout);\n+\n+\treturn 0;\n+}\n+\n struct update_clone_data {\n \tconst struct submodule *sub;\n \tstruct object_id oid;\n@@ -2080,6 +2140,7 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list, 0},\n \t{\"name\", module_name, 0},\n \t{\"clone\", module_clone, 0},\n+\t{\"update-module-mode\", module_update_module_mode, 0},\n \t{\"update-clone\", update_clone, 0},\n \t{\"ensure-core-worktree\", ensure_core_worktree, 0},\n \t{\"relative-path\", resolve_relative_path, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 19d010eac06..19c9f1215e1 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -537,27 +537,13 @@ cmd_update()\n \n \t\tgit submodule--helper ensure-core-worktree \"$sm_path\"\n \n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\t\tif ! test -z \"$update\"\n-\t\tthen\n-\t\t\tupdate_module=$update\n-\t\telse\n-\t\t\tupdate_module=$(git config submodule.\"$name\".update)\n-\t\t\tif test -z \"$update_module\"\n-\t\t\tthen\n-\t\t\t\tupdate_module=\"checkout\"\n-\t\t\tfi\n-\t\tfi\n+\t\tupdate_module=$(git submodule--helper update-module-mode $just_cloned \"$sm_path\" $update)\n \n \t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n \n \t\tif test $just_cloned -eq 1\n \t\tthen\n \t\t\tsubsha1=\n-\t\t\tcase \"$update_module\" in\n-\t\t\tmerge | rebase | none)\n-\t\t\t\tupdate_module=checkout ;;\n-\t\t\tesac\n \t\telse\n \t\t\tsubsha1=$(sanitize_submodule_env; cd \"$sm_path\" &&\n \t\t\t\tgit rev-parse --verify HEAD) ||\n-- \n2.18.0.132.g195c49a2227\n\n"},{"id":"354453","messageId":"xmqqwot7ulg9.fsf@gitster-ct.c.googlers.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"Re: [PATCH 0/7] Resend of sb/submodule-update-in-c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-03T22:36:22Z","receivedAt":"2018-08-03T22:36:26Z","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> * Introduce new patch\n>   \"submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree\"\n>   that resolves the conflict with earlier versions of this series with\n>   sb/submodule-core-worktree\n> * This series is based on master, which already contains \n>   sb/submodule-core-worktree\n\nThanks; as this is not a bugfix but a new way of implementing the\nthing, it is good to base it on 'master'.\n\nWill queue.\n"},{"id":"355165","messageId":"20180810214703.GB211322@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-7-sbeller@google.com","subject":"Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-08-10T21:47:03Z","receivedAt":"2018-08-10T21:47:08Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 08/03, Stefan Beller wrote:\n> e98317508c0 (submodule: ensure core.worktree is set after update,\n> 2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir\n> as that ensures both the 'core.worktree' configuration is set as well as\n> setting up correct gitlink file pointing at the git directory.\n> \n> We do not need to check for the gitlink in this part of the cmd_update\n> in git-submodule.sh, as the initial call to update-clone will have ensured\n> that. So we can reduce the work to only (check and potentially) set the\n> 'core.worktree' setting.\n> \n> While at it move the check from shell to C as that proves to be useful in\n> a follow up patch, as we do not need the 'name' in shell now.\n> \n> Signed-off-by: Stefan Beller <sbeller@google.com>\n> ---\n>  builtin/submodule--helper.c | 64 +++++++++++++++++++++++--------------\n>  git-submodule.sh            |  7 ++--\n>  2 files changed, 42 insertions(+), 29 deletions(-)\n> \n> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\n> index 8b1088ab58a..e7635d5d9ab 100644\n> --- a/builtin/submodule--helper.c\n> +++ b/builtin/submodule--helper.c\n> @@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, const char *prefix)\n>  \treturn 0;\n>  }\n>  \n> +static int ensure_core_worktree(int argc, const char **argv, const char *prefix)\n> +{\n> +\tconst struct submodule *sub;\n> +\tconst char *path;\n> +\tchar *cw;\n> +\tstruct repository subrepo;\n> +\n> +\tif (argc != 2)\n> +\t\tBUG(\"submodule--helper connect-gitdir-workingtree <name> <path>\");\n> +\n> +\tpath = argv[1];\n> +\n> +\tsub = submodule_from_path(the_repository, &null_oid, path);\n> +\tif (!sub)\n> +\t\tBUG(\"We could get the submodule handle before?\");\n> +\n> +\tif (repo_submodule_init(&subrepo, the_repository, path))\n> +\t\tdie(_(\"could not get a repository handle for submodule '%s'\"), path);\n> +\n> +\tif (!repo_config_get_string(&subrepo, \"core.worktree\", &cw)) {\n> +\t\tchar *cfg_file, *abs_path;\n> +\t\tconst char *rel_path;\n> +\t\tstruct strbuf sb = STRBUF_INIT;\n> +\n> +\t\tcfg_file = xstrfmt(\"%s/config\", subrepo.gitdir);\n\nAs I mentioned here:\nhttps://public-inbox.org/git/20180807230637.247200-1-bmwill@google.com/T/#t\n\nThis lines should probably be more like:\n\n  cfg_file = repo_git_path(&subrepo, \"config\");\n\n> +\n> +\t\tabs_path = absolute_pathdup(path);\n> +\t\trel_path = relative_path(abs_path, subrepo.gitdir, &sb);\n> +\n> +\t\tgit_config_set_in_file(cfg_file, \"core.worktree\", rel_path);\n> +\n> +\t\tfree(cfg_file);\n> +\t\tfree(abs_path);\n> +\t\tstrbuf_release(&sb);\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n>  static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n>  {\n>  \tint i;\n> @@ -2029,29 +2068,6 @@ static int check_name(int argc, const char **argv, const char *prefix)\n>  \treturn 0;\n>  }\n>  \n> -static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)\n> -{\n> -\tstruct strbuf sb = STRBUF_INIT;\n> -\tconst char *name, *path;\n> -\tchar *sm_gitdir;\n> -\n> -\tif (argc != 3)\n> -\t\tBUG(\"submodule--helper connect-gitdir-workingtree <name> <path>\");\n> -\n> -\tname = argv[1];\n> -\tpath = argv[2];\n> -\n> -\tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n> -\tsm_gitdir = absolute_pathdup(sb.buf);\n> -\n> -\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n> -\n> -\tstrbuf_release(&sb);\n> -\tfree(sm_gitdir);\n> -\n> -\treturn 0;\n> -}\n> -\n>  #define SUPPORT_SUPER_PREFIX (1<<0)\n>  \n>  struct cmd_struct {\n> @@ -2065,7 +2081,7 @@ static struct cmd_struct commands[] = {\n>  \t{\"name\", module_name, 0},\n>  \t{\"clone\", module_clone, 0},\n>  \t{\"update-clone\", update_clone, 0},\n> -\t{\"connect-gitdir-workingtree\", connect_gitdir_workingtree, 0},\n> +\t{\"ensure-core-worktree\", ensure_core_worktree, 0},\n>  \t{\"relative-path\", resolve_relative_path, 0},\n>  \t{\"resolve-relative-url\", resolve_relative_url, 0},\n>  \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n> diff --git a/git-submodule.sh b/git-submodule.sh\n> index 8caaf274e25..19d010eac06 100755\n> --- a/git-submodule.sh\n> +++ b/git-submodule.sh\n> @@ -535,6 +535,8 @@ cmd_update()\n>  \tdo\n>  \t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n>  \n> +\t\tgit submodule--helper ensure-core-worktree \"$sm_path\"\n> +\n>  \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n>  \t\tif ! test -z \"$update\"\n>  \t\tthen\n> @@ -577,11 +579,6 @@ cmd_update()\n>  \t\t\tdie \"$(eval_gettext \"Unable to find current \\${remote_name}/\\${branch} revision in submodule path '\\$sm_path'\")\"\n>  \t\tfi\n>  \n> -\t\tif ! $(git config -f \"$(git rev-parse --git-common-dir)/modules/$name/config\" core.worktree) 2>/dev/null\n> -\t\tthen\n> -\t\t\tgit submodule--helper connect-gitdir-workingtree \"$name\" \"$sm_path\"\n> -\t\tfi\n> -\n>  \t\tif test \"$subsha1\" != \"$sha1\" || test -n \"$force\"\n>  \t\tthen\n>  \t\t\tsubforce=$force\n> -- \n> 2.18.0.132.g195c49a2227\n> \n\n-- \nBrandon Williams\n"},{"id":"355166","messageId":"CAGZ79kb+QyCuBw+e8ShU3Ts9GL+bhzb=i2F+5B0jb9eWk5Sj1w@mail.gmail.com","threadId":"49027","inReplyTo":"20180810214703.GB211322@google.com","subject":"Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-10T21:52:22Z","receivedAt":"2018-08-10T21:52:37Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"> > +             cfg_file = xstrfmt(\"%s/config\", subrepo.gitdir);\n>\n> As I mentioned here:\n> https://public-inbox.org/git/20180807230637.247200-1-bmwill@google.com/T/#t\n>\n> This lines should probably be more like:\n>\n>   cfg_file = repo_git_path(&subrepo, \"config\");\n>\n\nWhy? You did not mention the benefits for writing it this way\nhere or on the reference. Care to elaborate?\n"},{"id":"355170","messageId":"20180810220251.GC211322@google.com","threadId":"49027","inReplyTo":"CAGZ79kb+QyCuBw+e8ShU3Ts9GL+bhzb=i2F+5B0jb9eWk5Sj1w@mail.gmail.com","subject":"Re: [PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree","fromName":"Brandon Williams","fromEmail":"bmwill@google.com","sentAt":"2018-08-10T22:02:51Z","receivedAt":"2018-08-10T22:02:57Z","isPatch":true,"sender":{"key":"bwilliams.eng@gmail.com","avatar":null},"body":"On 08/10, Stefan Beller wrote:\n> > > +             cfg_file = xstrfmt(\"%s/config\", subrepo.gitdir);\n> >\n> > As I mentioned here:\n> > https://public-inbox.org/git/20180807230637.247200-1-bmwill@google.com/T/#t\n> >\n> > This lines should probably be more like:\n> >\n> >   cfg_file = repo_git_path(&subrepo, \"config\");\n> >\n> \n> Why? You did not mention the benefits for writing it this way\n> here or on the reference. Care to elaborate?\n\nIts more future proof especially because we have the difference bettwen\ncommondir and gitdir for worktrees.  Using the \"repo_git_path\" function\nhandles path rewritting when using worktrees.  Here (when working with\nworktrees) \"subrepo.gitdir\" refers to the worktree specific gitdir while\n\"subrepo.commondir\" refers to the global common gitdir where the\nrepository config actually lives.\n\n-- \nBrandon Williams\n"},{"id":"355495","messageId":"20180813224235.154580-1-sbeller@google.com","threadId":"49027","inReplyTo":"20180803222322.261813-1-sbeller@google.com","subject":"[PATCH 0/7] Resend of sb/submodule-update-in-c","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:28Z","receivedAt":"2018-08-13T22:42:42Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Thanks Brandon for pointing out to use repo_git_path instead of\nmanually constructing the path.\n\nThat is the only change in this resend.\n\nThanks,\nStefan\n\nStefan Beller (7):\n  git-submodule.sh: align error reporting for update mode to use path\n  git-submodule.sh: rename unused variables\n  builtin/submodule--helper: factor out submodule updating\n  builtin/submodule--helper: store update_clone information in a struct\n  builtin/submodule--helper: factor out method to update a single\n    submodule\n  submodule--helper: replace connect-gitdir-workingtree by\n    ensure-core-worktree\n  submodule--helper: introduce new update-module-mode helper\n\n builtin/submodule--helper.c | 216 ++++++++++++++++++++++++++----------\n git-submodule.sh            |  29 +----\n 2 files changed, 164 insertions(+), 81 deletions(-)\n\n(I am not yet using format-patches internal range diff version,\nbut  the paste below is manually crafted; the patch numbers are off, as\nthe fix was done in the second to last patch)\n\n./git-range-diff origin/sb/submodule-update-in-c...\n\n1:  1c866b9831d ! 1:  7bb6249dea9 submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree\n    @@ -49,7 +49,7 @@\n     +\t\tconst char *rel_path;\n     +\t\tstruct strbuf sb = STRBUF_INIT;\n     +\n    -+\t\tcfg_file = xstrfmt(\"%s/config\", subrepo.gitdir);\n    ++\t\tcfg_file = repo_git_path(&subrepo, \"config\");\n     +\n     +\t\tabs_path = absolute_pathdup(path);\n     +\t\trel_path = relative_path(abs_path, subrepo.gitdir, &sb);\n2:  5a3587e9c25 = 2:  23dc45cee2d submodule--helper: introduce new update-module-mode helper\n"},{"id":"355496","messageId":"20180813224235.154580-2-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 1/7] git-submodule.sh: align error reporting for update mode to use path","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:29Z","receivedAt":"2018-08-13T22:42:46Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"All other error messages in cmd_update are reporting the submodule based\non its path, so let's do that for invalid update modes, too.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n git-submodule.sh | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8b5ad59bdee..5a58812645d 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -632,7 +632,7 @@ cmd_update()\n \t\t\t\tmust_die_on_failure=yes\n \t\t\t\t;;\n \t\t\t*)\n-\t\t\t\tdie \"$(eval_gettext \"Invalid update mode '$update_module' for submodule '$name'\")\"\n+\t\t\t\tdie \"$(eval_gettext \"Invalid update mode '$update_module' for submodule path '$path'\")\"\n \t\t\tesac\n \n \t\t\tif (sanitize_submodule_env; cd \"$sm_path\" && $command \"$sha1\")\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355497","messageId":"20180813224235.154580-3-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 2/7] git-submodule.sh: rename unused variables","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:30Z","receivedAt":"2018-08-13T22:42:49Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The 'mode' variable is not used in cmd_update for its original purpose,\nrename it to 'dummy' as it only serves the purpose to abort quickly\ndocumenting this knowledge.\n\nThe variable 'stage' is also not used any more in cmd_update, so remove it.\n\nThis went unnoticed as first each function used the commonly used\nsubmodule listing, which was converted in 74703a1e4df (submodule: rewrite\n`module_list` shell function in C, 2015-09-02). When cmd_update was\nusing its own function starting in 48308681b07 (git submodule update:\nhave a dedicated helper for cloning, 2016-02-29), its removal was missed.\n\nA later patch in this series also touches the communication between\nthe submodule helper and git-submodule.sh, but let's have this as\na preparatory patch, as it eases the next patch, which stores the\nraw data instead of the line printed for this communication.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 5 ++---\n git-submodule.sh            | 4 ++--\n 2 files changed, 4 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex a3c4564c6c8..da700c88963 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1573,9 +1573,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tneeds_cloning = !file_exists(sb.buf);\n \n \tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"%06o %s %d %d\\t%s\\n\", ce->ce_mode,\n-\t\t\toid_to_hex(&ce->oid), ce_stage(ce),\n-\t\t\tneeds_cloning, ce->name);\n+\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n+\t\t    oid_to_hex(&ce->oid), needs_cloning, ce->name);\n \tstring_list_append(&suc->projectlines, sb.buf);\n \n \tif (!needs_cloning)\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 5a58812645d..8caaf274e25 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -531,9 +531,9 @@ cmd_update()\n \t\t\"$@\" || echo \"#unmatched\" $?\n \t} | {\n \terr=\n-\twhile read -r mode sha1 stage just_cloned sm_path\n+\twhile read -r quickabort sha1 just_cloned sm_path\n \tdo\n-\t\tdie_if_unmatched \"$mode\" \"$sha1\"\n+\t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n \n \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n \t\tif ! test -z \"$update\"\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355498","messageId":"20180813224235.154580-4-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 3/7] builtin/submodule--helper: factor out submodule updating","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:31Z","receivedAt":"2018-08-13T22:42:52Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"Separate the command line parsing from the actual execution of the command\nwithin the repository. For now there is not a lot of execution as\nmost of it is still in git-submodule.sh.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 59 +++++++++++++++++++++----------------\n 1 file changed, 33 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex da700c88963..32f00ca6f87 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1474,6 +1474,8 @@ struct submodule_update_clone {\n \t/* failed clones to be retried again */\n \tconst struct cache_entry **failed_clones;\n \tint failed_clones_nr, failed_clones_alloc;\n+\n+\tint max_jobs;\n };\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \\\n@@ -1716,11 +1718,36 @@ static int git_update_clone_config(const char *var, const char *value,\n \treturn 0;\n }\n \n+static int update_submodules(struct submodule_update_clone *suc)\n+{\n+\tstruct string_list_item *item;\n+\n+\trun_processes_parallel(suc->max_jobs,\n+\t\t\t       update_clone_get_next_task,\n+\t\t\t       update_clone_start_failure,\n+\t\t\t       update_clone_task_finished,\n+\t\t\t       suc);\n+\n+\t/*\n+\t * We saved the output and put it out all at once now.\n+\t * That means:\n+\t * - the listener does not have to interleave their (checkout)\n+\t *   work with our fetching.  The writes involved in a\n+\t *   checkout involve more straightforward sequential I/O.\n+\t * - the listener can avoid doing any work if fetching failed.\n+\t */\n+\tif (suc->quickstop)\n+\t\treturn 1;\n+\n+\tfor_each_string_list_item(item, &suc->projectlines)\n+\t\tfprintf(stdout, \"%s\", item->string);\n+\n+\treturn 0;\n+}\n+\n static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tconst char *update = NULL;\n-\tint max_jobs = 1;\n-\tstruct string_list_item *item;\n \tstruct pathspec pathspec;\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n \n@@ -1742,7 +1769,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"depth\", &suc.depth, \"<depth>\",\n \t\t\t   N_(\"Create a shallow clone truncated to the \"\n \t\t\t      \"specified number of revisions\")),\n-\t\tOPT_INTEGER('j', \"jobs\", &max_jobs,\n+\t\tOPT_INTEGER('j', \"jobs\", &suc.max_jobs,\n \t\t\t    N_(\"parallel jobs\")),\n \t\tOPT_BOOL(0, \"recommend-shallow\", &suc.recommend_shallow,\n \t\t\t    N_(\"whether the initial clone should follow the shallow recommendation\")),\n@@ -1758,8 +1785,8 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t};\n \tsuc.prefix = prefix;\n \n-\tupdate_clone_config_from_gitmodules(&max_jobs);\n-\tgit_config(git_update_clone_config, &max_jobs);\n+\tupdate_clone_config_from_gitmodules(&suc.max_jobs);\n+\tgit_config(git_update_clone_config, &suc.max_jobs);\n \n \targc = parse_options(argc, argv, prefix, module_update_clone_options,\n \t\t\t     git_submodule_helper_usage, 0);\n@@ -1774,27 +1801,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tif (pathspec.nr)\n \t\tsuc.warn_if_uninitialized = 1;\n \n-\trun_processes_parallel(max_jobs,\n-\t\t\t       update_clone_get_next_task,\n-\t\t\t       update_clone_start_failure,\n-\t\t\t       update_clone_task_finished,\n-\t\t\t       &suc);\n-\n-\t/*\n-\t * We saved the output and put it out all at once now.\n-\t * That means:\n-\t * - the listener does not have to interleave their (checkout)\n-\t *   work with our fetching.  The writes involved in a\n-\t *   checkout involve more straightforward sequential I/O.\n-\t * - the listener can avoid doing any work if fetching failed.\n-\t */\n-\tif (suc.quickstop)\n-\t\treturn 1;\n-\n-\tfor_each_string_list_item(item, &suc.projectlines)\n-\t\tfprintf(stdout, \"%s\", item->string);\n-\n-\treturn 0;\n+\treturn update_submodules(&suc);\n }\n \n static int resolve_relative_path(int argc, const char **argv, const char *prefix)\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355499","messageId":"20180813224235.154580-5-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 4/7] builtin/submodule--helper: store update_clone information in a struct","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:32Z","receivedAt":"2018-08-13T22:42:55Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"The information that is printed for update_submodules in\n'submodule--helper update-clone' and consumed by 'git submodule update'\nis stored as a string per submodule. This made sense at the time of\n48308681b07 (git submodule update: have a dedicated helper for cloning,\n2016-02-29), but as we want to migrate the rest of the submodule update\ninto C, we're better off having access to the raw information in a helper\nstruct.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 37 +++++++++++++++++++++++++++----------\n 1 file changed, 27 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 32f00ca6f87..40b94dd622e 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1446,6 +1446,12 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+struct update_clone_data {\n+\tconst struct submodule *sub;\n+\tstruct object_id oid;\n+\tunsigned just_cloned;\n+};\n+\n struct submodule_update_clone {\n \t/* index into 'list', the list of submodules to look into for cloning */\n \tint current;\n@@ -1465,8 +1471,9 @@ struct submodule_update_clone {\n \tconst char *recursive_prefix;\n \tconst char *prefix;\n \n-\t/* Machine-readable status lines to be consumed by git-submodule.sh */\n-\tstruct string_list projectlines;\n+\t/* to be consumed by git-submodule.sh */\n+\tstruct update_clone_data *update_clone;\n+\tint update_clone_nr; int update_clone_alloc;\n \n \t/* If we want to stop as fast as possible and return an error */\n \tunsigned quickstop : 1;\n@@ -1480,7 +1487,7 @@ struct submodule_update_clone {\n #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \\\n \tSUBMODULE_UPDATE_STRATEGY_INIT, 0, 0, -1, STRING_LIST_INIT_DUP, 0, \\\n \tNULL, NULL, NULL, \\\n-\tSTRING_LIST_INIT_DUP, 0, NULL, 0, 0}\n+\tNULL, 0, 0, 0, NULL, 0, 0, 0}\n \n \n static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n@@ -1574,10 +1581,12 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tstrbuf_addf(&sb, \"%s/.git\", ce->name);\n \tneeds_cloning = !file_exists(sb.buf);\n \n-\tstrbuf_reset(&sb);\n-\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n-\t\t    oid_to_hex(&ce->oid), needs_cloning, ce->name);\n-\tstring_list_append(&suc->projectlines, sb.buf);\n+\tALLOC_GROW(suc->update_clone, suc->update_clone_nr + 1,\n+\t\t   suc->update_clone_alloc);\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_nr++;\n \n \tif (!needs_cloning)\n \t\tgoto cleanup;\n@@ -1720,7 +1729,8 @@ static int git_update_clone_config(const char *var, const char *value,\n \n static int update_submodules(struct submodule_update_clone *suc)\n {\n-\tstruct string_list_item *item;\n+\tint i;\n+\tstruct strbuf sb = STRBUF_INIT;\n \n \trun_processes_parallel(suc->max_jobs,\n \t\t\t       update_clone_get_next_task,\n@@ -1739,9 +1749,16 @@ static int update_submodules(struct submodule_update_clone *suc)\n \tif (suc->quickstop)\n \t\treturn 1;\n \n-\tfor_each_string_list_item(item, &suc->projectlines)\n-\t\tfprintf(stdout, \"%s\", item->string);\n+\tfor (i = 0; i < suc->update_clone_nr; i++) {\n+\t\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n+\t\t\toid_to_hex(&suc->update_clone[i].oid),\n+\t\t\tsuc->update_clone[i].just_cloned,\n+\t\t\tsuc->update_clone[i].sub->path);\n+\t\tfprintf(stdout, \"%s\", sb.buf);\n+\t\tstrbuf_reset(&sb);\n+\t}\n \n+\tstrbuf_release(&sb);\n \treturn 0;\n }\n \n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355500","messageId":"20180813224235.154580-6-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 5/7] builtin/submodule--helper: factor out method to update a single submodule","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:33Z","receivedAt":"2018-08-13T22:42:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"In a later patch we'll find this method handy.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 20 ++++++++++----------\n 1 file changed, 10 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 40b94dd622e..8b1088ab58a 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1727,10 +1727,17 @@ static int git_update_clone_config(const char *var, const char *value,\n \treturn 0;\n }\n \n+static void update_submodule(struct update_clone_data *ucd)\n+{\n+\tfprintf(stdout, \"dummy %s %d\\t%s\\n\",\n+\t\toid_to_hex(&ucd->oid),\n+\t\tucd->just_cloned,\n+\t\tucd->sub->path);\n+}\n+\n static int update_submodules(struct submodule_update_clone *suc)\n {\n \tint i;\n-\tstruct strbuf sb = STRBUF_INIT;\n \n \trun_processes_parallel(suc->max_jobs,\n \t\t\t       update_clone_get_next_task,\n@@ -1749,16 +1756,9 @@ static int update_submodules(struct submodule_update_clone *suc)\n \tif (suc->quickstop)\n \t\treturn 1;\n \n-\tfor (i = 0; i < suc->update_clone_nr; i++) {\n-\t\tstrbuf_addf(&sb, \"dummy %s %d\\t%s\\n\",\n-\t\t\toid_to_hex(&suc->update_clone[i].oid),\n-\t\t\tsuc->update_clone[i].just_cloned,\n-\t\t\tsuc->update_clone[i].sub->path);\n-\t\tfprintf(stdout, \"%s\", sb.buf);\n-\t\tstrbuf_reset(&sb);\n-\t}\n+\tfor (i = 0; i < suc->update_clone_nr; i++)\n+\t\tupdate_submodule(&suc->update_clone[i]);\n \n-\tstrbuf_release(&sb);\n \treturn 0;\n }\n \n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355501","messageId":"20180813224235.154580-7-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 6/7] submodule--helper: replace connect-gitdir-workingtree by ensure-core-worktree","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:34Z","receivedAt":"2018-08-13T22:43:00Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"e98317508c0 (submodule: ensure core.worktree is set after update,\n2018-06-18) was overly aggressive in calling connect_work_tree_and_git_dir\nas that ensures both the 'core.worktree' configuration is set as well as\nsetting up correct gitlink file pointing at the git directory.\n\nWe do not need to check for the gitlink in this part of the cmd_update\nin git-submodule.sh, as the initial call to update-clone will have ensured\nthat. So we can reduce the work to only (check and potentially) set the\n'core.worktree' setting.\n\nWhile at it move the check from shell to C as that proves to be useful in\na follow up patch, as we do not need the 'name' in shell now.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 64 +++++++++++++++++++++++--------------\n git-submodule.sh            |  7 ++--\n 2 files changed, 42 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 8b1088ab58a..648e1330c15 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1964,6 +1964,45 @@ static int push_check(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static int ensure_core_worktree(int argc, const char **argv, const char *prefix)\n+{\n+\tconst struct submodule *sub;\n+\tconst char *path;\n+\tchar *cw;\n+\tstruct repository subrepo;\n+\n+\tif (argc != 2)\n+\t\tBUG(\"submodule--helper connect-gitdir-workingtree <name> <path>\");\n+\n+\tpath = argv[1];\n+\n+\tsub = submodule_from_path(the_repository, &null_oid, path);\n+\tif (!sub)\n+\t\tBUG(\"We could get the submodule handle before?\");\n+\n+\tif (repo_submodule_init(&subrepo, the_repository, path))\n+\t\tdie(_(\"could not get a repository handle for submodule '%s'\"), path);\n+\n+\tif (!repo_config_get_string(&subrepo, \"core.worktree\", &cw)) {\n+\t\tchar *cfg_file, *abs_path;\n+\t\tconst char *rel_path;\n+\t\tstruct strbuf sb = STRBUF_INIT;\n+\n+\t\tcfg_file = repo_git_path(&subrepo, \"config\");\n+\n+\t\tabs_path = absolute_pathdup(path);\n+\t\trel_path = relative_path(abs_path, subrepo.gitdir, &sb);\n+\n+\t\tgit_config_set_in_file(cfg_file, \"core.worktree\", rel_path);\n+\n+\t\tfree(cfg_file);\n+\t\tfree(abs_path);\n+\t\tstrbuf_release(&sb);\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -2029,29 +2068,6 @@ static int check_name(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static int connect_gitdir_workingtree(int argc, const char **argv, const char *prefix)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *name, *path;\n-\tchar *sm_gitdir;\n-\n-\tif (argc != 3)\n-\t\tBUG(\"submodule--helper connect-gitdir-workingtree <name> <path>\");\n-\n-\tname = argv[1];\n-\tpath = argv[2];\n-\n-\tstrbuf_addf(&sb, \"%s/modules/%s\", get_git_dir(), name);\n-\tsm_gitdir = absolute_pathdup(sb.buf);\n-\n-\tconnect_work_tree_and_git_dir(path, sm_gitdir, 0);\n-\n-\tstrbuf_release(&sb);\n-\tfree(sm_gitdir);\n-\n-\treturn 0;\n-}\n-\n #define SUPPORT_SUPER_PREFIX (1<<0)\n \n struct cmd_struct {\n@@ -2065,7 +2081,7 @@ static struct cmd_struct commands[] = {\n \t{\"name\", module_name, 0},\n \t{\"clone\", module_clone, 0},\n \t{\"update-clone\", update_clone, 0},\n-\t{\"connect-gitdir-workingtree\", connect_gitdir_workingtree, 0},\n+\t{\"ensure-core-worktree\", ensure_core_worktree, 0},\n \t{\"relative-path\", resolve_relative_path, 0},\n \t{\"resolve-relative-url\", resolve_relative_url, 0},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 8caaf274e25..19d010eac06 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -535,6 +535,8 @@ cmd_update()\n \tdo\n \t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n \n+\t\tgit submodule--helper ensure-core-worktree \"$sm_path\"\n+\n \t\tname=$(git submodule--helper name \"$sm_path\") || exit\n \t\tif ! test -z \"$update\"\n \t\tthen\n@@ -577,11 +579,6 @@ cmd_update()\n \t\t\tdie \"$(eval_gettext \"Unable to find current \\${remote_name}/\\${branch} revision in submodule path '\\$sm_path'\")\"\n \t\tfi\n \n-\t\tif ! $(git config -f \"$(git rev-parse --git-common-dir)/modules/$name/config\" core.worktree) 2>/dev/null\n-\t\tthen\n-\t\t\tgit submodule--helper connect-gitdir-workingtree \"$name\" \"$sm_path\"\n-\t\tfi\n-\n \t\tif test \"$subsha1\" != \"$sha1\" || test -n \"$force\"\n \t\tthen\n \t\t\tsubforce=$force\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355502","messageId":"20180813224235.154580-8-sbeller@google.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"[PATCH 7/7] submodule--helper: introduce new update-module-mode helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-13T22:42:35Z","receivedAt":"2018-08-13T22:43:03Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"This chews off a bit of the shell part of the update command in\ngit-submodule.sh. When writing the C code, keep in mind that the\nsubmodule--helper part will go away eventually and we want to have\na C function that is able to determine the submodule update strategy,\nit as a nicety, make determine_submodule_update_strategy accessible\nfor arbitrary repositories.\n\nSigned-off-by: Stefan Beller <sbeller@google.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 61 +++++++++++++++++++++++++++++++++++++\n git-submodule.sh            | 16 +---------\n 2 files changed, 62 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 648e1330c15..5c9d1fb496d 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1446,6 +1446,66 @@ static int module_clone(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n+static void determine_submodule_update_strategy(struct repository *r,\n+\t\t\t\t\t\tint just_cloned,\n+\t\t\t\t\t\tconst char *path,\n+\t\t\t\t\t\tconst char *update,\n+\t\t\t\t\t\tstruct submodule_update_strategy *out)\n+{\n+\tconst struct submodule *sub = submodule_from_path(r, &null_oid, path);\n+\tchar *key;\n+\tconst char *val;\n+\n+\tkey = xstrfmt(\"submodule.%s.update\", sub->name);\n+\n+\tif (update) {\n+\t\ttrace_printf(\"parsing update\");\n+\t\tif (parse_submodule_update_strategy(update, out) < 0)\n+\t\t\tdie(_(\"Invalid update mode '%s' for submodule path '%s'\"),\n+\t\t\t\tupdate, path);\n+\t} else if (!repo_config_get_string_const(r, key, &val)) {\n+\t\tif (parse_submodule_update_strategy(val, out) < 0)\n+\t\t\tdie(_(\"Invalid update mode '%s' configured for submodule path '%s'\"),\n+\t\t\t\tval, path);\n+\t} else if (sub->update_strategy.type != SM_UPDATE_UNSPECIFIED) {\n+\t\ttrace_printf(\"loaded thing\");\n+\t\tout->type = sub->update_strategy.type;\n+\t\tout->command = sub->update_strategy.command;\n+\t} else\n+\t\tout->type = SM_UPDATE_CHECKOUT;\n+\n+\tif (just_cloned &&\n+\t    (out->type == SM_UPDATE_MERGE ||\n+\t     out->type == SM_UPDATE_REBASE ||\n+\t     out->type == SM_UPDATE_NONE))\n+\t\tout->type = SM_UPDATE_CHECKOUT;\n+\n+\tfree(key);\n+}\n+\n+static int module_update_module_mode(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *path, *update = NULL;\n+\tint just_cloned;\n+\tstruct submodule_update_strategy update_strategy = { .type = SM_UPDATE_CHECKOUT };\n+\n+\tif (argc < 3 || argc > 4)\n+\t\tdie(\"submodule--helper update-module-clone expects <just-cloned> <path> [<update>]\");\n+\n+\tjust_cloned = git_config_int(\"just_cloned\", argv[1]);\n+\tpath = argv[2];\n+\n+\tif (argc == 4)\n+\t\tupdate = argv[3];\n+\n+\tdetermine_submodule_update_strategy(the_repository,\n+\t\t\t\t\t    just_cloned, path, update,\n+\t\t\t\t\t    &update_strategy);\n+\tfputs(submodule_strategy_to_string(&update_strategy), stdout);\n+\n+\treturn 0;\n+}\n+\n struct update_clone_data {\n \tconst struct submodule *sub;\n \tstruct object_id oid;\n@@ -2080,6 +2140,7 @@ static struct cmd_struct commands[] = {\n \t{\"list\", module_list, 0},\n \t{\"name\", module_name, 0},\n \t{\"clone\", module_clone, 0},\n+\t{\"update-module-mode\", module_update_module_mode, 0},\n \t{\"update-clone\", update_clone, 0},\n \t{\"ensure-core-worktree\", ensure_core_worktree, 0},\n \t{\"relative-path\", resolve_relative_path, 0},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex 19d010eac06..19c9f1215e1 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -537,27 +537,13 @@ cmd_update()\n \n \t\tgit submodule--helper ensure-core-worktree \"$sm_path\"\n \n-\t\tname=$(git submodule--helper name \"$sm_path\") || exit\n-\t\tif ! test -z \"$update\"\n-\t\tthen\n-\t\t\tupdate_module=$update\n-\t\telse\n-\t\t\tupdate_module=$(git config submodule.\"$name\".update)\n-\t\t\tif test -z \"$update_module\"\n-\t\t\tthen\n-\t\t\t\tupdate_module=\"checkout\"\n-\t\t\tfi\n-\t\tfi\n+\t\tupdate_module=$(git submodule--helper update-module-mode $just_cloned \"$sm_path\" $update)\n \n \t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n \n \t\tif test $just_cloned -eq 1\n \t\tthen\n \t\t\tsubsha1=\n-\t\t\tcase \"$update_module\" in\n-\t\t\tmerge | rebase | none)\n-\t\t\t\tupdate_module=checkout ;;\n-\t\t\tesac\n \t\telse\n \t\t\tsubsha1=$(sanitize_submodule_env; cd \"$sm_path\" &&\n \t\t\t\tgit rev-parse --verify HEAD) ||\n-- \n2.18.0.865.gffc8e1a3cd6-goog\n\n"},{"id":"355619","messageId":"xmqqva8cy85f.fsf@gitster-ct.c.googlers.com","threadId":"49027","inReplyTo":"20180813224235.154580-1-sbeller@google.com","subject":"Re: [PATCH 0/7] Resend of sb/submodule-update-in-c","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-14T21:01:48Z","receivedAt":"2018-08-14T21:01:52Z","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> Thanks Brandon for pointing out to use repo_git_path instead of\n> manually constructing the path.\n>\n> That is the only change in this resend.\n\nRcpt.  Hopefully this is now ready for 'next'?\n"},{"id":"355628","messageId":"CAGZ79kbaNegk6kFD2Ks2S1ejd9f-8mLB2E_xNr1B3iQkn2EVPA@mail.gmail.com","threadId":"49027","inReplyTo":"xmqqva8cy85f.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 0/7] Resend of sb/submodule-update-in-c","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-14T21:24:13Z","receivedAt":"2018-08-14T21:24:27Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"gist\nOn Tue, Aug 14, 2018 at 2:01 PM Junio C Hamano <gitster@pobox.com> wrote:\n>\n> Stefan Beller <sbeller@google.com> writes:\n>\n> > Thanks Brandon for pointing out to use repo_git_path instead of\n> > manually constructing the path.\n> >\n> > That is the only change in this resend.\n>\n> Rcpt.  Hopefully this is now ready for 'next'?\n\nI don't know any reasons opposing its progression.\n\nSo, Yes, I think it is.\n\nStefan\n"},{"id":"355996","messageId":"CACsJy8BWTd5LEtZ00z7a1sOwx3n=RfPDqguNb+zTW0CZUUyJaA@mail.gmail.com","threadId":"49027","inReplyTo":"20180813224235.154580-8-sbeller@google.com","subject":"Re: [PATCH 7/7] submodule--helper: introduce new update-module-mode helper","fromName":"Duy Nguyen","fromEmail":"pclouds@gmail.com","sentAt":"2018-08-18T16:10:44Z","receivedAt":"2018-08-18T16:11:11Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, Aug 14, 2018 at 12:45 AM Stefan Beller <sbeller@google.com> wrote:\n> +static int module_update_module_mode(int argc, const char **argv, const char *prefix)\n> +{\n> +       const char *path, *update = NULL;\n> +       int just_cloned;\n> +       struct submodule_update_strategy update_strategy = { .type = SM_UPDATE_CHECKOUT };\n> +\n> +       if (argc < 3 || argc > 4)\n> +               die(\"submodule--helper update-module-clone expects <just-cloned> <path> [<update>]\");\n\nMaybe _() ?\n-- \nDuy\n"},{"id":"356132","messageId":"CAGZ79kYALb4=uth1mMFdYLQCz=Z0m0VJDaGe5zWmXbYNDFui-Q@mail.gmail.com","threadId":"49027","inReplyTo":"CACsJy8BWTd5LEtZ00z7a1sOwx3n=RfPDqguNb+zTW0CZUUyJaA@mail.gmail.com","subject":"Re: [PATCH 7/7] submodule--helper: introduce new update-module-mode helper","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2018-08-20T19:44:43Z","receivedAt":"2018-08-20T19:44:57Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Sat, Aug 18, 2018 at 9:11 AM Duy Nguyen <pclouds@gmail.com> wrote:\n>\n> On Tue, Aug 14, 2018 at 12:45 AM Stefan Beller <sbeller@google.com> wrote:\n> > +static int module_update_module_mode(int argc, const char **argv, const char *prefix)\n> > +{\n> > +       const char *path, *update = NULL;\n> > +       int just_cloned;\n> > +       struct submodule_update_strategy update_strategy = { .type = SM_UPDATE_CHECKOUT };\n> > +\n> > +       if (argc < 3 || argc > 4)\n> > +               die(\"submodule--helper update-module-clone expects <just-cloned> <path> [<update>]\");\n>\n> Maybe _() ?\n\nI would rather not, as the submodule--helper is \"internal only\" and these die()\ncalls could be clarified via\n\n    #define BUG_IN_CALLING_SH(x) die(x)\n\nAfter the conversion to C is done, all these submodule helpers would go away,\nso I'd not burden the translators too much?\n\nThanks,\nStefan\n"}]}