Re: [PATCH 1/2] submodule--helper: set alternateLocation for cloned submodules
From: Stefan Beller <hidden>
Date: 2016-12-08 00:02:46
On Wed, Dec 7, 2016 at 2:49 PM, [off-list ref] 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)
quoted hunk ↗ jump to hunk
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