From: Glen Choo <chooglen@xxxxxxxxxx>
There are two locations in prepare_to_clone_next_submodule() that
manually calculate the submodule display path, but should just use
do_get_submodule_displaypath() for consistency.
Do this replacement and reorder the code slightly to avoid computing
the display path twice.
This code was never tested, and adding tests shows that both these sites
have been computing the display path incorrectly ever since they were
introduced in 48308681b0 (git submodule update: have a dedicated helper
for cloning, 2016-02-29) [1]:
- The first hunk puts a "/" between recursive_prefix and ce->name, but
recursive_prefix already ends with "/".
- The second hunk calls relative_path() on recursive_prefix and
ce->name, but relative_path() only makes sense when both paths share
the same base directory. This is never the case here:
- recursive_prefix is the path from the topmost superproject to the
current submodule
- ce->name is the path from the root of the current submodule to its
submodule.
so, e.g. recursive_prefix="super" and ce->name="submodule" produces
displayname="../super" instead of "super/submodule".
While we're fixing the display names, also fix inconsistent quoting of
the submodule name.
[1] I verified this by applying the tests to 48308681b0.
Signed-off-by: Glen Choo <chooglen@xxxxxxxxxx>
---
builtin/submodule--helper.c | 22 +++++---------
t/t7406-submodule-update.sh | 59 +++++++++++++++++++++++++++++++++++++
2 files changed, 66 insertions(+), 15 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index c597df7528e..63c661b26a6 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1949,30 +1949,22 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
const char *update_string;
enum submodule_update_type update_type;
char *key;
- struct strbuf displaypath_sb = STRBUF_INIT;
struct strbuf sb = STRBUF_INIT;
- const char *displaypath = NULL;
+ char *displaypath = NULL;
int needs_cloning = 0;
int need_free_url = 0;
+ displaypath = do_get_submodule_displaypath(ce->name,
+ suc->update_data->prefix,
+ suc->update_data->recursive_prefix);
+
if (ce_stage(ce)) {
- if (suc->update_data->recursive_prefix)
- strbuf_addf(&sb, "%s/%s", suc->update_data->recursive_prefix, ce->name);
- else
- strbuf_addstr(&sb, ce->name);
- strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);
- strbuf_addch(out, '\n');
+ strbuf_addf(out, _("Skipping unmerged submodule '%s'\n"), displaypath);
goto cleanup;
}
sub = submodule_from_path(the_repository, null_oid(), ce->name);
- if (suc->update_data->recursive_prefix)
- displaypath = relative_path(suc->update_data->recursive_prefix,
- ce->name, &displaypath_sb);
- else
- displaypath = ce->name;
-
if (!sub) {
next_submodule_warn_missing(suc, out, displaypath);
goto cleanup;
@@ -2062,7 +2054,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,
"--no-single-branch");
cleanup:
- strbuf_release(&displaypath_sb);
+ free(displaypath);
strbuf_release(&sb);
if (need_free_url)
free((void*)url);
diff --git a/t/t7406-submodule-update.sh b/t/t7406-submodule-update.sh
index 43f779d751c..e1dc3b1041b 100755
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1116,4 +1116,63 @@ test_expect_success 'submodule update --filter sets partial clone settings' '
test_cmp_config -C super-filter/submodule blob:none remote.origin.partialclonefilter
'
+# NEEDSWORK: Clean up the tests so that we can reuse the test setup.
+# Don't reuse the existing repos because the earlier tests have
+# intentionally disruptive configurations.
+test_expect_success 'setup clean recursive superproject' '
+ git init bottom &&
+ test_commit -C bottom "bottom" &&
+ git init middle &&
+ git -C middle submodule add ../bottom bottom &&
+ git -C middle commit -m "middle" &&
+ git init top &&
+ git -C top submodule add ../middle middle &&
+ git -C top commit -m "top" &&
+ git clone --recurse-submodules top top-clean
+'
+
+test_expect_success 'submodule update should skip unmerged submodules' '
+ test_when_finished "rm -fr top-cloned" &&
+ cp -r top-clean top-cloned &&
+
+ # Create an upstream commit in each repo
+ test_commit -C bottom upstream_commit &&
+ (cd middle &&
+ git -C bottom fetch &&
+ git -C bottom checkout -f FETCH_HEAD &&
+ git add bottom &&
+ git commit -m "upstream_commit"
+ ) &&
+ (cd top &&
+ git -C middle fetch &&
+ git -C middle checkout -f FETCH_HEAD &&
+ git add middle &&
+ git commit -m "upstream_commit"
+ ) &&
+
+ # Create a downstream conflict
+ (cd top-cloned/middle &&
+ test_commit -C bottom downstream_commit &&
+ git add bottom &&
+ git commit -m "downstream_commit" &&
+ git fetch --recurse-submodules origin &&
+ test_must_fail git merge origin/main
+ ) &&
+ # Make the update of "middle" a no-op, otherwise we error out
+ # because of its unmerged state
+ test_config -C top-cloned submodule.middle.update !true &&
+ git -C top-cloned submodule update --recursive 2>actual.err &&
+ grep "Skipping unmerged submodule .middle/bottom." actual.err
+'
+
+test_expect_success 'submodule update --recursive skip submodules with strategy=none' '
+ test_when_finished "rm -fr top-cloned" &&
+ cp -r top-clean top-cloned &&
+
+ test_commit -C top-cloned/middle/bottom downstream_commit &&
+ git -C top-cloned/middle config submodule.bottom.update none &&
+ git -C top-cloned submodule update --recursive 2>actual.err &&
+ grep "Skipping submodule .middle/bottom." actual.err
+'
+
test_done
--
gitgitgadget
