Re: [PATCH 1/2] unpack-trees: lose USE_THE_REPOSITORY

[Patrick Steinhardt <ps@pks.im>]

On Mon, Nov 09, 2026 at 11:30:06AM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> >> @@ -345,15 +394,16 @@ static void load_gitmodules_file(struct index_state *index,
> >>  				 struct checkout *state)
> >>  {
> >>  	int pos = index_name_pos(index, GITMODULES_FILE, strlen(GITMODULES_FILE));
> >> +	struct repository *repo = unpack_repo(index, NULL);
> >
> > This conversion here raises the question what we want to do in the case
> > where the caller has both structures available, but we don't end up
> > passing them to the pointer. This could in theory hide a bug where we
> > have mismatching repositories when the entry points don't verify that
> > property.
> 
> Puzzled.  Here we do have an istate but have no unpack_trees_options
> available, so it is natural to pass NULL for the latter to tell the
> validator "we do not have unpack_trees_options".

It is natural, but the calling context _does_ have options available.
It's just that the caller of this function here decided to not pass it
down because we previously didn't need it. In fact, every new callsite
of `unpack_repo()` does have options available in their call stack.

> Are you worried about a case where a programmer who writes an
> unpack_repo() call passes NULL by mistake efven when istate or
> unpack_trees_options are available?

All I'm pointing out is that there are cases where we could be as
defensive as everywhere else if we started passing down the options to
calling contexts that don't have it yet. I wouldn't say I'm "worried"
though. The proposed patch is very defensive overall, and I'm not overly
concerned about us missing that defense in some cases.

Patrick