@matheusbsbs / matheus-bagliano / commits / 31f898397b

refs: drop unused params from the reflog iterator callback

The ref and reflog iterators share much of the same underlying code to iterate over the corresponding entries. This results in some weird code because the reflog iterator also exposes an object ID as well as a flag to the callback function. Neither of these fields do refer to the reflog though -- they refer to the corresponding ref with the same name. This is quite misleading. In practice at least the object ID cannot really be implemented in any other way as a reflog does not have a specific object ID in the first place. This is further stressed by the fact that none of the callbacks except for our test helper make use of these fields. Split up the infrastucture so that ref and reflog iterators use separate callback signatures. This allows us to drop the nonsensical fields from the reflog iterator. Note that internally, the backends still use the same shared infra to iterate over both types. As the backends should never end up being called directly anyway, this is not much of a problem and thus kept as-is for simplicity's sake. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 21, 2024 at 13:37 UTC 31f898397bb2f44692b8bcc4fd64fffaf3b59c48

11 files changed +65 -54

builtin/fsck.c

+1 -3

@@ -509,9 +509,7 @@ static int fsck_handle_reflog_ent(struct object_id *ooid, struct object_id *noid

509 return 0;

510 }

511

512 -static int fsck_handle_reflog(const char *logname,

513 - const struct object_id *oid UNUSED,

514 - int flag UNUSED, void *cb_data)

512 +static int fsck_handle_reflog(const char *logname, void *cb_data)

513 {

514 struct strbuf refname = STRBUF_INIT;

515

builtin/reflog.c

+1 -2

@@ -60,8 +60,7 @@ struct worktree_reflogs {

60 struct string_list reflogs;

61 };

63 -static int collect_reflog(const char *ref, const struct object_id *oid UNUSED,

64 - int flags UNUSED, void *cb_data)

63 +static int collect_reflog(const char *ref, void *cb_data)

64 {

65 struct worktree_reflogs *cb = cb_data;

66 struct worktree *worktree = cb->worktree;

refs.c

+19 -4

@@ -2512,18 +2512,33 @@ cleanup:

2512 return ret;

2513 }

2514

2515 -int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data)

2515 +struct do_for_each_reflog_help {

2516 + each_reflog_fn *fn;

2517 + void *cb_data;

2518 +};

2519 +

2520 +static int do_for_each_reflog_helper(struct repository *r UNUSED,

2521 + const char *refname,

2522 + const struct object_id *oid UNUSED,

2523 + int flags,

2524 + void *cb_data)

2525 +{

2526 + struct do_for_each_reflog_help *hp = cb_data;

2527 + return hp->fn(refname, hp->cb_data);

2528 +}

2529 +

2530 +int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data)

2531 {

2532 struct ref_iterator *iter;

2518 - struct do_for_each_ref_help hp = { fn, cb_data };

2533 + struct do_for_each_reflog_help hp = { fn, cb_data };

2534

2535 iter = refs->be->reflog_iterator_begin(refs);

2536

2537 return do_for_each_repo_ref_iterator(the_repository, iter,

2523 - do_for_each_ref_helper, &hp);

2538 + do_for_each_reflog_helper, &hp);

2539 }

2540

2526 -int for_each_reflog(each_ref_fn fn, void *cb_data)

2541 +int for_each_reflog(each_reflog_fn fn, void *cb_data)

2542 {

2543 return refs_for_each_reflog(get_main_ref_store(the_repository), fn, cb_data);

2544 }

refs.h

+9 -2

@@ -534,12 +534,19 @@ int for_each_reflog_ent(const char *refname, each_reflog_ent_fn fn, void *cb_dat

534 /* youngest entry first */

535 int for_each_reflog_ent_reverse(const char *refname, each_reflog_ent_fn fn, void *cb_data);

536

537 +/*

538 + * The signature for the callback function for the {refs_,}for_each_reflog()

539 + * functions below. The memory pointed to by the refname argument is only

540 + * guaranteed to be valid for the duration of a single callback invocation.

541 + */

542 +typedef int each_reflog_fn(const char *refname, void *cb_data);

543 +

544 /*

545 * Calls the specified function for each reflog file until it returns nonzero,

546 * and returns the value. Reflog file order is unspecified.

547 */

541 -int refs_for_each_reflog(struct ref_store *refs, each_ref_fn fn, void *cb_data);

542 -int for_each_reflog(each_ref_fn fn, void *cb_data);

548 +int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_data);

549 +int for_each_reflog(each_reflog_fn fn, void *cb_data);

550

551 #define REFNAME_ALLOW_ONELEVEL 1

552 #define REFNAME_REFSPEC_PATTERN 2

refs/files-backend.c

+1 -7

@@ -2115,10 +2115,8 @@ static int files_for_each_reflog_ent(struct ref_store *ref_store,

2115

2116 struct files_reflog_iterator {

2117 struct ref_iterator base;

2118 -

2118 struct ref_store *ref_store;

2119 struct dir_iterator *dir_iterator;

2121 - struct object_id oid;

2120 };

2121

2122 static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)

@@ -2129,8 +2127,6 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)

2127 int ok;

2128

2129 while ((ok = dir_iterator_advance(diter)) == ITER_OK) {

2132 - int flags;

2133 -

2130 if (!S_ISREG(diter->st.st_mode))

2131 continue;

2132 if (diter->basename[0] == '.')

@@ -2140,14 +2136,12 @@ static int files_reflog_iterator_advance(struct ref_iterator *ref_iterator)

2136

2137 if (!refs_resolve_ref_unsafe(iter->ref_store,

2138 diter->relative_path, 0,

2143 - &iter->oid, &flags)) {

2139 + NULL, NULL)) {

2140 error("bad ref for %s", diter->path.buf);

2141 continue;

2142 }

2143

2144 iter->base.refname = diter->relative_path;

2149 - iter->base.oid = &iter->oid;

2150 - iter->base.flags = flags;

2145 return ITER_OK;

2146 }

2147

refs/reftable-backend.c

+1 -7

@@ -1594,7 +1594,6 @@ struct reftable_reflog_iterator {

1594 struct reftable_ref_store *refs;

1595 struct reftable_iterator iter;

1596 struct reftable_log_record log;

1597 - struct object_id oid;

1597 char *last_name;

1598 int err;

1599 };

@@ -1605,8 +1604,6 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)

1604 (struct reftable_reflog_iterator *)ref_iterator;

1605

1606 while (!iter->err) {

1608 - int flags;

1609 -

1607 iter->err = reftable_iterator_next_log(&iter->iter, &iter->log);

1608 if (iter->err)

1609 break;

@@ -1620,7 +1617,7 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)

1617 continue;

1618

1619 if (!refs_resolve_ref_unsafe(&iter->refs->base, iter->log.refname,

1623 - 0, &iter->oid, &flags)) {

1620 + 0, NULL, NULL)) {

1621 error(_("bad ref for %s"), iter->log.refname);

1622 continue;

1623 }

@@ -1628,8 +1625,6 @@ static int reftable_reflog_iterator_advance(struct ref_iterator *ref_iterator)

1625 free(iter->last_name);

1626 iter->last_name = xstrdup(iter->log.refname);

1627 iter->base.refname = iter->log.refname;

1631 - iter->base.oid = &iter->oid;

1632 - iter->base.flags = flags;

1628

1629 break;

1630 }

@@ -1682,7 +1677,6 @@ static struct reftable_reflog_iterator *reflog_iterator_for_stack(struct reftabl

1677 iter = xcalloc(1, sizeof(*iter));

1678 base_ref_iterator_init(&iter->base, &reftable_reflog_iterator_vtable);

1679 iter->refs = refs;

1685 - iter->base.oid = &iter->oid;

1680

1681 ret = refs->err;

1682 if (ret)

revision.c

+1 -3

@@ -1686,9 +1686,7 @@ static int handle_one_reflog_ent(struct object_id *ooid, struct object_id *noid,

1686 return 0;

1687 }

1688

1689 -static int handle_one_reflog(const char *refname_in_wt,

1690 - const struct object_id *oid UNUSED,

1691 - int flag UNUSED, void *cb_data)

1689 +static int handle_one_reflog(const char *refname_in_wt, void *cb_data)

1690 {

1691 struct all_refs_cb *cb = cb_data;

1692 struct strbuf refname = STRBUF_INIT;

t/helper/test-ref-store.c

+12 -6

@@ -221,15 +221,21 @@ static int cmd_verify_ref(struct ref_store *refs, const char **argv)

221 return ret;

222 }

223

224 +static int each_reflog(const char *refname, void *cb_data UNUSED)

225 +{

226 + printf("%s\n", refname);

227 + return 0;

228 +}

229 +

230 static int cmd_for_each_reflog(struct ref_store *refs,

231 const char **argv UNUSED)

232 {

227 - return refs_for_each_reflog(refs, each_ref, NULL);

233 + return refs_for_each_reflog(refs, each_reflog, NULL);

234 }

235

230 -static int each_reflog(struct object_id *old_oid, struct object_id *new_oid,

231 - const char *committer, timestamp_t timestamp,

232 - int tz, const char *msg, void *cb_data UNUSED)

236 +static int each_reflog_ent(struct object_id *old_oid, struct object_id *new_oid,

237 + const char *committer, timestamp_t timestamp,

238 + int tz, const char *msg, void *cb_data UNUSED)

239 {

240 printf("%s %s %s %" PRItime " %+05d%s%s", oid_to_hex(old_oid),

241 oid_to_hex(new_oid), committer, timestamp, tz,

@@ -241,14 +247,14 @@ static int cmd_for_each_reflog_ent(struct ref_store *refs, const char **argv)

247 {

248 const char *refname = notnull(*argv++, "refname");

249

244 - return refs_for_each_reflog_ent(refs, refname, each_reflog, refs);

250 + return refs_for_each_reflog_ent(refs, refname, each_reflog_ent, refs);

251 }

252

253 static int cmd_for_each_reflog_ent_reverse(struct ref_store *refs, const char **argv)

254 {

255 const char *refname = notnull(*argv++, "refname");

256

251 - return refs_for_each_reflog_ent_reverse(refs, refname, each_reflog, refs);

257 + return refs_for_each_reflog_ent_reverse(refs, refname, each_reflog_ent, refs);

258 }

259

260 static int cmd_reflog_exists(struct ref_store *refs, const char **argv)

t/t0600-reffiles-backend.sh

+12 -12

@@ -287,23 +287,23 @@ test_expect_success 'for_each_reflog()' '

287 mkdir -p .git/worktrees/wt/logs/refs/bisect &&

288 echo $ZERO_OID > .git/worktrees/wt/logs/refs/bisect/wt-random &&

289

290 - $RWT for-each-reflog | cut -d" " -f 2- >actual &&

290 + $RWT for-each-reflog >actual &&

291 cat >expected <<-\EOF &&

292 - HEAD 0x1

293 - PSEUDO-WT 0x0

294 - refs/bisect/wt-random 0x0

295 - refs/heads/main 0x0

296 - refs/heads/wt-main 0x0

292 + HEAD

293 + PSEUDO-WT

294 + refs/bisect/wt-random

295 + refs/heads/main

296 + refs/heads/wt-main

297 EOF

298 test_cmp expected actual &&

299

300 - $RMAIN for-each-reflog | cut -d" " -f 2- >actual &&

300 + $RMAIN for-each-reflog >actual &&

301 cat >expected <<-\EOF &&

302 - HEAD 0x1

303 - PSEUDO-MAIN 0x0

304 - refs/bisect/random 0x0

305 - refs/heads/main 0x0

306 - refs/heads/wt-main 0x0

302 + HEAD

303 + PSEUDO-MAIN

304 + refs/bisect/random

305 + refs/heads/main

306 + refs/heads/wt-main

307 EOF

308 test_cmp expected actual

309 '

t/t1405-main-ref-store.sh

+4 -4

@@ -74,11 +74,11 @@ test_expect_success 'verify_ref(new-main)' '

74 '

76 test_expect_success 'for_each_reflog()' '

77 - $RUN for-each-reflog | cut -d" " -f 2- >actual &&

77 + $RUN for-each-reflog >actual &&

78 cat >expected <<-\EOF &&

79 - HEAD 0x1

80 - refs/heads/main 0x0

81 - refs/heads/new-main 0x0

79 + HEAD

80 + refs/heads/main

81 + refs/heads/new-main

82 EOF

83 test_cmp expected actual

84 '

t/t1406-submodule-ref-store.sh

+4 -4

@@ -63,11 +63,11 @@ test_expect_success 'verify_ref(new-main)' '

63 '

65 test_expect_success 'for_each_reflog()' '

66 - $RUN for-each-reflog | cut -d" " -f 2- >actual &&

66 + $RUN for-each-reflog >actual &&

67 cat >expected <<-\EOF &&

68 - HEAD 0x1

69 - refs/heads/main 0x0

70 - refs/heads/new-main 0x0

68 + HEAD

69 + refs/heads/main

70 + refs/heads/new-main

71 EOF

72 test_cmp expected actual

73 '