From: Junio C Hamano <gitster@pobox.com>
Date: Tue, 19 May 2026 05:56:51 GMT
Subject: Re: [PATCH] commit: fall back to full read when maybe_tree is NULL
Message-ID: <xmqqcxys7xi4.fsf@gitster.g>
In-Reply-To: <20260519050513.GA1635924@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

> It also means we have to reimplement a bit of the commit parsing. We
> can't just use parse_commit_buffer() here, because it expects an
> unparsed struct and wants to load everything, including parent links.
> But we don't know if the parent list has been munged during traversal,
> so it's not safe for us to touch it. Fortunately, it's quite easy to
> load just the tree, as it is always the first line of the commit object.

I was hoping that existing code to parse out the tree in
parse_commit_buffer() will become a call into this new helper
function, so that we avoid duplicating the logic.

> Moreover, this strategy does nothing if we lose access to the graph file
> unexpectedly (e.g., due to a system error).

Or simultaneous repack may lose the file from the filesystem,
perhaps?

> +static void load_tree_from_commit_contents(struct repository *r, struct commit *commit)
> +{
> +	enum object_type type;
> +	unsigned long size;
> +	char *buf;
> +	const char *p;
> +	struct object_id tree_oid;
> +
> +	buf = odb_read_object(r->objects, &commit->object.oid, &type, &size);
> +	if (!buf)
> +		return;
> +
> +	if (type == OBJ_COMMIT &&
> +	    skip_prefix(buf, "tree ", &p) &&
> +	    !parse_oid_hex(p, &tree_oid, &p) &&
> +	    *p == '\n')
> +		set_commit_tree(commit, lookup_tree(r, &tree_oid));
> +
> +	free(buf);
> +}

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?

Thanks.