On Fri, Jul 22, 2016 at 12:15 PM, Johannes Sixt <j6t@xxxxxxxx> wrote:
> '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@xxxxxxxx>
> ---
> builtin/submodule--helper.c | 2 +-
> t/t5815-submodule-protos.sh | 4 ++--
> t/t7400-submodule-basic.sh | 4 ++--
> 3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index b22352b..6f6d67a 100644
> --- 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 {
> - idx = suc->current - suc->list.nr;
> + idx -= suc->list.nr;
The fix is the same as in
http://thread.gmane.org/gmane.comp.version-control.git/299995
There we have an additional check, which may make sense to use here as well,
specifically when having the patch 1 which propagates the exit code.
The approach to tests is different though. I like yours better than mine,
as it doesn't add more tests, but strengthens existing tests.
> 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 112cf40..06f55a1 100755
> --- 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"
> '
>
> -test_expect_failure 'clone with recurse-submodules fails' '
> +test_expect_success 'clone with recurse-submodules fails' '
> 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
> '
>
> -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
> '
>
> diff --git a/t/t7400-submodule-basic.sh b/t/t7400-submodule-basic.sh
> index 7c8b90b..b77cce8 100755
> --- 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_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 &&
> @@ -361,7 +361,7 @@ test_expect_failure 'update should fail when path is used by a file' '
> 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 &&
> --
> 2.9.0.443.ga8520ad
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html