Re: [PATCH 6/8] git submodule update: have a dedicated helper for cloning
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 3, 2016, 23:24 UTC
- Message-ID
- <xmqqr3gtjs23.fsf@gitster.mtv.corp.google.com>
- In-Reply-To
- <1454435497-26429-7-git-send-email-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
Show 8 quoted lines
> + if (ce_stage(ce)) {
> + if (pp->recursive_prefix)
> + strbuf_addf(err, "Skipping unmerged submodule %s/%s\n",
> + pp->recursive_prefix, ce->name);
> + else
> + strbuf_addf(err, "Skipping unmerged submodule %s\n",
> + ce->name);
> + continue;This raised my eyebrow somewhat, until I realized that it is OK because module_list prepared by the caller has only one of the unmerged entries for the same path to avoid duplicates.
Show 8 quoted lines
> + }
> +
> + sub = submodule_from_path(null_sha1, ce->name);
> + if (!sub) {
> + strbuf_addf(err, "BUG: internal error managing submodules. "
> + "The cache could not locate '%s'", ce->name);
> + pp->print_unmatched = 1;
> + continue;Interesting.
I am wondering if this check should go to module_list_compute().
Show 24 quoted lines
> + }
> +
> + if (pp->recursive_prefix)
> + displaypath = relative_path(pp->recursive_prefix, ce->name, &sb);
> + else
> + displaypath = ce->name;
> +
> + if (pp->update)
> + update_module = pp->update;
> + if (!update_module)
> + update_module = sub->update;
> + if (!update_module)
> + update_module = "checkout";
> + if (!strcmp(update_module, "none")) {
> + strbuf_addf(err, "Skipping submodule '%s'\n", displaypath);
> + continue;
> + }
> +
> + /*
> + * Looking up the url in .git/config.
> + * We must not fall back to .gitmodules as we only want to process
> + * configured submodules.
> + */
> + strbuf_reset(&sb);Doesn't this invalidate displaypath, when pp->recursive_prefix is in effect? It still seems to be used to create an error message just a few lines below...
Show 12 quoted lines
> + strbuf_addf(&sb, "submodule.%s.url", sub->name);
> + git_config_get_string(sb.buf, &url);
> + if (!url) {
> + /*
> + * Only mention uninitialized submodules when its
> + * path have been specified
> + */
> + if (pp->pathspec.nr)
> + strbuf_addf(err, _("Submodule path '%s' not initialized\n"
> + "Maybe you want to use 'update --init'?"), displaypath);
> + continue;
> + }