Re: [PATCH v2 6/7] update-ref: add support for symref-update

From: Karthik Nayak <karthik.188@gmail.com>
Date: Apr 21, 2024, 19:00 UTC
Message-ID: <CAOLa=ZTA94HModfa40So7Uk7WAG99G+XabvKfOY93RCL6Xac5Q@mail.gmail.com>
In-Reply-To: <ZiI8IFdfBHEqr02C@tanuki>

Patrick Steinhardt <ps@pks.im> writes: [snip]

Show 18 quoted lines

>> +	/*
>> +	 * Since the user can also send in an old-oid, we try to parse
>> +	 * it as such too.
>> +	 */
>> +	if (old_ref && read_ref(old_ref, NULL)) {
>> +		if (!repo_get_oid(the_repository, old_ref, &old_oid)) {
>> +			old_ref = NULL;
>> +			have_old = 1;
>> +		} else
>> +			die("symref-update %s: invalid <old-ref> or <old-oid>", refname);
>> +	}
>
> So we first try to parse it as a ref, and then as an object ID? Wouldn't
> it preferable to try it the other way round and first check whether it
> is a valid object ID? That would likely be cheaper, even though it may
> be premature optimization.
>
> Patrick

I think the issue is `repo_get_oid` would also parse a refname to an OID. Whereas we want to first check and keep refnames and only if it isn't a refname, we'd want to parse it as an OID.

Also, why do you say it is cheaper?