Re: [PATCH 17/19] submodule: use submodule repos for object lookup
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Oct 11, 2018, 22:40 UTC
- Message-ID
- <20181011224052.191281-1-jonathantanmy@google.com>
- In-Reply-To
- <20181011211754.31369-18-sbeller@google.com>
Show 6 quoted lines
> +/* > + * 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).
Show 10 quoted lines
> +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?
Show 6 quoted lines
> @@ -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.