On Fri, Apr 19, 2024 at 01:09:39PM -0500, Karthik Nayak wrote:
> 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>
> > [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.

Should they really take precedence, or should it be forbidden to pass
both old target and old object ID or new target and new object ID,
respectively? I'd rather claim the latter, and that should be detected
such that there is no bad surprises here.

Patrick