Re: [PATCH 6/7] refs: allow multiple reflog entries for the same refname
From: Christian Couder <hidden>
Date: 2024-12-11 10:44:31
On Mon, Dec 9, 2024 at 12:11 PM Karthik Nayak [off-list ref] wrote:
The reference transaction only allows a update for a given reference to
s/a update/an update/ or: s/a update/a single update/
avoid conflicts. This, however, isn't an issue for reflogs. There are no conflicts to be resolved in reflogs and when migrating reflogs between backends we'd have multiple reflog entries for the same refname.
quoted hunk ↗ jump to hunk
@@ -1302,6 +1303,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data struct ident_split committer_ident = {0}; size_t logs_nr = 0, logs_alloc = 0, i; const char *committer_info; + struct strintmap logs_ts;
Here a comment might help explain what logs_ts is used for.
quoted hunk ↗ jump to hunk
int ret = 0; committer_info = git_committer_info(0);@@ -1310,6 +1312,8 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data QSORT(arg->updates, arg->updates_nr, transaction_update_cmp); + strintmap_init(&logs_ts, ts);
I am not sure I understand what logs_ts is used for and why its default value is set to ts. Also ts is an uint64_t while the second argument to strintmap_init() is an int. I wonder if it could be an issue especially on 32 bits platforms.
quoted hunk ↗ jump to hunk
reftable_writer_set_limits(writer, ts, ts); for (i = 0; i < arg->updates_nr; i++) {@@ -1391,6 +1395,7 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data if (create_reflog) { struct ident_split c; + uint64_t update_index; ALLOC_GROW(logs, logs_nr + 1, logs_alloc); log = &logs[logs_nr++];@@ -1405,7 +1410,11 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data } fill_reftable_log_record(log, &c); - log->update_index = ts; + + update_index = strintmap_get(&logs_ts, u->refname); + log->update_index = update_index; + strintmap_set(&logs_ts, u->refname, update_index+1);
s/update_index+1/update_index + 1/
Also is the 'update_index' var really needed or could we just do:
log->update_index =
strintmap_get(&logs_ts, u->refname);
strintmap_set(&logs_ts, u->refname,
log->update_index + 1);
?
log->refname = xstrdup(u->refname);
memcpy(log->value.update.new_hash,
u->new_oid.hash, GIT_MAX_RAWSZ);