@matheusbsbs / matheus-bagliano / commits / e0a862fdaf

submodule helper: convert relative URL to absolute URL if needed

The submodule helper update_clone called by "git submodule update", clones submodules if needed. As submodules used to have the URL indicating if they were active, the step to resolve relative URLs was done in the "submodule init" step. Nowadays submodules can be configured active without calling an explicit init, e.g. via configuring submodule.active. When trying to obtain submodules that are set active this way, we'll fallback to the URL found in the .gitmodules, which may be relative to the superproject, but we do not resolve it, yet: git clone https://gerrit.googlesource.com/gerrit cd gerrit && grep url .gitmodules url = ../plugins/codemirror-editor ... git config submodule.active . git submodule update fatal: repository '../plugins/codemirror-editor' does not exist fatal: clone of '../plugins/codemirror-editor' into submodule path '/tmp/gerrit/plugins/codemirror-editor' failed Failed to clone 'plugins/codemirror-editor'. Retry scheduled [...] fatal: clone of '../plugins/codemirror-editor' into submodule path '/tmp/gerrit/plugins/codemirror-editor' failed Failed to clone 'plugins/codemirror-editor' a second time, aborting [...] To resolve the issue, factor out the function that resolves the relative URLs in "git submodule init" (in the submodule helper in the init_submodule function) and call it at the appropriate place in the update_clone helper. Reported-by: Jaewoong Jung <jungjw@google.com> Signed-off-by: Stefan Beller <sbeller@google.com> Reviewed-by: Jonathan Nieder <jrnieder@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Stefan Beller committed Oct 16, 2018 at 10:27 UTC e0a862fdaf7628ceba4ea290b91ffd5d3c46b75b

2 files changed +58 -17

builtin/submodule--helper.c

+34 -17

@@ -584,6 +584,26 @@ static int module_foreach(int argc, const char **argv, const char *prefix)

584 return 0;

585 }

586

587 +static char *compute_submodule_clone_url(const char *rel_url)

588 +{

589 + char *remoteurl, *relurl;

590 + char *remote = get_default_remote();

591 + struct strbuf remotesb = STRBUF_INIT;

592 +

593 + strbuf_addf(&remotesb, "remote.%s.url", remote);

594 + if (git_config_get_string(remotesb.buf, &remoteurl)) {

595 + warning(_("could not look up configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);

596 + remoteurl = xgetcwd();

597 + }

598 + relurl = relative_url(remoteurl, rel_url, NULL);

599 +

600 + free(remote);

601 + free(remoteurl);

602 + strbuf_release(&remotesb);

603 +

604 + return relurl;

605 +}

606 +

607 struct init_cb {

608 const char *prefix;

609 unsigned int flags;

@@ -634,21 +654,9 @@ static void init_submodule(const char *path, const char *prefix,

654 /* Possibly a url relative to parent */

655 if (starts_with_dot_dot_slash(url) ||

656 starts_with_dot_slash(url)) {

637 - char *remoteurl, *relurl;

638 - char *remote = get_default_remote();

639 - struct strbuf remotesb = STRBUF_INIT;

640 - strbuf_addf(&remotesb, "remote.%s.url", remote);

641 - free(remote);

642 -

643 - if (git_config_get_string(remotesb.buf, &remoteurl)) {

644 - warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);

645 - remoteurl = xgetcwd();

646 - }

647 - relurl = relative_url(remoteurl, url, NULL);

648 - strbuf_release(&remotesb);

649 - free(remoteurl);

650 - free(url);

651 - url = relurl;

657 + char *oldurl = url;

658 + url = compute_submodule_clone_url(oldurl);

659 + free(oldurl);

660 }

661

662 if (git_config_set_gently(sb.buf, url))

@@ -1515,6 +1523,7 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,

1523 struct strbuf sb = STRBUF_INIT;

1524 const char *displaypath = NULL;

1525 int needs_cloning = 0;

1526 + int need_free_url = 0;

1527

1528 if (ce_stage(ce)) {

1529 if (suc->recursive_prefix)

@@ -1563,8 +1572,14 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,

1572

1573 strbuf_reset(&sb);

1574 strbuf_addf(&sb, "submodule.%s.url", sub->name);

1566 - if (repo_config_get_string_const(the_repository, sb.buf, &url))

1567 - url = sub->url;

1575 + if (repo_config_get_string_const(the_repository, sb.buf, &url)) {

1576 + if (starts_with_dot_slash(sub->url) ||

1577 + starts_with_dot_dot_slash(sub->url)) {

1578 + url = compute_submodule_clone_url(sub->url);

1579 + need_free_url = 1;

1580 + } else

1581 + url = sub->url;

1582 + }

1583

1584 strbuf_reset(&sb);

1585 strbuf_addf(&sb, "%s/.git", ce->name);

@@ -1609,6 +1624,8 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce,

1624 cleanup:

1625 strbuf_reset(&displaypath_sb);

1626 strbuf_reset(&sb);

1627 + if (need_free_url)

1628 + free((void*)url);

1629

1630 return needs_cloning;

1631 }

t/t7400-submodule-basic.sh

+24

@@ -1224,6 +1224,30 @@ test_expect_success 'submodule update and setting submodule.<name>.active' '

1224 test_cmp expect actual

1225 '

1226

1227 +test_expect_success 'clone active submodule without submodule url set' '

1228 + test_when_finished "rm -rf test/test" &&

1229 + mkdir test &&

1230 + # another dir breaks accidental relative paths still being correct

1231 + git clone file://"$pwd"/multisuper test/test &&

1232 + (

1233 + cd test/test &&

1234 + git config submodule.active "." &&

1235 +

1236 + # do not pass --init flag, as the submodule is already active:

1237 + git submodule update &&

1238 + git submodule status >actual_raw &&

1239 +

1240 + cut -c 1,43- actual_raw >actual &&

1241 + cat >expect <<-\EOF &&

1242 + sub0 (test2)

1243 + sub1 (test2)

1244 + sub2 (test2)

1245 + sub3 (test2)

1246 + EOF

1247 + test_cmp expect actual

1248 + )

1249 +'

1250 +

1251 test_expect_success 'clone --recurse-submodules with a pathspec works' '

1252 test_when_finished "rm -rf multisuper_clone" &&

1253 cat >expected <<-\EOF &&