From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 19 Apr 2024 09:40:17 GMT
Subject: Re: [PATCH v2 1/7] refs: accept symref values in `ref_transaction[_add]_update`
Message-ID: <ZiI8AfkJL4O3K7Xk@tanuki>
In-Reply-To: <20240412095908.1134387-2-knayak@gitlab.com>

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.

[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`?

Patrick