On 25/07/28 03:08PM, Patrick Steinhardt wrote:
> With `refs_for_each_reflog_ent()` callers can iterate through all the
> reflog entries for a given reference. The callback that is being invoked
> for each such entry does not receive the name of the reference that we
> are currently iterating through. This isn't really a limiting factor, as
> callers can simply pass the name via the callback data.
>
> But this layout sometimes does make for a bit of an awkward calling
> pattern. One example: when iterating through all reflogs, and for each
> reflog we iterate through all refnames, we have to do some extra book
> keeping to track which reference name we are currently yielding reflog
> entries for.
Making the reference name part of the callback signature seems
reasonable here. For the above mentioned example, it will certainly
simplify things quite a bit.
> Change the signature of the callback function so that the reference name
> of the reflog gets passed through to it. Adapt callers accordingly and
> start using the new parameter in trivial cases. The next commit will
> refactor the reference migration logic to make use of this parameter so
> that we can simplify its logic a bit.
>
> Signed-off-by: Patrick Steinhardt <ps@xxxxxx>
> ---
> builtin/fsck.c | 9 ++++-----
> builtin/gc.c | 3 ++-
> builtin/stash.c | 6 ++++--
> commit.c | 3 ++-
> object-name.c | 3 ++-
> reflog-walk.c | 7 ++++---
> reflog.c | 3 ++-
> reflog.h | 3 ++-
> refs.c | 19 +++++++++----------
> refs.h | 1 +
> refs/debug.c | 5 +++--
> refs/files-backend.c | 15 +++++++++------
> refs/reftable-backend.c | 2 +-
> remote.c | 6 ++++--
> revision.c | 3 ++-
> t/helper/test-ref-store.c | 3 ++-
> wt-status.c | 6 ++++--
> 17 files changed, 57 insertions(+), 40 deletions(-)
>
[snip]
> diff --git a/refs.h b/refs.h
> index 99b58d0b73c..a39f873b1fe 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -559,6 +559,7 @@ int refs_delete_reflog(struct ref_store *refs, const char *refname);
> * functions.
> */
> typedef int each_reflog_ent_fn(
> + const char *refname,
The callback function now propagates the reference name and each
reference backend is updated accordingly. This patch looks good.
> struct object_id *old_oid, struct object_id *new_oid,
> const char *committer, timestamp_t timestamp,
> int tz, const char *msg, void *cb_data);
> diff --git a/refs/debug.c b/refs/debug.c
> index 485e3079d7a..5e113db307a 100644
> --- a/refs/debug.c
> +++ b/refs/debug.c
> @@ -276,7 +276,8 @@ struct debug_reflog {
> void *cb_data;
> };
>
> -static int debug_print_reflog_ent(struct object_id *old_oid,
> +static int debug_print_reflog_ent(const char *refname,
> + struct object_id *old_oid,
> struct object_id *new_oid,
> const char *committer, timestamp_t timestamp,
> int tz, const char *msg, void *cb_data)
> @@ -291,7 +292,7 @@ static int debug_print_reflog_ent(struct object_id *old_oid,
> if (new_oid)
> oid_to_hex_r(n, new_oid);
>
> - ret = dbg->fn(old_oid, new_oid, committer, timestamp, tz, msg,
> + ret = dbg->fn(refname, old_oid, new_oid, committer, timestamp, tz, msg,
> dbg->cb_data);
> trace_printf_key(&trace_refs,
> "reflog_ent %s (ret %d): %s -> %s, %s %ld \"%.*s\"\n",
> diff --git a/refs/files-backend.c b/refs/files-backend.c
> index 3ebe0323d4e..24d0a8ebde0 100644
> --- a/refs/files-backend.c
> +++ b/refs/files-backend.c
> @@ -2109,7 +2109,9 @@ static int files_delete_reflog(struct ref_store *ref_store,
> return ret;
> }
>
> -static int show_one_reflog_ent(struct files_ref_store *refs, struct strbuf *sb,
> +static int show_one_reflog_ent(struct files_ref_store *refs,
> + const char *refname,
> + struct strbuf *sb,
> each_reflog_ent_fn fn, void *cb_data)
> {
> struct object_id ooid, noid;
> @@ -2136,7 +2138,7 @@ static int show_one_reflog_ent(struct files_ref_store *refs, struct strbuf *sb,
> message += 6;
> else
> message += 7;
> - return fn(&ooid, &noid, p, timestamp, tz, message, cb_data);
> + return fn(refname, &ooid, &noid, p, timestamp, tz, message, cb_data);
> }
>
> static char *find_beginning_of_line(char *bob, char *scan)
> @@ -2220,7 +2222,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
> strbuf_splice(&sb, 0, 0, bp + 1, endp - (bp + 1));
> scanp = bp;
> endp = bp + 1;
> - ret = show_one_reflog_ent(refs, &sb, fn, cb_data);
> + ret = show_one_reflog_ent(refs, refname, &sb, fn, cb_data);
> strbuf_reset(&sb);
> if (ret)
> break;
> @@ -2232,7 +2234,7 @@ static int files_for_each_reflog_ent_reverse(struct ref_store *ref_store,
> * Process it, and we can end the loop.
> */
> strbuf_splice(&sb, 0, 0, buf, endp - buf);
> - ret = show_one_reflog_ent(refs, &sb, fn, cb_data);
> + ret = show_one_reflog_ent(refs, refname, &sb, fn, cb_data);
> strbuf_reset(&sb);
> break;
> }
> @@ -2282,7 +2284,7 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,
> return -1;
>
> while (!ret && !strbuf_getwholeline(&sb, logfp, '\n'))
> - ret = show_one_reflog_ent(refs, &sb, fn, cb_data);
> + ret = show_one_reflog_ent(refs, refname, &sb, fn, cb_data);
> fclose(logfp);
> strbuf_release(&sb);
> return ret;
> @@ -3323,7 +3325,8 @@ struct expire_reflog_cb {
> dry_run:1;
> };
>
> -static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,
> +static int expire_reflog_ent(const char *refname UNUSED,
> + struct object_id *ooid, struct object_id *noid,
> const char *email, timestamp_t timestamp, int tz,
> const char *message, void *cb_data)
> {
> diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
> index 99fafd75ebe..25a1d516184 100644
> --- a/refs/reftable-backend.c
> +++ b/refs/reftable-backend.c
> @@ -2148,7 +2148,7 @@ static int yield_log_record(struct reftable_ref_store *refs,
>
> full_committer = fmt_ident(log->value.update.name, log->value.update.email,
> WANT_COMMITTER_IDENT, NULL, IDENT_NO_DATE);
> - return fn(&old_oid, &new_oid, full_committer,
> + return fn(log->refname, &old_oid, &new_oid, full_committer,
> log->value.update.time, log->value.update.tz_offset,
> log->value.update.message, cb_data);
> }
