On Wed, Dec 7, 2016 at 2:49 PM, <[email protected]> wrote:
> Notes:
> Resolved issues pointed by Stefan Beller except of
> the one about loosened path check, which he aggreed
> to be relaxed for this case.
I am sorry to have given an incomplete review at the first time. :/
More below.
> + /* setup alternateLocation and alternateErrorStrategy in the cloned
> submodule if needed */
> + git_config_get_string("submodule.alternateLocation", &sm_alternate);
> + if (sm_alternate) {
> + git_config_set_in_file(p, "submodule.alternateLocation",
> + sm_alternate);
> + }
For a single command, we usually omit the braces for
an if clause, i.e.
if (foo)
bar(...);
/* code goes on */
> + git_config_get_string("submodule.alternateErrorStrategy",
> &error_strategy);
> + if (error_strategy) {
> + git_config_set_in_file(p, "submodule.alternateErrorStrategy",
> + error_strategy);
> + }
> +
> + free(sm_alternate);
> + free(error_strategy);
> +
The indentation seems a bit off for the free here?
(The main nit that motivated me writing the email)
> strbuf_release(&sb);
> strbuf_release(&rel_path);
> free(sm_gitdir);
> diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh
> index 1c1e289..ef7771b 100755
> --- a/t/t7408-submodule-reference.sh
> +++ b/t/t7408-submodule-reference.sh
> @@ -125,4 +125,76 @@ test_expect_success 'ignoring missing submodule
> alternates passes clone and subm
> )
> '
>
> +test_expect_success 'preparing second superproject with a nested submodule' '
> + test_create_repo supersuper &&
> + (
> + cd supersuper &&
> + echo "I am super super." >file &&
> + git add file &&
> + git commit -m B-super-super-initial
> + git submodule add "file://$base_dir/super" subwithsub &&
> + git commit -m B-super-super-added &&
> + git submodule update --init --recursive &&
> + git repack -ad
> + )
> +'
> +
> +# At this point there are three root-level positories: A, B, super and super2
> +
> +test_expect_success 'nested submodule alternate in works and is actually
> used' '
> + test_when_finished "rm -rf supersuper-clone" &&
> + git clone --recursive --reference supersuper supersuper
> supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # immediate submodule has alternate:
> + test_alternate_is_used
> .git/modules/subwithsub/objects/info/alternates subwithsub
> + # nested submodule also has alternate:
> + test_alternate_is_used
> .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> + )
> +'
> +
> +test_expect_success 'missing nested submodule alternate fails clone and
> submodule update' '
> + test_when_finished "rm -rf supersuper-clone supersuper2" &&
> + git clone supersuper supersuper2 &&
> + (
> + cd supersuper2 &&
> + git submodule update --init
> + ) &&
> + test_must_fail git clone --recursive --reference supersuper2
> supersuper2 supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # update of the submodule fails
> + test_must_fail git submodule update --init --recursive &&
> + # immediate submodule has alternate:
> + test_alternate_is_used
> .git/modules/subwithsub/objects/info/alternates subwithsub
> + # but nested submodule has no alternate:
> + test_must_fail test_alternate_is_used
> .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> + )
> +'
> +
> +test_expect_success 'missing nested submodule alternate in
> --reference-if-able mode' '
> + test_when_finished "rm -rf supersuper-clone supersuper2" &&
> + git clone supersuper supersuper2 &&
> + (
> + cd supersuper2 &&
> + git submodule update --init
> + ) &&
> + git clone --recursive --reference-if-able supersuper2 supersuper2
> supersuper-clone &&
> + (
> + cd supersuper-clone &&
> + # test superproject has alternates setup correctly
> + test_alternate_is_used .git/objects/info/alternates . &&
> + # update of the submodule fails
> + test_must_fail git submodule update --init --recursive &&
> + # immediate submodule has alternate:
> + test_alternate_is_used
> .git/modules/subwithsub/objects/info/alternates subwithsub
> + # but nested submodule has no alternate:
> + test_must_fail test_alternate_is_used
> .git/modules/subwithsub/modules/sub/objects/info/alternates subwithsub/sub
> + )
Both the first and the last part are the same in the last two tests,
the only difference is the line with git clone --reference ...
Maybe we can use a function somehow to make this a bit more
obvious?
Otherwise the tests look good to me.
Thanks,
Stefan
