On Mon, Dec 9, 2024 at 12:11 PM Karthik Nayak <karthik.188@xxxxxxxxx> wrote: > > Introduce a new function `ref_transaction_update_reflog`, for clients to > add a reflog update to a transaction. While the existing function > `ref_transaction_update` also allows clients to add a reflog entry, this > function does a few things more, It: > - Enforces that only a reflog entry is added and does not update the > ref itself. > - Allows the users to also provide the committer information. This > means clients can add reflog entries with custom committer > information. > A follow up commit will utilize this function to add reflog support to > `git refs migrate`. > > Signed-off-by: Karthik Nayak <karthik.188@xxxxxxxxx> > --- > refs.c | 89 +++++++++++++++++++++++++++++++++++++------------ > refs.h | 12 +++++++ > refs/files-backend.c | 48 +++++++++++++++----------- > refs/refs-internal.h | 16 +++++---- > refs/reftable-backend.c | 6 ++-- > 5 files changed, 122 insertions(+), 49 deletions(-) > > 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. > + 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); > + } > + > 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); > 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); > return -1; > } [...] > 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. > 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".