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

From: Junio C Hamano <gitster@pobox.com>
Date: Sep 28, 2021, 22:49 UTC
Message-ID: <xmqqh7e4iacw.fsf@gitster.g>
In-Reply-To: <da0c9c2d44da40e25c2c5b68697e3461cefd63a5.1632859148.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

Show 15 quoted lines

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

Show 13 quoted lines

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