Thread (79 messages) 79 messages, 4 authors, 2024-12-20

Re: [PATCH 5/7] refs: introduce the `ref_transaction_update_reflog` function

From: karthik nayak <hidden>
Date: 2024-12-11 18:06:35

Christian Couder [off-list ref] writes:

[snip]
quoted
diff --git a/refs.c b/refs.c
index 732c236a3fd0cf324cc172b48d3d54f6dbadf4a4..602a65873181a90751def525608a7fa7bea59562 100644
--- a/refs.c
+++ b/refs.c
@@ -1160,13 +1160,15 @@ void ref_transaction_free(struct ref_transaction *transaction)
        free(transaction);
 }

-struct ref_update *ref_transaction_add_update(
-               struct ref_transaction *transaction,
-               const char *refname, unsigned int flags,
-               const struct object_id *new_oid,
-               const struct object_id *old_oid,
-               const char *new_target, const char *old_target,
-               const char *msg)
+struct ref_update *ref_transaction_add_update(struct ref_transaction *transaction,
+                                             const char *refname,
+                                             unsigned int flags,
+                                             const struct object_id *new_oid,
+                                             const struct object_id *old_oid,
+                                             const char *new_target,
+                                             const char *old_target,
+                                             const char *committer_info,
This change (adding a 'const char *committer_info' argument to
ref_transaction_add_update()) is not described in the commit message
and it requires a number of changes to the callers of this function,
so I think it might want to be in its own preparatory commit before
this one.
I think this is a great suggestion, it would reduce the congnitive load
of the commit and make it easier to review. Will do.
quoted
+                                             const char *msg)
 {
        struct ref_update *update;
@@ -1190,8 +1192,15 @@ struct ref_update *ref_transaction_add_update(
                oidcpy(&update->new_oid, new_oid);
        if ((flags & REF_HAVE_OLD) && old_oid)
                oidcpy(&update->old_oid, old_oid);
-       if (!(flags & REF_SKIP_CREATE_REFLOG))
+       if (!(flags & REF_SKIP_CREATE_REFLOG)) {
+               if (committer_info) {
+                       struct strbuf sb = STRBUF_INIT;
+                       strbuf_addstr(&sb, committer_info);
+                       update->committer_info = strbuf_detach(&sb, NULL);
Maybe:
                      update->committer_info = xstrdup(committer_info);
Indeed, I thought there was a better way. This is what I needed to have done.
quoted
+               }
+
                update->msg = normalize_reflog_message(msg);
+       }

        return update;
 }
@@ -1199,20 +1208,29 @@ struct ref_update *ref_transaction_add_update(
 static int transaction_refname_verification(const char *refname,
                                            const struct object_id *new_oid,
                                            unsigned int flags,
+                                           unsigned int reflog,
                                            struct strbuf *err)
 {
        if (flags & REF_SKIP_REFNAME_VERIFICATION)
                return 0;

        if (is_pseudo_ref(refname)) {
-               strbuf_addf(err, _("refusing to update pseudoref '%s'"),
-                           refname);
+               if (reflog)
+                       strbuf_addf(err, _("refusing to update reflog for pseudoref '%s'"),
+                                   refname);
+               else
+                       strbuf_addf(err, _("refusing to update pseudoref '%s'"),
+                                   refname);
Maybe:

              const char *what = reflog ? "reflog for pseudoref" : "pseudoref";
              strbuf_addf(err, _("refusing to update %s '%s'"), what, refname);
Much nicer, will add.
quoted
                return -1;
        } else if ((new_oid && !is_null_oid(new_oid)) ?
                 check_refname_format(refname, REFNAME_ALLOW_ONELEVEL) :
                 !refname_is_safe(refname)) {
-               strbuf_addf(err, _("refusing to update ref with bad name '%s'"),
-                           refname);
+               if (reflog)
+                       strbuf_addf(err, _("refusing to update reflog with bad name '%s'"),
+                                   refname);
+               else
+                       strbuf_addf(err, _("refusing to update ref with bad name '%s'"),
+                                   refname);
Maybe:

              const char *what = reflog ? "reflog with bad name" :
"ref with bad name";
              strbuf_addf(err, _("refusing to update %s '%s'"), what, refname);
Similar, will do.
quoted
                return -1;
        }
[...]
quoted
 int ref_transaction_create(struct ref_transaction *transaction,
-                          const char *refname,
-                          const struct object_id *new_oid,
-                          const char *new_target,
-                          unsigned int flags, const char *msg,
-                          struct strbuf *err)
+                          const char *refname, const struct object_id *new_oid,
+                          const char *new_target, unsigned int flags,
+                          const char *msg, struct strbuf *err)
This looks like a wrapping or indenting only change. If you really
want to do it, it should probably be in its own preparatory commit.
I think it was the auto linter, will remove.
quoted
index a5bedf48cf6de91005a7e8d0bf58ca98350397a6..b86d2cd87be33f7bb1b31fce711d6c7c8d9491c9 100644
--- a/refs.h
+++ b/refs.h
@@ -727,6 +727,18 @@ int ref_transaction_update(struct ref_transaction *transaction,
                           unsigned int flags, const char *msg,
                           struct strbuf *err);

+/*
+ * Similar to `ref_transaction_update`, but this function is only for adding
+ * a reflog updates.
"a reflog update" or "reflog updates".
Makes sense. Thanks.

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help