Re: [PATCH 1/4] submodule-config: add submodules_of_tree() helper

From: Jonathan Tan <jonathantanmy@google.com>
Date: Nov 23, 2021, 02:12 UTC
Message-ID: <20211123021223.3472472-1-jonathantanmy@google.com>
In-Reply-To: <20211122223252.19922-2-chooglen@google.com>

Glen Choo <chooglen@google.com> writes:

Show 10 quoted lines

> +struct submodule_entry_list *
> +submodules_of_tree(struct repository *r, const struct object_id *treeish_name)
> +{
> +	struct tree_desc tree;
> +	struct name_entry entry;
> +	struct submodule_entry_list *ret;
> +
> +	CALLOC_ARRAY(ret, 1);
> +	fill_tree_descriptor(r, &tree, treeish_name);
> +	while (tree_entry(&tree, &entry)) {

I think that tree_entry() doesn't recurse into subtrees, but in any case we should test this. (I looked at patch 4 and I think that the submodules are always in the root tree.)

This reminded me of a similar thing when fetching submodules recursively and we needed the "before" and "after" of submodule gitlinks. You can look at the code (collect_changed_submodules_cb() and the functions that use it in submodule.c) but it may not be useful - in particular, that uses diff since we need to see differences there, but we don't need that here.

I'll review the other patches tomorrow.