Subject: Re: [PATCH v2 4/9] refs: teach refs_for_each_ref() arbitrary repos

From: Junio C Hamano <[email-masked]>
Date: Tue, 28 Sep 2021 15:49:51 -0700

Jonathan Tan <[email-masked]> writes:

>  	} else if (!(flags & OPT_CACHED)) {
>  		struct object_id oid;
> -		struct ref_store *refs = get_submodule_ref_store(path);
> +		struct repository subrepo;
>  
> -		if (!refs) {
> +		if (repo_submodule_init(&subrepo, the_repository, path, null_oid())) {
>  			print_status(flags, '-', path, ce_oid, displaypath);
>  			goto cleanup;
>  		}
> -		if (refs_head_ref(refs, handle_submodule_head_ref, &oid))
> +		if (refs_head_ref(&subrepo, handle_submodule_head_ref, &oid))
>  			die(_("could not resolve HEAD ref inside the "
>  			      "submodule '%s'"), path);
> +		repo_clear(&subrepo);

While this makes perfect sense, if we extended the ref_store to know
what repository it belongs to, I suspect that we don't have to
change anything in a "user" codepath like this one.
get_submodule_ref_store() would repare a ref store that is bound to
the submodule repository, refs_head_ref() and other helpers that
take a ref_store would not have to gain an extra "repository"
parameter (because it is known via the ref_store) and does the
iteration in the right repository, etc...

> @@ -1018,9 +1019,12 @@ static void generate_submodule_summary(struct summary_cb *info,
>  
>  	if (!info->cached && oideq(&p->oid_dst, null_oid())) {
>  		if (S_ISGITLINK(p->mod_dst)) {
> -			struct ref_store *refs = get_submodule_ref_store(p->sm_path);
> -			if (refs)
> -				refs_head_ref(refs, handle_submodule_head_ref, &p->oid_dst);
> +			struct repository subrepo;
> +
> +			if (!repo_submodule_init(&subrepo, the_repository, p->sm_path, null_oid())) {
> +				refs_head_ref(&subrepo, handle_submodule_head_ref, &p->oid_dst);
> +				repo_clear(&subrepo);
> +			}

The story looks the same here, too.