Re: [PATCH v7 17/20] submodule: move core cmd_update() logic to C
Here is the diff between this tree (+) and that of v5's [1] (-).
[1] https://lore.kernel.org/git/patch-v5-9.9-e8e57606ee9-20220128T125206Z-avarab@gmail.com
----- >8 --------- >8 --------- >8 --------- >8 --------- >8 ----
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 1c28b6f479..b895e88a64 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2003,6 +2003,7 @@ struct update_data {
const char *recursive_prefix;
const char *sm_path;
const char *displaypath;
+ const char *update_default;
struct object_id oid;
struct object_id suboid;
struct submodule_update_strategy update_strategy;
@@ -2355,15 +2356,8 @@ static int run_update_command(struct update_data *ud, int subforce)
struct child_process cp = CHILD_PROCESS_INIT;
char *oid = oid_to_hex(&ud->oid);
int must_die_on_failure = 0;
- struct submodule_update_strategy strategy = SUBMODULE_UPDATE_STRATEGY_INIT;
- if (ud->update_strategy.type == SM_UPDATE_UNSPECIFIED || ud->just_cloned)
- determine_submodule_update_strategy(the_repository, ud->just_cloned,
- ud->sm_path, NULL, &strategy);
- else
- strategy = ud->update_strategy;
-
- switch (strategy.type) {
+ switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
cp.git_cmd = 1;
strvec_pushl(&cp.args, "checkout", "-q", NULL);
@@ -2386,45 +2380,41 @@ static int run_update_command(struct update_data *ud, int subforce)
break;
case SM_UPDATE_COMMAND:
cp.use_shell = 1;
- strvec_push(&cp.args, strategy.command);
+ strvec_push(&cp.args, ud->update_strategy.command);
must_die_on_failure = 1;
break;
default:
BUG("unexpected update strategy type: %s",
- submodule_strategy_to_string(&strategy));
+ submodule_strategy_to_string(&ud->update_strategy));
}
strvec_push(&cp.args, oid);
cp.dir = xstrdup(ud->sm_path);
prepare_submodule_repo_env(&cp.env_array);
if (run_command(&cp)) {
- switch (strategy.type) {
+ switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
die_message(_("Unable to checkout '%s' in submodule path '%s'"),
oid, ud->displaypath);
break;
case SM_UPDATE_REBASE:
- if (!must_die_on_failure)
- break;
- die(_("Unable to rebase '%s' in submodule path '%s'"),
+ die_message(_("Unable to rebase '%s' in submodule path '%s'"),
oid, ud->displaypath);
break;
case SM_UPDATE_MERGE:
- if (!must_die_on_failure)
- break;
- die(_("Unable to merge '%s' in submodule path '%s'"),
+ die_message(_("Unable to merge '%s' in submodule path '%s'"),
oid, ud->displaypath);
break;
case SM_UPDATE_COMMAND:
- if (!must_die_on_failure)
- break;
- die(_("Execution of '%s %s' failed in submodule path '%s'"),
- strategy.command, oid, ud->displaypath);
+ die_message(_("Execution of '%s %s' failed in submodule path '%s'"),
+ ud->update_strategy.command, oid, ud->displaypath);
break;
default:
BUG("unexpected update strategy type: %s",
- submodule_strategy_to_string(&strategy));
+ submodule_strategy_to_string(&ud->update_strategy));
}
+ if (must_die_on_failure)
+ exit(128);
/* the command failed, but update must continue */
return 1;
@@ -2433,7 +2423,7 @@ static int run_update_command(struct update_data *ud, int subforce)
if (ud->quiet)
return 0;
- switch (strategy.type) {
+ switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
printf(_("Submodule path '%s': checked out '%s'\n"),
ud->displaypath, oid);
@@ -2448,11 +2438,11 @@ static int run_update_command(struct update_data *ud, int subforce)
break;
case SM_UPDATE_COMMAND:
printf(_("Submodule path '%s': '%s %s'\n"),
- ud->displaypath, strategy.command, oid);
+ ud->displaypath, ud->update_strategy.command, oid);
break;
default:
BUG("unexpected update strategy type: %s",
- submodule_strategy_to_string(&strategy));
+ submodule_strategy_to_string(&ud->update_strategy));
}
return 0;
@@ -2832,10 +2822,12 @@ static int module_set_branch(int argc, const char **argv, const char *prefix)
static void update_data_to_args(struct update_data *update_data, struct strvec *args)
{
- const char *update = submodule_strategy_to_string(&update_data->update_strategy);
-
strvec_pushl(args, "submodule--helper", "update", "--recursive", NULL);
strvec_pushf(args, "--jobs=%d", update_data->max_jobs);
+ /*
+ * NEEDSWORK: the equivalent code in git-submodule.sh does not
+ * pass --prefix, so this shouldn't either
+ */
if (update_data->prefix)
strvec_pushl(args, "--prefix", update_data->prefix, NULL);
if (update_data->recursive_prefix)
@@ -2859,8 +2851,8 @@ static void update_data_to_args(struct update_data *update_data, struct strvec *
strvec_push(args, "--require-init");
if (update_data->depth)
strvec_pushf(args, "--depth=%d", update_data->depth);
- if (update)
- strvec_pushl(args, "--update", update, NULL);
+ if (update_data->update_default)
+ strvec_pushl(args, "--update", update_data->update_default, NULL);
if (update_data->references.nr) {
struct string_list_item *item;
for_each_string_list_item(item, &update_data->references)
@@ -2890,6 +2882,11 @@ static int update_submodule(struct update_data *update_data)
update_data->prefix);
free(prefixed_path);
+ determine_submodule_update_strategy(the_repository, update_data->just_cloned,
+ update_data->sm_path, update_data->update_default,
+ &update_data->update_strategy);
+
+ /* NEEDSWORK: fix the style issues e.g. braces */
if (update_data->just_cloned) {
oidcpy(&update_data->suboid, null_oid());
} else {
@@ -3000,10 +2997,10 @@ static int update_submodules(struct update_data *update_data)
static int module_update(int argc, const char **argv, const char *prefix)
{
- const char *update = NULL;
struct pathspec pathspec;
struct update_data opt = UPDATE_DATA_INIT;
+ /* NEEDSWORK: update names and strings */
struct option module_update_clone_options[] = {
OPT__FORCE(&opt.force, N_("force checkout updates"), 0),
OPT_BOOL(0, "init", &opt.init,
@@ -3021,7 +3018,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
N_("path"),
N_("path into the working tree, across nested "
"submodule boundaries")),
- OPT_STRING(0, "update", &update,
+ OPT_STRING(0, "update", &opt.update_default,
N_("string"),
N_("rebase, merge, checkout or none")),
OPT_STRING_LIST(0, "reference", &opt.references, N_("repo"),
@@ -3058,8 +3055,8 @@ static int module_update(int argc, const char **argv, const char *prefix)
oidcpy(&opt.oid, null_oid());
oidcpy(&opt.suboid, null_oid());
- if (update)
- if (parse_submodule_update_strategy(update,
+ if (opt.update_default)
+ if (parse_submodule_update_strategy(opt.update_default,
&opt.update_strategy) < 0)
die(_("bad value for update parameter"));
@@ -3490,6 +3487,7 @@ static struct cmd_struct commands[] = {
{"sync", module_sync, SUPPORT_SUPER_PREFIX},
{"deinit", module_deinit, 0},
{"summary", module_summary, SUPPORT_SUPER_PREFIX},
+ /* NEEDSWORK: remote-branch is also obsolete */
{"remote-branch", resolve_remote_submodule_branch, 0},
{"push-check", push_check, 0},
{"absorb-git-dirs", absorb_git_dirs, SUPPORT_SUPER_PREFIX},
diff --git a/git-submodule.sh b/git-submodule.sh
index bcd8b92aab..d176469fb1 100755
--- a/git-submodule.sh
+++ b/git-submodule.sh
@@ -50,6 +50,7 @@ single_branch=
jobs=
recommend_shallow=
+# NEEDSWORK this is now unused
die_if_unmatched ()
{
if test "$1" = "#unmatched"
@@ -347,6 +348,8 @@ cmd_update()
shift
done
+ # NEEDSWORK --super-prefix isn't actually supported by this
+ # command - we just pass the $prefix to --recursive-prefix.
git ${wt_prefix:+-C "$wt_prefix"} ${prefix:+--super-prefix "$prefix"} submodule--helper update \
${GIT_QUIET:+--quiet} \
${force:+--force} \
Glen Choo <chooglen@google.com>