submodule--helper update: don't override 'checkout' exit code

When "git submodule update" runs it might call "checkout", "merge", "rebase", or a custom command. Ever since run_update_command() was added in c51f8f94e5b (submodule--helper: run update procedures from C, 2021-08-24) we'd either exit immediately if the "submodule.<name>.update" method failed, or in the case of "checkout" continue trying to update other submodules. This code used to use the magical "2" return code, but in 55b3f12cb54 (submodule update: use die_message(), 2022-03-15) it was made to exit(128), which in preceding commits has been changed to return that 128 code to the top-level. Let's "libify" this code even more by not having it arbitrarily override the return code. In practice this doesn't change anything as the code "git checkout" would return on any normal failure is "1", but we'll now in principle properly abort the operation if "git checkout" were to exit with 128. It would make sense to follow-up this change with a change to allow the "submodule.<name>.update = !..." (SM_UPDATE_COMMAND) method the same liberties as "checkout", and perhaps to do the same with a failed "merge" or "rebase". But let's leave that for now. 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 a03c01de2fbb1e83138d83095ddacd1417f49b66
1 file changed +3 -6
builtin/submodule--helper.c
+3 -6
@@ -2133,6 +2133,7 @@ static int run_update_command(const struct update_data *ud, int subforce)
2133 {
2134 struct child_process cp = CHILD_PROCESS_INIT;
2135 char *oid = oid_to_hex(&ud->oid);
2136 + int ret;
2137
2138 switch (ud->update_strategy.type) {
2139 case SM_UPDATE_CHECKOUT:
@@ -2165,15 +2166,12 @@ static int run_update_command(const struct update_data *ud, int subforce)
2166
2167 cp.dir = xstrdup(ud->sm_path);
2168 prepare_submodule_repo_env(&cp.env);
2168 - if (run_command(&cp)) {
2169 - int ret;
2170 -
2169 + if ((ret = run_command(&cp))) {
2170 switch (ud->update_strategy.type) {
2171 case SM_UPDATE_CHECKOUT:
2172 die_message(_("Unable to checkout '%s' in submodule path '%s'"),
2173 oid, ud->displaypath);
2175 - /* the command failed, but update must continue */
2176 - ret = 1;
2174 + /* No "ret" assignment, use "git checkout"'s */
2175 break;
2176 case SM_UPDATE_REBASE:
2177 ret = die_message(_("Unable to rebase '%s' in submodule path '%s'"),
@@ -2496,7 +2494,6 @@ static int update_submodules(struct update_data *update_data)
2494 ret = code;
2495 if (ret == 128)
2496 goto cleanup;
2499 - ret = 1;
2497 }
2498
2499 cleanup: