On Fri, Dec 13, 2024 at 11:36:53AM +0100, Karthik Nayak wrote:
> diff --git a/refs.c b/refs.c
> index 9f539369bc94a25594adc3e95847f2fe72f58a08..f19292d50f0003881220e8f7cfcf6c7eb4b2e749 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -2708,6 +2710,53 @@ static int migrate_one_ref(const char *refname, const char *referent UNUSED, con
> return ret;
> }
>
> +struct reflog_migration_data {
> + unsigned int *index;
> + const char *refname;
> + struct ref_store *old_refs;
> + struct ref_transaction *transaction;
> + struct strbuf *errbuf;
> +};
> +
> +static int migrate_one_reflog_entry(struct object_id *old_oid,
> + struct object_id *new_oid,
> + const char *committer,
> + timestamp_t timestamp, int tz,
> + const char *msg, void *cb_data)
> +{
> + struct reflog_migration_data *data = cb_data;
> + struct strbuf sb = STRBUF_INIT;
> + const char *date;
> + int ret;
> +
> + date = show_date(timestamp, tz, DATE_MODE(NORMAL));
> + /* committer contains name and email */
> + strbuf_addstr(&sb, fmt_ident("", committer, WANT_BLANK_IDENT, date, 0));
> +
> + ret = ref_transaction_update_reflog(data->transaction, data->refname,
> + new_oid, old_oid, sb.buf,
> + REF_HAVE_NEW | REF_HAVE_OLD, msg,
> + (*data->index)++, data->errbuf);
This is where we now increment the reflog index to ensure a proper
ordering.
> + strbuf_release(&sb);
> +
> + return ret;
> +}
We're now allocating one buffer per reflog entry. We may want to
optimize this by having a scratch buffer in `migration_data`, which we
could then pass on via `reflog_migration_data`.
> @@ -2910,6 +2940,11 @@ int repo_migrate_ref_storage_format(struct repository *repo,
> if (ret < 0)
> goto done;
>
> + data.reflog_index = 1;
I'm a bit surprised that we initialize the relfog entry here, because
that means we now have a globally increasing counter across all reflogs.
Couldn't we initialize the index per reflog instead? It ultimately does
not really matter, but feels like the more obvious design to me
Also, is there any specific reason why we start at 1 and not 0? Just curious.
Patrick
