{"thread":{"id":"57567","subject":"[PATCH 0/7] Finish converting \"submodule update\" to C","startedAt":"2022-03-15T21:09:40Z","lastAt":"2022-03-15T21:10:01Z","messageCount":8,"participants":["Glen Choo"],"isPatch":true,"patchVersion":1,"patchTotal":7},"messages":[{"id":"451431","messageId":"20220315210925.79289-1-chooglen@google.com","threadId":"57567","inReplyTo":null,"subject":"[PATCH 0/7] Finish converting \"submodule update\" to C","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:18Z","receivedAt":"2022-03-15T21:09:40Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Original series: https://lore.kernel.org/git/20220210092833.55360-1-chooglen@google.com\n\"git submodule update\" in C part 1: https://lore.kernel.org/git/20220301000816.56177-1-chooglen@google.com/\nBased off 'gc/submodule-update-part-1'.\n\n= Overview\n\nThis is part 2 of 2 series that supersedes ar/submodule-update. This\nfinishes the conversion of \"git submodule update\" to C by:\n\n- combining the constituent \"git submodule--helper\" commands\n  (\"update-clone\" and \"run-update-procedure\") to create \"git\n  submodule--helper update\"\n- converting the --recursive flag to C by teaching \"git\n  submodule--helper update\" to invoke itself\n\nThis also fixes a test failure that Junio noted in\nhttps://lore.kernel.org/git/xmqqr18770pc.fsf@gitster.g (see patch 6 for\nmore details).\n\n= Known conflicts\n\nThis series is missing some functionality introduced in the in-flight\nes/superproject-aware-submodules [1], namely, that \"git submodule\nupdate\" sets `submodule.hasSuperproject` on all submodules. I intended\nfor this to go _after_ es/superproject-aware-submodules, and in that\nseries, Emily and I tried to simplify this conflict by preemptively\ndoing this in C (see that series' v9 cover letter [1]).\n\nThat discussion is still ongoing, but it seems helpful to send this\nseries anyway (thanks to all who weighed in over IRC [2]). At any rate,\nneither series really depends on the other, so this series is still\nindependently reviewable, but with the caveat that I may add another\npatch that adds the missing behavior (it will probably look something\nlike [3]).\n\nAlternatively, this series might graduate first. In which case, I'll\ncontinue working with Emily to fix the conflict.\n\n= Patch organization\n\nThis series largely follows the remaining ar/submodule-update patches\nthat weren't included in part 1 and some additional clean ups. One\nnotable difference vis-a-vis ar/submodule-update is that the CLI args\nfor \"update-clone\" are no longer copied across structs (see patch 4 for\nmore details).\n\n- Patches 1-2 teaches \"run-update-procedure\" to exit and report its\n  failures in a more idiomatic manner instead of relying on\n  git-submodule.sh.\n- Patches 3-4 prepare for \"run-update-procedure\" and\n  \"update-clone\"'s args to be merged into a single struct.\n- Patches 5-6 finish the process of implementing `submodule--helper\n  update`.\n- Patch 7 cleans up a forward declaration that kept the diffs small\n  while we were in the process of conversion, but is no longer needed.\n\n[1] https://lore.kernel.org/git/20220310004423.2627181-1-emilyshaffer@google.com\n[2] https://colabti.org/irclogger/irclogger_log/git-devel?date=2022-03-14\n[3] https://lore.kernel.org/git/kl6l1qz9s6tu.fsf@chooglen-macbookpro.roam.corp.google.com\n\nAtharva Raykar (2):\n  submodule--helper: run update using child process struct\n  submodule: move core cmd_update() logic to C\n\nGlen Choo (4):\n  submodule update: use die_message()\n  submodule--helper: teach update_data more options\n  submodule--helper: reduce logic in run_update_procedure()\n  submodule--helper: remove forward declaration\n\nÆvar Arnfjörð Bjarmason (1):\n  builtin/submodule--helper.c: rename option struct to \"opt\"\n\n builtin/submodule--helper.c | 691 ++++++++++++++++++------------------\n git-submodule.sh            | 105 +-----\n 2 files changed, 361 insertions(+), 435 deletions(-)\n\n\nbase-commit: c9d256249375c7b8a1773139791448860b5789ff\n-- \n2.33.GIT\n\n"},{"id":"451432","messageId":"20220315210925.79289-2-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 1/7] submodule--helper: run update using child process struct","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:19Z","receivedAt":"2022-03-15T21:09:42Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Atharva Raykar <raykar.ath@gmail.com>\n\nWe switch to using the run-command API function that takes a\n'struct child process', since we are using a lot of the options. This\nwill also make it simple to switch over to using 'capture_command()'\nwhen we start handling the output of the command completely in C.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Shourya Shukla <periperidip@gmail.com>\nSigned-off-by: Atharva Raykar <raykar.ath@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n builtin/submodule--helper.c | 34 ++++++++++++++++------------------\n 1 file changed, 16 insertions(+), 18 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex bef9ab22d4..95ef113d16 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2325,47 +2325,45 @@ static int fetch_in_submodule(const char *module_path, int depth, int quiet, str\n \n static int run_update_command(struct update_data *ud, int subforce)\n {\n-\tstruct strvec args = STRVEC_INIT;\n-\tstruct strvec child_env = STRVEC_INIT;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n \tchar *oid = oid_to_hex(&ud->oid);\n \tint must_die_on_failure = 0;\n-\tint git_cmd;\n \n \tswitch (ud->update_strategy.type) {\n \tcase SM_UPDATE_CHECKOUT:\n-\t\tgit_cmd = 1;\n-\t\tstrvec_pushl(&args, \"checkout\", \"-q\", NULL);\n+\t\tcp.git_cmd = 1;\n+\t\tstrvec_pushl(&cp.args, \"checkout\", \"-q\", NULL);\n \t\tif (subforce)\n-\t\t\tstrvec_push(&args, \"-f\");\n+\t\t\tstrvec_push(&cp.args, \"-f\");\n \t\tbreak;\n \tcase SM_UPDATE_REBASE:\n-\t\tgit_cmd = 1;\n-\t\tstrvec_push(&args, \"rebase\");\n+\t\tcp.git_cmd = 1;\n+\t\tstrvec_push(&cp.args, \"rebase\");\n \t\tif (ud->quiet)\n-\t\t\tstrvec_push(&args, \"--quiet\");\n+\t\t\tstrvec_push(&cp.args, \"--quiet\");\n \t\tmust_die_on_failure = 1;\n \t\tbreak;\n \tcase SM_UPDATE_MERGE:\n-\t\tgit_cmd = 1;\n-\t\tstrvec_push(&args, \"merge\");\n+\t\tcp.git_cmd = 1;\n+\t\tstrvec_push(&cp.args, \"merge\");\n \t\tif (ud->quiet)\n-\t\t\tstrvec_push(&args, \"--quiet\");\n+\t\t\tstrvec_push(&cp.args, \"--quiet\");\n \t\tmust_die_on_failure = 1;\n \t\tbreak;\n \tcase SM_UPDATE_COMMAND:\n-\t\tgit_cmd = 0;\n-\t\tstrvec_push(&args, ud->update_strategy.command);\n+\t\tcp.use_shell = 1;\n+\t\tstrvec_push(&cp.args, ud->update_strategy.command);\n \t\tmust_die_on_failure = 1;\n \t\tbreak;\n \tdefault:\n \t\tBUG(\"unexpected update strategy type: %s\",\n \t\t    submodule_strategy_to_string(&ud->update_strategy));\n \t}\n-\tstrvec_push(&args, oid);\n+\tstrvec_push(&cp.args, oid);\n \n-\tprepare_submodule_repo_env(&child_env);\n-\tif (run_command_v_opt_cd_env(args.v, git_cmd ? RUN_GIT_CMD : RUN_USING_SHELL,\n-\t\t\t\t     ud->sm_path, child_env.v)) {\n+\tcp.dir = xstrdup(ud->sm_path);\n+\tprepare_submodule_repo_env(&cp.env_array);\n+\tif (run_command(&cp)) {\n \t\tswitch (ud->update_strategy.type) {\n \t\tcase SM_UPDATE_CHECKOUT:\n \t\t\tprintf(_(\"Unable to checkout '%s' in submodule path '%s'\"),\n-- \n2.33.GIT\n\n"},{"id":"451433","messageId":"20220315210925.79289-3-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 2/7] submodule update: use die_message()","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:20Z","receivedAt":"2022-03-15T21:09:45Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Use die_message() to print the \"fatal: \" prefix instead of doing it in\ngit-submodule.sh and remove a now-unnecessary exit code from \"git\nsubmodule--helper run-update-procedure\".\n\nAlso, since die_message() adds the newline for us, replace an invocation\nof die_with_status() with printf + exit invocations that do not add a\nnewline, but are otherwise identical to die_with_status().\n\nHelped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Glen Choo <chooglen@google.com>\n---\nThis was spun out of\nhttps://lore.kernel.org/git/patch-v5-9.9-e8e57606ee9-20220128T125206Z-avarab@gmail.com\n\n builtin/submodule--helper.c | 33 ++++++++++++++-------------------\n git-submodule.sh            |  9 +++++----\n 2 files changed, 19 insertions(+), 23 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 95ef113d16..af1d90af7c 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2366,40 +2366,35 @@ static int run_update_command(struct update_data *ud, int subforce)\n \tif (run_command(&cp)) {\n \t\tswitch (ud->update_strategy.type) {\n \t\tcase SM_UPDATE_CHECKOUT:\n-\t\t\tprintf(_(\"Unable to checkout '%s' in submodule path '%s'\"),\n-\t\t\t       oid, ud->displaypath);\n+\t\t\tdie_message(_(\"Unable to checkout '%s' in submodule path '%s'\"),\n+\t\t\t\t    oid, ud->displaypath);\n \t\t\tbreak;\n \t\tcase SM_UPDATE_REBASE:\n-\t\t\tprintf(_(\"Unable to rebase '%s' in submodule path '%s'\"),\n-\t\t\t       oid, ud->displaypath);\n+\t\t\tdie_message(_(\"Unable to rebase '%s' in submodule path '%s'\"),\n+\t\t\t    oid, ud->displaypath);\n \t\t\tbreak;\n \t\tcase SM_UPDATE_MERGE:\n-\t\t\tprintf(_(\"Unable to merge '%s' in submodule path '%s'\"),\n-\t\t\t       oid, ud->displaypath);\n+\t\t\tdie_message(_(\"Unable to merge '%s' in submodule path '%s'\"),\n+\t\t\t    oid, ud->displaypath);\n \t\t\tbreak;\n \t\tcase SM_UPDATE_COMMAND:\n-\t\t\tprintf(_(\"Execution of '%s %s' failed in submodule path '%s'\"),\n-\t\t\t       ud->update_strategy.command, oid, ud->displaypath);\n+\t\t\tdie_message(_(\"Execution of '%s %s' failed in submodule path '%s'\"),\n+\t\t\t    ud->update_strategy.command, oid, ud->displaypath);\n \t\t\tbreak;\n \t\tdefault:\n \t\t\tBUG(\"unexpected update strategy type: %s\",\n \t\t\t    submodule_strategy_to_string(&ud->update_strategy));\n \t\t}\n-\t\t/*\n-\t\t * NEEDSWORK: We are currently printing to stdout with error\n-\t\t * return so that the shell caller handles the error output\n-\t\t * properly. Once we start handling the error messages within\n-\t\t * C, we should use die() instead.\n-\t\t */\n \t\tif (must_die_on_failure)\n-\t\t\treturn 2;\n-\t\t/*\n-\t\t * This signifies to the caller in shell that the command\n-\t\t * failed without dying\n-\t\t */\n+\t\t\texit(128);\n+\n+\t\t/* the command failed, but update must continue */\n \t\treturn 1;\n \t}\n \n+\tif (ud->quiet)\n+\t\treturn 0;\n+\n \tswitch (ud->update_strategy.type) {\n \tcase SM_UPDATE_CHECKOUT:\n \t\tprintf(_(\"Submodule path '%s': checked out '%s'\\n\"),\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex aa8bdfca9d..a84143daab 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -404,7 +404,7 @@ cmd_update()\n \t\t# exit codes for run-update-procedure:\n \t\t# 0: update was successful, say command output\n \t\t# 1: update procedure failed, but should not die\n-\t\t# 2 or 128: subcommand died during execution\n+\t\t# 128: subcommand died during execution\n \t\t# 3: no update procedure was run\n \t\tres=\"$?\"\n \t\tcase $res in\n@@ -412,11 +412,12 @@ cmd_update()\n \t\t\tsay \"$out\"\n \t\t\t;;\n \t\t1)\n-\t\t\terr=\"${err};fatal: $out\"\n+\t\t\terr=\"${err};$out\"\n \t\t\tcontinue\n \t\t\t;;\n-\t\t2|128)\n-\t\t\tdie_with_status $res \"fatal: $out\"\n+\t\t128)\n+\t\t\tprintf >&2 \"$out\"\n+\t\t\texit $res\n \t\t\t;;\n \t\tesac\n \n-- \n2.33.GIT\n\n"},{"id":"451434","messageId":"20220315210925.79289-4-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 3/7] builtin/submodule--helper.c: rename option struct to \"opt\"","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:21Z","receivedAt":"2022-03-15T21:09:47Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n\nIn a later commit, update_clone()'s options will be stored in a struct\nupdate_data instead of submodule_update_clone. Preemptively rename the\noptions struct to \"opt\" to shrink that commit's diff.\n\nHelped-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Glen Choo <chooglen@google.com>\n---\nThis is the successor to\nhttps://lore.kernel.org/git/20220210092833.55360-7-chooglen@google.com.\nIn that patch, we noted that renaming both functions' variables anchors\nthe final diff better, but patch 6 in this series shows that\nrun_update_procedure() is altogether deleted, so there's no point\nchanging that function.\n\n builtin/submodule--helper.c | 52 ++++++++++++++++++-------------------\n 1 file changed, 26 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex af1d90af7c..0e3cafb500 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2498,40 +2498,40 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n {\n \tconst char *update = NULL;\n \tstruct pathspec pathspec;\n-\tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n+\tstruct submodule_update_clone opt = SUBMODULE_UPDATE_CLONE_INIT;\n \tstruct list_objects_filter_options filter_options;\n \tint ret;\n \n \tstruct option module_update_clone_options[] = {\n-\t\tOPT_BOOL(0, \"init\", &suc.init,\n+\t\tOPT_BOOL(0, \"init\", &opt.init,\n \t\t\t N_(\"initialize uninitialized submodules before update\")),\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree\")),\n-\t\tOPT_STRING(0, \"recursive-prefix\", &suc.recursive_prefix,\n+\t\tOPT_STRING(0, \"recursive-prefix\", &opt.recursive_prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree, across nested \"\n \t\t\t      \"submodule boundaries\")),\n \t\tOPT_STRING(0, \"update\", &update,\n \t\t\t   N_(\"string\"),\n \t\t\t   N_(\"rebase, merge, checkout or none\")),\n-\t\tOPT_STRING_LIST(0, \"reference\", &suc.references, N_(\"repo\"),\n+\t\tOPT_STRING_LIST(0, \"reference\", &opt.references, N_(\"repo\"),\n \t\t\t   N_(\"reference repository\")),\n-\t\tOPT_BOOL(0, \"dissociate\", &suc.dissociate,\n+\t\tOPT_BOOL(0, \"dissociate\", &opt.dissociate,\n \t\t\t   N_(\"use --reference only while cloning\")),\n-\t\tOPT_STRING(0, \"depth\", &suc.depth, \"<depth>\",\n+\t\tOPT_STRING(0, \"depth\", &opt.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\", &suc.max_jobs,\n+\t\tOPT_INTEGER('j', \"jobs\", &opt.max_jobs,\n \t\t\t    N_(\"parallel jobs\")),\n-\t\tOPT_BOOL(0, \"recommend-shallow\", &suc.recommend_shallow,\n+\t\tOPT_BOOL(0, \"recommend-shallow\", &opt.recommend_shallow,\n \t\t\t    N_(\"whether the initial clone should follow the shallow recommendation\")),\n-\t\tOPT__QUIET(&suc.quiet, N_(\"don't print cloning progress\")),\n-\t\tOPT_BOOL(0, \"progress\", &suc.progress,\n+\t\tOPT__QUIET(&opt.quiet, N_(\"don't print cloning progress\")),\n+\t\tOPT_BOOL(0, \"progress\", &opt.progress,\n \t\t\t    N_(\"force cloning progress\")),\n-\t\tOPT_BOOL(0, \"require-init\", &suc.require_init,\n+\t\tOPT_BOOL(0, \"require-init\", &opt.require_init,\n \t\t\t   N_(\"disallow cloning into non-empty directory\")),\n-\t\tOPT_BOOL(0, \"single-branch\", &suc.single_branch,\n+\t\tOPT_BOOL(0, \"single-branch\", &opt.single_branch,\n \t\t\t N_(\"clone only one branch, HEAD or --branch\")),\n \t\tOPT_PARSE_LIST_OBJECTS_FILTER(&filter_options),\n \t\tOPT_END()\n@@ -2546,16 +2546,16 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\t\" [--recursive] [--[no-]single-branch] [--] [<path>...]\"),\n \t\tNULL\n \t};\n-\tsuc.prefix = prefix;\n+\topt.prefix = prefix;\n \n-\tupdate_clone_config_from_gitmodules(&suc.max_jobs);\n-\tgit_config(git_update_clone_config, &suc.max_jobs);\n+\tupdate_clone_config_from_gitmodules(&opt.max_jobs);\n+\tgit_config(git_update_clone_config, &opt.max_jobs);\n \n \tmemset(&filter_options, 0, sizeof(filter_options));\n \targc = parse_options(argc, argv, prefix, module_update_clone_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n-\tif (filter_options.choice && !suc.init) {\n+\tif (filter_options.choice && !opt.init) {\n \t\t/*\n \t\t * NEEDSWORK: Don't use usage_with_options() because the\n \t\t * usage string is for \"git submodule update\", but the\n@@ -2567,25 +2567,25 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tusage(git_submodule_helper_usage[0]);\n \t}\n \n-\tsuc.filter_options = &filter_options;\n+\topt.filter_options = &filter_options;\n \n \tif (update)\n-\t\tif (parse_submodule_update_strategy(update, &suc.update) < 0)\n+\t\tif (parse_submodule_update_strategy(update, &opt.update) < 0)\n \t\t\tdie(_(\"bad value for update parameter\"));\n \n-\tif (module_list_compute(argc, argv, prefix, &pathspec, &suc.list) < 0) {\n+\tif (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {\n \t\tlist_objects_filter_release(&filter_options);\n \t\treturn 1;\n \t}\n \n \tif (pathspec.nr)\n-\t\tsuc.warn_if_uninitialized = 1;\n+\t\topt.warn_if_uninitialized = 1;\n \n-\tif (suc.init) {\n+\tif (opt.init) {\n \t\tstruct module_list list = MODULE_LIST_INIT;\n \t\tstruct init_cb info = INIT_CB_INIT;\n \n-\t\tif (module_list_compute(argc, argv, suc.prefix,\n+\t\tif (module_list_compute(argc, argv, opt.prefix,\n \t\t\t\t\t&pathspec, &list) < 0)\n \t\t\treturn 1;\n \n@@ -2596,15 +2596,15 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\tif (!argc && git_config_get_value_multi(\"submodule.active\"))\n \t\t\tmodule_list_active(&list);\n \n-\t\tinfo.prefix = suc.prefix;\n-\t\tinfo.superprefix = suc.recursive_prefix;\n-\t\tif (suc.quiet)\n+\t\tinfo.prefix = opt.prefix;\n+\t\tinfo.superprefix = opt.recursive_prefix;\n+\t\tif (opt.quiet)\n \t\t\tinfo.flags |= OPT_QUIET;\n \n \t\tfor_each_listed_submodule(&list, init_submodule_cb, &info);\n \t}\n \n-\tret = update_submodules(&suc);\n+\tret = update_submodules(&opt);\n \tlist_objects_filter_release(&filter_options);\n \treturn ret;\n }\n-- \n2.33.GIT\n\n"},{"id":"451435","messageId":"20220315210925.79289-5-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 4/7] submodule--helper: teach update_data more options","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:22Z","receivedAt":"2022-03-15T21:09:54Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Refactor 'struct update_data' to hold the parsed args needed by \"git\nsubmodule--helper update\" and refactor \"update-clone\" and\n\"run-update-procedure\" (the functions that will be combined to form\n\"update\") to use these options.\n\nFor \"run-update-procedure\", 'struct update_data' already holds its args,\nso only arg parsing code needs to be updated.\n\nFor \"update-clone\", move its args from 'struct submodule_update_clone'\ninto 'struct update_data', and replace them with a pointer to 'struct\nupdate_data'. Its other members hold the submodule iteration state of\n\"update-clone\", so those are unchanged.\n\nIncidentally, since we reformat the designated initializers of the\naffected structs, also reformat MODULE_CLONE_DATA_INIT for consistency.\n\nSigned-off-by: Glen Choo <chooglen@google.com>\n---\nFor reviewers who have also looked at ar/submodule-update, note that\nthis patch changes how the CLI args are stored. In that series, the\nupdate_clone logic continues to read the CLI args from 'struct\nsubmodule_update_clone' (the CLI args are copied over from 'struct\nupdate_data'). In this series, however, all of these CLI args are moved\nfrom 'struct submodule_update_clone' to 'struct update_data'.\n\nThis patch also incorporates some reformatting from\nhttps://lore.kernel.org/git/patch-v5-5.9-fa815e37f9f-20220128T125206Z-avarab@gmail.com,\nbut that patch is now largely obsolete because the initializers are\nquite different now.\n\n builtin/submodule--helper.c | 150 ++++++++++++++++++------------------\n git-submodule.sh            |   2 +-\n 2 files changed, 76 insertions(+), 76 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 0e3cafb500..04874ce111 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1643,7 +1643,10 @@ struct module_clone_data {\n \tunsigned int require_init: 1;\n \tint single_branch;\n };\n-#define MODULE_CLONE_DATA_INIT { .reference = STRING_LIST_INIT_NODUP, .single_branch = -1 }\n+#define MODULE_CLONE_DATA_INIT { \\\n+\t.reference = STRING_LIST_INIT_NODUP, \\\n+\t.single_branch = -1, \\\n+}\n \n struct submodule_alternate_setup {\n \tconst char *submodule_name;\n@@ -1967,26 +1970,11 @@ struct update_clone_data {\n };\n \n struct submodule_update_clone {\n-\t/* index into 'list', the list of submodules to look into for cloning */\n+\t/* index into 'update_data.list', the list of submodules to look into for cloning */\n \tint current;\n-\tstruct module_list list;\n-\tunsigned warn_if_uninitialized : 1;\n-\n-\t/* update parameter passed via commandline */\n-\tstruct submodule_update_strategy update;\n \n \t/* configuration parameters which are passed on to the children */\n-\tint progress;\n-\tint quiet;\n-\tint recommend_shallow;\n-\tstruct string_list references;\n-\tint dissociate;\n-\tunsigned require_init;\n-\tconst char *depth;\n-\tconst char *recursive_prefix;\n-\tconst char *prefix;\n-\tint single_branch;\n-\tstruct list_objects_filter_options *filter_options;\n+\tstruct update_data *update_data;\n \n \t/* to be consumed by git-submodule.sh */\n \tstruct update_clone_data *update_clone;\n@@ -1998,34 +1986,45 @@ 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-\tunsigned int init;\n };\n-#define SUBMODULE_UPDATE_CLONE_INIT { \\\n-\t.list = MODULE_LIST_INIT, \\\n-\t.update = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n-\t.recommend_shallow = -1, \\\n-\t.references = STRING_LIST_INIT_DUP, \\\n-\t.single_branch = -1, \\\n-\t.max_jobs = 1, \\\n-}\n+#define SUBMODULE_UPDATE_CLONE_INIT { 0 }\n \n struct update_data {\n+\tconst char *prefix;\n \tconst char *recursive_prefix;\n \tconst char *sm_path;\n \tconst char *displaypath;\n+\tconst char *update_default;\n \tstruct object_id oid;\n \tstruct object_id suboid;\n+\tstruct string_list references;\n \tstruct submodule_update_strategy update_strategy;\n+\tstruct list_objects_filter_options *filter_options;\n+\tstruct module_list list;\n \tint depth;\n+\tint max_jobs;\n+\tint single_branch;\n+\tint recommend_shallow;\n+\tunsigned int require_init;\n \tunsigned int force;\n \tunsigned int quiet;\n \tunsigned int nofetch;\n \tunsigned int just_cloned;\n \tunsigned int remote;\n+\tunsigned int progress;\n+\tunsigned int dissociate;\n+\tunsigned int init;\n+\tunsigned int warn_if_uninitialized;\n };\n-#define UPDATE_DATA_INIT { .update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT }\n+#define UPDATE_DATA_INIT { \\\n+\t.update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n+\t.list = MODULE_LIST_INIT, \\\n+\t.recommend_shallow = -1, \\\n+\t.references = STRING_LIST_INIT_DUP, \\\n+\t.single_branch = -1, \\\n+\t.max_jobs = 1, \\\n+\t.warn_if_uninitialized = 1, \\\n+}\n \n static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n \t\tstruct strbuf *out, const char *displaypath)\n@@ -2034,7 +2033,7 @@ static void next_submodule_warn_missing(struct submodule_update_clone *suc,\n \t * Only mention uninitialized submodules when their\n \t * paths have been specified.\n \t */\n-\tif (suc->warn_if_uninitialized) {\n+\tif (suc->update_data->warn_if_uninitialized) {\n \t\tstrbuf_addf(out,\n \t\t\t_(\"Submodule path '%s' not initialized\"),\n \t\t\tdisplaypath);\n@@ -2066,8 +2065,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tint need_free_url = 0;\n \n \tif (ce_stage(ce)) {\n-\t\tif (suc->recursive_prefix)\n-\t\t\tstrbuf_addf(&sb, \"%s/%s\", suc->recursive_prefix, ce->name);\n+\t\tif (suc->update_data->recursive_prefix)\n+\t\t\tstrbuf_addf(&sb, \"%s/%s\", suc->update_data->recursive_prefix, ce->name);\n \t\telse\n \t\t\tstrbuf_addstr(&sb, ce->name);\n \t\tstrbuf_addf(out, _(\"Skipping unmerged submodule %s\"), sb.buf);\n@@ -2077,8 +2076,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \n \tsub = submodule_from_path(the_repository, null_oid(), ce->name);\n \n-\tif (suc->recursive_prefix)\n-\t\tdisplaypath = relative_path(suc->recursive_prefix,\n+\tif (suc->update_data->recursive_prefix)\n+\t\tdisplaypath = relative_path(suc->update_data->recursive_prefix,\n \t\t\t\t\t    ce->name, &displaypath_sb);\n \telse\n \t\tdisplaypath = ce->name;\n@@ -2096,8 +2095,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \t}\n \tfree(key);\n \n-\tif (suc->update.type == SM_UPDATE_NONE\n-\t    || (suc->update.type == SM_UPDATE_UNSPECIFIED\n+\tif (suc->update_data->update_strategy.type == SM_UPDATE_NONE\n+\t    || (suc->update_data->update_strategy.type == SM_UPDATE_UNSPECIFIED\n \t\t&& update_type == SM_UPDATE_NONE)) {\n \t\tstrbuf_addf(out, _(\"Skipping submodule '%s'\"), displaypath);\n \t\tstrbuf_addch(out, '\\n');\n@@ -2141,33 +2140,33 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,\n \tchild->err = -1;\n \tstrvec_push(&child->args, \"submodule--helper\");\n \tstrvec_push(&child->args, \"clone\");\n-\tif (suc->progress)\n+\tif (suc->update_data->progress)\n \t\tstrvec_push(&child->args, \"--progress\");\n-\tif (suc->quiet)\n+\tif (suc->update_data->quiet)\n \t\tstrvec_push(&child->args, \"--quiet\");\n-\tif (suc->prefix)\n-\t\tstrvec_pushl(&child->args, \"--prefix\", suc->prefix, NULL);\n-\tif (suc->recommend_shallow && sub->recommend_shallow == 1)\n+\tif (suc->update_data->prefix)\n+\t\tstrvec_pushl(&child->args, \"--prefix\", suc->update_data->prefix, NULL);\n+\tif (suc->update_data->recommend_shallow && sub->recommend_shallow == 1)\n \t\tstrvec_push(&child->args, \"--depth=1\");\n-\tif (suc->filter_options && suc->filter_options->choice)\n+\telse if (suc->update_data->depth)\n+\t\tstrvec_pushf(&child->args, \"--depth=%d\", suc->update_data->depth);\n+\tif (suc->update_data->filter_options && suc->update_data->filter_options->choice)\n \t\tstrvec_pushf(&child->args, \"--filter=%s\",\n-\t\t\t     expand_list_objects_filter_spec(suc->filter_options));\n-\tif (suc->require_init)\n+\t\t\t     expand_list_objects_filter_spec(suc->update_data->filter_options));\n+\tif (suc->update_data->require_init)\n \t\tstrvec_push(&child->args, \"--require-init\");\n \tstrvec_pushl(&child->args, \"--path\", sub->path, NULL);\n \tstrvec_pushl(&child->args, \"--name\", sub->name, NULL);\n \tstrvec_pushl(&child->args, \"--url\", url, NULL);\n-\tif (suc->references.nr) {\n+\tif (suc->update_data->references.nr) {\n \t\tstruct string_list_item *item;\n-\t\tfor_each_string_list_item(item, &suc->references)\n+\t\tfor_each_string_list_item(item, &suc->update_data->references)\n \t\t\tstrvec_pushl(&child->args, \"--reference\", item->string, NULL);\n \t}\n-\tif (suc->dissociate)\n+\tif (suc->update_data->dissociate)\n \t\tstrvec_push(&child->args, \"--dissociate\");\n-\tif (suc->depth)\n-\t\tstrvec_push(&child->args, suc->depth);\n-\tif (suc->single_branch >= 0)\n-\t\tstrvec_push(&child->args, suc->single_branch ?\n+\tif (suc->update_data->single_branch >= 0)\n+\t\tstrvec_push(&child->args, suc->update_data->single_branch ?\n \t\t\t\t\t      \"--single-branch\" :\n \t\t\t\t\t      \"--no-single-branch\");\n \n@@ -2189,8 +2188,8 @@ static int update_clone_get_next_task(struct child_process *child,\n \tconst struct cache_entry *ce;\n \tint index;\n \n-\tfor (; suc->current < suc->list.nr; suc->current++) {\n-\t\tce = suc->list.entries[suc->current];\n+\tfor (; suc->current < suc->update_data->list.nr; suc->current++) {\n+\t\tce = suc->update_data->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@@ -2205,7 +2204,7 @@ static int update_clone_get_next_task(struct child_process *child,\n \t * stragglers again, which we can imagine as an extension of the\n \t * entry list.\n \t */\n-\tindex = suc->current - suc->list.nr;\n+\tindex = suc->current - suc->update_data->list.nr;\n \tif (index < suc->failed_clones_nr) {\n \t\tint *p;\n \t\tce = suc->failed_clones[index];\n@@ -2250,8 +2249,8 @@ static int update_clone_task_finished(int result,\n \tif (!result)\n \t\treturn 0;\n \n-\tif (idx < suc->list.nr) {\n-\t\tce  = suc->list.entries[idx];\n+\tif (idx < suc->update_data->list.nr) {\n+\t\tce  = suc->update_data->list.entries[idx];\n \t\tstrbuf_addf(err, _(\"Failed to clone '%s'. Retry scheduled\"),\n \t\t\t    ce->name);\n \t\tstrbuf_addch(err, '\\n');\n@@ -2261,7 +2260,7 @@ static int update_clone_task_finished(int result,\n \t\tsuc->failed_clones[suc->failed_clones_nr++] = ce;\n \t\treturn 0;\n \t} else {\n-\t\tidx -= suc->list.nr;\n+\t\tidx -= suc->update_data->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@@ -2468,13 +2467,15 @@ static void update_submodule(struct update_clone_data *ucd)\n \t\tucd->sub->path);\n }\n \n-static int update_submodules(struct submodule_update_clone *suc)\n+static int update_submodules(struct update_data *update_data)\n {\n \tint i;\n+\tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n \n-\trun_processes_parallel_tr2(suc->max_jobs, update_clone_get_next_task,\n+\tsuc.update_data = update_data;\n+\trun_processes_parallel_tr2(suc.update_data->max_jobs, update_clone_get_next_task,\n \t\t\t\t   update_clone_start_failure,\n-\t\t\t\t   update_clone_task_finished, suc, \"submodule\",\n+\t\t\t\t   update_clone_task_finished, &suc, \"submodule\",\n \t\t\t\t   \"parallel/update\");\n \n \t/*\n@@ -2485,41 +2486,40 @@ static int update_submodules(struct submodule_update_clone *suc)\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+\tif (suc.quickstop)\n \t\treturn 1;\n \n-\tfor (i = 0; i < suc->update_clone_nr; i++)\n-\t\tupdate_submodule(&suc->update_clone[i]);\n+\tfor (i = 0; i < suc.update_clone_nr; i++)\n+\t\tupdate_submodule(&suc.update_clone[i]);\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 \tstruct pathspec pathspec;\n-\tstruct submodule_update_clone opt = SUBMODULE_UPDATE_CLONE_INIT;\n+\tstruct update_data opt = UPDATE_DATA_INIT;\n \tstruct list_objects_filter_options filter_options;\n \tint ret;\n \n \tstruct option module_update_clone_options[] = {\n \t\tOPT_BOOL(0, \"init\", &opt.init,\n \t\t\t N_(\"initialize uninitialized submodules before update\")),\n-\t\tOPT_STRING(0, \"prefix\", &prefix,\n+\t\tOPT_STRING(0, \"prefix\", &opt.prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree\")),\n \t\tOPT_STRING(0, \"recursive-prefix\", &opt.recursive_prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree, across nested \"\n \t\t\t      \"submodule boundaries\")),\n-\t\tOPT_STRING(0, \"update\", &update,\n+\t\tOPT_STRING(0, \"update\", &opt.update_default,\n \t\t\t   N_(\"string\"),\n \t\t\t   N_(\"rebase, merge, checkout or none\")),\n \t\tOPT_STRING_LIST(0, \"reference\", &opt.references, N_(\"repo\"),\n \t\t\t   N_(\"reference repository\")),\n \t\tOPT_BOOL(0, \"dissociate\", &opt.dissociate,\n \t\t\t   N_(\"use --reference only while cloning\")),\n-\t\tOPT_STRING(0, \"depth\", &opt.depth, \"<depth>\",\n+\t\tOPT_INTEGER(0, \"depth\", &opt.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\", &opt.max_jobs,\n@@ -2546,7 +2546,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \t\t\" [--recursive] [--[no-]single-branch] [--] [<path>...]\"),\n \t\tNULL\n \t};\n-\topt.prefix = prefix;\n \n \tupdate_clone_config_from_gitmodules(&opt.max_jobs);\n \tgit_config(git_update_clone_config, &opt.max_jobs);\n@@ -2569,8 +2568,9 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \n \topt.filter_options = &filter_options;\n \n-\tif (update)\n-\t\tif (parse_submodule_update_strategy(update, &opt.update) < 0)\n+\tif (opt.update_default)\n+\t\tif (parse_submodule_update_strategy(opt.update_default,\n+\t\t\t\t\t\t    &opt.update_strategy) < 0)\n \t\t\tdie(_(\"bad value for update parameter\"));\n \n \tif (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {\n@@ -2611,7 +2611,7 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \n static int run_update_procedure(int argc, const char **argv, const char *prefix)\n {\n-\tchar *prefixed_path, *update = NULL;\n+\tchar *prefixed_path;\n \tstruct update_data update_data = UPDATE_DATA_INIT;\n \n \tstruct option options[] = {\n@@ -2627,7 +2627,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)\n \t\tOPT_STRING(0, \"prefix\", &prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree\")),\n-\t\tOPT_STRING(0, \"update\", &update,\n+\t\tOPT_STRING(0, \"update\", &update_data.update_default,\n \t\t\t   N_(\"string\"),\n \t\t\t   N_(\"rebase, merge, checkout or none\")),\n \t\tOPT_STRING(0, \"recursive-prefix\", &update_data.recursive_prefix, N_(\"path\"),\n@@ -2661,7 +2661,7 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)\n \tupdate_data.displaypath = get_submodule_displaypath(prefixed_path, prefix);\n \n \tdetermine_submodule_update_strategy(the_repository, update_data.just_cloned,\n-\t\t\t\t\t    update_data.sm_path, update,\n+\t\t\t\t\t    update_data.sm_path, update_data.update_default,\n \t\t\t\t\t    &update_data.update_strategy);\n \n \tfree(prefixed_path);\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex a84143daab..b63a2aefa7 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -366,7 +366,7 @@ cmd_update()\n \t\t${update:+--update \"$update\"} \\\n \t\t${reference:+\"$reference\"} \\\n \t\t${dissociate:+\"--dissociate\"} \\\n-\t\t${depth:+--depth \"$depth\"} \\\n+\t\t${depth:+\"$depth\"} \\\n \t\t${require_init:+--require-init} \\\n \t\t$single_branch \\\n \t\t$recommend_shallow \\\n-- \n2.33.GIT\n\n"},{"id":"451436","messageId":"20220315210925.79289-6-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 5/7] submodule--helper: reduce logic in run_update_procedure()","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:23Z","receivedAt":"2022-03-15T21:09:56Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"A later commit will combine the \"update-clone\" and\n\"run-update-procedure\" commands, so run_update_procedure() will be\nremoved. Prepare for this by moving as much logic as possible out of\nrun_update_procedure() and into update_submodule2().\n\nSigned-off-by: Glen Choo <chooglen@google.com>\n---\n builtin/submodule--helper.c | 30 +++++++++++++++++-------------\n 1 file changed, 17 insertions(+), 13 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 04874ce111..85f37c4458 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2611,7 +2611,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \n static int run_update_procedure(int argc, const char **argv, const char *prefix)\n {\n-\tchar *prefixed_path;\n \tstruct update_data update_data = UPDATE_DATA_INIT;\n \n \tstruct option options[] = {\n@@ -2653,18 +2652,6 @@ static int run_update_procedure(int argc, const char **argv, const char *prefix)\n \n \tupdate_data.sm_path = argv[0];\n \n-\tif (update_data.recursive_prefix)\n-\t\tprefixed_path = xstrfmt(\"%s%s\", update_data.recursive_prefix, update_data.sm_path);\n-\telse\n-\t\tprefixed_path = xstrdup(update_data.sm_path);\n-\n-\tupdate_data.displaypath = get_submodule_displaypath(prefixed_path, prefix);\n-\n-\tdetermine_submodule_update_strategy(the_repository, update_data.just_cloned,\n-\t\t\t\t\t    update_data.sm_path, update_data.update_default,\n-\t\t\t\t\t    &update_data.update_strategy);\n-\n-\tfree(prefixed_path);\n \treturn update_submodule2(&update_data);\n }\n \n@@ -3044,7 +3031,24 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)\n /* NEEDSWORK: this is a temporary name until we delete update_submodule() */\n static int update_submodule2(struct update_data *update_data)\n {\n+\tchar *prefixed_path;\n+\n \tensure_core_worktree(update_data->sm_path);\n+\n+\tif (update_data->recursive_prefix)\n+\t\tprefixed_path = xstrfmt(\"%s%s\", update_data->recursive_prefix,\n+\t\t\t\t\tupdate_data->sm_path);\n+\telse\n+\t\tprefixed_path = xstrdup(update_data->sm_path);\n+\n+\tupdate_data->displaypath = get_submodule_displaypath(prefixed_path,\n+\t\t\t\t\t\t\t     update_data->prefix);\n+\tfree(prefixed_path);\n+\n+\tdetermine_submodule_update_strategy(the_repository, update_data->just_cloned,\n+\t\t\t\t\t    update_data->sm_path, update_data->update_default,\n+\t\t\t\t\t    &update_data->update_strategy);\n+\n \tif (update_data->just_cloned)\n \t\toidcpy(&update_data->suboid, null_oid());\n \telse if (resolve_gitlink_ref(update_data->sm_path, \"HEAD\", &update_data->suboid))\n-- \n2.33.GIT\n\n"},{"id":"451437","messageId":"20220315210925.79289-7-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 6/7] submodule: move core cmd_update() logic to C","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:24Z","receivedAt":"2022-03-15T21:09:58Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"From: Atharva Raykar <raykar.ath@gmail.com>\n\nThis patch completes the conversion past the flag parsing of\n`submodule update` by introducing a helper subcommand called\n`submodule--helper update`. The behaviour of `submodule update` should\nremain the same after this patch.\n\nPrior to this patch, `submodule update` was implemented by piping the\noutput of `update-clone` (which clones all missing submodules, then\nprints relevant information for all submodules) into\n`run-update-procedure` (which reads the information and updates the\nsubmodule tree).\n\nWith `submodule--helper update`, we iterate over the submodules and\nupdate the submodule tree in the same process. This reuses most of\nexisting code structure, except that `update_submodule()` now updates\nthe submodule tree (instead of printing submodule information to be\nconsumed by another process).\n\nRecursing on a submodule is done by calling a subprocess that launches\n`submodule--helper update`, with a modified `--recursive-prefix` and\n`--prefix` parameter.\n\nMentored-by: Christian Couder <christian.couder@gmail.com>\nMentored-by: Shourya Shukla <periperidip@gmail.com>\nSigned-off-by: Atharva Raykar <raykar.ath@gmail.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\nSigned-off-by: Glen Choo <chooglen@google.com>\n---\nFor the curious, the bug that was in ar/submodule-update\n(<xmqqr18770pc.fsf@gitster.g>), comes from a typo that caused\n`--single-branch` to be set incorrectly in the recursive `update`:\n\n\tif (update_data->single_branch >= 0)\n\t\tstrvec_push(args, \"--single-branch\");\n\nThis has been corrected to:\n\n\tif (update_data->single_branch >= 0)\n\t\tstrvec_push(args, update_data->single_branch ?\n\t\t\t\t    \"--single-branch\" :\n\t\t\t\t    \"--no-single-branch\");\n\n(-1 means \"--[no-]single-branch\" was not specified)\n\nt3207 only failed because of a second-order effect - the test sets up\ntracking based on the remote's \"fetch\" refspec, but `--single-branch`\ncaused the wrong refspec to be written. This might suggest that more\ndirect tests of recursive `update` are needed.\n\n builtin/submodule--helper.c | 229 +++++++++++++++++++-----------------\n git-submodule.sh            | 104 ++--------------\n 2 files changed, 131 insertions(+), 202 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 85f37c4458..842d9ecaa8 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -1976,7 +1976,7 @@ struct submodule_update_clone {\n \t/* configuration parameters which are passed on to the children */\n \tstruct update_data *update_data;\n \n-\t/* to be consumed by git-submodule.sh */\n+\t/* to be consumed by update_submodule() */\n \tstruct update_clone_data *update_clone;\n \tint update_clone_nr; int update_clone_alloc;\n \n@@ -1992,10 +1992,8 @@ struct submodule_update_clone {\n struct update_data {\n \tconst char *prefix;\n \tconst char *recursive_prefix;\n-\tconst char *sm_path;\n \tconst char *displaypath;\n \tconst char *update_default;\n-\tstruct object_id oid;\n \tstruct object_id suboid;\n \tstruct string_list references;\n \tstruct submodule_update_strategy update_strategy;\n@@ -2009,12 +2007,17 @@ struct update_data {\n \tunsigned int force;\n \tunsigned int quiet;\n \tunsigned int nofetch;\n-\tunsigned int just_cloned;\n \tunsigned int remote;\n \tunsigned int progress;\n \tunsigned int dissociate;\n \tunsigned int init;\n \tunsigned int warn_if_uninitialized;\n+\tunsigned int recursive;\n+\n+\t/* copied over from update_clone_data */\n+\tstruct object_id oid;\n+\tunsigned int just_cloned;\n+\tconst char *sm_path;\n };\n #define UPDATE_DATA_INIT { \\\n \t.update_strategy = SUBMODULE_UPDATE_STRATEGY_INIT, \\\n@@ -2419,7 +2422,7 @@ static int run_update_command(struct update_data *ud, int subforce)\n \treturn 0;\n }\n \n-static int do_run_update_procedure(struct update_data *ud)\n+static int run_update_procedure(struct update_data *ud)\n {\n \tint subforce = is_null_oid(&ud->suboid) || ud->force;\n \n@@ -2449,27 +2452,10 @@ static int do_run_update_procedure(struct update_data *ud)\n \treturn run_update_command(ud, subforce);\n }\n \n-/*\n- * NEEDSWORK: As we convert \"git submodule update\" to C,\n- * update_submodule2() will invoke more and more functions, making it\n- * difficult to preserve the function ordering without forward\n- * declarations.\n- *\n- * When the conversion is complete, this forward declaration will be\n- * unnecessary and should be removed.\n- */\n-static int update_submodule2(struct update_data *update_data);\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_submodule(struct update_data *update_data);\n static int update_submodules(struct update_data *update_data)\n {\n-\tint i;\n+\tint i, res = 0;\n \tstruct submodule_update_clone suc = SUBMODULE_UPDATE_CLONE_INIT;\n \n \tsuc.update_data = update_data;\n@@ -2486,25 +2472,44 @@ static int update_submodules(struct update_data *update_data)\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+\tif (suc.quickstop) {\n+\t\tres = 1;\n+\t\tgoto cleanup;\n+\t}\n \n-\tfor (i = 0; i < suc.update_clone_nr; i++)\n-\t\tupdate_submodule(&suc.update_clone[i]);\n+\tfor (i = 0; i < suc.update_clone_nr; i++) {\n+\t\tstruct update_clone_data ucd = suc.update_clone[i];\n \n-\treturn 0;\n+\t\toidcpy(&update_data->oid, &ucd.oid);\n+\t\tupdate_data->just_cloned = ucd.just_cloned;\n+\t\tupdate_data->sm_path = ucd.sub->path;\n+\n+\t\tif (update_submodule(update_data))\n+\t\t\tres = 1;\n+\t}\n+\n+cleanup:\n+\tstring_list_clear(&update_data->references, 0);\n+\treturn res;\n }\n \n-static int update_clone(int argc, const char **argv, const char *prefix)\n+static int module_update(int argc, const char **argv, const char *prefix)\n {\n \tstruct pathspec pathspec;\n \tstruct update_data opt = UPDATE_DATA_INIT;\n \tstruct list_objects_filter_options filter_options;\n \tint ret;\n \n-\tstruct option module_update_clone_options[] = {\n+\tstruct option module_update_options[] = {\n+\t\tOPT__FORCE(&opt.force, N_(\"force checkout updates\"), 0),\n \t\tOPT_BOOL(0, \"init\", &opt.init,\n \t\t\t N_(\"initialize uninitialized submodules before update\")),\n+\t\tOPT_BOOL(0, \"remote\", &opt.remote,\n+\t\t\t N_(\"use SHA-1 of submodule's remote tracking branch\")),\n+\t\tOPT_BOOL(0, \"recursive\", &opt.recursive,\n+\t\t\t N_(\"traverse submodules recursively\")),\n+\t\tOPT_BOOL('N', \"no-fetch\", &opt.nofetch,\n+\t\t\t N_(\"don't fetch new objects from the remote site\")),\n \t\tOPT_STRING(0, \"prefix\", &opt.prefix,\n \t\t\t   N_(\"path\"),\n \t\t\t   N_(\"path into the working tree\")),\n@@ -2551,19 +2556,12 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \tgit_config(git_update_clone_config, &opt.max_jobs);\n \n \tmemset(&filter_options, 0, sizeof(filter_options));\n-\targc = parse_options(argc, argv, prefix, module_update_clone_options,\n+\targc = parse_options(argc, argv, prefix, module_update_options,\n \t\t\t     git_submodule_helper_usage, 0);\n \n \tif (filter_options.choice && !opt.init) {\n-\t\t/*\n-\t\t * NEEDSWORK: Don't use usage_with_options() because the\n-\t\t * usage string is for \"git submodule update\", but the\n-\t\t * options are for \"git submodule--helper update-clone\".\n-\t\t *\n-\t\t * This will no longer be an issue when \"update-clone\"\n-\t\t * is replaced by \"git submodule--helper update\".\n-\t\t */\n-\t\tusage(git_submodule_helper_usage[0]);\n+\t\tusage_with_options(git_submodule_helper_usage,\n+\t\t\t\t   module_update_options);\n \t}\n \n \topt.filter_options = &filter_options;\n@@ -2609,63 +2607,6 @@ static int update_clone(int argc, const char **argv, const char *prefix)\n \treturn ret;\n }\n \n-static int run_update_procedure(int argc, const char **argv, const char *prefix)\n-{\n-\tstruct update_data update_data = UPDATE_DATA_INIT;\n-\n-\tstruct option options[] = {\n-\t\tOPT__QUIET(&update_data.quiet,\n-\t\t\t   N_(\"suppress output for update by rebase or merge\")),\n-\t\tOPT__FORCE(&update_data.force, N_(\"force checkout updates\"),\n-\t\t\t   0),\n-\t\tOPT_BOOL('N', \"no-fetch\", &update_data.nofetch,\n-\t\t\t N_(\"don't fetch new objects from the remote site\")),\n-\t\tOPT_BOOL(0, \"just-cloned\", &update_data.just_cloned,\n-\t\t\t N_(\"overrides update mode in case the repository is a fresh clone\")),\n-\t\tOPT_INTEGER(0, \"depth\", &update_data.depth, N_(\"depth for shallow fetch\")),\n-\t\tOPT_STRING(0, \"prefix\", &prefix,\n-\t\t\t   N_(\"path\"),\n-\t\t\t   N_(\"path into the working tree\")),\n-\t\tOPT_STRING(0, \"update\", &update_data.update_default,\n-\t\t\t   N_(\"string\"),\n-\t\t\t   N_(\"rebase, merge, checkout or none\")),\n-\t\tOPT_STRING(0, \"recursive-prefix\", &update_data.recursive_prefix, N_(\"path\"),\n-\t\t\t   N_(\"path into the working tree, across nested \"\n-\t\t\t      \"submodule boundaries\")),\n-\t\tOPT_CALLBACK_F(0, \"oid\", &update_data.oid, N_(\"sha1\"),\n-\t\t\t       N_(\"SHA1 expected by superproject\"), PARSE_OPT_NONEG,\n-\t\t\t       parse_opt_object_id),\n-\t\tOPT_BOOL(0, \"remote\", &update_data.remote,\n-\t\t\t N_(\"use SHA-1 of submodule's remote tracking branch\")),\n-\t\tOPT_END()\n-\t};\n-\n-\tconst char *const usage[] = {\n-\t\tN_(\"git submodule--helper run-update-procedure [<options>] <path>\"),\n-\t\tNULL\n-\t};\n-\n-\targc = parse_options(argc, argv, prefix, options, usage, 0);\n-\n-\tif (argc != 1)\n-\t\tusage_with_options(usage, options);\n-\n-\tupdate_data.sm_path = argv[0];\n-\n-\treturn update_submodule2(&update_data);\n-}\n-\n-static int resolve_relative_path(int argc, const char **argv, const char *prefix)\n-{\n-\tstruct strbuf sb = STRBUF_INIT;\n-\tif (argc != 3)\n-\t\tdie(\"submodule--helper relative-path takes exactly 2 arguments, got %d\", argc);\n-\n-\tprintf(\"%s\", relative_path(argv[1], argv[2], &sb));\n-\tstrbuf_release(&sb);\n-\treturn 0;\n-}\n-\n static const char *remote_submodule_branch(const char *path)\n {\n \tconst struct submodule *sub;\n@@ -3028,8 +2969,53 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-/* NEEDSWORK: this is a temporary name until we delete update_submodule() */\n-static int update_submodule2(struct update_data *update_data)\n+static void update_data_to_args(struct update_data *update_data, struct strvec *args)\n+{\n+\tstrvec_pushl(args, \"submodule--helper\", \"update\", \"--recursive\", NULL);\n+\tstrvec_pushf(args, \"--jobs=%d\", update_data->max_jobs);\n+\tif (update_data->recursive_prefix)\n+\t\tstrvec_pushl(args, \"--recursive-prefix\",\n+\t\t\t     update_data->recursive_prefix, NULL);\n+\tif (update_data->quiet)\n+\t\tstrvec_push(args, \"--quiet\");\n+\tif (update_data->force)\n+\t\tstrvec_push(args, \"--force\");\n+\tif (update_data->init)\n+\t\tstrvec_push(args, \"--init\");\n+\tif (update_data->remote)\n+\t\tstrvec_push(args, \"--remote\");\n+\tif (update_data->nofetch)\n+\t\tstrvec_push(args, \"--no-fetch\");\n+\tif (update_data->dissociate)\n+\t\tstrvec_push(args, \"--dissociate\");\n+\tif (update_data->progress)\n+\t\tstrvec_push(args, \"--progress\");\n+\tif (update_data->require_init)\n+\t\tstrvec_push(args, \"--require-init\");\n+\tif (update_data->depth)\n+\t\tstrvec_pushf(args, \"--depth=%d\", update_data->depth);\n+\tif (update_data->update_default)\n+\t\tstrvec_pushl(args, \"--update\", update_data->update_default, NULL);\n+\tif (update_data->references.nr) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, &update_data->references)\n+\t\t\tstrvec_pushl(args, \"--reference\", item->string, NULL);\n+\t}\n+\tif (update_data->filter_options && update_data->filter_options->choice)\n+\t\tstrvec_pushf(args, \"--filter=%s\",\n+\t\t\t\texpand_list_objects_filter_spec(\n+\t\t\t\t\tupdate_data->filter_options));\n+\tif (update_data->recommend_shallow == 0)\n+\t\tstrvec_push(args, \"--no-recommend-shallow\");\n+\telse if (update_data->recommend_shallow == 1)\n+\t\tstrvec_push(args, \"--recommend-shallow\");\n+\tif (update_data->single_branch >= 0)\n+\t\tstrvec_push(args, update_data->single_branch ?\n+\t\t\t\t    \"--single-branch\" :\n+\t\t\t\t    \"--no-single-branch\");\n+}\n+\n+static int update_submodule(struct update_data *update_data)\n {\n \tchar *prefixed_path;\n \n@@ -3075,9 +3061,44 @@ static int update_submodule2(struct update_data *update_data)\n \t}\n \n \tif (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)\n-\t\treturn do_run_update_procedure(update_data);\n+\t\tif (run_update_procedure(update_data))\n+\t\t\treturn 1;\n+\n+\tif (update_data->recursive) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct update_data next = *update_data;\n+\t\tint res;\n+\n+\t\tif (update_data->recursive_prefix)\n+\t\t\tprefixed_path = xstrfmt(\"%s%s/\", update_data->recursive_prefix,\n+\t\t\t\t\t\tupdate_data->sm_path);\n+\t\telse\n+\t\t\tprefixed_path = xstrfmt(\"%s/\", update_data->sm_path);\n+\n+\t\tnext.recursive_prefix = get_submodule_displaypath(prefixed_path,\n+\t\t\t\t\t\t\t\t  update_data->prefix);\n+\t\tnext.prefix = NULL;\n+\t\toidcpy(&next.oid, null_oid());\n+\t\toidcpy(&next.suboid, null_oid());\n+\n+\t\tcp.dir = update_data->sm_path;\n+\t\tcp.git_cmd = 1;\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n+\t\tupdate_data_to_args(&next, &cp.args);\n \n-\treturn 3;\n+\t\t/* die() if child process die()'d */\n+\t\tres = run_command(&cp);\n+\t\tif (!res)\n+\t\t\treturn 0;\n+\t\tdie_message(_(\"Failed to recurse into submodule path '%s'\"),\n+\t\t\t    update_data->displaypath);\n+\t\tif (res == 128)\n+\t\t\texit(res);\n+\t\telse if (res)\n+\t\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n }\n \n struct add_data {\n@@ -3465,9 +3486,7 @@ static struct cmd_struct commands[] = {\n \t{\"name\", module_name, 0},\n \t{\"clone\", module_clone, 0},\n \t{\"add\", module_add, SUPPORT_SUPER_PREFIX},\n-\t{\"update-clone\", update_clone, 0},\n-\t{\"run-update-procedure\", run_update_procedure, 0},\n-\t{\"relative-path\", resolve_relative_path, 0},\n+\t{\"update\", module_update, 0},\n \t{\"resolve-relative-url-test\", resolve_relative_url_test, 0},\n \t{\"foreach\", module_foreach, SUPPORT_SUPER_PREFIX},\n \t{\"init\", module_init, SUPPORT_SUPER_PREFIX},\ndiff --git a/git-submodule.sh b/git-submodule.sh\nindex b63a2aefa7..fd0b4a2c94 100755\n--- a/git-submodule.sh\n+++ b/git-submodule.sh\n@@ -51,14 +51,6 @@ jobs=\n recommend_shallow=\n filter=\n \n-die_if_unmatched ()\n-{\n-\tif test \"$1\" = \"#unmatched\"\n-\tthen\n-\t\texit ${2:-1}\n-\tfi\n-}\n-\n isnumber()\n {\n \tn=$(($1 + 0)) 2>/dev/null && test \"$n\" = \"$1\"\n@@ -356,11 +348,14 @@ cmd_update()\n \t\tshift\n \tdone\n \n-\t{\n-\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper update-clone \\\n+\tgit ${wt_prefix:+-C \"$wt_prefix\"} submodule--helper update \\\n \t\t${GIT_QUIET:+--quiet} \\\n+\t\t${force:+--force} \\\n \t\t${progress:+\"--progress\"} \\\n+\t\t${remote:+--remote} \\\n+\t\t${recursive:+--recursive} \\\n \t\t${init:+--init} \\\n+\t\t${nofetch:+--no-fetch} \\\n \t\t${wt_prefix:+--prefix \"$wt_prefix\"} \\\n \t\t${prefix:+--recursive-prefix \"$prefix\"} \\\n \t\t${update:+--update \"$update\"} \\\n@@ -368,98 +363,13 @@ cmd_update()\n \t\t${dissociate:+\"--dissociate\"} \\\n \t\t${depth:+\"$depth\"} \\\n \t\t${require_init:+--require-init} \\\n+\t\t${dissociate:+\"--dissociate\"} \\\n \t\t$single_branch \\\n \t\t$recommend_shallow \\\n \t\t$jobs \\\n \t\t$filter \\\n \t\t-- \\\n-\t\t\"$@\" || echo \"#unmatched\" $?\n-\t} | {\n-\terr=\n-\twhile read -r quickabort sha1 just_cloned sm_path\n-\tdo\n-\t\tdie_if_unmatched \"$quickabort\" \"$sha1\"\n-\n-\t\tdisplaypath=$(git submodule--helper relative-path \"$prefix$sm_path\" \"$wt_prefix\")\n-\n-\t\tif test $just_cloned -eq 0\n-\t\tthen\n-\t\t\tjust_cloned=\n-\t\tfi\n-\n-\t\tout=$(git submodule--helper run-update-procedure \\\n-\t\t\t  ${wt_prefix:+--prefix \"$wt_prefix\"} \\\n-\t\t\t  ${GIT_QUIET:+--quiet} \\\n-\t\t\t  ${force:+--force} \\\n-\t\t\t  ${just_cloned:+--just-cloned} \\\n-\t\t\t  ${nofetch:+--no-fetch} \\\n-\t\t\t  ${depth:+\"$depth\"} \\\n-\t\t\t  ${update:+--update \"$update\"} \\\n-\t\t\t  ${prefix:+--recursive-prefix \"$prefix\"} \\\n-\t\t\t  ${sha1:+--oid \"$sha1\"} \\\n-\t\t\t  ${remote:+--remote} \\\n-\t\t\t  \"--\" \\\n-\t\t\t  \"$sm_path\")\n-\n-\t\t# exit codes for run-update-procedure:\n-\t\t# 0: update was successful, say command output\n-\t\t# 1: update procedure failed, but should not die\n-\t\t# 128: subcommand died during execution\n-\t\t# 3: no update procedure was run\n-\t\tres=\"$?\"\n-\t\tcase $res in\n-\t\t0)\n-\t\t\tsay \"$out\"\n-\t\t\t;;\n-\t\t1)\n-\t\t\terr=\"${err};$out\"\n-\t\t\tcontinue\n-\t\t\t;;\n-\t\t128)\n-\t\t\tprintf >&2 \"$out\"\n-\t\t\texit $res\n-\t\t\t;;\n-\t\tesac\n-\n-\t\tif test -n \"$recursive\"\n-\t\tthen\n-\t\t\t(\n-\t\t\t\tprefix=$(git submodule--helper relative-path \"$prefix$sm_path/\" \"$wt_prefix\")\n-\t\t\t\twt_prefix=\n-\t\t\t\tsanitize_submodule_env\n-\t\t\t\tcd \"$sm_path\" &&\n-\t\t\t\teval cmd_update\n-\t\t\t)\n-\t\t\tres=$?\n-\t\t\tif test $res -gt 0\n-\t\t\tthen\n-\t\t\t\tdie_msg=\"fatal: $(eval_gettext \"Failed to recurse into submodule path '\\$displaypath'\")\"\n-\t\t\t\tif test $res -ne 2\n-\t\t\t\tthen\n-\t\t\t\t\terr=\"${err};$die_msg\"\n-\t\t\t\t\tcontinue\n-\t\t\t\telse\n-\t\t\t\t\tdie_with_status $res \"$die_msg\"\n-\t\t\t\tfi\n-\t\t\tfi\n-\t\tfi\n-\tdone\n-\n-\tif test -n \"$err\"\n-\tthen\n-\t\tOIFS=$IFS\n-\t\tIFS=';'\n-\t\tfor e in $err\n-\t\tdo\n-\t\t\tif test -n \"$e\"\n-\t\t\tthen\n-\t\t\t\techo >&2 \"$e\"\n-\t\t\tfi\n-\t\tdone\n-\t\tIFS=$OIFS\n-\t\texit 1\n-\tfi\n-\t}\n+\t\t\"$@\"\n }\n \n #\n-- \n2.33.GIT\n\n"},{"id":"451438","messageId":"20220315210925.79289-8-chooglen@google.com","threadId":"57567","inReplyTo":"20220315210925.79289-1-chooglen@google.com","subject":"[PATCH 7/7] submodule--helper: remove forward declaration","fromName":"Glen Choo","fromEmail":"chooglen@google.com","sentAt":"2022-03-15T21:09:25Z","receivedAt":"2022-03-15T21:10:01Z","isPatch":true,"sender":{"key":"glencbz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58092771?v=4"},"body":"Rearrange functions so that submodule_update() no longer needs to be\nforward declared.\n\nSigned-off-by: Glen Choo <chooglen@google.com>\n---\n builtin/submodule--helper.c | 393 ++++++++++++++++++------------------\n 1 file changed, 196 insertions(+), 197 deletions(-)\n\ndiff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c\nindex 842d9ecaa8..11289f0ae5 100644\n--- a/builtin/submodule--helper.c\n+++ b/builtin/submodule--helper.c\n@@ -2452,7 +2452,202 @@ static int run_update_procedure(struct update_data *ud)\n \treturn run_update_command(ud, subforce);\n }\n \n-static int update_submodule(struct update_data *update_data);\n+static const char *remote_submodule_branch(const char *path)\n+{\n+\tconst struct submodule *sub;\n+\tconst char *branch = NULL;\n+\tchar *key;\n+\n+\tsub = submodule_from_path(the_repository, null_oid(), path);\n+\tif (!sub)\n+\t\treturn NULL;\n+\n+\tkey = xstrfmt(\"submodule.%s.branch\", sub->name);\n+\tif (repo_config_get_string_tmp(the_repository, key, &branch))\n+\t\tbranch = sub->branch;\n+\tfree(key);\n+\n+\tif (!branch)\n+\t\treturn \"HEAD\";\n+\n+\tif (!strcmp(branch, \".\")) {\n+\t\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n+\n+\t\tif (!refname)\n+\t\t\tdie(_(\"No such ref: %s\"), \"HEAD\");\n+\n+\t\t/* detached HEAD */\n+\t\tif (!strcmp(refname, \"HEAD\"))\n+\t\t\tdie(_(\"Submodule (%s) branch configured to inherit \"\n+\t\t\t      \"branch from superproject, but the superproject \"\n+\t\t\t      \"is not on any branch\"), sub->name);\n+\n+\t\tif (!skip_prefix(refname, \"refs/heads/\", &refname))\n+\t\t\tdie(_(\"Expecting a full ref name, got %s\"), refname);\n+\t\treturn refname;\n+\t}\n+\n+\treturn branch;\n+}\n+\n+static void ensure_core_worktree(const char *path)\n+{\n+\tconst char *cw;\n+\tstruct repository subrepo;\n+\n+\tif (repo_submodule_init(&subrepo, the_repository, path, null_oid()))\n+\t\tdie(_(\"could not get a repository handle for submodule '%s'\"), path);\n+\n+\tif (!repo_config_get_string_tmp(&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+\n+static void update_data_to_args(struct update_data *update_data, struct strvec *args)\n+{\n+\tstrvec_pushl(args, \"submodule--helper\", \"update\", \"--recursive\", NULL);\n+\tstrvec_pushf(args, \"--jobs=%d\", update_data->max_jobs);\n+\tif (update_data->recursive_prefix)\n+\t\tstrvec_pushl(args, \"--recursive-prefix\",\n+\t\t\t     update_data->recursive_prefix, NULL);\n+\tif (update_data->quiet)\n+\t\tstrvec_push(args, \"--quiet\");\n+\tif (update_data->force)\n+\t\tstrvec_push(args, \"--force\");\n+\tif (update_data->init)\n+\t\tstrvec_push(args, \"--init\");\n+\tif (update_data->remote)\n+\t\tstrvec_push(args, \"--remote\");\n+\tif (update_data->nofetch)\n+\t\tstrvec_push(args, \"--no-fetch\");\n+\tif (update_data->dissociate)\n+\t\tstrvec_push(args, \"--dissociate\");\n+\tif (update_data->progress)\n+\t\tstrvec_push(args, \"--progress\");\n+\tif (update_data->require_init)\n+\t\tstrvec_push(args, \"--require-init\");\n+\tif (update_data->depth)\n+\t\tstrvec_pushf(args, \"--depth=%d\", update_data->depth);\n+\tif (update_data->update_default)\n+\t\tstrvec_pushl(args, \"--update\", update_data->update_default, NULL);\n+\tif (update_data->references.nr) {\n+\t\tstruct string_list_item *item;\n+\t\tfor_each_string_list_item(item, &update_data->references)\n+\t\t\tstrvec_pushl(args, \"--reference\", item->string, NULL);\n+\t}\n+\tif (update_data->filter_options && update_data->filter_options->choice)\n+\t\tstrvec_pushf(args, \"--filter=%s\",\n+\t\t\t\texpand_list_objects_filter_spec(\n+\t\t\t\t\tupdate_data->filter_options));\n+\tif (update_data->recommend_shallow == 0)\n+\t\tstrvec_push(args, \"--no-recommend-shallow\");\n+\telse if (update_data->recommend_shallow == 1)\n+\t\tstrvec_push(args, \"--recommend-shallow\");\n+\tif (update_data->single_branch >= 0)\n+\t\tstrvec_push(args, update_data->single_branch ?\n+\t\t\t\t    \"--single-branch\" :\n+\t\t\t\t    \"--no-single-branch\");\n+}\n+\n+static int update_submodule(struct update_data *update_data)\n+{\n+\tchar *prefixed_path;\n+\n+\tensure_core_worktree(update_data->sm_path);\n+\n+\tif (update_data->recursive_prefix)\n+\t\tprefixed_path = xstrfmt(\"%s%s\", update_data->recursive_prefix,\n+\t\t\t\t\tupdate_data->sm_path);\n+\telse\n+\t\tprefixed_path = xstrdup(update_data->sm_path);\n+\n+\tupdate_data->displaypath = get_submodule_displaypath(prefixed_path,\n+\t\t\t\t\t\t\t     update_data->prefix);\n+\tfree(prefixed_path);\n+\n+\tdetermine_submodule_update_strategy(the_repository, update_data->just_cloned,\n+\t\t\t\t\t    update_data->sm_path, update_data->update_default,\n+\t\t\t\t\t    &update_data->update_strategy);\n+\n+\tif (update_data->just_cloned)\n+\t\toidcpy(&update_data->suboid, null_oid());\n+\telse if (resolve_gitlink_ref(update_data->sm_path, \"HEAD\", &update_data->suboid))\n+\t\tdie(_(\"Unable to find current revision in submodule path '%s'\"),\n+\t\t\tupdate_data->displaypath);\n+\n+\tif (update_data->remote) {\n+\t\tchar *remote_name = get_default_remote_submodule(update_data->sm_path);\n+\t\tconst char *branch = remote_submodule_branch(update_data->sm_path);\n+\t\tchar *remote_ref = xstrfmt(\"refs/remotes/%s/%s\", remote_name, branch);\n+\n+\t\tif (!update_data->nofetch) {\n+\t\t\tif (fetch_in_submodule(update_data->sm_path, update_data->depth,\n+\t\t\t\t\t      0, NULL))\n+\t\t\t\tdie(_(\"Unable to fetch in submodule path '%s'\"),\n+\t\t\t\t    update_data->sm_path);\n+\t\t}\n+\n+\t\tif (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))\n+\t\t\tdie(_(\"Unable to find %s revision in submodule path '%s'\"),\n+\t\t\t    remote_ref, update_data->sm_path);\n+\n+\t\tfree(remote_ref);\n+\t}\n+\n+\tif (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)\n+\t\tif (run_update_procedure(update_data))\n+\t\t\treturn 1;\n+\n+\tif (update_data->recursive) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct update_data next = *update_data;\n+\t\tint res;\n+\n+\t\tif (update_data->recursive_prefix)\n+\t\t\tprefixed_path = xstrfmt(\"%s%s/\", update_data->recursive_prefix,\n+\t\t\t\t\t\tupdate_data->sm_path);\n+\t\telse\n+\t\t\tprefixed_path = xstrfmt(\"%s/\", update_data->sm_path);\n+\n+\t\tnext.recursive_prefix = get_submodule_displaypath(prefixed_path,\n+\t\t\t\t\t\t\t\t  update_data->prefix);\n+\t\tnext.prefix = NULL;\n+\t\toidcpy(&next.oid, null_oid());\n+\t\toidcpy(&next.suboid, null_oid());\n+\n+\t\tcp.dir = update_data->sm_path;\n+\t\tcp.git_cmd = 1;\n+\t\tprepare_submodule_repo_env(&cp.env_array);\n+\t\tupdate_data_to_args(&next, &cp.args);\n+\n+\t\t/* die() if child process die()'d */\n+\t\tres = run_command(&cp);\n+\t\tif (!res)\n+\t\t\treturn 0;\n+\t\tdie_message(_(\"Failed to recurse into submodule path '%s'\"),\n+\t\t\t    update_data->displaypath);\n+\t\tif (res == 128)\n+\t\t\texit(res);\n+\t\telse if (res)\n+\t\t\treturn 1;\n+\t}\n+\n+\treturn 0;\n+}\n+\n static int update_submodules(struct update_data *update_data)\n {\n \tint i, res = 0;\n@@ -2607,44 +2802,6 @@ static int module_update(int argc, const char **argv, const char *prefix)\n \treturn ret;\n }\n \n-static const char *remote_submodule_branch(const char *path)\n-{\n-\tconst struct submodule *sub;\n-\tconst char *branch = NULL;\n-\tchar *key;\n-\n-\tsub = submodule_from_path(the_repository, null_oid(), path);\n-\tif (!sub)\n-\t\treturn NULL;\n-\n-\tkey = xstrfmt(\"submodule.%s.branch\", sub->name);\n-\tif (repo_config_get_string_tmp(the_repository, key, &branch))\n-\t\tbranch = sub->branch;\n-\tfree(key);\n-\n-\tif (!branch)\n-\t\treturn \"HEAD\";\n-\n-\tif (!strcmp(branch, \".\")) {\n-\t\tconst char *refname = resolve_ref_unsafe(\"HEAD\", 0, NULL, NULL);\n-\n-\t\tif (!refname)\n-\t\t\tdie(_(\"No such ref: %s\"), \"HEAD\");\n-\n-\t\t/* detached HEAD */\n-\t\tif (!strcmp(refname, \"HEAD\"))\n-\t\t\tdie(_(\"Submodule (%s) branch configured to inherit \"\n-\t\t\t      \"branch from superproject, but the superproject \"\n-\t\t\t      \"is not on any branch\"), sub->name);\n-\n-\t\tif (!skip_prefix(refname, \"refs/heads/\", &refname))\n-\t\t\tdie(_(\"Expecting a full ref name, got %s\"), refname);\n-\t\treturn refname;\n-\t}\n-\n-\treturn branch;\n-}\n-\n static int push_check(int argc, const char **argv, const char *prefix)\n {\n \tstruct remote *remote;\n@@ -2722,32 +2879,6 @@ static int push_check(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static void ensure_core_worktree(const char *path)\n-{\n-\tconst char *cw;\n-\tstruct repository subrepo;\n-\n-\tif (repo_submodule_init(&subrepo, the_repository, path, null_oid()))\n-\t\tdie(_(\"could not get a repository handle for submodule '%s'\"), path);\n-\n-\tif (!repo_config_get_string_tmp(&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-\n static int absorb_git_dirs(int argc, const char **argv, const char *prefix)\n {\n \tint i;\n@@ -2969,138 +3100,6 @@ static int module_create_branch(int argc, const char **argv, const char *prefix)\n \treturn 0;\n }\n \n-static void update_data_to_args(struct update_data *update_data, struct strvec *args)\n-{\n-\tstrvec_pushl(args, \"submodule--helper\", \"update\", \"--recursive\", NULL);\n-\tstrvec_pushf(args, \"--jobs=%d\", update_data->max_jobs);\n-\tif (update_data->recursive_prefix)\n-\t\tstrvec_pushl(args, \"--recursive-prefix\",\n-\t\t\t     update_data->recursive_prefix, NULL);\n-\tif (update_data->quiet)\n-\t\tstrvec_push(args, \"--quiet\");\n-\tif (update_data->force)\n-\t\tstrvec_push(args, \"--force\");\n-\tif (update_data->init)\n-\t\tstrvec_push(args, \"--init\");\n-\tif (update_data->remote)\n-\t\tstrvec_push(args, \"--remote\");\n-\tif (update_data->nofetch)\n-\t\tstrvec_push(args, \"--no-fetch\");\n-\tif (update_data->dissociate)\n-\t\tstrvec_push(args, \"--dissociate\");\n-\tif (update_data->progress)\n-\t\tstrvec_push(args, \"--progress\");\n-\tif (update_data->require_init)\n-\t\tstrvec_push(args, \"--require-init\");\n-\tif (update_data->depth)\n-\t\tstrvec_pushf(args, \"--depth=%d\", update_data->depth);\n-\tif (update_data->update_default)\n-\t\tstrvec_pushl(args, \"--update\", update_data->update_default, NULL);\n-\tif (update_data->references.nr) {\n-\t\tstruct string_list_item *item;\n-\t\tfor_each_string_list_item(item, &update_data->references)\n-\t\t\tstrvec_pushl(args, \"--reference\", item->string, NULL);\n-\t}\n-\tif (update_data->filter_options && update_data->filter_options->choice)\n-\t\tstrvec_pushf(args, \"--filter=%s\",\n-\t\t\t\texpand_list_objects_filter_spec(\n-\t\t\t\t\tupdate_data->filter_options));\n-\tif (update_data->recommend_shallow == 0)\n-\t\tstrvec_push(args, \"--no-recommend-shallow\");\n-\telse if (update_data->recommend_shallow == 1)\n-\t\tstrvec_push(args, \"--recommend-shallow\");\n-\tif (update_data->single_branch >= 0)\n-\t\tstrvec_push(args, update_data->single_branch ?\n-\t\t\t\t    \"--single-branch\" :\n-\t\t\t\t    \"--no-single-branch\");\n-}\n-\n-static int update_submodule(struct update_data *update_data)\n-{\n-\tchar *prefixed_path;\n-\n-\tensure_core_worktree(update_data->sm_path);\n-\n-\tif (update_data->recursive_prefix)\n-\t\tprefixed_path = xstrfmt(\"%s%s\", update_data->recursive_prefix,\n-\t\t\t\t\tupdate_data->sm_path);\n-\telse\n-\t\tprefixed_path = xstrdup(update_data->sm_path);\n-\n-\tupdate_data->displaypath = get_submodule_displaypath(prefixed_path,\n-\t\t\t\t\t\t\t     update_data->prefix);\n-\tfree(prefixed_path);\n-\n-\tdetermine_submodule_update_strategy(the_repository, update_data->just_cloned,\n-\t\t\t\t\t    update_data->sm_path, update_data->update_default,\n-\t\t\t\t\t    &update_data->update_strategy);\n-\n-\tif (update_data->just_cloned)\n-\t\toidcpy(&update_data->suboid, null_oid());\n-\telse if (resolve_gitlink_ref(update_data->sm_path, \"HEAD\", &update_data->suboid))\n-\t\tdie(_(\"Unable to find current revision in submodule path '%s'\"),\n-\t\t\tupdate_data->displaypath);\n-\n-\tif (update_data->remote) {\n-\t\tchar *remote_name = get_default_remote_submodule(update_data->sm_path);\n-\t\tconst char *branch = remote_submodule_branch(update_data->sm_path);\n-\t\tchar *remote_ref = xstrfmt(\"refs/remotes/%s/%s\", remote_name, branch);\n-\n-\t\tif (!update_data->nofetch) {\n-\t\t\tif (fetch_in_submodule(update_data->sm_path, update_data->depth,\n-\t\t\t\t\t      0, NULL))\n-\t\t\t\tdie(_(\"Unable to fetch in submodule path '%s'\"),\n-\t\t\t\t    update_data->sm_path);\n-\t\t}\n-\n-\t\tif (resolve_gitlink_ref(update_data->sm_path, remote_ref, &update_data->oid))\n-\t\t\tdie(_(\"Unable to find %s revision in submodule path '%s'\"),\n-\t\t\t    remote_ref, update_data->sm_path);\n-\n-\t\tfree(remote_ref);\n-\t}\n-\n-\tif (!oideq(&update_data->oid, &update_data->suboid) || update_data->force)\n-\t\tif (run_update_procedure(update_data))\n-\t\t\treturn 1;\n-\n-\tif (update_data->recursive) {\n-\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n-\t\tstruct update_data next = *update_data;\n-\t\tint res;\n-\n-\t\tif (update_data->recursive_prefix)\n-\t\t\tprefixed_path = xstrfmt(\"%s%s/\", update_data->recursive_prefix,\n-\t\t\t\t\t\tupdate_data->sm_path);\n-\t\telse\n-\t\t\tprefixed_path = xstrfmt(\"%s/\", update_data->sm_path);\n-\n-\t\tnext.recursive_prefix = get_submodule_displaypath(prefixed_path,\n-\t\t\t\t\t\t\t\t  update_data->prefix);\n-\t\tnext.prefix = NULL;\n-\t\toidcpy(&next.oid, null_oid());\n-\t\toidcpy(&next.suboid, null_oid());\n-\n-\t\tcp.dir = update_data->sm_path;\n-\t\tcp.git_cmd = 1;\n-\t\tprepare_submodule_repo_env(&cp.env_array);\n-\t\tupdate_data_to_args(&next, &cp.args);\n-\n-\t\t/* die() if child process die()'d */\n-\t\tres = run_command(&cp);\n-\t\tif (!res)\n-\t\t\treturn 0;\n-\t\tdie_message(_(\"Failed to recurse into submodule path '%s'\"),\n-\t\t\t    update_data->displaypath);\n-\t\tif (res == 128)\n-\t\t\texit(res);\n-\t\telse if (res)\n-\t\t\treturn 1;\n-\t}\n-\n-\treturn 0;\n-}\n-\n struct add_data {\n \tconst char *prefix;\n \tconst char *branch;\n-- \n2.33.GIT\n\n"}]}