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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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".





[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux