Re: [PATCH v3 12/12] hash: stop depending on `the_repository` in `null_oid()`

[Junio C Hamano <gitster@pobox.com>]

Elijah Newren <newren@gmail.com> writes:

>> @@ -4304,7 +4304,7 @@ static int process_entry(struct merge_options *opt,
>>                 /* Deleted on both sides */
>>                 ci->merged.is_null = 1;
>>                 ci->merged.result.mode = 0;
>> -               oidcpy(&ci->merged.result.oid, null_oid());
>> +               oidcpy(&ci->merged.result.oid, null_oid(the_hash_algo));
>>                 assert(!ci->df_conflict);
>>                 ci->merged.clean = !ci->path_conflict;
>>         }
>
> What you have is an improvement since it's at least making things
> explicit, but these should really be opt->repo->hash_algo.

I think this is following the typical pattern in recent topics of
this sort, i.e. first make things explicit and use "the" singleton
instance, with the plan to find if there are instances more
appropriate in the context.  When we passed a repo instance through
the call chain, the caller at the root may have passed the_repository
down the call chain, to make sure everybody down below will be
bug-to-bug compatible (because they either used the_repository or
used a helper that implicitly always used the_repository).  This is
doing the same, and your "but these should really be" belongs to the
"with the plan to find" phase, which may come later but outside the
scope of this series.

Incidentally, that is the reason why the use of superproject
repository instead of the_repository was singled out in the proposed
commit log message.  It should have been mechanical replacement to
make this step mechanical and easier to audit, but with another
topic in flight that semantically conflicts the changes in ths
patch, it took an exception.