From: Patrick Steinhardt <ps@pks•im>
To: Karthik Nayak <karthik.188@gmail•com>
Cc: git@vger•kernel.org, Christian Couder <chriscool@tuxfamily•org>
Subject: Re: [PATCH v2 8/8] refs: add support for migrating reflogs
Date: Fri, 13 Dec 2024 13:24:34 +0100 [thread overview]
Message-ID: <Z1wngv1Kox_QQcwK@pks.im> (raw)
In-Reply-To: <20241213-320-git-refs-migrate-reflogs-v2-8-f28312cdb6c0@gmail.com>
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
next prev parent reply other threads:[~2024-12-13 12:24 UTC|newest]
Thread overview: 93+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-09 11:07 [PATCH 0/7] refs: add reflog support to `git refs migrate` Karthik Nayak
2024-12-09 11:07 ` [PATCH 1/7] refs: include committer info in `ref_update` struct Karthik Nayak
2024-12-10 16:51 ` Christian Couder
2024-12-11 10:13 ` karthik nayak
2024-12-09 11:07 ` [PATCH 2/7] refs: add `index` field to `struct ref_udpate` Karthik Nayak
2024-12-09 11:07 ` [PATCH 3/7] refs/files: add count field to ref_lock Karthik Nayak
2024-12-10 17:22 ` Christian Couder
2024-12-11 10:18 ` karthik nayak
2024-12-11 9:05 ` Christian Couder
2024-12-11 10:26 ` karthik nayak
2024-12-09 11:07 ` [PATCH 4/7] refs: extract out refname verification in transactions Karthik Nayak
2024-12-11 9:26 ` Christian Couder
2024-12-11 10:31 ` karthik nayak
2024-12-11 14:26 ` Patrick Steinhardt
2024-12-09 11:07 ` [PATCH 5/7] refs: introduce the `ref_transaction_update_reflog` function Karthik Nayak
2024-12-11 10:10 ` Christian Couder
2024-12-11 18:06 ` karthik nayak
2024-12-11 14:26 ` Patrick Steinhardt
2024-12-11 18:09 ` karthik nayak
2024-12-09 11:07 ` [PATCH 6/7] refs: allow multiple reflog entries for the same refname Karthik Nayak
2024-12-11 10:44 ` Christian Couder
2024-12-12 14:52 ` karthik nayak
2024-12-11 14:26 ` Patrick Steinhardt
2024-12-12 14:47 ` karthik nayak
2024-12-09 11:07 ` [PATCH 7/7] refs: add support for migrating reflogs Karthik Nayak
2024-12-11 14:26 ` Patrick Steinhardt
2024-12-12 14:04 ` karthik nayak
2024-12-10 12:13 ` [PATCH 0/7] refs: add reflog support to `git refs migrate` Junio C Hamano
2024-12-10 17:42 ` karthik nayak
2024-12-10 18:03 ` karthik nayak
2024-12-13 10:36 ` [PATCH v2 0/8] " Karthik Nayak
2024-12-13 10:36 ` [PATCH v2 1/8] refs: include committer info in `ref_update` struct Karthik Nayak
2024-12-13 10:36 ` [PATCH v2 2/8] refs: add `index` field to `struct ref_udpate` Karthik Nayak
2024-12-13 10:36 ` [PATCH v2 3/8] refs/files: add count field to ref_lock Karthik Nayak
2024-12-13 10:36 ` [PATCH v2 4/8] refs: extract out refname verification in transactions Karthik Nayak
2024-12-13 10:36 ` [PATCH v2 5/8] refs: add `committer_info` to `ref_transaction_add_update()` Karthik Nayak
2024-12-13 12:24 ` Patrick Steinhardt
2024-12-13 19:43 ` karthik nayak
2024-12-19 19:31 ` Toon Claes
2024-12-20 11:31 ` karthik nayak
2024-12-13 10:36 ` [PATCH v2 6/8] refs: introduce the `ref_transaction_update_reflog` function Karthik Nayak
2024-12-13 11:44 ` Christian Couder
2024-12-13 19:49 ` karthik nayak
2024-12-13 12:24 ` Patrick Steinhardt
2024-12-13 10:36 ` [PATCH v2 7/8] refs: allow multiple reflog entries for the same refname Karthik Nayak
2024-12-13 12:24 ` Patrick Steinhardt
2024-12-13 20:02 ` karthik nayak
2024-12-13 10:36 ` [PATCH v2 8/8] refs: add support for migrating reflogs Karthik Nayak
2024-12-13 12:24 ` Patrick Steinhardt [this message]
2024-12-15 11:09 ` karthik nayak
2024-12-15 16:25 ` [PATCH v3 0/8] refs: add reflog support to `git refs migrate` Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 1/8] refs: include committer info in `ref_update` struct Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 2/8] refs: add `index` field to `struct ref_udpate` Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 3/8] refs/files: add count field to ref_lock Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 4/8] refs: extract out refname verification in transactions Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 5/8] refs: add `committer_info` to `ref_transaction_add_update()` Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 6/8] refs: introduce the `ref_transaction_update_reflog` function Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 7/8] refs: allow multiple reflog entries for the same refname Karthik Nayak
2024-12-15 16:25 ` [PATCH v3 8/8] refs: add support for migrating reflogs Karthik Nayak
2024-12-16 7:25 ` Patrick Steinhardt
2024-12-16 15:50 ` Junio C Hamano
2024-12-16 15:59 ` karthik nayak
2024-12-15 23:54 ` [PATCH v3 0/8] refs: add reflog support to `git refs migrate` Junio C Hamano
2024-12-16 14:33 ` karthik nayak
2024-12-16 16:32 ` Junio C Hamano
2024-12-16 16:44 ` [PATCH v4 " Karthik Nayak
2024-12-16 16:44 ` [PATCH v4 1/8] refs: include committer info in `ref_update` struct Karthik Nayak
2024-12-16 16:44 ` [PATCH v4 2/8] refs: add `index` field to `struct ref_udpate` Karthik Nayak
2024-12-19 19:28 ` Toon Claes
2024-12-20 10:09 ` karthik nayak
2024-12-16 16:44 ` [PATCH v4 3/8] refs/files: add count field to ref_lock Karthik Nayak
2024-12-16 16:44 ` [PATCH v4 4/8] refs: extract out refname verification in transactions Karthik Nayak
2024-12-19 19:29 ` Toon Claes
2024-12-20 10:30 ` karthik nayak
2024-12-16 16:44 ` [PATCH v4 5/8] refs: add `committer_info` to `ref_transaction_add_update()` Karthik Nayak
2024-12-19 19:30 ` Toon Claes
2024-12-20 10:44 ` karthik nayak
2024-12-16 16:44 ` [PATCH v4 6/8] refs: introduce the `ref_transaction_update_reflog` function Karthik Nayak
2024-12-19 19:32 ` Toon Claes
2024-12-19 20:25 ` Junio C Hamano
2024-12-20 10:55 ` karthik nayak
2024-12-20 12:58 ` [PATCH] refs: mark invalid refname message for translation Karthik Nayak
2024-12-20 15:53 ` Junio C Hamano
2024-12-24 10:34 ` Toon Claes
2024-12-16 16:44 ` [PATCH v4 7/8] refs: allow multiple reflog entries for the same refname Karthik Nayak
2024-12-19 19:33 ` Toon Claes
2024-12-20 11:15 ` karthik nayak
2024-12-16 16:44 ` [PATCH v4 8/8] refs: add support for migrating reflogs Karthik Nayak
2024-12-17 6:59 ` [PATCH v4 0/8] refs: add reflog support to `git refs migrate` Patrick Steinhardt
2024-12-17 9:35 ` karthik nayak
2024-12-17 21:28 ` Junio C Hamano
2024-12-19 19:32 ` Toon Claes
2024-12-20 11:23 ` karthik nayak
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z1wngv1Kox_QQcwK@pks.im \
--to=ps@pks$(echo .)im \
--cc=chriscool@tuxfamily$(echo .)org \
--cc=git@vger$(echo .)kernel.org \
--cc=karthik.188@gmail$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox