submodule-helper: fix indexing in clone retry error reporting path

'git submodule--helper update-clone' has logic to retry failed clones
a second time. For this purpose, there is a list of submodules to clone,
and a second list that is filled with the submodules to retry. Within
these lists, the submodules are identified by an index as if both lists
were just appended.

This works nicely except when the second clone attempt fails as well. To
report an error, the identifying index must be adjusted by an offset so
that it can be used as an index into the second list. However, the
calculation uses the logical total length of the lists so that the result
always points one past the end of the second list.

Pick the correct index.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
Acked-by: Stefan Beller <sbeller@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index b22352b6e1e4c40d1a6e182f5c12abf6351f71fb..6f6d67a4694a83f18f86f5de2d9d64fd4e449a5e 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -795,7 +795,7 @@ static int update_clone_task_finished(int result,
                suc->failed_clones[suc->failed_clones_nr++] = ce;
                return 0;
        } else {
                suc->failed_clones[suc->failed_clones_nr++] = ce;
                return 0;
        } else {

-               idx = suc->current - suc->list.nr;
+               idx -= suc->list.nr;

                ce  = suc->failed_clones[idx];
                strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
                            ce->name);
                ce  = suc->failed_clones[idx];
                strbuf_addf(err, _("Failed to clone '%s' a second time, aborting"),
                            ce->name);

diff --git a/t/t5815-submodule-protos.sh b/t/t5815-submodule-protos.sh
index 112cf40233d6eb7847b14f495fb0bc43f101428f..06f55a1b8a0b57a1dad151d591e9469c81588775 100755 (executable)
--- a/t/t5815-submodule-protos.sh
+++ b/t/t5815-submodule-protos.sh
@@ -18,7 +18,7 @@ test_expect_success 'setup repository with submodules' '
        git commit -m "add submodules"
 '
 
        git commit -m "add submodules"
 '
 

-test_expect_failure 'clone with recurse-submodules fails' '
+test_expect_success 'clone with recurse-submodules fails' '

        test_must_fail git clone --recurse-submodules . dst
 '
 
        test_must_fail git clone --recurse-submodules . dst
 '
 


@@ -32,7 +32,7 @@ test_expect_success 'update of ssh allowed' '
        git -C dst submodule update ssh-module
 '
 
        git -C dst submodule update ssh-module
 '
 

-test_expect_failure 'update of ext not allowed' '
+test_expect_success 'update of ext not allowed' '

        test_must_fail git -C dst submodule update ext-module
 '
 
        test_must_fail git -C dst submodule update ext-module
 '
 

diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
index fba2659a22a9f73e30b1f473ee83cf6c625f4f6a..3570f7bb8c8955a1bf519eda3278392d249e6fd1 100755 (executable)
--- a/t/t7400-submodule-basic.sh
+++ b/t/t7400-submodule-basic.sh
@@ -352,7 +352,7 @@ test_expect_success 'sync should fail with unknown submodule' '
        test_failure_with_unknown_submodule sync
 '
 
        test_failure_with_unknown_submodule sync
 '
 

-test_expect_failure 'update should fail when path is used by a file' '
+test_expect_success 'update should fail when path is used by a file' '

        echo hello >expect &&
 
        echo "hello" >init &&
        echo hello >expect &&
 
        echo "hello" >init &&


@@ -361,7 +361,7 @@ test_expect_failure 'update should fail when path is used by a file' '
        test_cmp expect init
 '
 
        test_cmp expect init
 '
 

-test_expect_failure 'update should fail when path is used by a nonempty directory' '
+test_expect_success 'update should fail when path is used by a nonempty directory' '

        echo hello >expect &&
 
        rm -fr init &&
        echo hello >expect &&
 
        rm -fr init &&