@matheusbsbs / matheus-bagliano / commits / 6870cdc32a

submodule--helper: use "code" in run_update_command()

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@gmail.com> Reviewed-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 1, 2022 at 01:18 UTC 6870cdc32aaa2b5786b77898522d932cf592a7f1
1 file changed +13 -14
builtin/submodule--helper.c
+13 -14
@@ -2127,7 +2127,6 @@ static int run_update_command(const struct update_data *ud, int subforce)
2127 {
2128 struct child_process cp = CHILD_PROCESS_INIT;
2129 char *oid = oid_to_hex(&ud->oid);
2130 - int must_die_on_failure = 0;
2130
2131 switch (ud->update_strategy.type) {
2132 case SM_UPDATE_CHECKOUT:
@@ -2141,19 +2140,16 @@ static int run_update_command(const struct update_data *ud, int subforce)
2140 strvec_push(&cp.args, "rebase");
2141 if (ud->quiet)
2142 strvec_push(&cp.args, "--quiet");
2144 - must_die_on_failure = 1;
2143 break;
2144 case SM_UPDATE_MERGE:
2145 cp.git_cmd = 1;
2146 strvec_push(&cp.args, "merge");
2147 if (ud->quiet)
2148 strvec_push(&cp.args, "--quiet");
2151 - must_die_on_failure = 1;
2149 break;
2150 case SM_UPDATE_COMMAND:
2151 cp.use_shell = 1;
2152 strvec_push(&cp.args, ud->update_strategy.command);
2156 - must_die_on_failure = 1;
2153 break;
2154 default:
2155 BUG("unexpected update strategy type: %d",
@@ -2164,32 +2160,35 @@ static int run_update_command(const struct update_data *ud, int subforce)
2160 cp.dir = xstrdup(ud->sm_path);
2161 prepare_submodule_repo_env(&cp.env);
2162 if (run_command(&cp)) {
2163 + int ret;
2164 +
2165 switch (ud->update_strategy.type) {
2166 case SM_UPDATE_CHECKOUT:
2167 die_message(_("Unable to checkout '%s' in submodule path '%s'"),
2168 oid, ud->displaypath);
2169 + /* the command failed, but update must continue */
2170 + ret = 1;
2171 break;
2172 case SM_UPDATE_REBASE:
2173 - die_message(_("Unable to rebase '%s' in submodule path '%s'"),
2174 - oid, ud->displaypath);
2173 + ret = die_message(_("Unable to rebase '%s' in submodule path '%s'"),
2174 + oid, ud->displaypath);
2175 break;
2176 case SM_UPDATE_MERGE:
2177 - die_message(_("Unable to merge '%s' in submodule path '%s'"),
2178 - oid, ud->displaypath);
2177 + ret = die_message(_("Unable to merge '%s' in submodule path '%s'"),
2178 + oid, ud->displaypath);
2179 break;
2180 case SM_UPDATE_COMMAND:
2181 - die_message(_("Execution of '%s %s' failed in submodule path '%s'"),
2182 - ud->update_strategy.command, oid, ud->displaypath);
2181 + ret = die_message(_("Execution of '%s %s' failed in submodule path '%s'"),
2182 + ud->update_strategy.command, oid, ud->displaypath);
2183 break;
2184 default:
2185 BUG("unexpected update strategy type: %d",
2186 ud->update_strategy.type);
2187 }
2188 - if (must_die_on_failure)
2189 - exit(128);
2188
2191 - /* the command failed, but update must continue */
2192 - return 1;
2189 + if (ret == 128)
2190 + exit(ret);
2191 + return ret;
2192 }
2193
2194 if (ud->quiet)