Re: [PATCH] commit: fall back to full read when maybe_tree is NULL
- From
Derrick Stolee <stolee@gmail.com>
- Date
- May 20, 2026, 16:22 UTC
- Message-ID
- <478ff417-d5d0-458f-b5cd-472373eed7b2@gmail.com>
- In-Reply-To
- <20260519061534.GA1709881@coredump.intra.peff.net>
On 5/19/2026 2:15 AM, Jeff King wrote:
> On Tue, May 19, 2026 at 02:56:51PM +0900, Junio C Hamano wrote:
Show 28 quoted lines
>> Looks quite straight-forward. Don't you need to pay attention to >> r->hash_algo and call parse_oid_hex_algop() instead? >> >> Or are we pretty much sure that "r" is always "the_repository" here, >> in which case parse_oid_hex() that uses "the_hash_algo" would be >> sufficient? > > No, I didn't even think about it, since the use of the_hash_algo is > hidden behind the function. We definitely should use the hash algo from > "r", since we have access to it. I'm not even sure if you can have repos > of two different hashes loaded in the same process at this point, but > certainly it is the correct long-term direction. > > Here's a re-roll with the one-line fixup: > > diff --git a/commit.c b/commit.c > index cfc87ad185..499a9602ad 100644 > --- a/commit.c > +++ b/commit.c > @@ -448,7 +448,7 @@ static void load_tree_from_commit_contents(struct repository *r, struct commit * > > if (type == OBJ_COMMIT && > skip_prefix(buf, "tree ", &p) && > - !parse_oid_hex(p, &tree_oid, &p) && > + !parse_oid_hex_algop(p, &tree_oid, &p, r->hash_algo) && > *p == '\n') > set_commit_tree(commit, lookup_tree(r, &tree_oid)); >
I figured that this was already tested via the test variable that runs the test with SHA256, but the multi-repo case is an interesting one that I'm sure would catch us at some point in the future.
I'm happy with the re-roll here.
Thanks, -Stolee