Subject: Re: [PATCH v2 08/20] hash: require hash algorithm in `empty_tree_oid_hex()`

From: Patrick Steinhardt <[email-masked]>
Date: Fri, 14 Jun 2024 07:23:10 +0200

On Thu, Jun 13, 2024 at 08:39:18AM -0700, Junio C Hamano wrote:
> Phillip Wood <[email-masked]> writes:
> > diff --git a/sequencer.c b/sequencer.c
> >> index 68d62a12ff..823691e379 100644
> >> --- a/sequencer.c
> >> +++ b/sequencer.c
> >> @@ -2263,7 +2263,7 @@ static int do_pick_commit(struct repository *r,
> >>   			unborn = 1;
> >>   		} else if (unborn)
> >>   			oidcpy(&head, the_hash_algo->empty_tree);
> >> -		if (index_differs_from(r, unborn ? empty_tree_oid_hex() : "HEAD",
> >> +		if (index_differs_from(r, unborn ? empty_tree_oid_hex(the_repository->hash_algo) : "HEAD",
> >
> > The hunk fragment shows that we already have a struct repository
> > instance in "r" which we should use instead of "the_repository" here.
> 
> Yes, but the same "it is better to make a faithful conversion first,
> corrections separately in a later step" would apply.

Yeah, this is what I'm aiming for. In large patch series like this I
think it increases the risk of regression quite significantly if we also
try to deviate from the preceding code and do the "right" thing, even if
it is seemingly obvious. So I rather do a faithful conversion that does
not change the behaviour and leave conversion away from `the_repository`
to later steps after this series.

I can amend the commit message to say so.

Patrick