submodule--helper update: use display path helper

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.

Until the preceding commit this code had never been tested, with our
newly added tests we can see 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".

[1] I verified this by applying the tests to 48308681b0.

Signed-off-by: Glen Choo <chooglen@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index d60f6cd9de0a8ce5040236bdc55a9ef60004d9a8..08ca18af49ecdd1f6fcfc9a298e156cf3c569b96 100644 (file)
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1947,30 +1947,21 @@ 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 update_data *ud = suc->update_data;
+       char *displaypath = do_get_submodule_displaypath(ce->name, ud->prefix,
+                                                        ud->recursive_prefix);
        struct strbuf sb = STRBUF_INIT;
-       const char *displaypath = NULL;
        int needs_cloning = 0;
        int need_free_url = 0;
 
        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_addf(out, _("Skipping unmerged submodule %s"), displaypath);
                strbuf_addch(out, '\n');
                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;
@@ -2060,7 +2051,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 9a076e025f33f9377e1ccf4872d128b9ed0be7f2..6cc07460dd2a1f1f8b06010f06a5915ef88ce8eb 100755 (executable)
--- a/t/t7406-submodule-update.sh
+++ b/t/t7406-submodule-update.sh
@@ -1160,7 +1160,7 @@ test_expect_success 'submodule update should skip unmerged submodules' '
        test_config -C top-cloned submodule.middle.update !true &&
        git -C top-cloned submodule update --recursive 2>actual.err &&
        cat >expect.err <<-\EOF &&
-       Skipping unmerged submodule middle//bottom
+       Skipping unmerged submodule middle/bottom
        EOF
        test_cmp expect.err actual.err
 '
@@ -1173,7 +1173,7 @@ test_expect_success 'submodule update --recursive skip submodules with strategy=
        git -C top-cloned/middle config submodule.bottom.update none &&
        git -C top-cloned submodule update --recursive 2>actual.err &&
        cat >expect.err <<-\EOF &&
-       Skipping submodule '\''../middle/'\''
+       Skipping submodule '\''middle/bottom'\''
        EOF
        test_cmp expect.err actual.err
 '