@matheusbsbs / matheus-bagliano / commits / 618b8445d9

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>

Glen Choo committed Jun 30, 2022 at 19:11 UTC 618b8445d92c60d9e1e4609e7c56a941bb3862b9

2 files changed +7 -16

builtin/submodule--helper.c

+5 -14

@@ -1947,30 +1947,21 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,

1947 const char *update_string;

1948 enum submodule_update_type update_type;

1949 char *key;

1950 - struct strbuf displaypath_sb = STRBUF_INIT;

1950 + struct update_data *ud = suc->update_data;

1951 + char *displaypath = do_get_submodule_displaypath(ce->name, ud->prefix,

1952 + ud->recursive_prefix);

1953 struct strbuf sb = STRBUF_INIT;

1952 - const char *displaypath = NULL;

1954 int needs_cloning = 0;

1955 int need_free_url = 0;

1956

1957 if (ce_stage(ce)) {

1957 - if (suc->update_data->recursive_prefix)

1958 - strbuf_addf(&sb, "%s/%s", suc->update_data->recursive_prefix, ce->name);

1959 - else

1960 - strbuf_addstr(&sb, ce->name);

1961 - strbuf_addf(out, _("Skipping unmerged submodule %s"), sb.buf);

1958 + strbuf_addf(out, _("Skipping unmerged submodule %s"), displaypath);

1959 strbuf_addch(out, '\n');

1960 goto cleanup;

1961 }

1962

1963 sub = submodule_from_path(the_repository, null_oid(), ce->name);

1964

1968 - if (suc->update_data->recursive_prefix)

1969 - displaypath = relative_path(suc->update_data->recursive_prefix,

1970 - ce->name, &displaypath_sb);

1971 - else

1972 - displaypath = ce->name;

1973 -

1965 if (!sub) {

1966 next_submodule_warn_missing(suc, out, displaypath);

1967 goto cleanup;

@@ -2060,7 +2051,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,

2051 "--no-single-branch");

2052

2053 cleanup:

2063 - strbuf_release(&displaypath_sb);

2054 + free(displaypath);

2055 strbuf_release(&sb);

2056 if (need_free_url)

2057 free((void*)url);

t/t7406-submodule-update.sh

+2 -2

@@ -1160,7 +1160,7 @@ test_expect_success 'submodule update should skip unmerged submodules' '

1160 test_config -C top-cloned submodule.middle.update !true &&

1161 git -C top-cloned submodule update --recursive 2>actual.err &&

1162 cat >expect.err <<-\EOF &&

1163 - Skipping unmerged submodule middle//bottom

1163 + Skipping unmerged submodule middle/bottom

1164 EOF

1165 test_cmp expect.err actual.err

1166 '

@@ -1173,7 +1173,7 @@ test_expect_success 'submodule update --recursive skip submodules with strategy=

1173 git -C top-cloned/middle config submodule.bottom.update none &&

1174 git -C top-cloned submodule update --recursive 2>actual.err &&

1175 cat >expect.err <<-\EOF &&

1176 - Skipping submodule '\''../middle/'\''

1176 + Skipping submodule '\''middle/bottom'\''

1177 EOF

1178 test_cmp expect.err actual.err

1179 '