From 5fc84755f1478ea8c0f05b8d7be67a3c96a24bff Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 14 Jun 2018 10:31:07 -0700 Subject: [PATCH 1/2] submodule: fix NULL correctness in renamed broken submodules When fetching with recursing into submodules, the fetch logic inspects the superproject which submodules actually need to be fetched. This is tricky for submodules that were renamed in the fetched range of commits. This was implemented in c68f8375760 (implement fetching of moved submodules, 2017-10-16), and this patch fixes a mistake in the logic there. When the warning is printed, the `name` might be NULL as default_name_or_path can return NULL, so fix the warning to use the path as obtained from the diff machinery, as that is not NULL. While at it, make sure we only attempt to load the submodule if a git directory of the submodule is found as default_name_or_path will return NULL in case the git directory cannot be found. Note that passing NULL to submodule_from_name is just a semantic error, as submodule_from_name accepts NULL as a value, but then the return value is not the submodule that was asked for, but some arbitrary other submodule. (Cf. 'config_from' in submodule-config.c: "If any parameter except the cache is a NULL pointer just return the first submodule. Can be used to check whether there are any submodules parsed.") Reported-by: Duy Nguyen Helped-by: Duy Nguyen Helped-by: Heiko Voigt Signed-off-by: Stefan Beller Acked-by: Heiko Voigt Signed-off-by: Junio C Hamano --- submodule.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/submodule.c b/submodule.c index 939d6870ecd..0998ea23458 100644 --- a/submodule.c +++ b/submodule.c @@ -740,12 +740,14 @@ static void collect_changed_submodules_cb(struct diff_queue_struct *q, else { name = default_name_or_path(p->two->path); /* make sure name does not collide with existing one */ - submodule = submodule_from_name(the_repository, commit_oid, name); + if (name) + submodule = submodule_from_name(the_repository, + commit_oid, name); if (submodule) { warning("Submodule in commit %s at path: " "'%s' collides with a submodule named " "the same. Skipping it.", - oid_to_hex(commit_oid), name); + oid_to_hex(commit_oid), p->two->path); name = NULL; } } From c3749f6e597a7b8140181fa5efc95f3f08595f8c Mon Sep 17 00:00:00 2001 From: Stefan Beller Date: Thu, 14 Jun 2018 10:37:30 -0700 Subject: [PATCH 2/2] t5526: test recursive submodules when fetching moved submodules The topic merged in 0c7ecb7c311 (Merge branch 'sb/submodule-move-nested', 2018-05-08) provided support for moving nested submodules. Remove the NEEDSWORK comment and implement the nested submodules test as the comment hinted at. Signed-off-by: Stefan Beller Acked-by: Heiko Voigt Signed-off-by: Junio C Hamano --- t/t5526-fetch-submodules.sh | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/t/t5526-fetch-submodules.sh b/t/t5526-fetch-submodules.sh index 9cc4b569c05..359e03ff836 100755 --- a/t/t5526-fetch-submodules.sh +++ b/t/t5526-fetch-submodules.sh @@ -574,11 +574,7 @@ test_expect_success "fetch new commits when submodule got renamed" ' git clone . downstream_rename && ( cd downstream_rename && - git submodule update --init && -# NEEDSWORK: we omitted --recursive for the submodule update here since -# that does not work. See test 7001 for mv "moving nested submodules" -# for details. Once that is fixed we should add the --recursive option -# here. + git submodule update --init --recursive && git checkout -b rename && git mv submodule submodule_renamed && (