Christian Couder [off-list ref] writes:

On Fri, Dec 13, 2024 at 11:36 AM Karthik Nayak [off-list ref] wrote:

quoted

+int ref_transaction_update_reflog(struct ref_transaction *transaction,
+                                 const char *refname,
+                                 const struct object_id *new_oid,
+                                 const struct object_id *old_oid,
+                                 const char *committer_info, unsigned int flags,
+                                 const char *msg, unsigned int index,
+                                 struct strbuf *err)
+{
+       struct ref_update *update;
+
+       assert(err);
+
+       if (!transaction_refname_valid(refname, new_oid, flags, 1, err))
+               return -1;
+
+       flags |= REF_LOG_ONLY | REF_NO_DEREF;

If we could switch the above lines like this:

      flags |= REF_LOG_ONLY | REF_NO_DEREF;

      if (!transaction_refname_valid(refname, new_oid, flags, 1, err))
               return -1;

maybe we wouldn't need transaction_refname_valid() to take an
'unsigned int reflog' argument and we could instead use 'flags &
REF_LOG_ONLY' inside that function?

The issue is the that this changes existing behavior, since
`ref_transaction_update()` can also be called with the `REF_LOG_ONLY`
flag set.

But, I think it is a worthwhile change, because earlier even for reflog
updates we would show 'refusing to update ref ...' as the message. I'll
add this in and also make a note in the commit message.

Thanks

quoted

+       update = ref_transaction_add_update(transaction, refname, flags,
+                                           new_oid, old_oid, NULL, NULL,
+                                           committer_info, msg);
+       /*
+        * While we do set the old_oid value, we unset the flag to skip
+        * old_oid verification which only makes sense for refs.
+        */
+       update->flags &= ~REF_HAVE_OLD;
+       update->index = index;
+
        return 0;
 }