Subject: Re: [PATCH v5 7/7] builtin/clone: teach git-clone(1) the --revision= option

From: Toon Claes <[email-masked]>
Date: Wed, 05 Feb 2025 17:43:17 +0100

Patrick Steinhardt <[email-masked]> writes:

> On Tue, Feb 04, 2025 at 10:34:06PM +0100, Toon Claes wrote:
>> @@ -580,8 +581,9 @@ static void update_head(const struct ref *our, const struct ref *remote,
>>  			install_branch_config(0, head, remote_name, our->name);
>>  		}
>>  	} else if (our) {
>> -		struct commit *c = lookup_commit_reference(the_repository,
>> -							   &our->old_oid);
>> +		struct commit *c = lookup_commit_or_die(&our->old_oid,
>> +						        our->name);
>> +
>>  		/* --branch specifies a non-branch (i.e. tags), detach HEAD */
>>  		refs_update_ref(get_main_ref_store(the_repository), msg,
>>  				"HEAD", &c->object.oid, NULL, REF_NO_DEREF,
>
> I wonder: is this fixing a potential segfault? If so, it might make
> sense to split this out into a separate commit and provide a test that
> demonstrates the issue. If it doesn't, then the change shouldn't be
> necessary, unless I misunderstand.

This potential segfault would occur for the added test case:
'clone with --revision being a tree hash'

So I can't really split out this change in a separate commit, but the
change is still needed.

-- 
Toon