> +/*
> + * Initialize 'out' based on the provided submodule path.
> + *
> + * Unlike repo_submodule_init, this tolerates submodules not present
> + * in .gitmodules. NEEDSWORK: The repo_submodule_init behavior is
> + * preferrable. This function exists only to preserve historical behavior.
What do you mean by "The repo_submodule_init behavior is preferable"? If
we need to preserve historical behavior, then it seems that the most
preferable one is something that meets our needs (like open_submodule()
in this patch).
> +static int open_submodule(struct repository *out, const char *path)
> +{
> + struct strbuf sb = STRBUF_INIT;
> +
> + if (submodule_to_gitdir(&sb, path) || repo_init(out, sb.buf, NULL)) {
> + strbuf_release(&sb);
> + return -1;
> + }
> +
> + out->submodule_prefix = xstrdup(path);
Do we need to set submodule_prefix?
> @@ -507,7 +533,7 @@ static void show_submodule_header(struct diff_options *o,
> const char *path,
> else if (is_null_oid(two))
> message = "(submodule deleted)";
>
> - if (add_submodule_odb(path)) {
> + if (open_submodule(sub, path) < 0) {
This function, as a side effect, writes the open repository to "sub" for
its caller to use. I think it's better if its callers open "sub" and
then pass it to show_submodule_header() if successful. If opening the
submodule is expected to fail sometimes (like it seems here), then we
can allow callers to also pass NULL, and document what happens when NULL
is passed.
Also, repositories open in this way should also be freed.
