Patrick Steinhardt writes: > On Fri, Apr 19, 2024 at 01:09:39PM -0500, Karthik Nayak wrote: >> Patrick Steinhardt writes: >> >> > On Fri, Apr 12, 2024 at 11:59:02AM +0200, Karthik Nayak wrote: >> >> From: Karthik Nayak >> > [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 I think after that email, I agreed to Phillip's suggestion [1] and now I've added an explicit check for this. [1]: https://lore.kernel.org/git/CAOLa=ZSwtOQXwbgregzKMtVA30wUCH8R=8D7u1+KGnsGEDD1oA@mail.gmail.com/