Apply some DRY principles in run_update_command() and don't have two
"switch" statements over "ud->update_strategy.type" determine the same
thing.
First we were setting "must_die_on_failure = 1" in all cases except
"SM_UPDATE_CHECKOUT" (and we'd BUG(...) out on the rest). This code
was added in c51f8f94e5b (submodule--helper: run update procedures
from C, 2021-08-24).
Then we'd duplicate same "switch" logic when we were using the
"must_die_on_failure" variable.
Let's instead have the "case" branches in that inner "switch"
determine whether or not the "update must continue" by picking an exit
code.
This also mostly avoids hardcoding the "128" exit code, instead we can
make use of the return value of the die_message() function, which
we've been calling here since 55b3f12cb54 (submodule update: use
die_message(), 2022-03-15). We're still hardcoding it to determine if
we "exit()", but subsequent commit(s) will address that.
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
builtin/submodule--helper.c | 27 +++++++++++++--------------
1 file changed, 13 insertions(+), 14 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d787c0fead4..bac45e2bab7 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -2145,7 +2145,6 @@ 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;
switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
@@ -2159,19 +2158,16 @@ static int run_update_command(struct update_data *ud, int subforce)
strvec_push(&cp.args, "rebase");
if (ud->quiet)
strvec_push(&cp.args, "--quiet");
- must_die_on_failure = 1;
break;
case SM_UPDATE_MERGE:
cp.git_cmd = 1;
strvec_push(&cp.args, "merge");
if (ud->quiet)
strvec_push(&cp.args, "--quiet");
- must_die_on_failure = 1;
break;
case SM_UPDATE_COMMAND:
cp.use_shell = 1;
strvec_push(&cp.args, ud->update_strategy.command);
- must_die_on_failure = 1;
break;
default:
BUG("unexpected update strategy type: %d",
@@ -2182,32 +2178,35 @@ static int run_update_command(struct update_data *ud, int subforce)
cp.dir = xstrdup(ud->sm_path);
prepare_submodule_repo_env(&cp.env);
if (run_command(&cp)) {
+ int ret;
+
switch (ud->update_strategy.type) {
case SM_UPDATE_CHECKOUT:
die_message(_("Unable to checkout '%s' in submodule path '%s'"),
oid, ud->displaypath);
+ /* the command failed, but update must continue */
+ ret = 1;
break;
case SM_UPDATE_REBASE:
- die_message(_("Unable to rebase '%s' in submodule path '%s'"),
- oid, ud->displaypath);
+ ret = die_message(_("Unable to rebase '%s' in submodule path '%s'"),
+ oid, ud->displaypath);
break;
case SM_UPDATE_MERGE:
- die_message(_("Unable to merge '%s' in submodule path '%s'"),
- oid, ud->displaypath);
+ ret = die_message(_("Unable to merge '%s' in submodule path '%s'"),
+ oid, ud->displaypath);
break;
case SM_UPDATE_COMMAND:
- die_message(_("Execution of '%s %s' failed in submodule path '%s'"),
- ud->update_strategy.command, oid, ud->displaypath);
+ ret = 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: %d",
ud->update_strategy.type);
}
- if (must_die_on_failure)
- exit(128);
- /* the command failed, but update must continue */
- return 1;
+ if (ret == 128)
+ exit(ret);
+ return ret;
}
if (ud->quiet)
--
2.37.1.1233.ge8b09efaedc
