From 249b340887ce6f91227a98710a93d004551bc0e7 Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Wed, 30 Oct 2024 16:44:18 +0100 Subject: [PATCH 1/2] refs: include committer info in `ref_update` struct The reference backends obtain the committer information from `git_committer_info(0)` when adding a reflog. In the upcoming patches we want to introduce support for migrating reflogs between the reference backends. To do this, we need to provide an interface to creating reflogs, including custom committer information. To do this, let's add a new field `committer_info` to the `ref_update` struct, which is then used by the reference backends. If there is no `committer_info` provided, the reference backends default to using `git_committer_info(0)`. We cannot set the field to `git_committer_info(0)` since the values are dynamic and must be obtained when the reflog is being committed. Signed-off-by: Karthik Nayak --- refs/files-backend.c | 32 ++++++++++++++++++-------------- refs/refs-internal.h | 1 + refs/reftable-backend.c | 9 ++++++++- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index 0824c0b8a94..e1eb24432bc 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -1851,6 +1851,9 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid, struct strbuf sb = STRBUF_INIT; int ret = 0; + if (!committer) + committer = git_committer_info(0); + strbuf_addf(&sb, "%s %s %s", oid_to_hex(old_oid), oid_to_hex(new_oid), committer); if (msg && *msg) { strbuf_addch(&sb, '\t'); @@ -1864,8 +1867,10 @@ static int log_ref_write_fd(int fd, const struct object_id *old_oid, } static int files_log_ref_write(struct files_ref_store *refs, - const char *refname, const struct object_id *old_oid, - const struct object_id *new_oid, const char *msg, + const char *refname, + const struct object_id *old_oid, + const struct object_id *new_oid, + const char *committer_info, const char *msg, int flags, struct strbuf *err) { int logfd, result; @@ -1882,8 +1887,7 @@ static int files_log_ref_write(struct files_ref_store *refs, if (logfd < 0) return 0; - result = log_ref_write_fd(logfd, old_oid, new_oid, - git_committer_info(0), msg); + result = log_ref_write_fd(logfd, old_oid, new_oid, committer_info, msg); if (result) { struct strbuf sb = STRBUF_INIT; int save_errno = errno; @@ -1967,8 +1971,7 @@ static int commit_ref_update(struct files_ref_store *refs, files_assert_main_repository(refs, "commit_ref_update"); clear_loose_ref_cache(refs); - if (files_log_ref_write(refs, lock->ref_name, - &lock->old_oid, oid, + if (files_log_ref_write(refs, lock->ref_name, &lock->old_oid, oid, NULL, logmsg, flags, err)) { char *old_msg = strbuf_detach(err, NULL); strbuf_addf(err, "cannot update the ref '%s': %s", @@ -2000,8 +2003,8 @@ static int commit_ref_update(struct files_ref_store *refs, if (head_ref && (head_flag & REF_ISSYMREF) && !strcmp(head_ref, lock->ref_name)) { struct strbuf log_err = STRBUF_INIT; - if (files_log_ref_write(refs, "HEAD", - &lock->old_oid, oid, + if (files_log_ref_write(refs, "HEAD", &lock->old_oid, + oid, git_committer_info(0), logmsg, flags, &log_err)) { error("%s", log_err.buf); strbuf_release(&log_err); @@ -2943,23 +2946,24 @@ static int parse_and_write_reflog(struct files_ref_store *refs, { if (update->new_target) { /* - * We want to get the resolved OID for the target, to ensure - * that the correct value is added to the reflog. + * We want to get the resolved OID for the target, to + * ensure that the correct value is added to the reflog. */ if (!refs_resolve_ref_unsafe(&refs->base, update->new_target, RESOLVE_REF_READING, &update->new_oid, NULL)) { /* - * TODO: currently we skip creating reflogs for dangling - * symref updates. It would be nice to capture this as - * zero oid updates however. + * TODO: currently we skip creating reflogs for + * dangling symref updates. It would be nice to + * capture this as zero oid updates however. */ return 0; } } if (files_log_ref_write(refs, lock->ref_name, &lock->old_oid, - &update->new_oid, update->msg, update->flags, err)) { + &update->new_oid, update->committer_info, + update->msg, update->flags, err)) { char *old_msg = strbuf_detach(err, NULL); strbuf_addf(err, "cannot update the ref '%s': %s", diff --git a/refs/refs-internal.h b/refs/refs-internal.h index 2313c830d8f..50e21fd62b7 100644 --- a/refs/refs-internal.h +++ b/refs/refs-internal.h @@ -113,6 +113,7 @@ struct ref_update { void *backend_data; unsigned int type; char *msg; + const char *committer_info; /* * If this ref_update was split off of a symref update via diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c index 38eb14d591e..7f580dd863e 100644 --- a/refs/reftable-backend.c +++ b/refs/reftable-backend.c @@ -1377,11 +1377,18 @@ static int write_transaction_table(struct reftable_writer *writer, void *cb_data } if (create_reflog) { + struct ident_split c = committer_ident; + ALLOC_GROW(logs, logs_nr + 1, logs_alloc); log = &logs[logs_nr++]; memset(log, 0, sizeof(*log)); - fill_reftable_log_record(log, &committer_ident); + if (u->committer_info) + if (split_ident_line(&c, u->committer_info, + strlen(u->committer_info))) + BUG("failed splitting committer info"); + + fill_reftable_log_record(log, &c); log->update_index = ts; log->refname = xstrdup(u->refname); memcpy(log->value.update.new_hash, -- GitLab From 3f5062edb3e90e50dfb97eb274770d044f2838be Mon Sep 17 00:00:00 2001 From: Karthik Nayak Date: Fri, 8 Nov 2024 16:10:18 +0100 Subject: [PATCH 2/2] files-backend: add count to ref_lock Whenever we want to update a ref in the file-backend, we obtain a lock for the corresponding file path. Even when we want to create/update a reflog, we still obtain the same lock, which is on the ref. This works, because generally reflogs are updated alongside the ref. We want to add reflog update to ref transaction in the upcoming patches. This means, in a particular transaction we want to have ref update and reflog updates. For refs, in a given transaction there can only be one update. But, we can theoretically have multiple reflog updates in a given transaction. This is because there is no conflicts in regards to reflogs. The current flow will not support this, because we currently treat refs & reflogs as a single entity and capture the lock together. To separate this, we add a count field to ref_lock. With this, multiple updates can hold onto a single ref_lock and the lock will only be released when all of them release the lock. This patch only adds the `count` field to `ref_lock` and adds the logic to increment and decrement the lock. In the next commit, we'll separate the reflog update logic from ref updates and utilize this functionality. Signed-off-by: Karthik Nayak --- refs/files-backend.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/refs/files-backend.c b/refs/files-backend.c index e1eb24432bc..35f335fb63b 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -69,6 +69,8 @@ struct ref_lock { char *ref_name; struct lock_file lk; struct object_id old_oid; + /* count keeps track of users of the lock */ + uint count; }; struct files_ref_store { @@ -632,9 +634,12 @@ int parse_loose_ref_contents(const struct git_hash_algo *algop, static void unlock_ref(struct ref_lock *lock) { - rollback_lock_file(&lock->lk); - free(lock->ref_name); - free(lock); + lock->count--; + if (!lock->count) { + rollback_lock_file(&lock->lk); + free(lock->ref_name); + free(lock); + } } /* @@ -690,6 +695,7 @@ static int lock_raw_ref(struct files_ref_store *refs, *lock_p = CALLOC_ARRAY(lock, 1); lock->ref_name = xstrdup(refname); + lock->count = 1; files_ref_path(refs, &ref_file, refname); retry: @@ -1163,6 +1169,7 @@ static struct ref_lock *lock_ref_oid_basic(struct files_ref_store *refs, goto error_return; lock->ref_name = xstrdup(refname); + lock->count = 1; if (raceproof_create_file(ref_file.buf, create_reflock, &lock->lk)) { unable_to_lock_message(ref_file.buf, errno, err); -- GitLab