Patrick Steinhardt <ps@pks.im> writes:

> On Fri, Apr 12, 2024 at 11:59:02AM +0200, Karthik Nayak wrote:
>> From: Karthik Nayak <karthik.188@gmail.com>
>>
>> The `ref_transaction[_add]_update` functions obtain ref information and
>> flags to create a `ref_update` and add it to the transaction at hand.
>>
>> To extend symref support in transactions, we need to also accept the
>> old and new ref values and process it. In this commit, let's add the
>> required paramaters to the function and modify all call sites.
>>
>> The two paramaters added are `new_ref` and `old_ref`. The `new_ref` is
>
> Would `new_target` and `old_target` be easier to understand? `new_ref`
> and `old_ref` to me sound as if they might also apply to the ref itself,
> for example when doing a rename.
>

I guess that makes sense, I'll add it.

> [snip]
>> diff --git a/refs/refs-internal.h b/refs/refs-internal.h
>> index 56641aa57a..4c5fe02687 100644
>> --- a/refs/refs-internal.h
>> +++ b/refs/refs-internal.h
>> @@ -124,6 +124,19 @@ struct ref_update {
>>  	 */
>>  	struct object_id old_oid;
>>
>> +	/*
>> +	 * If (flags & REF_SYMREF_UPDATE), set the reference to this
>> +	 * value (or delete it, if `new_ref` is an empty string).
>> +	 */
>> +	const char *new_ref;
>> +
>> +	/*
>> +	 * If (type & REF_SYMREF_UPDATE), check that the reference
>> +	 * previously had this value (or didn't previously exist,
>> +	 * if `old_ref` is an empty string).
>> +	 */
>> +	const char *old_ref;
>
> I think one important bit of information here would be how to handle the
> update from a plain ref to a symref or vice versa. Would I set both
> `REF_SYMREF_UPDATE` and `REF_HAVE_NEW`/`REF_HAVE_OLD`?

I'll now remove `REF_SYMREF_UPDATE` and add documentation around the
usage on `new_target` and `old_target`, where if either of them are set,
they take precedence over `old_oid` and `new_oid`. The `new_target` will
ensure that the ref is now a symbolic ref which points to the
`new_target` value. While the `old_target` will treat the ref as a
symbolic ref and check its old value.

`REF_HAVE_NEW`/`REF_HAVE_OLD` should however never be set by users of
ref.c, this is set internally. See REF_TRANSACTION_UPDATE_ALLOWED_FLAGS.