Stefan Beller <[email protected]> writes:

> +             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.

> +             }
> +
> +             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().

> +             }
> +
> +             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...

> +             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;
> +             }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to