{"thread":{"id":"57005","subject":"[PATCH 00/12] reflog + gc: refactor, progress output, reftable-readyness","startedAt":"2021-11-30T21:38:23Z","lastAt":"2021-12-22T04:07:20Z","messageCount":37,"participants":["Ævar Arnfjörð Bjarmason","Junio C Hamano","Han-Wen Nienhuys"],"isPatch":true,"patchVersion":1,"patchTotal":12},"messages":[{"id":"442694","messageId":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":null,"subject":"[PATCH 00/12] reflog + gc: refactor, progress output, reftable-readyness","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:02Z","receivedAt":"2021-11-30T21:38:23Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This is a follow-up topic I had for my \"errno\" refs API changes that\njust landed[1]. I initially thought I'd split this into multiple\nsubmissions, but I think it's probably better to consider it in\nunison.\n\nComments on individual patches below:\n\n1. https://lore.kernel.org/git/cover-v2-00.21-00000000000-20211016T093845Z-avarab@gmail.com/\n\nÆvar Arnfjörð Bjarmason (12):\n  reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()\n  reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()\n  reflog: change one->many worktree->refnames to use a string_list\n  reflog expire: don't do negative comparison on enum values\n  reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL\n  reflog expire: don't use lookup_commit_reference_gently()\n  reflog: reduce scope of \"struct rev_info\"\n  refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN\n\nThis is all refactorings to make the reflog-related code smaller,\neasier to read, using string-list instead of a custom home-grown flex\narray API etc, and to make it clear what variables and data is used\nwhere. This really helps for subsequent steps.\n\n  reflog + refs-backend: move \"verbose\" out of the backend\n\nAs noted in\nhttps://lore.kernel.org/git/211123.864k83w3y4.gmgdl@evledraar.gmail.com/\nthis makes the \"verbose\" API independent, so the reftable backend\nwe'll have soon won't need to do anything special to support it.\n\nIt used to conflict with Han-Wen's topic, but in his v2 he picked\nanother approach.\n\n  reflog expire: add progress output on --stale-fix\n  gc + reflog: emit progress output from \"reflog expire --all\"\n  gc + reflog: don't stall before initial \"git gc\" progress output\n\nThese are all progress additions to \"reflog expire\" including fixing\nthe long-standing oddity that on large repos \"git gc\"'s progress seems\nto simply hang before reaching \"Enumerating objects\".\n\nAs noted in 12/12 this is partly a bit of a hack, but I've got other\nin-flight progress.c API changes that'll eventually lead to supporting\n\"nested\" progress bars, which this code will be one of the first\nconsumers of.\n\n Documentation/git-reflog.txt |   8 ++\n builtin/gc.c                 |   4 +-\n builtin/reflog.c             | 260 ++++++++++++++++++++++-------------\n refs.h                       |   3 +-\n refs/files-backend.c         |  44 +++---\n 5 files changed, 194 insertions(+), 125 deletions(-)\n\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442695","messageId":"patch-01.12-99e8a639163-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 01/12] reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:03Z","receivedAt":"2021-11-30T21:38:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cb_data\" we pass to the count_reflog_ent() to be the\n&cb.cmd itself, instead of passing &cb and having the callback lookup\ncb->cmd.\n\nThis makes it clear that the \"cb\" itself is the same memzero'd\nstructure on each iteration of the for-loop that uses &cb, except for\nthe \"cmd\" member.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 175c83e7cc2..4c15d71f3e9 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -662,20 +662,18 @@ static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *email, timestamp_t timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n-\tstruct expire_reflog_policy_cb *cb = cb_data;\n-\tif (!cb->cmd.expire_total || timestamp < cb->cmd.expire_total)\n-\t\tcb->cmd.recno++;\n+\tstruct cmd_reflog_expire_cb *cb = cb_data;\n+\tif (!cb->expire_total || timestamp < cb->expire_total)\n+\t\tcb->recno++;\n \treturn 0;\n }\n \n static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n {\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct cmd_reflog_expire_cb cmd = { 0 };\n \tint i, status = 0;\n \tunsigned int flags = 0;\n \n-\tmemset(&cb, 0, sizeof(cb));\n-\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n@@ -703,6 +701,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tconst char *spec = strstr(argv[i], \"@{\");\n \t\tchar *ep, *ref;\n \t\tint recno;\n+\t\tstruct expire_reflog_policy_cb cb = { 0 };\n \n \t\tif (!spec) {\n \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\n@@ -716,14 +715,15 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \n \t\trecno = strtoul(spec + 2, &ep, 10);\n \t\tif (*ep == '}') {\n-\t\t\tcb.cmd.recno = -recno;\n-\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n+\t\t\tcmd.recno = -recno;\n+\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cmd);\n \t\t} else {\n-\t\t\tcb.cmd.expire_total = approxidate(spec + 2);\n-\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n-\t\t\tcb.cmd.expire_total = 0;\n+\t\t\tcmd.expire_total = approxidate(spec + 2);\n+\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cmd);\n+\t\t\tcmd.expire_total = 0;\n \t\t}\n \n+\t\tcb.cmd = cmd;\n \t\tstatus |= reflog_expire(ref, flags,\n \t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\tshould_expire_reflog_ent,\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442696","messageId":"patch-02.12-c424b26b4fe-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 02/12] reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:04Z","receivedAt":"2021-11-30T21:38:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As with the preceding change for \"reflog delete\", change the \"cb_data\"\nwe pass to callbacks to be &cb.cmd itself, instead of passing &cb and\nhaving the callback lookup cb->cmd.\n\nThis makes it clear that the \"cb\" itself is the same memzero'd\nstructure on each iteration of the for-loops that use &cb, except for\nthe \"cmd\" member.\n\nThe \"struct expire_reflog_policy_cb\" we pass to reflog_expire() will\nhave the members that aren't \"cmd\" modified by the callbacks, but\nbefore we invoke them everything except \"cmd\" is zero'd out.\n\nThis included the \"tip_commit\", \"mark_list\" and \"tips\". It might have\nlooked as though we were re-using those between iterations, but the\nfirst thing we did in reflog_expiry_prepare() was to either NULL them,\nor clobber them with another value.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 31 +++++++++++++++----------------\n 1 file changed, 15 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 4c15d71f3e9..6989492bf5c 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -357,7 +357,6 @@ static void reflog_expiry_prepare(const char *refname,\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n-\t\tcb->tip_commit = NULL;\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n \t\tcb->tip_commit = lookup_commit_reference_gently(the_repository,\n@@ -371,8 +370,6 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n-\tcb->mark_list = NULL;\n-\tcb->tips = NULL;\n \tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n \t\t\tstruct commit_list *elem;\n@@ -541,7 +538,7 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c\n \n static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n {\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct cmd_reflog_expire_cb cmd = { 0 };\n \ttimestamp_t now = time(NULL);\n \tint i, status, do_all, all_worktrees = 1;\n \tint explicit_expiry = 0;\n@@ -553,10 +550,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tsave_commit_buffer = 0;\n \tdo_all = status = 0;\n-\tmemset(&cb, 0, sizeof(cb));\n \n-\tcb.cmd.expire_total = default_reflog_expire;\n-\tcb.cmd.expire_unreachable = default_reflog_expire_unreachable;\n+\tcmd.expire_total = default_reflog_expire;\n+\tcmd.expire_unreachable = default_reflog_expire_unreachable;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -564,17 +560,17 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n \t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (skip_prefix(arg, \"--expire=\", &arg)) {\n-\t\t\tif (parse_expiry_date(arg, &cb.cmd.expire_total))\n+\t\t\tif (parse_expiry_date(arg, &cmd.expire_total))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_TOTAL;\n \t\t}\n \t\telse if (skip_prefix(arg, \"--expire-unreachable=\", &arg)) {\n-\t\t\tif (parse_expiry_date(arg, &cb.cmd.expire_unreachable))\n+\t\t\tif (parse_expiry_date(arg, &cmd.expire_unreachable))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_UNREACH;\n \t\t}\n \t\telse if (!strcmp(arg, \"--stale-fix\"))\n-\t\t\tcb.cmd.stalefix = 1;\n+\t\t\tcmd.stalefix = 1;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n@@ -600,14 +596,14 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * even in older repository.  We cannot trust what's reachable\n \t * from reflog if the repository was pruned with older git.\n \t */\n-\tif (cb.cmd.stalefix) {\n-\t\trepo_init_revisions(the_repository, &cb.cmd.revs, prefix);\n-\t\tcb.cmd.revs.do_not_die_on_missing_tree = 1;\n-\t\tcb.cmd.revs.ignore_missing = 1;\n-\t\tcb.cmd.revs.ignore_missing_links = 1;\n+\tif (cmd.stalefix) {\n+\t\trepo_init_revisions(the_repository, &cmd.revs, prefix);\n+\t\tcmd.revs.do_not_die_on_missing_tree = 1;\n+\t\tcmd.revs.ignore_missing = 1;\n+\t\tcmd.revs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&cb.cmd.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&cmd.revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n@@ -629,6 +625,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n+\t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n \n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n \t\t\tstatus |= reflog_expire(e->reflog, flags,\n@@ -643,6 +640,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tfor (; i < argc; i++) {\n \t\tchar *ref;\n+\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n+\n \t\tif (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) {\n \t\t\tstatus |= error(_(\"%s points nowhere!\"), argv[i]);\n \t\t\tcontinue;\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442697","messageId":"patch-03.12-5a54b04a13e-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 03/12] reflog: change one->many worktree->refnames to use a string_list","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:05Z","receivedAt":"2021-11-30T21:38:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the FLEX_ARRAY pattern added in bda3a31cc79 (reflog-expire:\nAvoid creating new files in a directory inside readdir(3) loop,\n2008-01-25) the string-list API instead.\n\nThis does not change any behavior, allows us to delete much of this\ncode as it's replaced by things we get from the string-list API for\nfree, as a result we need just one struct to keep track of this data,\ninstead of two.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 48 +++++++++++++++++++-----------------------------\n 1 file changed, 19 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 6989492bf5c..0fb46ade19f 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -48,16 +48,9 @@ struct expire_reflog_policy_cb {\n \tstruct commit_list *tips;\n };\n \n-struct collected_reflog {\n-\tstruct object_id oid;\n-\tchar reflog[FLEX_ARRAY];\n-};\n-\n-struct collect_reflog_cb {\n-\tstruct collected_reflog **e;\n-\tint alloc;\n-\tint nr;\n-\tstruct worktree *wt;\n+struct worktree_reflogs {\n+\tstruct worktree *worktree;\n+\tstruct string_list reflogs;\n };\n \n /* Remember to update object flag allocation in object.h */\n@@ -403,24 +396,20 @@ static void reflog_expiry_cleanup(void *cb_data)\n \n static int collect_reflog(const char *ref, const struct object_id *oid, int unused, void *cb_data)\n {\n-\tstruct collected_reflog *e;\n-\tstruct collect_reflog_cb *cb = cb_data;\n+\tstruct worktree_reflogs *cb = cb_data;\n+\tstruct worktree *worktree = cb->worktree;\n \tstruct strbuf newref = STRBUF_INIT;\n \n \t/*\n \t * Avoid collecting the same shared ref multiple times because\n \t * they are available via all worktrees.\n \t */\n-\tif (!cb->wt->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n+\tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n \t\treturn 0;\n \n-\tstrbuf_worktree_ref(cb->wt, &newref, ref);\n-\tFLEX_ALLOC_STR(e, reflog, newref.buf);\n-\tstrbuf_release(&newref);\n+\tstrbuf_worktree_ref(worktree, &newref, ref);\n+\tstring_list_append(&cb->reflogs, strbuf_detach(&newref, NULL));\n \n-\toidcpy(&e->oid, oid);\n-\tALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);\n-\tcb->e[cb->nr++] = e;\n \treturn 0;\n }\n \n@@ -609,33 +598,34 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (do_all) {\n-\t\tstruct collect_reflog_cb collected;\n+\t\tstruct worktree_reflogs collected = {\n+\t\t\t.reflogs = STRING_LIST_INIT_NODUP,\n+\t\t};\n+\t\tstruct string_list_item *item;\n \t\tstruct worktree **worktrees, **p;\n-\t\tint i;\n \n-\t\tmemset(&collected, 0, sizeof(collected));\n \t\tworktrees = get_worktrees();\n \t\tfor (p = worktrees; *p; p++) {\n \t\t\tif (!all_worktrees && !(*p)->is_current)\n \t\t\t\tcontinue;\n-\t\t\tcollected.wt = *p;\n+\t\t\tcollected.worktree = *p;\n \t\t\trefs_for_each_reflog(get_worktree_ref_store(*p),\n \t\t\t\t\t     collect_reflog, &collected);\n \t\t}\n \t\tfree_worktrees(worktrees);\n-\t\tfor (i = 0; i < collected.nr; i++) {\n-\t\t\tstruct collected_reflog *e = collected.e[i];\n+\n+\t\tfor_each_string_list_item(item, &collected.reflogs) {\n \t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n \n-\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n-\t\t\tstatus |= reflog_expire(e->reflog, flags,\n+\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n+\t\t\tstatus |= reflog_expire(item->string, flags,\n \t\t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\t\tshould_expire_reflog_ent,\n \t\t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t\t&cb);\n-\t\t\tfree(e);\n \t\t}\n-\t\tfree(collected.e);\n+\t\tcollected.reflogs.strdup_strings = 1;\n+\t\tstring_list_clear(&collected.reflogs, 0);\n \t}\n \n \tfor (; i < argc; i++) {\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442698","messageId":"patch-04.12-a7a2dfd1406-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 04/12] reflog expire: don't do negative comparison on enum values","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:06Z","receivedAt":"2021-11-30T21:38:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code added in 03cb91b18cc (reflog --expire-unreachable: special\ncase entries in \"HEAD\" reflog, 2010-04-09) to not do positive instead\nof negative comparisons on enum values, i.e. not to assume that \"x !=\nUE_ALWAYS\" means \"(x == UE_HEAD || x || UE_NORMAL)\".\n\nThat assumption is true now, but we'd introduce subtle bugs here if\nthat were to change, now the compiler will notice and error out on\nsuch errors.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 57 ++++++++++++++++++++++++++++--------------------\n 1 file changed, 33 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 0fb46ade19f..f8a24f1aa26 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -303,10 +303,15 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \t\treturn 1;\n \n \tif (timestamp < cb->cmd.expire_unreachable) {\n-\t\tif (cb->unreachable_expire_kind == UE_ALWAYS)\n-\t\t\treturn 1;\n-\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n+\t\tswitch (cb->unreachable_expire_kind) {\n+\t\tcase UE_ALWAYS:\n \t\t\treturn 1;\n+\t\tcase UE_NORMAL:\n+\t\tcase UE_HEAD:\n+\t\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n+\t\t\t\treturn 1;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n@@ -348,6 +353,7 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\t\t  void *cb_data)\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct commit_list *elem;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n@@ -363,34 +369,37 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n-\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\n-\t\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n-\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n-\t\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n-\t\t} else {\n-\t\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n-\t\t}\n-\t\tcb->mark_limit = cb->cmd.expire_total;\n-\t\tmark_reachable(cb);\n+\tswitch (cb->unreachable_expire_kind) {\n+\tcase UE_ALWAYS:\n+\t\treturn;\n+\tcase UE_HEAD:\n+\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n+\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n+\t\tbreak;\n+\tcase UE_NORMAL:\n+\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n \t}\n+\tcb->mark_limit = cb->cmd.expire_total;\n+\tmark_reachable(cb);\n }\n \n static void reflog_expiry_cleanup(void *cb_data)\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct commit_list *elem;\n \n-\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n-\t\t\t\tclear_commit_marks(elem->item, REACHABLE);\n-\t\t\tfree_commit_list(cb->tips);\n-\t\t} else {\n-\t\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n-\t\t}\n+\tswitch (cb->unreachable_expire_kind) {\n+\tcase UE_ALWAYS:\n+\t\treturn;\n+\tcase UE_HEAD:\n+\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\tclear_commit_marks(elem->item, REACHABLE);\n+\t\tfree_commit_list(cb->tips);\n+\t\tbreak;\n+\tcase UE_NORMAL:\n+\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n+\t\tbreak;\n \t}\n }\n \n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442699","messageId":"patch-05.12-de162a476c1-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 05/12] reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:07Z","receivedAt":"2021-11-30T21:38:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an intermediate variable for \"tip_commit\" in\nreflog_expiry_prepare(), and only add it to the struct if we're\nhandling the UE_NORMAL case.\n\nThe code behaves the same way as before, but this makes the control\nflow clearer, and the shorter name allows us to fold a 4-line i/else\nint a one-line terany instead.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex f8a24f1aa26..ec0c6051135 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -354,16 +354,14 @@ static void reflog_expiry_prepare(const char *refname,\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \tstruct commit_list *elem;\n+\tstruct commit *commit = NULL;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n-\t\tcb->tip_commit = lookup_commit_reference_gently(the_repository,\n-\t\t\t\t\t\t\t\toid, 1);\n-\t\tif (!cb->tip_commit)\n-\t\t\tcb->unreachable_expire_kind = UE_ALWAYS;\n-\t\telse\n-\t\t\tcb->unreachable_expire_kind = UE_NORMAL;\n+\t\tcommit = lookup_commit_reference_gently(the_repository,\n+\t\t\t\t\t\t\toid, 1);\n+\t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n@@ -378,7 +376,9 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n \t\tbreak;\n \tcase UE_NORMAL:\n-\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n+\t\tcommit_list_insert(commit, &cb->mark_list);\n+\t\t/* For reflog_expiry_cleanup() below */\n+\t\tcb->tip_commit = commit;\n \t}\n \tcb->mark_limit = cb->cmd.expire_total;\n \tmark_reachable(cb);\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442700","messageId":"patch-06.12-eb3dd3fa8b9-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 06/12] reflog expire: don't use lookup_commit_reference_gently()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:08Z","receivedAt":"2021-11-30T21:38:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In the initial implementation of \"git reflog\" in 4264dc15e19 (git\nreflog expire, 2006-12-19) we had this\nlookup_commit_reference_gently().\n\nI don't think we've ever found tags that we need to recursively\ndereference in reflogs, so this should at least be changed to a\n\"lookup commit\" as I'm doing here, although I can't think of a way\nwhere it mattered in practice.\n\nI also think we'd probably like to just die here if we have a NULL\nobject, but as this code needs to handle potentially broken\nrepositories let's just show an \"error\" but continue, the non-quiet\nlookup_commit() will do for us. None of our tests cover the case where\n\"commit\" is NULL after this lookup.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex ec0c6051135..29dcd91abca 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -359,8 +359,7 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n-\t\tcommit = lookup_commit_reference_gently(the_repository,\n-\t\t\t\t\t\t\toid, 1);\n+\t\tcommit = lookup_commit(the_repository, oid);\n \t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442701","messageId":"patch-07.12-3aab4a4a436-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 07/12] reflog: reduce scope of \"struct rev_info\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:09Z","receivedAt":"2021-11-30T21:38:51Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cmd.stalefix\" handling added in 1389d9ddaa6 (reflog expire\n--fix-stale, 2007-01-06) to use a locally scoped \"struct\nrev_info\". This code relies on mark_reachable_objects() twiddling\nflags in the walked objects.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 29dcd91abca..48e4f5887b0 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -28,7 +28,6 @@ static timestamp_t default_reflog_expire;\n static timestamp_t default_reflog_expire_unreachable;\n \n struct cmd_reflog_expire_cb {\n-\tstruct rev_info revs;\n \tint stalefix;\n \ttimestamp_t expire_total;\n \ttimestamp_t expire_unreachable;\n@@ -594,13 +593,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * from reflog if the repository was pruned with older git.\n \t */\n \tif (cmd.stalefix) {\n-\t\trepo_init_revisions(the_repository, &cmd.revs, prefix);\n-\t\tcmd.revs.do_not_die_on_missing_tree = 1;\n-\t\tcmd.revs.ignore_missing = 1;\n-\t\tcmd.revs.ignore_missing_links = 1;\n+\t\tstruct rev_info revs;\n+\n+\t\trepo_init_revisions(the_repository, &revs, prefix);\n+\t\trevs.do_not_die_on_missing_tree = 1;\n+\t\trevs.ignore_missing = 1;\n+\t\trevs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&cmd.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442702","messageId":"patch-09.12-6a8f3915898-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 09/12] reflog + refs-backend: move \"verbose\" out of the backend","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:11Z","receivedAt":"2021-11-30T21:38:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the handling of the \"verbose\" flag entirely out of\n\"refs/files-backend.c\" and into \"builtin/reflog.c\". This allows the\nbackend to stop knowing about the EXPIRE_REFLOGS_VERBOSE flag.\n\nThe expire_reflog_ent() function shouldn't need to deal with the\nimplementation detail of whether or not we're emitting verbose output,\nby doing this the --verbose output becomes backend-agnostic, so\nreftable will get the same output.\n\nI think the output is rather bad currently, and should e.g. be\nimplemented with some better future mode of progress.[ch], but that's\na topic for another improvement.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c     | 42 +++++++++++++++++++++++++++++++-----------\n refs.h               |  3 +--\n refs/files-backend.c | 44 ++++++++++++++++++++------------------------\n 3 files changed, 52 insertions(+), 37 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 48e4f5887b0..a77c0d96dce 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -45,6 +45,8 @@ struct expire_reflog_policy_cb {\n \tstruct cmd_reflog_expire_cb cmd;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n+\tunsigned int dry_run:1,\n+\t\t     verbose:1;\n };\n \n struct worktree_reflogs {\n@@ -294,29 +296,38 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \tstruct commit *old_commit, *new_commit;\n \n \tif (timestamp < cb->cmd.expire_total)\n-\t\treturn 1;\n+\t\tgoto expire;\n \n \told_commit = new_commit = NULL;\n \tif (cb->cmd.stalefix &&\n \t    (!keep_entry(&old_commit, ooid) || !keep_entry(&new_commit, noid)))\n-\t\treturn 1;\n+\t\tgoto expire;\n \n \tif (timestamp < cb->cmd.expire_unreachable) {\n \t\tswitch (cb->unreachable_expire_kind) {\n \t\tcase UE_ALWAYS:\n-\t\t\treturn 1;\n+\t\t\tgoto expire;\n \t\tcase UE_NORMAL:\n \t\tcase UE_HEAD:\n \t\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n-\t\t\t\treturn 1;\n+\t\t\t\tgoto expire;\n \t\t\tbreak;\n \t\t}\n \t}\n \n \tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n-\t\treturn 1;\n+\t\tgoto expire;\n+\n+\tif (cb->verbose)\n+\t\tprintf(\"keep %s\", message);\n \n \treturn 0;\n+expire:\n+\tif (cb->dry_run)\n+\t\tprintf(\"would prune %s\", message);\n+\telse if (cb->verbose)\n+\t\tprintf(\"prune %s\", message);\n+\treturn 1;\n }\n \n static int push_tip_to_list(const char *refname, const struct object_id *oid,\n@@ -539,6 +550,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tint i, status, do_all, all_worktrees = 1;\n \tint explicit_expiry = 0;\n \tunsigned int flags = 0;\n+\tint verbose = 0;\n \n \tdefault_reflog_expire_unreachable = now - 30 * 24 * 3600;\n \tdefault_reflog_expire = now - 90 * 24 * 3600;\n@@ -576,7 +588,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--single-worktree\"))\n \t\t\tall_worktrees = 0;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n+\t\t\tverbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -599,10 +611,10 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\trevs.do_not_die_on_missing_tree = 1;\n \t\trevs.ignore_missing = 1;\n \t\trevs.ignore_missing_links = 1;\n-\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\tif (verbose)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n \t\tmark_reachable_objects(&revs, 0, 0, NULL);\n-\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\tif (verbose)\n \t\t\tputchar('\\n');\n \t}\n \n@@ -624,7 +636,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \n \t\tfor_each_string_list_item(item, &collected.reflogs) {\n-\t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n+\t\t\tstruct expire_reflog_policy_cb cb = {\n+\t\t\t\t.cmd = cmd,\n+\t\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n+\t\t\t\t.verbose = verbose,\n+\t\t\t};\n \n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n \t\t\tstatus |= reflog_expire(item->string, flags,\n@@ -671,6 +687,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \tstruct cmd_reflog_expire_cb cmd = { 0 };\n \tint i, status = 0;\n \tunsigned int flags = 0;\n+\tint verbose = 0;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -681,7 +698,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--updateref\"))\n \t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n+\t\t\tverbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -699,7 +716,10 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tconst char *spec = strstr(argv[i], \"@{\");\n \t\tchar *ep, *ref;\n \t\tint recno;\n-\t\tstruct expire_reflog_policy_cb cb = { 0 };\n+\t\tstruct expire_reflog_policy_cb cb = {\n+\t\t\t.verbose = verbose,\n+\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n+\t\t};\n \n \t\tif (!spec) {\n \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\ndiff --git a/refs.h b/refs.h\nindex 45c34e99e3a..0c3374b405c 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -786,8 +786,7 @@ enum ref_type ref_type(const char *refname);\n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n \tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n-\tEXPIRE_REFLOGS_VERBOSE = 1 << 2,\n-\tEXPIRE_REFLOGS_REWRITE = 1 << 3\n+\tEXPIRE_REFLOGS_REWRITE = 1 << 2,\n };\n \n /*\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 451c4e2a052..c154c3c4a23 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3080,11 +3080,12 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,\n }\n \n struct expire_reflog_cb {\n-\tunsigned int flags;\n \treflog_expiry_should_prune_fn *should_prune_fn;\n \tvoid *policy_cb;\n \tFILE *newlog;\n \tstruct object_id last_kept_oid;\n+\tunsigned int rewrite:1,\n+\t\t     dry_run:1;\n };\n \n static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n@@ -3092,33 +3093,27 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \t\t\t     const char *message, void *cb_data)\n {\n \tstruct expire_reflog_cb *cb = cb_data;\n-\tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n+\treflog_expiry_should_prune_fn *fn = cb->should_prune_fn;\n \n-\tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n+\tif (cb->rewrite)\n \t\tooid = &cb->last_kept_oid;\n \n-\tif ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,\n-\t\t\t\t   message, policy_cb)) {\n-\t\tif (cb->flags & EXPIRE_REFLOGS_DRY_RUN)\n-\t\t\tprintf(\"would prune %s\", message);\n-\t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"prune %s\", message);\n-\t} else {\n-\t\tif (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\",\n-\t\t\t\toid_to_hex(ooid), oid_to_hex(noid),\n-\t\t\t\temail, timestamp, tz, message);\n-\t\t\toidcpy(&cb->last_kept_oid, noid);\n-\t\t}\n-\t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"keep %s\", message);\n-\t}\n+\tif (fn(ooid, noid, email, timestamp, tz, message, cb->policy_cb))\n+\t\treturn 0;\n+\n+\tif (cb->dry_run)\n+\t\treturn 0; /* --dry-run */\n+\n+\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\", oid_to_hex(ooid),\n+\t\toid_to_hex(noid), email, timestamp, tz, message);\n+\toidcpy(&cb->last_kept_oid, noid);\n+\n \treturn 0;\n }\n \n static int files_reflog_expire(struct ref_store *ref_store,\n \t\t\t       const char *refname,\n-\t\t\t       unsigned int flags,\n+\t\t\t       unsigned int expire_flags,\n \t\t\t       reflog_expiry_prepare_fn prepare_fn,\n \t\t\t       reflog_expiry_should_prune_fn should_prune_fn,\n \t\t\t       reflog_expiry_cleanup_fn cleanup_fn,\n@@ -3136,7 +3131,8 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \tconst struct object_id *oid;\n \n \tmemset(&cb, 0, sizeof(cb));\n-\tcb.flags = flags;\n+\tcb.rewrite = !!(expire_flags & EXPIRE_REFLOGS_REWRITE);\n+\tcb.dry_run = !!(expire_flags & EXPIRE_REFLOGS_DRY_RUN);\n \tcb.policy_cb = policy_cb_data;\n \tcb.should_prune_fn = should_prune_fn;\n \n@@ -3172,7 +3168,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \tfiles_reflog_path(refs, &log_file_sb, refname);\n \tlog_file = strbuf_detach(&log_file_sb, NULL);\n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\tif (!cb.dry_run) {\n \t\t/*\n \t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n \t\t * no locking implications, we use the lock_file\n@@ -3199,7 +3195,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \trefs_for_each_reflog_ent(ref_store, refname, expire_reflog_ent, &cb);\n \t(*cleanup_fn)(cb.policy_cb);\n \n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\tif (!cb.dry_run) {\n \t\t/*\n \t\t * It doesn't make sense to adjust a reference pointed\n \t\t * to by a symbolic ref based on expiring entries in\n@@ -3209,7 +3205,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \t\t */\n \t\tint update = 0;\n \n-\t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n+\t\tif ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t    !is_null_oid(&cb.last_kept_oid)) {\n \t\t\tint ignore_errno;\n \t\t\tint type;\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442703","messageId":"patch-11.12-794e6e677a8-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 11/12] gc + reflog: emit progress output from \"reflog expire --all\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:13Z","receivedAt":"2021-11-30T21:38:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Emit progress output on \"git reflog expire --all\", which is what \"git\ngc\" runs. On my git.git checkout I'll now get:\n\n    $ time GIT_PROGRESS_DELAY=0 ~/g/git/git reflog expire --all --dry-run\n    Enumerating reflogs: 23782, done.\n    Expiring reflogs: 100% (23782/23782), done.\n\n    real    0m3.264s\n    user    0m2.308s\n    sys     0m0.941s\n\nThe \"Enumerating reflogs\" is too fast to appear for me except with\nGIT_PROGRESS_DELAY=0. We'll also emit this at the top of \"git gc\"\noutput:\n\n    $ ~/g/git/git --exec-path=$PWD gc\n    Expiring reflogs: 100% (23782/23782), done.\n    Enumerating objects: [...]\n\nThis goes a long way (but not quite, a subsequent commit will) to\naddressing the seeming halting of \"git gc\" on startup. That usually\nhappens because of the \"HEAD\" case in \"reflog expire --all\" and\nunreachable().\n\nNote that this code isn't going to be affected by the sort of bug we\nhad to fix in 6b89a34c89f (gc: fix regression in 7b0f229222 impacting\n--quiet, 2018-09-19).\n\nThis is because \"git gc\" even with \"--auto\" won't detach until after\nit runs \"git reflog -expire\", so whatever output we emit will never\nend up in the gc.log. We should still obey its --quiet to mean our\n--no-progress, but we don't need a special-case for \"daemonized\" as\nwrite_commit_graph_reachable() does.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/gc.c     |  4 +++-\n builtin/reflog.c | 19 ++++++++++++++++++-\n 2 files changed, 21 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex bcef6a4c8da..872209e083e 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -593,8 +593,10 @@ int cmd_gc(int argc, const char **argv, const char *prefix)\n \t\tif (aggressive_window > 0)\n \t\t\tstrvec_pushf(&repack, \"--window=%d\", aggressive_window);\n \t}\n-\tif (quiet)\n+\tif (quiet) {\n+\t\tstrvec_push(&reflog, \"--no-progress\");\n \t\tstrvec_push(&repack, \"-q\");\n+\t}\n \n \tif (auto_gc) {\n \t\t/*\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex cf0ef68d82d..1a2a210ecf1 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -53,6 +53,7 @@ struct expire_reflog_policy_cb {\n struct worktree_reflogs {\n \tstruct worktree *worktree;\n \tstruct string_list reflogs;\n+\tstruct progress *progress;\n };\n \n /* Remember to update object flag allocation in object.h */\n@@ -324,7 +325,7 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \n \treturn 0;\n expire:\n-\tif (cb->dry_run)\n+\tif (cb->verbose && cb->dry_run)\n \t\tprintf(\"would prune %s\", message);\n \telse if (cb->verbose)\n \t\tprintf(\"prune %s\", message);\n@@ -426,6 +427,7 @@ static int collect_reflog(const char *ref, const struct object_id *oid, int unus\n \tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n \t\treturn 0;\n \n+\tdisplay_progress(cb->progress, cb->reflogs.nr + 1);\n \tstrbuf_worktree_ref(worktree, &newref, ref);\n \tstring_list_append(&cb->reflogs, strbuf_detach(&newref, NULL));\n \n@@ -627,11 +629,17 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (do_all) {\n+\t\tstruct progress *progress = NULL;\n \t\tstruct worktree_reflogs collected = {\n \t\t\t.reflogs = STRING_LIST_INIT_NODUP,\n \t\t};\n \t\tstruct string_list_item *item;\n \t\tstruct worktree **worktrees, **p;\n+\t\tuint64_t progress_cnt;\n+\n+\t\tif (show_progress)\n+\t\t\tcollected.progress = start_delayed_progress(_(\"Enumerating reflogs\"),\n+\t\t\t\t\t\t\t\t    0);\n \n \t\tworktrees = get_worktrees();\n \t\tfor (p = worktrees; *p; p++) {\n@@ -642,6 +650,13 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t     collect_reflog, &collected);\n \t\t}\n \t\tfree_worktrees(worktrees);\n+\t\tstop_progress(&collected.progress);\n+\n+\t\tif (show_progress) {\n+\t\t\tprogress_cnt = 0;\n+\t\t\tprogress = start_delayed_progress(_(\"Expiring reflogs\"),\n+\t\t\t\t\t\t\t  collected.reflogs.nr);\n+\t\t}\n \n \t\tfor_each_string_list_item(item, &collected.reflogs) {\n \t\t\tstruct expire_reflog_policy_cb cb = {\n@@ -650,6 +665,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\t\t.verbose = verbose,\n \t\t\t};\n \n+\t\t\tdisplay_progress(progress, ++progress_cnt);\n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n \t\t\tstatus |= reflog_expire(item->string, flags,\n \t\t\t\t\t\treflog_expiry_prepare,\n@@ -657,6 +673,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t\t&cb);\n \t\t}\n+\t\tstop_progress(&progress);\n \t\tcollected.reflogs.strdup_strings = 1;\n \t\tstring_list_clear(&collected.reflogs, 0);\n \t}\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442704","messageId":"patch-10.12-f54dee1f1cc-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 10/12] reflog expire: add progress output on --stale-fix","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:12Z","receivedAt":"2021-11-30T21:38:56Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add progress output when the \"git reflog expire --stale-fix\" option is\nused. This output was previously only emitted under --verbose, but we\nshouldn't treat it the same way as the actually verbose \"--verbose\"\noutput emitted in should_expire_reflog_ent().\n\nNote that this code isn't going to be affected by the sort of bug we\nhad to fix in 6b89a34c89f (gc: fix regression in 7b0f229222 impacting\n--quiet, 2018-09-19). I.e. \"git gc\" won't call it with the\n\"--stale-fix\" flag, that option is purely used as a one-off.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Documentation/git-reflog.txt |  8 ++++++++\n builtin/reflog.c             | 18 +++++++++++++-----\n 2 files changed, 21 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt\nindex ff487ff77d3..1735bbea9fb 100644\n--- a/Documentation/git-reflog.txt\n+++ b/Documentation/git-reflog.txt\n@@ -69,6 +69,14 @@ Options for `show`\n Options for `expire`\n ~~~~~~~~~~~~~~~~~~~~\n \n+--progress::\n+--no-progress::\n+\tProgress status is reported on the standard error stream by\n+\tdefault when it is attached to a terminal. The `--progress\n+\tflag enables progress reporting even if not attached to a\n+\tterminal. Supplying `--no-progress` will suppress all progress\n+\toutput.\n+\n --all::\n \tProcess the reflogs of all references.\n \ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex a77c0d96dce..cf0ef68d82d 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -11,6 +11,7 @@\n #include \"revision.h\"\n #include \"reachable.h\"\n #include \"worktree.h\"\n+#include \"progress.h\"\n \n /* NEEDSWORK: switch to using parse_options */\n static const char reflog_expire_usage[] =\n@@ -551,6 +552,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tint explicit_expiry = 0;\n \tunsigned int flags = 0;\n \tint verbose = 0;\n+\tint show_progress = -1;\n \n \tdefault_reflog_expire_unreachable = now - 30 * 24 * 3600;\n \tdefault_reflog_expire = now - 90 * 24 * 3600;\n@@ -579,6 +581,10 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t}\n \t\telse if (!strcmp(arg, \"--stale-fix\"))\n \t\t\tcmd.stalefix = 1;\n+\t\telse if (!strcmp(arg, \"--progress\"))\n+\t\t\tshow_progress = 1;\n+\t\telse if (!strcmp(arg, \"--no-progress\"))\n+\t\t\tshow_progress = 0;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n@@ -598,6 +604,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse\n \t\t\tbreak;\n \t}\n+\tif (show_progress == -1)\n+\t\tshow_progress = isatty(2);\n \n \t/*\n \t * We can trust the commits and objects reachable from refs\n@@ -606,16 +614,16 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t */\n \tif (cmd.stalefix) {\n \t\tstruct rev_info revs;\n+\t\tstruct progress *progress = NULL;\n \n+\t\tif (show_progress)\n+\t\t\tprogress = start_delayed_progress(_(\"Marking reachable objects\"), 0);\n \t\trepo_init_revisions(the_repository, &revs, prefix);\n \t\trevs.do_not_die_on_missing_tree = 1;\n \t\trevs.ignore_missing = 1;\n \t\trevs.ignore_missing_links = 1;\n-\t\tif (verbose)\n-\t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&revs, 0, 0, NULL);\n-\t\tif (verbose)\n-\t\t\tputchar('\\n');\n+\t\tmark_reachable_objects(&revs, 0, 0, progress);\n+\t\tstop_progress(&progress);\n \t}\n \n \tif (do_all) {\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442705","messageId":"patch-08.12-adbec242a7a-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 08/12] refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:10Z","receivedAt":"2021-11-30T21:38:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"It's not possible for \"cb->newlog\" to be NULL if\n!EXPIRE_REFLOGS_DRY_RUN, since files_reflog_expire() would have\nerror()'d and taken the \"goto failure\" branch if it couldn't open the\nfile. By not using the \"newlog\" field private to \"file-backend.c\"'s\n\"struct expire_reflog_cb\", we can move this verbosity logging to\n\"builtin/reflog.c\" in a subsequent commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs/files-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 4b14f30d48f..451c4e2a052 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3099,12 +3099,12 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \n \tif ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,\n \t\t\t\t   message, policy_cb)) {\n-\t\tif (!cb->newlog)\n+\t\tif (cb->flags & EXPIRE_REFLOGS_DRY_RUN)\n \t\t\tprintf(\"would prune %s\", message);\n \t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n-\t\tif (cb->newlog) {\n+\t\tif (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\t\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\",\n \t\t\t\toid_to_hex(ooid), oid_to_hex(noid),\n \t\t\t\temail, timestamp, tz, message);\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"442706","messageId":"patch-12.12-fc2b15d0abe-20211130T213319Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH 12/12] gc + reflog: don't stall before initial \"git gc\" progress output","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-30T21:38:14Z","receivedAt":"2021-11-30T21:39:02Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"git reflog expire --all\" progress output to show progress\non mark_reachable() in addition to the \"Expiring reflogs\" output added\nin the preceding commit.\n\nThis is a major UI improvement to the \"git gc\" progress output, which\nin large repositories would seem to hang before the \"initial\"\n\"Enumerating objects\" phase. E.g. on a linux.git checkout I've got it\npreviously took me around 10 seconds before I saw any output.\n\nWith the change to add the \"Expiring reflogs\" output in the preceding\ncommit that ~10 second delay was perhaps ~8-9, now it's pretty much\nguaranteed to show output within the first couple of seconds (our\ndefault progress delay).\n\nWhy? Because as we iterate through our reflogs to expire in\nreflog_exire() we'd always check the reflog for \"HEAD\" first, and as\nwe hadn't previously marked anything REACHABLE in mark_reachable()\nwould almost certainly end up needing to walk a substantial amount of\nhistory to mark commits. In linux.git north of 1 million commits.\n\nNow we'll instead show:\n\n    $ git gc\n    Iterating (un)reachable HEAD: 1137354, done.\n    Expiring reflogs: 100% (45/45), done.\n    ^Enmerating objects: 662499^C\n    [...]\n\nThe implementation is a bit of a hack. Ideally we'd support nested\nprogress bars, and the ability to \"attach\" a sub-progress bar to a\nrunning one. I.e. to optimistically show either:\n\n    Expiring reflogs: X (Y/Z)\n\nOr:\n\n    Expiring reflogs: X (Y/Z) iterating unreachable HEAD: 123456\n\nIn this case the problem is that if we have done our initial big\n\"REACHABLE\" iteration we're usually going to process the reflog quite\nquickly, but if we haven't we'd stall.\n\nSo as a hack pass down a \"show_progress\" to reflog_expiry_prepare(),\nwhich will only be shown if we haven't processed the UE_HEAD\ncase. We'll then start a progress bar, which when passed through to\nmark_reachable() will almost certainly result in a substantial initial\niteration.\n\nThen when we've done that initial walk we'll stop that progress bar in\nour main loop, and start the \"Expiring reflogs\" progress bar. We'll\nusually start it at 2/X, since our \"HEAD\" was the 1/X usurped by the\n\"Iterating (un)reachable HEAD\" progress bar.\n\nThe \"usually\" would assume that we wouldn't hit the\n\"cb->no_reachable_progress = 1\" case being added here. I.e. if our\nconfiguration is such that we're not going to do the UE_HEAD walk\nwe'll just fall back on only using \"Expiring reflogs\". We'll still\nstart the count at the 2nd reflog, but it shouldn't matter, on e.g. my\nlinux.git checkout it doesn't, the progress bar goes by too fast to\nnotice.\n\nIt would have been nicer to be able to compute the\n\"cb->unreachable_expire_kind\" before we call reflog_expire(), but to\ndo that we'd need the \"oid\", which we'll only know once we lock it.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 53 +++++++++++++++++++++++++++++++++---------------\n 1 file changed, 37 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 1a2a210ecf1..ef4f039a20c 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -47,7 +47,11 @@ struct expire_reflog_policy_cb {\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n \tunsigned int dry_run:1,\n-\t\t     verbose:1;\n+\t\t     verbose:1,\n+\t\t     show_progress:1,\n+\t\t     no_reachable_progress:1;\n+\tstruct progress *reachable_progress;\n+\tuint64_t reachable_progress_cnt;\n };\n \n struct worktree_reflogs {\n@@ -235,6 +239,8 @@ static void mark_reachable(struct expire_reflog_policy_cb *cb)\n \twhile (pending) {\n \t\tstruct commit_list *parent;\n \t\tstruct commit *commit = pop_commit(&pending);\n+\n+\t\tdisplay_progress(cb->reachable_progress, ++cb->reachable_progress_cnt);\n \t\tif (commit->object.flags & REACHABLE)\n \t\t\tcontinue;\n \t\tif (parse_commit(commit))\n@@ -375,16 +381,27 @@ static void reflog_expiry_prepare(const char *refname,\n \t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n-\tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n+\tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total) {\n+\t\tif (cb->show_progress &&\n+\t\t    cb->unreachable_expire_kind == UE_HEAD)\n+\t\t\tcb->no_reachable_progress = 1;\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n+\t}\n \n \tswitch (cb->unreachable_expire_kind) {\n \tcase UE_ALWAYS:\n \t\treturn;\n \tcase UE_HEAD:\n+\t\tif (cb->show_progress) {\n+\t\t\tconst char *s = _(\"Iterating (un)reachable HEAD\");\n+\t\t\tcb->reachable_progress = start_delayed_progress(s, 0);\n+\t\t}\n \t\tfor_each_ref(push_tip_to_list, &cb->tips);\n-\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\tfor (elem = cb->tips; elem; elem = elem->next) {\n+\t\t\tdisplay_progress(cb->reachable_progress,\n+\t\t\t\t\t ++cb->reachable_progress_cnt);\n \t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n+\t\t}\n \t\tbreak;\n \tcase UE_NORMAL:\n \t\tcommit_list_insert(commit, &cb->mark_list);\n@@ -633,9 +650,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tstruct worktree_reflogs collected = {\n \t\t\t.reflogs = STRING_LIST_INIT_NODUP,\n \t\t};\n-\t\tstruct string_list_item *item;\n \t\tstruct worktree **worktrees, **p;\n-\t\tuint64_t progress_cnt;\n+\t\tint show_head_progress = show_progress;\n+\t\tsize_t j;\n \n \t\tif (show_progress)\n \t\t\tcollected.progress = start_delayed_progress(_(\"Enumerating reflogs\"),\n@@ -652,27 +669,31 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \t\tstop_progress(&collected.progress);\n \n-\t\tif (show_progress) {\n-\t\t\tprogress_cnt = 0;\n-\t\t\tprogress = start_delayed_progress(_(\"Expiring reflogs\"),\n-\t\t\t\t\t\t\t  collected.reflogs.nr);\n-\t\t}\n-\n-\t\tfor_each_string_list_item(item, &collected.reflogs) {\n+\t\tfor (j = 0; j < collected.reflogs.nr; j++) {\n+\t\t\tconst char *string = collected.reflogs.items[j].string;\n \t\t\tstruct expire_reflog_policy_cb cb = {\n \t\t\t\t.cmd = cmd,\n \t\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n \t\t\t\t.verbose = verbose,\n+\t\t\t\t.show_progress = show_head_progress,\n \t\t\t};\n-\n-\t\t\tdisplay_progress(progress, ++progress_cnt);\n-\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n-\t\t\tstatus |= reflog_expire(item->string, flags,\n+\t\t\tdisplay_progress(progress, j + 1);\n+\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, string);\n+\t\t\tstatus |= reflog_expire(string, flags,\n \t\t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\t\tshould_expire_reflog_ent,\n \t\t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t\t&cb);\n+\n+\t\t\tif (show_head_progress &&\n+\t\t\t    (cb.reachable_progress || cb.no_reachable_progress)) {\n+\t\t\t\tstop_progress(&cb.reachable_progress);\n+\t\t\t\tshow_head_progress = 0;\n+\t\t\t\tprogress = start_delayed_progress(_(\"Expiring reflogs\"),\n+\t\t\t\t\t\t\t\t  collected.reflogs.nr);\n+\t\t\t}\n \t\t}\n+\t\tdisplay_progress(progress, collected.reflogs.nr); /* only HEAD? */\n \t\tstop_progress(&progress);\n \t\tcollected.reflogs.strdup_strings = 1;\n \t\tstring_list_clear(&collected.reflogs, 0);\n-- \n2.34.1.877.g7d5b0a3b8a6\n\n"},{"id":"444209","messageId":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com","subject":"[PATCH v2 0/9] reftable prep: have builtin/reflog.c handle \"--verbose\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:09Z","receivedAt":"2021-12-16T13:45:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series refactors various small bits of builtin/reflog.c\n(e.g. using a \"struct string_list\" now), and finally makes it handle\nthe \"--verbose\" output, instead of telling the files backend to emit\nthat same verbose output.\n\nThis means that when we start to integrate \"reftable\" the new backend\nwon't need to implement verbose reflog output, it will just work.\n\nThis is a sort-of v2[1]. I ejected the changes at the end to add\nbetter --progress output to \"git reflog\". Those fixes are worthwhile,\nbut hopefully this smaller & easier to review series can be queued up\nfirst, we can do those UX improvements later.\n\n1. https://lore.kernel.org/git/cover-00.12-00000000000-20211130T213319Z-avarab@gmail.com/\n\nÆvar Arnfjörð Bjarmason (9):\n  reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()\n  reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()\n  reflog: change one->many worktree->refnames to use a string_list\n  reflog expire: don't do negative comparison on enum values\n  reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL\n  reflog expire: don't use lookup_commit_reference_gently()\n  reflog: reduce scope of \"struct rev_info\"\n  refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN\n  reflog + refs-backend: move \"verbose\" out of the backend\n\n builtin/reflog.c     | 208 +++++++++++++++++++++++--------------------\n refs.h               |   3 +-\n refs/files-backend.c |  44 +++++----\n 3 files changed, 134 insertions(+), 121 deletions(-)\n\nRange-diff against v1:\n 1:  99e8a639163 =  1:  22c8119640c reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()\n 2:  c424b26b4fe =  2:  b8e84538427 reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()\n 3:  5a54b04a13e =  3:  c0e190e46cf reflog: change one->many worktree->refnames to use a string_list\n 4:  a7a2dfd1406 =  4:  e42fac1b518 reflog expire: don't do negative comparison on enum values\n 5:  de162a476c1 =  5:  39263cd00ae reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL\n 6:  eb3dd3fa8b9 =  6:  c71aab5845e reflog expire: don't use lookup_commit_reference_gently()\n 7:  3aab4a4a436 =  7:  2fb33ef2546 reflog: reduce scope of \"struct rev_info\"\n 8:  adbec242a7a =  8:  f9fe6a2cfb0 refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN\n 9:  6a8f3915898 =  9:  28aa0aa6e30 reflog + refs-backend: move \"verbose\" out of the backend\n10:  f54dee1f1cc <  -:  ----------- reflog expire: add progress output on --stale-fix\n11:  794e6e677a8 <  -:  ----------- gc + reflog: emit progress output from \"reflog expire --all\"\n12:  fc2b15d0abe <  -:  ----------- gc + reflog: don't stall before initial \"git gc\" progress output\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444210","messageId":"patch-v2-1.9-22c8119640c-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 1/9] reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:10Z","receivedAt":"2021-12-16T13:45:25Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cb_data\" we pass to the count_reflog_ent() to be the\n&cb.cmd itself, instead of passing &cb and having the callback lookup\ncb->cmd.\n\nThis makes it clear that the \"cb\" itself is the same memzero'd\nstructure on each iteration of the for-loop that uses &cb, except for\nthe \"cmd\" member.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 175c83e7cc2..4c15d71f3e9 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -662,20 +662,18 @@ static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *email, timestamp_t timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n-\tstruct expire_reflog_policy_cb *cb = cb_data;\n-\tif (!cb->cmd.expire_total || timestamp < cb->cmd.expire_total)\n-\t\tcb->cmd.recno++;\n+\tstruct cmd_reflog_expire_cb *cb = cb_data;\n+\tif (!cb->expire_total || timestamp < cb->expire_total)\n+\t\tcb->recno++;\n \treturn 0;\n }\n \n static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n {\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct cmd_reflog_expire_cb cmd = { 0 };\n \tint i, status = 0;\n \tunsigned int flags = 0;\n \n-\tmemset(&cb, 0, sizeof(cb));\n-\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n@@ -703,6 +701,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tconst char *spec = strstr(argv[i], \"@{\");\n \t\tchar *ep, *ref;\n \t\tint recno;\n+\t\tstruct expire_reflog_policy_cb cb = { 0 };\n \n \t\tif (!spec) {\n \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\n@@ -716,14 +715,15 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \n \t\trecno = strtoul(spec + 2, &ep, 10);\n \t\tif (*ep == '}') {\n-\t\t\tcb.cmd.recno = -recno;\n-\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n+\t\t\tcmd.recno = -recno;\n+\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cmd);\n \t\t} else {\n-\t\t\tcb.cmd.expire_total = approxidate(spec + 2);\n-\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n-\t\t\tcb.cmd.expire_total = 0;\n+\t\t\tcmd.expire_total = approxidate(spec + 2);\n+\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cmd);\n+\t\t\tcmd.expire_total = 0;\n \t\t}\n \n+\t\tcb.cmd = cmd;\n \t\tstatus |= reflog_expire(ref, flags,\n \t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\tshould_expire_reflog_ent,\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444211","messageId":"patch-v2-2.9-b8e84538427-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 2/9] reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:11Z","receivedAt":"2021-12-16T13:45:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As with the preceding change for \"reflog delete\", change the \"cb_data\"\nwe pass to callbacks to be &cb.cmd itself, instead of passing &cb and\nhaving the callback lookup cb->cmd.\n\nThis makes it clear that the \"cb\" itself is the same memzero'd\nstructure on each iteration of the for-loops that use &cb, except for\nthe \"cmd\" member.\n\nThe \"struct expire_reflog_policy_cb\" we pass to reflog_expire() will\nhave the members that aren't \"cmd\" modified by the callbacks, but\nbefore we invoke them everything except \"cmd\" is zero'd out.\n\nThis included the \"tip_commit\", \"mark_list\" and \"tips\". It might have\nlooked as though we were re-using those between iterations, but the\nfirst thing we did in reflog_expiry_prepare() was to either NULL them,\nor clobber them with another value.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 31 +++++++++++++++----------------\n 1 file changed, 15 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 4c15d71f3e9..6989492bf5c 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -357,7 +357,6 @@ static void reflog_expiry_prepare(const char *refname,\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n-\t\tcb->tip_commit = NULL;\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n \t\tcb->tip_commit = lookup_commit_reference_gently(the_repository,\n@@ -371,8 +370,6 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n-\tcb->mark_list = NULL;\n-\tcb->tips = NULL;\n \tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n \t\t\tstruct commit_list *elem;\n@@ -541,7 +538,7 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c\n \n static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n {\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct cmd_reflog_expire_cb cmd = { 0 };\n \ttimestamp_t now = time(NULL);\n \tint i, status, do_all, all_worktrees = 1;\n \tint explicit_expiry = 0;\n@@ -553,10 +550,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tsave_commit_buffer = 0;\n \tdo_all = status = 0;\n-\tmemset(&cb, 0, sizeof(cb));\n \n-\tcb.cmd.expire_total = default_reflog_expire;\n-\tcb.cmd.expire_unreachable = default_reflog_expire_unreachable;\n+\tcmd.expire_total = default_reflog_expire;\n+\tcmd.expire_unreachable = default_reflog_expire_unreachable;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -564,17 +560,17 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n \t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (skip_prefix(arg, \"--expire=\", &arg)) {\n-\t\t\tif (parse_expiry_date(arg, &cb.cmd.expire_total))\n+\t\t\tif (parse_expiry_date(arg, &cmd.expire_total))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_TOTAL;\n \t\t}\n \t\telse if (skip_prefix(arg, \"--expire-unreachable=\", &arg)) {\n-\t\t\tif (parse_expiry_date(arg, &cb.cmd.expire_unreachable))\n+\t\t\tif (parse_expiry_date(arg, &cmd.expire_unreachable))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_UNREACH;\n \t\t}\n \t\telse if (!strcmp(arg, \"--stale-fix\"))\n-\t\t\tcb.cmd.stalefix = 1;\n+\t\t\tcmd.stalefix = 1;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n@@ -600,14 +596,14 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * even in older repository.  We cannot trust what's reachable\n \t * from reflog if the repository was pruned with older git.\n \t */\n-\tif (cb.cmd.stalefix) {\n-\t\trepo_init_revisions(the_repository, &cb.cmd.revs, prefix);\n-\t\tcb.cmd.revs.do_not_die_on_missing_tree = 1;\n-\t\tcb.cmd.revs.ignore_missing = 1;\n-\t\tcb.cmd.revs.ignore_missing_links = 1;\n+\tif (cmd.stalefix) {\n+\t\trepo_init_revisions(the_repository, &cmd.revs, prefix);\n+\t\tcmd.revs.do_not_die_on_missing_tree = 1;\n+\t\tcmd.revs.ignore_missing = 1;\n+\t\tcmd.revs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&cb.cmd.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&cmd.revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n@@ -629,6 +625,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n+\t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n \n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n \t\t\tstatus |= reflog_expire(e->reflog, flags,\n@@ -643,6 +640,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tfor (; i < argc; i++) {\n \t\tchar *ref;\n+\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n+\n \t\tif (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) {\n \t\t\tstatus |= error(_(\"%s points nowhere!\"), argv[i]);\n \t\t\tcontinue;\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444212","messageId":"patch-v2-3.9-c0e190e46cf-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 3/9] reflog: change one->many worktree->refnames to use a string_list","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:12Z","receivedAt":"2021-12-16T13:45:26Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the FLEX_ARRAY pattern added in bda3a31cc79 (reflog-expire:\nAvoid creating new files in a directory inside readdir(3) loop,\n2008-01-25) the string-list API instead.\n\nThis does not change any behavior, allows us to delete much of this\ncode as it's replaced by things we get from the string-list API for\nfree, as a result we need just one struct to keep track of this data,\ninstead of two.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 48 +++++++++++++++++++-----------------------------\n 1 file changed, 19 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 6989492bf5c..0fb46ade19f 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -48,16 +48,9 @@ struct expire_reflog_policy_cb {\n \tstruct commit_list *tips;\n };\n \n-struct collected_reflog {\n-\tstruct object_id oid;\n-\tchar reflog[FLEX_ARRAY];\n-};\n-\n-struct collect_reflog_cb {\n-\tstruct collected_reflog **e;\n-\tint alloc;\n-\tint nr;\n-\tstruct worktree *wt;\n+struct worktree_reflogs {\n+\tstruct worktree *worktree;\n+\tstruct string_list reflogs;\n };\n \n /* Remember to update object flag allocation in object.h */\n@@ -403,24 +396,20 @@ static void reflog_expiry_cleanup(void *cb_data)\n \n static int collect_reflog(const char *ref, const struct object_id *oid, int unused, void *cb_data)\n {\n-\tstruct collected_reflog *e;\n-\tstruct collect_reflog_cb *cb = cb_data;\n+\tstruct worktree_reflogs *cb = cb_data;\n+\tstruct worktree *worktree = cb->worktree;\n \tstruct strbuf newref = STRBUF_INIT;\n \n \t/*\n \t * Avoid collecting the same shared ref multiple times because\n \t * they are available via all worktrees.\n \t */\n-\tif (!cb->wt->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n+\tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n \t\treturn 0;\n \n-\tstrbuf_worktree_ref(cb->wt, &newref, ref);\n-\tFLEX_ALLOC_STR(e, reflog, newref.buf);\n-\tstrbuf_release(&newref);\n+\tstrbuf_worktree_ref(worktree, &newref, ref);\n+\tstring_list_append(&cb->reflogs, strbuf_detach(&newref, NULL));\n \n-\toidcpy(&e->oid, oid);\n-\tALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);\n-\tcb->e[cb->nr++] = e;\n \treturn 0;\n }\n \n@@ -609,33 +598,34 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (do_all) {\n-\t\tstruct collect_reflog_cb collected;\n+\t\tstruct worktree_reflogs collected = {\n+\t\t\t.reflogs = STRING_LIST_INIT_NODUP,\n+\t\t};\n+\t\tstruct string_list_item *item;\n \t\tstruct worktree **worktrees, **p;\n-\t\tint i;\n \n-\t\tmemset(&collected, 0, sizeof(collected));\n \t\tworktrees = get_worktrees();\n \t\tfor (p = worktrees; *p; p++) {\n \t\t\tif (!all_worktrees && !(*p)->is_current)\n \t\t\t\tcontinue;\n-\t\t\tcollected.wt = *p;\n+\t\t\tcollected.worktree = *p;\n \t\t\trefs_for_each_reflog(get_worktree_ref_store(*p),\n \t\t\t\t\t     collect_reflog, &collected);\n \t\t}\n \t\tfree_worktrees(worktrees);\n-\t\tfor (i = 0; i < collected.nr; i++) {\n-\t\t\tstruct collected_reflog *e = collected.e[i];\n+\n+\t\tfor_each_string_list_item(item, &collected.reflogs) {\n \t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n \n-\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n-\t\t\tstatus |= reflog_expire(e->reflog, flags,\n+\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n+\t\t\tstatus |= reflog_expire(item->string, flags,\n \t\t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\t\tshould_expire_reflog_ent,\n \t\t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t\t&cb);\n-\t\t\tfree(e);\n \t\t}\n-\t\tfree(collected.e);\n+\t\tcollected.reflogs.strdup_strings = 1;\n+\t\tstring_list_clear(&collected.reflogs, 0);\n \t}\n \n \tfor (; i < argc; i++) {\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444213","messageId":"patch-v2-4.9-e42fac1b518-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 4/9] reflog expire: don't do negative comparison on enum values","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:13Z","receivedAt":"2021-12-16T13:45:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code added in 03cb91b18cc (reflog --expire-unreachable: special\ncase entries in \"HEAD\" reflog, 2010-04-09) to not do positive instead\nof negative comparisons on enum values, i.e. not to assume that \"x !=\nUE_ALWAYS\" means \"(x == UE_HEAD || x || UE_NORMAL)\".\n\nThat assumption is true now, but we'd introduce subtle bugs here if\nthat were to change, now the compiler will notice and error out on\nsuch errors.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 57 ++++++++++++++++++++++++++++--------------------\n 1 file changed, 33 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 0fb46ade19f..f8a24f1aa26 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -303,10 +303,15 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \t\treturn 1;\n \n \tif (timestamp < cb->cmd.expire_unreachable) {\n-\t\tif (cb->unreachable_expire_kind == UE_ALWAYS)\n-\t\t\treturn 1;\n-\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n+\t\tswitch (cb->unreachable_expire_kind) {\n+\t\tcase UE_ALWAYS:\n \t\t\treturn 1;\n+\t\tcase UE_NORMAL:\n+\t\tcase UE_HEAD:\n+\t\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n+\t\t\t\treturn 1;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n@@ -348,6 +353,7 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\t\t  void *cb_data)\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct commit_list *elem;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n@@ -363,34 +369,37 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n-\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\n-\t\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n-\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n-\t\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n-\t\t} else {\n-\t\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n-\t\t}\n-\t\tcb->mark_limit = cb->cmd.expire_total;\n-\t\tmark_reachable(cb);\n+\tswitch (cb->unreachable_expire_kind) {\n+\tcase UE_ALWAYS:\n+\t\treturn;\n+\tcase UE_HEAD:\n+\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n+\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n+\t\tbreak;\n+\tcase UE_NORMAL:\n+\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n \t}\n+\tcb->mark_limit = cb->cmd.expire_total;\n+\tmark_reachable(cb);\n }\n \n static void reflog_expiry_cleanup(void *cb_data)\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct commit_list *elem;\n \n-\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n-\t\t\t\tclear_commit_marks(elem->item, REACHABLE);\n-\t\t\tfree_commit_list(cb->tips);\n-\t\t} else {\n-\t\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n-\t\t}\n+\tswitch (cb->unreachable_expire_kind) {\n+\tcase UE_ALWAYS:\n+\t\treturn;\n+\tcase UE_HEAD:\n+\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\tclear_commit_marks(elem->item, REACHABLE);\n+\t\tfree_commit_list(cb->tips);\n+\t\tbreak;\n+\tcase UE_NORMAL:\n+\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n+\t\tbreak;\n \t}\n }\n \n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444214","messageId":"patch-v2-5.9-39263cd00ae-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 5/9] reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:14Z","receivedAt":"2021-12-16T13:45:36Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an intermediate variable for \"tip_commit\" in\nreflog_expiry_prepare(), and only add it to the struct if we're\nhandling the UE_NORMAL case.\n\nThe code behaves the same way as before, but this makes the control\nflow clearer, and the shorter name allows us to fold a 4-line i/else\nint a one-line terany instead.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex f8a24f1aa26..ec0c6051135 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -354,16 +354,14 @@ static void reflog_expiry_prepare(const char *refname,\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \tstruct commit_list *elem;\n+\tstruct commit *commit = NULL;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n-\t\tcb->tip_commit = lookup_commit_reference_gently(the_repository,\n-\t\t\t\t\t\t\t\toid, 1);\n-\t\tif (!cb->tip_commit)\n-\t\t\tcb->unreachable_expire_kind = UE_ALWAYS;\n-\t\telse\n-\t\t\tcb->unreachable_expire_kind = UE_NORMAL;\n+\t\tcommit = lookup_commit_reference_gently(the_repository,\n+\t\t\t\t\t\t\toid, 1);\n+\t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n@@ -378,7 +376,9 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n \t\tbreak;\n \tcase UE_NORMAL:\n-\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n+\t\tcommit_list_insert(commit, &cb->mark_list);\n+\t\t/* For reflog_expiry_cleanup() below */\n+\t\tcb->tip_commit = commit;\n \t}\n \tcb->mark_limit = cb->cmd.expire_total;\n \tmark_reachable(cb);\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444215","messageId":"patch-v2-7.9-2fb33ef2546-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 7/9] reflog: reduce scope of \"struct rev_info\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:16Z","receivedAt":"2021-12-16T13:45:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cmd.stalefix\" handling added in 1389d9ddaa6 (reflog expire\n--fix-stale, 2007-01-06) to use a locally scoped \"struct\nrev_info\". This code relies on mark_reachable_objects() twiddling\nflags in the walked objects.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 29dcd91abca..48e4f5887b0 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -28,7 +28,6 @@ static timestamp_t default_reflog_expire;\n static timestamp_t default_reflog_expire_unreachable;\n \n struct cmd_reflog_expire_cb {\n-\tstruct rev_info revs;\n \tint stalefix;\n \ttimestamp_t expire_total;\n \ttimestamp_t expire_unreachable;\n@@ -594,13 +593,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * from reflog if the repository was pruned with older git.\n \t */\n \tif (cmd.stalefix) {\n-\t\trepo_init_revisions(the_repository, &cmd.revs, prefix);\n-\t\tcmd.revs.do_not_die_on_missing_tree = 1;\n-\t\tcmd.revs.ignore_missing = 1;\n-\t\tcmd.revs.ignore_missing_links = 1;\n+\t\tstruct rev_info revs;\n+\n+\t\trepo_init_revisions(the_repository, &revs, prefix);\n+\t\trevs.do_not_die_on_missing_tree = 1;\n+\t\trevs.ignore_missing = 1;\n+\t\trevs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&cmd.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444216","messageId":"patch-v2-6.9-c71aab5845e-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 6/9] reflog expire: don't use lookup_commit_reference_gently()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:15Z","receivedAt":"2021-12-16T13:45:37Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In the initial implementation of \"git reflog\" in 4264dc15e19 (git\nreflog expire, 2006-12-19) we had this\nlookup_commit_reference_gently().\n\nI don't think we've ever found tags that we need to recursively\ndereference in reflogs, so this should at least be changed to a\n\"lookup commit\" as I'm doing here, although I can't think of a way\nwhere it mattered in practice.\n\nI also think we'd probably like to just die here if we have a NULL\nobject, but as this code needs to handle potentially broken\nrepositories let's just show an \"error\" but continue, the non-quiet\nlookup_commit() will do for us. None of our tests cover the case where\n\"commit\" is NULL after this lookup.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex ec0c6051135..29dcd91abca 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -359,8 +359,7 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n-\t\tcommit = lookup_commit_reference_gently(the_repository,\n-\t\t\t\t\t\t\toid, 1);\n+\t\tcommit = lookup_commit(the_repository, oid);\n \t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444217","messageId":"patch-v2-8.9-f9fe6a2cfb0-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 8/9] refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:17Z","receivedAt":"2021-12-16T13:45:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"It's not possible for \"cb->newlog\" to be NULL if\n!EXPIRE_REFLOGS_DRY_RUN, since files_reflog_expire() would have\nerror()'d and taken the \"goto failure\" branch if it couldn't open the\nfile. By not using the \"newlog\" field private to \"file-backend.c\"'s\n\"struct expire_reflog_cb\", we can move this verbosity logging to\n\"builtin/reflog.c\" in a subsequent commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs/files-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 90b671025a7..5f8586a36e3 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3105,12 +3105,12 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \n \tif ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,\n \t\t\t\t   message, policy_cb)) {\n-\t\tif (!cb->newlog)\n+\t\tif (cb->flags & EXPIRE_REFLOGS_DRY_RUN)\n \t\t\tprintf(\"would prune %s\", message);\n \t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n-\t\tif (cb->newlog) {\n+\t\tif (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\t\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\",\n \t\t\t\toid_to_hex(ooid), oid_to_hex(noid),\n \t\t\t\temail, timestamp, tz, message);\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444218","messageId":"patch-v2-9.9-28aa0aa6e30-20211216T134028Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v2 9/9] reflog + refs-backend: move \"verbose\" out of the backend","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T13:45:18Z","receivedAt":"2021-12-16T13:45:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the handling of the \"verbose\" flag entirely out of\n\"refs/files-backend.c\" and into \"builtin/reflog.c\". This allows the\nbackend to stop knowing about the EXPIRE_REFLOGS_VERBOSE flag.\n\nThe expire_reflog_ent() function shouldn't need to deal with the\nimplementation detail of whether or not we're emitting verbose output,\nby doing this the --verbose output becomes backend-agnostic, so\nreftable will get the same output.\n\nI think the output is rather bad currently, and should e.g. be\nimplemented with some better future mode of progress.[ch], but that's\na topic for another improvement.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c     | 42 +++++++++++++++++++++++++++++++-----------\n refs.h               |  3 +--\n refs/files-backend.c | 44 ++++++++++++++++++++------------------------\n 3 files changed, 52 insertions(+), 37 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 48e4f5887b0..a77c0d96dce 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -45,6 +45,8 @@ struct expire_reflog_policy_cb {\n \tstruct cmd_reflog_expire_cb cmd;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n+\tunsigned int dry_run:1,\n+\t\t     verbose:1;\n };\n \n struct worktree_reflogs {\n@@ -294,29 +296,38 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \tstruct commit *old_commit, *new_commit;\n \n \tif (timestamp < cb->cmd.expire_total)\n-\t\treturn 1;\n+\t\tgoto expire;\n \n \told_commit = new_commit = NULL;\n \tif (cb->cmd.stalefix &&\n \t    (!keep_entry(&old_commit, ooid) || !keep_entry(&new_commit, noid)))\n-\t\treturn 1;\n+\t\tgoto expire;\n \n \tif (timestamp < cb->cmd.expire_unreachable) {\n \t\tswitch (cb->unreachable_expire_kind) {\n \t\tcase UE_ALWAYS:\n-\t\t\treturn 1;\n+\t\t\tgoto expire;\n \t\tcase UE_NORMAL:\n \t\tcase UE_HEAD:\n \t\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n-\t\t\t\treturn 1;\n+\t\t\t\tgoto expire;\n \t\t\tbreak;\n \t\t}\n \t}\n \n \tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n-\t\treturn 1;\n+\t\tgoto expire;\n+\n+\tif (cb->verbose)\n+\t\tprintf(\"keep %s\", message);\n \n \treturn 0;\n+expire:\n+\tif (cb->dry_run)\n+\t\tprintf(\"would prune %s\", message);\n+\telse if (cb->verbose)\n+\t\tprintf(\"prune %s\", message);\n+\treturn 1;\n }\n \n static int push_tip_to_list(const char *refname, const struct object_id *oid,\n@@ -539,6 +550,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tint i, status, do_all, all_worktrees = 1;\n \tint explicit_expiry = 0;\n \tunsigned int flags = 0;\n+\tint verbose = 0;\n \n \tdefault_reflog_expire_unreachable = now - 30 * 24 * 3600;\n \tdefault_reflog_expire = now - 90 * 24 * 3600;\n@@ -576,7 +588,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--single-worktree\"))\n \t\t\tall_worktrees = 0;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n+\t\t\tverbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -599,10 +611,10 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\trevs.do_not_die_on_missing_tree = 1;\n \t\trevs.ignore_missing = 1;\n \t\trevs.ignore_missing_links = 1;\n-\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\tif (verbose)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n \t\tmark_reachable_objects(&revs, 0, 0, NULL);\n-\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\tif (verbose)\n \t\t\tputchar('\\n');\n \t}\n \n@@ -624,7 +636,11 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \n \t\tfor_each_string_list_item(item, &collected.reflogs) {\n-\t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n+\t\t\tstruct expire_reflog_policy_cb cb = {\n+\t\t\t\t.cmd = cmd,\n+\t\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n+\t\t\t\t.verbose = verbose,\n+\t\t\t};\n \n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n \t\t\tstatus |= reflog_expire(item->string, flags,\n@@ -671,6 +687,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \tstruct cmd_reflog_expire_cb cmd = { 0 };\n \tint i, status = 0;\n \tunsigned int flags = 0;\n+\tint verbose = 0;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -681,7 +698,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--updateref\"))\n \t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n+\t\t\tverbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -699,7 +716,10 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tconst char *spec = strstr(argv[i], \"@{\");\n \t\tchar *ep, *ref;\n \t\tint recno;\n-\t\tstruct expire_reflog_policy_cb cb = { 0 };\n+\t\tstruct expire_reflog_policy_cb cb = {\n+\t\t\t.verbose = verbose,\n+\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n+\t\t};\n \n \t\tif (!spec) {\n \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\ndiff --git a/refs.h b/refs.h\nindex 92360e55a20..8f91a7f9ff2 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -820,8 +820,7 @@ enum ref_type ref_type(const char *refname);\n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n \tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n-\tEXPIRE_REFLOGS_VERBOSE = 1 << 2,\n-\tEXPIRE_REFLOGS_REWRITE = 1 << 3\n+\tEXPIRE_REFLOGS_REWRITE = 1 << 2,\n };\n \n /*\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5f8586a36e3..6178ad8c77c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3086,11 +3086,12 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,\n }\n \n struct expire_reflog_cb {\n-\tunsigned int flags;\n \treflog_expiry_should_prune_fn *should_prune_fn;\n \tvoid *policy_cb;\n \tFILE *newlog;\n \tstruct object_id last_kept_oid;\n+\tunsigned int rewrite:1,\n+\t\t     dry_run:1;\n };\n \n static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n@@ -3098,33 +3099,27 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \t\t\t     const char *message, void *cb_data)\n {\n \tstruct expire_reflog_cb *cb = cb_data;\n-\tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n+\treflog_expiry_should_prune_fn *fn = cb->should_prune_fn;\n \n-\tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n+\tif (cb->rewrite)\n \t\tooid = &cb->last_kept_oid;\n \n-\tif ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,\n-\t\t\t\t   message, policy_cb)) {\n-\t\tif (cb->flags & EXPIRE_REFLOGS_DRY_RUN)\n-\t\t\tprintf(\"would prune %s\", message);\n-\t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"prune %s\", message);\n-\t} else {\n-\t\tif (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\",\n-\t\t\t\toid_to_hex(ooid), oid_to_hex(noid),\n-\t\t\t\temail, timestamp, tz, message);\n-\t\t\toidcpy(&cb->last_kept_oid, noid);\n-\t\t}\n-\t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"keep %s\", message);\n-\t}\n+\tif (fn(ooid, noid, email, timestamp, tz, message, cb->policy_cb))\n+\t\treturn 0;\n+\n+\tif (cb->dry_run)\n+\t\treturn 0; /* --dry-run */\n+\n+\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\", oid_to_hex(ooid),\n+\t\toid_to_hex(noid), email, timestamp, tz, message);\n+\toidcpy(&cb->last_kept_oid, noid);\n+\n \treturn 0;\n }\n \n static int files_reflog_expire(struct ref_store *ref_store,\n \t\t\t       const char *refname,\n-\t\t\t       unsigned int flags,\n+\t\t\t       unsigned int expire_flags,\n \t\t\t       reflog_expiry_prepare_fn prepare_fn,\n \t\t\t       reflog_expiry_should_prune_fn should_prune_fn,\n \t\t\t       reflog_expiry_cleanup_fn cleanup_fn,\n@@ -3142,7 +3137,8 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \tconst struct object_id *oid;\n \n \tmemset(&cb, 0, sizeof(cb));\n-\tcb.flags = flags;\n+\tcb.rewrite = !!(expire_flags & EXPIRE_REFLOGS_REWRITE);\n+\tcb.dry_run = !!(expire_flags & EXPIRE_REFLOGS_DRY_RUN);\n \tcb.policy_cb = policy_cb_data;\n \tcb.should_prune_fn = should_prune_fn;\n \n@@ -3178,7 +3174,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \tfiles_reflog_path(refs, &log_file_sb, refname);\n \tlog_file = strbuf_detach(&log_file_sb, NULL);\n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\tif (!cb.dry_run) {\n \t\t/*\n \t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n \t\t * no locking implications, we use the lock_file\n@@ -3205,7 +3201,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \trefs_for_each_reflog_ent(ref_store, refname, expire_reflog_ent, &cb);\n \t(*cleanup_fn)(cb.policy_cb);\n \n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\tif (!cb.dry_run) {\n \t\t/*\n \t\t * It doesn't make sense to adjust a reference pointed\n \t\t * to by a symbolic ref based on expiring entries in\n@@ -3215,7 +3211,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \t\t */\n \t\tint update = 0;\n \n-\t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n+\t\tif ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t    !is_null_oid(&cb.last_kept_oid)) {\n \t\t\tint ignore_errno;\n \t\t\tint type;\n-- \n2.34.1.1020.gc80c40b6642\n\n"},{"id":"444271","messageId":"xmqq35ms89u2.fsf@gitster.g","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/9] reftable prep: have builtin/reflog.c handle \"--verbose\"","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-12-16T22:27:01Z","receivedAt":"2021-12-16T22:27:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> This series refactors various small bits of builtin/reflog.c\n> (e.g. using a \"struct string_list\" now), and finally makes it handle\n> the \"--verbose\" output, instead of telling the files backend to emit\n> that same verbose output.\n\nI was quite confused even after spending quite some time until I\nrealized that you are primarily talking about the \"-v\" option of\n\"git reflog expire\" (and \"delete\", perhaps) and not about \"git\nreflog -v\" having different output from the same command without\n\"-v\" option.\n\nI'll refrain from commenting on, or picking up, the series until\nHan-Wen has a chance to take a look and give his input as it is\nunclear if it helps or crashes with his plan.\n\n\n"},{"id":"444283","messageId":"211217.86ilvo9kxi.gmgdl@evledraar.gmail.com","threadId":"57005","inReplyTo":"xmqq35ms89u2.fsf@gitster.g","subject":"Re: [PATCH v2 0/9] reftable prep: have builtin/reflog.c handle \"--verbose\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-16T23:31:10Z","receivedAt":"2021-12-16T23:42:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Dec 16 2021, Junio C Hamano wrote:\n\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> This series refactors various small bits of builtin/reflog.c\n>> (e.g. using a \"struct string_list\" now), and finally makes it handle\n>> the \"--verbose\" output, instead of telling the files backend to emit\n>> that same verbose output.\n>\n> I was quite confused even after spending quite some time until I\n> realized that you are primarily talking about the \"-v\" option of\n> \"git reflog expire\" (and \"delete\", perhaps) and not about \"git\n> reflog -v\" having different output from the same command without\n> \"-v\" option.\n\nWill try to clear that up in an eventual re-roll, but considering this a\n\"look at only if re-rolling for other reasons\" for now.\n\n> I'll refrain from commenting on, or picking up, the series until\n> Han-Wen has a chance to take a look and give his input as it is\n> unclear if it helps or crashes with his plan.\n\nHe hasn't reviewed this series.\n\nBut I think I/he have looked at it sufficiently that you might want to\nconsider picking it up, particularly since he doesn't work on Friday's &\nwith the holidays etc. it might be a while.\n\nHe commented on this helping with his plans recently here:\n\n    https://lore.kernel.org/git/CAFQ2z_PSS9zOzR6nGYZ8DBK+6oOQzkJsEy_7y+NprwJ1OHNs7w@mail.gmail.com/\n\nThe snippet he's referring to can (last seen on-list, AFAICT) be seen\nhere:\n\n    https://lore.kernel.org/git/3d57f7c443082fd2a7f01aee003a9cd3ca2dd910.1629207607.git.gitgitgadget@gmail.com/\n\nWhich from my reading of the code (I haven't actually tried to combine\nthis with reftable/* for real) would Just Work when combined with this\nseries.\n\nI.e. he had \"dry_run\" implemented, but not \"verbose\". Now he'll get\n\"verbose\" for free. The \"XXX\" comment there about dry_run appears to be\na TODO about not doing needless work within the reflog backend.\n"},{"id":"444614","messageId":"CAFQ2z_McOfm545Xd8hF7YDgzyOjDmcGxpWZ6pQ-yaKAEWMMbgg@mail.gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/9] reftable prep: have builtin/reflog.c handle \"--verbose\"","fromName":"Han-Wen Nienhuys","fromEmail":"hanwen@google.com","sentAt":"2021-12-21T14:24:18Z","receivedAt":"2021-12-21T14:24:34Z","isPatch":true,"sender":{"key":"hanwen@google.com","avatar":"https://avatars.githubusercontent.com/u/31547?v=4"},"body":"On Thu, Dec 16, 2021 at 2:45 PM Ævar Arnfjörð Bjarmason\n<avarab@gmail.com> wrote:\n>\n> This series refactors various small bits of builtin/reflog.c\n> (e.g. using a \"struct string_list\" now), and finally makes it handle\n> the \"--verbose\" output, instead of telling the files backend to emit\n> that same verbose output.\n>\n> This means that when we start to integrate \"reftable\" the new backend\n> won't need to implement verbose reflog output, it will just work.\n>\n> This is a sort-of v2[1]. I ejected the changes at the end to add\n> better --progress output to \"git reflog\". Those fixes are worthwhile,\n> but hopefully this smaller & easier to review series can be queued up\n> first, we can do those UX improvements later.\n\nThanks for sending this.\n\nI looked over all patches separately. Overall, the series looks good to me.\n\n> int a one-line terany instead.\n\nternary.\n\n> \"don't do negative comparison on enum values\"\n\nI would describe it as \"use switch over enum values\", as this doesn't\ninvolve negative numbers.\n\n> collected.reflogs.strdup_strings = 1;\n\nThis puzzled me. Why isn't the init done as _DUP ? Warrants a comment\nat the least.\n\n>  .. goto expire\n> ..\n>    return 0;\n> expire:\n\n(personal opinion) this is going overboard with gotos and labels. Either\n\n  if ( .. ) {\n    expire = 1;\n    goto done;\n  }\ndone:\n  if (expire) {  print stuff }\n  return expire\n\nOr wrap the existing function (without changes) in a callback that\ndoes the print for you.\n\nyour call.\n\n-- \nHan-Wen Nienhuys - Google Munich\nI work 80%. Don't expect answers from me on Fridays.\n--\n\nGoogle Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich\n\nRegistergericht und -nummer: Hamburg, HRB 86891\n\nSitz der Gesellschaft: Hamburg\n\nGeschäftsführer: Paul Manicle, Halimah DeLaine Prado\n"},{"id":"444624","messageId":"211221.86lf0eq8qj.gmgdl@evledraar.gmail.com","threadId":"57005","inReplyTo":"CAFQ2z_McOfm545Xd8hF7YDgzyOjDmcGxpWZ6pQ-yaKAEWMMbgg@mail.gmail.com","subject":"Re: [PATCH v2 0/9] reftable prep: have builtin/reflog.c handle \"--verbose\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-21T15:21:06Z","receivedAt":"2021-12-21T15:27:04Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 21 2021, Han-Wen Nienhuys wrote:\n\n> On Thu, Dec 16, 2021 at 2:45 PM Ævar Arnfjörð Bjarmason\n> <avarab@gmail.com> wrote:\n>>\n>> This series refactors various small bits of builtin/reflog.c\n>> (e.g. using a \"struct string_list\" now), and finally makes it handle\n>> the \"--verbose\" output, instead of telling the files backend to emit\n>> that same verbose output.\n>>\n>> This means that when we start to integrate \"reftable\" the new backend\n>> won't need to implement verbose reflog output, it will just work.\n>>\n>> This is a sort-of v2[1]. I ejected the changes at the end to add\n>> better --progress output to \"git reflog\". Those fixes are worthwhile,\n>> but hopefully this smaller & easier to review series can be queued up\n>> first, we can do those UX improvements later.\n>\n> Thanks for sending this.\n>\n> I looked over all patches separately. Overall, the series looks good to me.\n>\n>> int a one-line terany instead.\n>\n> ternary.\n>\n>> \"don't do negative comparison on enum values\"\n\nWill fix.\n\n> I would describe it as \"use switch over enum values\", as this doesn't\n> involve negative numbers.\n\n*nod*\n\n>> collected.reflogs.strdup_strings = 1;\n>\n> This puzzled me. Why isn't the init done as _DUP ? Warrants a comment\n> at the least.\n\nWill comment on it. FWWI this is a common pattern with the string_list\nAPI.\n\nIf you declare it \"dup\" and push into it you'll end up double-duping,\nbut if you don't declare it dup'd and free it you'll leak memory. It\nwon't free() a non-duped list.\n\nThe other option is to declare it \"dup\" and then\n\"string_list_append_nodup\", will try and see...\n\n>>  .. goto expire\n>> ..\n>>    return 0;\n>> expire:\n>\n> (personal opinion) this is going overboard with gotos and labels. Either\n>\n>   if ( .. ) {\n>     expire = 1;\n>     goto done;\n>   }\n> done:\n>   if (expire) {  print stuff }\n>   return expire\n>\n> Or wrap the existing function (without changes) in a callback that\n> does the print for you.\n>\n> your call.\n>\n\nWill experiment & see, looks like a wrapper might be easiest here.\n\nThanks!\n"},{"id":"444724","messageId":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com","subject":"[PATCH v3 0/9] reftable prep: have builtin/reflog.c handle \"--verbose\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:39Z","receivedAt":"2021-12-22T04:07:08Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series refactors various small bits of builtin/reflog.c\n(e.g. using a \"struct string_list\" now), and finally makes it handle\nthe \"--verbose\" output, instead of telling the files backend to emit\nthat same verbose output.\n\nThis means that when we start to integrate \"reftable\" the new backend\nwon't need to implement verbose reflog output, it will just work.\n\nThis v3 addresses comments Han-Wen had on the v2[1] in [2]. Those are\nall addressed here.\n\nI thought that eliminating the \"goto\" as he suggested might lead to\nneedlessly verbose code, but I think this new approach is much\nbetter. We now have a \"verbose\" callback that's a wrapper around the\nnon-verbose version. For the common case of non-verbose we won't\nexecute any \"verbose\" code at all, which makes this code easier to\nfollow.\n\n1. https://lore.kernel.org/git/cover-v2-0.9-00000000000-20211216T134028Z-avarab@gmail.com/\n2. https://lore.kernel.org/git/CAFQ2z_McOfm545Xd8hF7YDgzyOjDmcGxpWZ6pQ-yaKAEWMMbgg@mail.gmail.com/\n\nÆvar Arnfjörð Bjarmason (9):\n  reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()\n  reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()\n  reflog: change one->many worktree->refnames to use a string_list\n  reflog expire: use \"switch\" over enum values\n  reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL\n  reflog expire: don't use lookup_commit_reference_gently()\n  reflog: reduce scope of \"struct rev_info\"\n  refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN\n  reflog + refs-backend: move \"verbose\" out of the backend\n\n builtin/reflog.c     | 223 +++++++++++++++++++++++++------------------\n refs.h               |   3 +-\n refs/files-backend.c |  44 ++++-----\n 3 files changed, 150 insertions(+), 120 deletions(-)\n\nRange-diff against v2:\n 1:  22c8119640c =  1:  7fac198f485 reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()\n 2:  b8e84538427 =  2:  1ffc8ef8a8b reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()\n 3:  c0e190e46cf !  3:  ba7679e6fc0 reflog: change one->many worktree->refnames to use a string_list\n    @@ Commit message\n         free, as a result we need just one struct to keep track of this data,\n         instead of two.\n     \n    +    The \"DUP\" -> \"string_list_append_nodup(..., strbuf_detach(...))\"\n    +    pattern here is the same as that used in a recent memory leak fix in\n    +    b202e51b154 (grep: fix a \"path_list\" memory leak, 2021-10-22).\n    +\n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## builtin/reflog.c ##\n    @@ builtin/reflog.c: static void reflog_expiry_cleanup(void *cb_data)\n     -\tFLEX_ALLOC_STR(e, reflog, newref.buf);\n     -\tstrbuf_release(&newref);\n     +\tstrbuf_worktree_ref(worktree, &newref, ref);\n    -+\tstring_list_append(&cb->reflogs, strbuf_detach(&newref, NULL));\n    ++\tstring_list_append_nodup(&cb->reflogs, strbuf_detach(&newref, NULL));\n      \n     -\toidcpy(&e->oid, oid);\n     -\tALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);\n    @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons\n      \tif (do_all) {\n     -\t\tstruct collect_reflog_cb collected;\n     +\t\tstruct worktree_reflogs collected = {\n    -+\t\t\t.reflogs = STRING_LIST_INIT_NODUP,\n    ++\t\t\t.reflogs = STRING_LIST_INIT_DUP,\n     +\t\t};\n     +\t\tstruct string_list_item *item;\n      \t\tstruct worktree **worktrees, **p;\n    @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons\n     -\t\t\tfree(e);\n      \t\t}\n     -\t\tfree(collected.e);\n    -+\t\tcollected.reflogs.strdup_strings = 1;\n     +\t\tstring_list_clear(&collected.reflogs, 0);\n      \t}\n      \n 4:  e42fac1b518 !  4:  74447de0413 reflog expire: don't do negative comparison on enum values\n    @@ Metadata\n     Author: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n      ## Commit message ##\n    -    reflog expire: don't do negative comparison on enum values\n    +    reflog expire: use \"switch\" over enum values\n     \n         Change code added in 03cb91b18cc (reflog --expire-unreachable: special\n    -    case entries in \"HEAD\" reflog, 2010-04-09) to not do positive instead\n    -    of negative comparisons on enum values, i.e. not to assume that \"x !=\n    -    UE_ALWAYS\" means \"(x == UE_HEAD || x || UE_NORMAL)\".\n    +    case entries in \"HEAD\" reflog, 2010-04-09) to use a \"switch\" statement\n    +    with an exhaustive list of \"case\" statements instead of doing numeric\n    +    comparisons against the enum labels.\n     \n    -    That assumption is true now, but we'd introduce subtle bugs here if\n    -    that were to change, now the compiler will notice and error out on\n    -    such errors.\n    +    Now we won't assume that \"x != UE_ALWAYS\" means \"(x == UE_HEAD || x ||\n    +    UE_NORMAL)\". That assumption is true now, but we'd introduce subtle\n    +    bugs here if that were to change, now the compiler will notice and\n    +    error out on such errors.\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n 5:  39263cd00ae !  5:  896ae9f73eb reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL\n    @@ Commit message\n     \n         The code behaves the same way as before, but this makes the control\n         flow clearer, and the shorter name allows us to fold a 4-line i/else\n    -    int a one-line terany instead.\n    +    into a one-line ternary instead.\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n 6:  c71aab5845e =  6:  cfa80e84c6d reflog expire: don't use lookup_commit_reference_gently()\n 7:  2fb33ef2546 =  7:  b3a62b9b177 reflog: reduce scope of \"struct rev_info\"\n 8:  f9fe6a2cfb0 =  8:  6748298a782 refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN\n 9:  28aa0aa6e30 !  9:  2fb9de8ae51 reflog + refs-backend: move \"verbose\" out of the backend\n    @@ builtin/reflog.c: struct expire_reflog_policy_cb {\n      \tstruct cmd_reflog_expire_cb cmd;\n      \tstruct commit *tip_commit;\n      \tstruct commit_list *tips;\n    -+\tunsigned int dry_run:1,\n    -+\t\t     verbose:1;\n    ++\tunsigned int dry_run:1;\n      };\n      \n      struct worktree_reflogs {\n     @@ builtin/reflog.c: static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n    - \tstruct commit *old_commit, *new_commit;\n    - \n    - \tif (timestamp < cb->cmd.expire_total)\n    --\t\treturn 1;\n    -+\t\tgoto expire;\n    - \n    - \told_commit = new_commit = NULL;\n    - \tif (cb->cmd.stalefix &&\n    - \t    (!keep_entry(&old_commit, ooid) || !keep_entry(&new_commit, noid)))\n    --\t\treturn 1;\n    -+\t\tgoto expire;\n    - \n    - \tif (timestamp < cb->cmd.expire_unreachable) {\n    - \t\tswitch (cb->unreachable_expire_kind) {\n    - \t\tcase UE_ALWAYS:\n    --\t\t\treturn 1;\n    -+\t\t\tgoto expire;\n    - \t\tcase UE_NORMAL:\n    - \t\tcase UE_HEAD:\n    - \t\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n    --\t\t\t\treturn 1;\n    -+\t\t\t\tgoto expire;\n    - \t\t\tbreak;\n    - \t\t}\n    - \t}\n    + \treturn 0;\n    + }\n      \n    - \tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n    --\t\treturn 1;\n    -+\t\tgoto expire;\n    ++static int should_expire_reflog_ent_verbose(struct object_id *ooid,\n    ++\t\t\t\t\t    struct object_id *noid,\n    ++\t\t\t\t\t    const char *email,\n    ++\t\t\t\t\t    timestamp_t timestamp, int tz,\n    ++\t\t\t\t\t    const char *message, void *cb_data)\n    ++{\n    ++\tstruct expire_reflog_policy_cb *cb = cb_data;\n    ++\tint expire;\n     +\n    -+\tif (cb->verbose)\n    ++\texpire = should_expire_reflog_ent(ooid, noid, email, timestamp, tz,\n    ++\t\t\t\t\t  message, cb);\n    ++\n    ++\tif (!expire)\n     +\t\tprintf(\"keep %s\", message);\n    - \n    - \treturn 0;\n    -+expire:\n    -+\tif (cb->dry_run)\n    ++\telse if (cb->dry_run)\n     +\t\tprintf(\"would prune %s\", message);\n    -+\telse if (cb->verbose)\n    ++\telse\n     +\t\tprintf(\"prune %s\", message);\n    -+\treturn 1;\n    - }\n    - \n    ++\n    ++\treturn expire;\n    ++}\n    ++\n      static int push_tip_to_list(const char *refname, const struct object_id *oid,\n    + \t\t\t    int flags, void *cb_data)\n    + {\n     @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n      \tint i, status, do_all, all_worktrees = 1;\n      \tint explicit_expiry = 0;\n      \tunsigned int flags = 0;\n     +\tint verbose = 0;\n    ++\treflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;\n      \n      \tdefault_reflog_expire_unreachable = now - 30 * 24 * 3600;\n      \tdefault_reflog_expire = now - 90 * 24 * 3600;\n    @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons\n      \t\telse if (!strcmp(arg, \"--\")) {\n      \t\t\ti++;\n      \t\t\tbreak;\n    +@@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n    + \t\t\tbreak;\n    + \t}\n    + \n    ++\tif (verbose)\n    ++\t\tshould_prune_fn = should_expire_reflog_ent_verbose;\n    ++\n    + \t/*\n    + \t * We can trust the commits and objects reachable from refs\n    + \t * even in older repository.  We cannot trust what's reachable\n     @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n      \t\trevs.do_not_die_on_missing_tree = 1;\n      \t\trevs.ignore_missing = 1;\n    @@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, cons\n     +\t\t\tstruct expire_reflog_policy_cb cb = {\n     +\t\t\t\t.cmd = cmd,\n     +\t\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n    -+\t\t\t\t.verbose = verbose,\n     +\t\t\t};\n      \n      \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n      \t\t\tstatus |= reflog_expire(item->string, flags,\n    + \t\t\t\t\t\treflog_expiry_prepare,\n    +-\t\t\t\t\t\tshould_expire_reflog_ent,\n    ++\t\t\t\t\t\tshould_prune_fn,\n    + \t\t\t\t\t\treflog_expiry_cleanup,\n    + \t\t\t\t\t\t&cb);\n    + \t\t}\n    +@@ builtin/reflog.c: static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n    + \t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);\n    + \t\tstatus |= reflog_expire(ref, flags,\n    + \t\t\t\t\treflog_expiry_prepare,\n    +-\t\t\t\t\tshould_expire_reflog_ent,\n    ++\t\t\t\t\tshould_prune_fn,\n    + \t\t\t\t\treflog_expiry_cleanup,\n    + \t\t\t\t\t&cb);\n    + \t\tfree(ref);\n     @@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n      \tstruct cmd_reflog_expire_cb cmd = { 0 };\n      \tint i, status = 0;\n      \tunsigned int flags = 0;\n     +\tint verbose = 0;\n    ++\treflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;\n      \n      \tfor (i = 1; i < argc; i++) {\n      \t\tconst char *arg = argv[i];\n    @@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, cons\n      \t\telse if (!strcmp(arg, \"--\")) {\n      \t\t\ti++;\n      \t\t\tbreak;\n    +@@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n    + \t\t\tbreak;\n    + \t}\n    + \n    ++\tif (verbose)\n    ++\t\tshould_prune_fn = should_expire_reflog_ent_verbose;\n    ++\n    + \tif (argc - i < 1)\n    + \t\treturn error(_(\"no reflog specified to delete\"));\n    + \n     @@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n      \t\tconst char *spec = strstr(argv[i], \"@{\");\n      \t\tchar *ep, *ref;\n      \t\tint recno;\n     -\t\tstruct expire_reflog_policy_cb cb = { 0 };\n     +\t\tstruct expire_reflog_policy_cb cb = {\n    -+\t\t\t.verbose = verbose,\n     +\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n     +\t\t};\n      \n      \t\tif (!spec) {\n      \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\n    +@@ builtin/reflog.c: static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n    + \t\tcb.cmd = cmd;\n    + \t\tstatus |= reflog_expire(ref, flags,\n    + \t\t\t\t\treflog_expiry_prepare,\n    +-\t\t\t\t\tshould_expire_reflog_ent,\n    ++\t\t\t\t\tshould_prune_fn,\n    + \t\t\t\t\treflog_expiry_cleanup,\n    + \t\t\t\t\t&cb);\n    + \t\tfree(ref);\n     \n      ## refs.h ##\n     @@ refs.h: enum ref_type ref_type(const char *refname);\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444723","messageId":"patch-v3-1.9-7fac198f485-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 1/9] reflog delete: narrow scope of \"cmd\" passed to count_reflog_ent()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:40Z","receivedAt":"2021-12-22T04:07:09Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cb_data\" we pass to the count_reflog_ent() to be the\n&cb.cmd itself, instead of passing &cb and having the callback lookup\ncb->cmd.\n\nThis makes it clear that the \"cb\" itself is the same memzero'd\nstructure on each iteration of the for-loop that uses &cb, except for\nthe \"cmd\" member.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 175c83e7cc2..4c15d71f3e9 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -662,20 +662,18 @@ static int count_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \t\tconst char *email, timestamp_t timestamp, int tz,\n \t\tconst char *message, void *cb_data)\n {\n-\tstruct expire_reflog_policy_cb *cb = cb_data;\n-\tif (!cb->cmd.expire_total || timestamp < cb->cmd.expire_total)\n-\t\tcb->cmd.recno++;\n+\tstruct cmd_reflog_expire_cb *cb = cb_data;\n+\tif (!cb->expire_total || timestamp < cb->expire_total)\n+\t\tcb->recno++;\n \treturn 0;\n }\n \n static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n {\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct cmd_reflog_expire_cb cmd = { 0 };\n \tint i, status = 0;\n \tunsigned int flags = 0;\n \n-\tmemset(&cb, 0, sizeof(cb));\n-\n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n@@ -703,6 +701,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tconst char *spec = strstr(argv[i], \"@{\");\n \t\tchar *ep, *ref;\n \t\tint recno;\n+\t\tstruct expire_reflog_policy_cb cb = { 0 };\n \n \t\tif (!spec) {\n \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\n@@ -716,14 +715,15 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \n \t\trecno = strtoul(spec + 2, &ep, 10);\n \t\tif (*ep == '}') {\n-\t\t\tcb.cmd.recno = -recno;\n-\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n+\t\t\tcmd.recno = -recno;\n+\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cmd);\n \t\t} else {\n-\t\t\tcb.cmd.expire_total = approxidate(spec + 2);\n-\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cb);\n-\t\t\tcb.cmd.expire_total = 0;\n+\t\t\tcmd.expire_total = approxidate(spec + 2);\n+\t\t\tfor_each_reflog_ent(ref, count_reflog_ent, &cmd);\n+\t\t\tcmd.expire_total = 0;\n \t\t}\n \n+\t\tcb.cmd = cmd;\n \t\tstatus |= reflog_expire(ref, flags,\n \t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\tshould_expire_reflog_ent,\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444725","messageId":"patch-v3-2.9-1ffc8ef8a8b-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 2/9] reflog expire: narrow scope of \"cb\" in cmd_reflog_expire()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:41Z","receivedAt":"2021-12-22T04:07:12Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"As with the preceding change for \"reflog delete\", change the \"cb_data\"\nwe pass to callbacks to be &cb.cmd itself, instead of passing &cb and\nhaving the callback lookup cb->cmd.\n\nThis makes it clear that the \"cb\" itself is the same memzero'd\nstructure on each iteration of the for-loops that use &cb, except for\nthe \"cmd\" member.\n\nThe \"struct expire_reflog_policy_cb\" we pass to reflog_expire() will\nhave the members that aren't \"cmd\" modified by the callbacks, but\nbefore we invoke them everything except \"cmd\" is zero'd out.\n\nThis included the \"tip_commit\", \"mark_list\" and \"tips\". It might have\nlooked as though we were re-using those between iterations, but the\nfirst thing we did in reflog_expiry_prepare() was to either NULL them,\nor clobber them with another value.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 31 +++++++++++++++----------------\n 1 file changed, 15 insertions(+), 16 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 4c15d71f3e9..6989492bf5c 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -357,7 +357,6 @@ static void reflog_expiry_prepare(const char *refname,\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n-\t\tcb->tip_commit = NULL;\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n \t\tcb->tip_commit = lookup_commit_reference_gently(the_repository,\n@@ -371,8 +370,6 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n-\tcb->mark_list = NULL;\n-\tcb->tips = NULL;\n \tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n \t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n \t\t\tstruct commit_list *elem;\n@@ -541,7 +538,7 @@ static void set_reflog_expiry_param(struct cmd_reflog_expire_cb *cb, int slot, c\n \n static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n {\n-\tstruct expire_reflog_policy_cb cb;\n+\tstruct cmd_reflog_expire_cb cmd = { 0 };\n \ttimestamp_t now = time(NULL);\n \tint i, status, do_all, all_worktrees = 1;\n \tint explicit_expiry = 0;\n@@ -553,10 +550,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tsave_commit_buffer = 0;\n \tdo_all = status = 0;\n-\tmemset(&cb, 0, sizeof(cb));\n \n-\tcb.cmd.expire_total = default_reflog_expire;\n-\tcb.cmd.expire_unreachable = default_reflog_expire_unreachable;\n+\tcmd.expire_total = default_reflog_expire;\n+\tcmd.expire_unreachable = default_reflog_expire_unreachable;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -564,17 +560,17 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tif (!strcmp(arg, \"--dry-run\") || !strcmp(arg, \"-n\"))\n \t\t\tflags |= EXPIRE_REFLOGS_DRY_RUN;\n \t\telse if (skip_prefix(arg, \"--expire=\", &arg)) {\n-\t\t\tif (parse_expiry_date(arg, &cb.cmd.expire_total))\n+\t\t\tif (parse_expiry_date(arg, &cmd.expire_total))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_TOTAL;\n \t\t}\n \t\telse if (skip_prefix(arg, \"--expire-unreachable=\", &arg)) {\n-\t\t\tif (parse_expiry_date(arg, &cb.cmd.expire_unreachable))\n+\t\t\tif (parse_expiry_date(arg, &cmd.expire_unreachable))\n \t\t\t\tdie(_(\"'%s' is not a valid timestamp\"), arg);\n \t\t\texplicit_expiry |= EXPIRE_UNREACH;\n \t\t}\n \t\telse if (!strcmp(arg, \"--stale-fix\"))\n-\t\t\tcb.cmd.stalefix = 1;\n+\t\t\tcmd.stalefix = 1;\n \t\telse if (!strcmp(arg, \"--rewrite\"))\n \t\t\tflags |= EXPIRE_REFLOGS_REWRITE;\n \t\telse if (!strcmp(arg, \"--updateref\"))\n@@ -600,14 +596,14 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * even in older repository.  We cannot trust what's reachable\n \t * from reflog if the repository was pruned with older git.\n \t */\n-\tif (cb.cmd.stalefix) {\n-\t\trepo_init_revisions(the_repository, &cb.cmd.revs, prefix);\n-\t\tcb.cmd.revs.do_not_die_on_missing_tree = 1;\n-\t\tcb.cmd.revs.ignore_missing = 1;\n-\t\tcb.cmd.revs.ignore_missing_links = 1;\n+\tif (cmd.stalefix) {\n+\t\trepo_init_revisions(the_repository, &cmd.revs, prefix);\n+\t\tcmd.revs.do_not_die_on_missing_tree = 1;\n+\t\tcmd.revs.ignore_missing = 1;\n+\t\tcmd.revs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&cb.cmd.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&cmd.revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n@@ -629,6 +625,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \t\tfor (i = 0; i < collected.nr; i++) {\n \t\t\tstruct collected_reflog *e = collected.e[i];\n+\t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n \n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n \t\t\tstatus |= reflog_expire(e->reflog, flags,\n@@ -643,6 +640,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \n \tfor (; i < argc; i++) {\n \t\tchar *ref;\n+\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n+\n \t\tif (!dwim_log(argv[i], strlen(argv[i]), NULL, &ref)) {\n \t\t\tstatus |= error(_(\"%s points nowhere!\"), argv[i]);\n \t\t\tcontinue;\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444726","messageId":"patch-v3-4.9-74447de0413-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 4/9] reflog expire: use \"switch\" over enum values","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:43Z","receivedAt":"2021-12-22T04:07:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change code added in 03cb91b18cc (reflog --expire-unreachable: special\ncase entries in \"HEAD\" reflog, 2010-04-09) to use a \"switch\" statement\nwith an exhaustive list of \"case\" statements instead of doing numeric\ncomparisons against the enum labels.\n\nNow we won't assume that \"x != UE_ALWAYS\" means \"(x == UE_HEAD || x ||\nUE_NORMAL)\". That assumption is true now, but we'd introduce subtle\nbugs here if that were to change, now the compiler will notice and\nerror out on such errors.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 57 ++++++++++++++++++++++++++++--------------------\n 1 file changed, 33 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 27851c6efb7..8d05660e644 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -303,10 +303,15 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \t\treturn 1;\n \n \tif (timestamp < cb->cmd.expire_unreachable) {\n-\t\tif (cb->unreachable_expire_kind == UE_ALWAYS)\n-\t\t\treturn 1;\n-\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n+\t\tswitch (cb->unreachable_expire_kind) {\n+\t\tcase UE_ALWAYS:\n \t\t\treturn 1;\n+\t\tcase UE_NORMAL:\n+\t\tcase UE_HEAD:\n+\t\t\tif (unreachable(cb, old_commit, ooid) || unreachable(cb, new_commit, noid))\n+\t\t\t\treturn 1;\n+\t\t\tbreak;\n+\t\t}\n \t}\n \n \tif (cb->cmd.recno && --(cb->cmd.recno) == 0)\n@@ -348,6 +353,7 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\t\t  void *cb_data)\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct commit_list *elem;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n@@ -363,34 +369,37 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n \t\tcb->unreachable_expire_kind = UE_ALWAYS;\n \n-\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\n-\t\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n-\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n-\t\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n-\t\t} else {\n-\t\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n-\t\t}\n-\t\tcb->mark_limit = cb->cmd.expire_total;\n-\t\tmark_reachable(cb);\n+\tswitch (cb->unreachable_expire_kind) {\n+\tcase UE_ALWAYS:\n+\t\treturn;\n+\tcase UE_HEAD:\n+\t\tfor_each_ref(push_tip_to_list, &cb->tips);\n+\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n+\t\tbreak;\n+\tcase UE_NORMAL:\n+\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n \t}\n+\tcb->mark_limit = cb->cmd.expire_total;\n+\tmark_reachable(cb);\n }\n \n static void reflog_expiry_cleanup(void *cb_data)\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tstruct commit_list *elem;\n \n-\tif (cb->unreachable_expire_kind != UE_ALWAYS) {\n-\t\tif (cb->unreachable_expire_kind == UE_HEAD) {\n-\t\t\tstruct commit_list *elem;\n-\t\t\tfor (elem = cb->tips; elem; elem = elem->next)\n-\t\t\t\tclear_commit_marks(elem->item, REACHABLE);\n-\t\t\tfree_commit_list(cb->tips);\n-\t\t} else {\n-\t\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n-\t\t}\n+\tswitch (cb->unreachable_expire_kind) {\n+\tcase UE_ALWAYS:\n+\t\treturn;\n+\tcase UE_HEAD:\n+\t\tfor (elem = cb->tips; elem; elem = elem->next)\n+\t\t\tclear_commit_marks(elem->item, REACHABLE);\n+\t\tfree_commit_list(cb->tips);\n+\t\tbreak;\n+\tcase UE_NORMAL:\n+\t\tclear_commit_marks(cb->tip_commit, REACHABLE);\n+\t\tbreak;\n \t}\n }\n \n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444727","messageId":"patch-v3-3.9-ba7679e6fc0-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 3/9] reflog: change one->many worktree->refnames to use a string_list","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:42Z","receivedAt":"2021-12-22T04:07:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the FLEX_ARRAY pattern added in bda3a31cc79 (reflog-expire:\nAvoid creating new files in a directory inside readdir(3) loop,\n2008-01-25) the string-list API instead.\n\nThis does not change any behavior, allows us to delete much of this\ncode as it's replaced by things we get from the string-list API for\nfree, as a result we need just one struct to keep track of this data,\ninstead of two.\n\nThe \"DUP\" -> \"string_list_append_nodup(..., strbuf_detach(...))\"\npattern here is the same as that used in a recent memory leak fix in\nb202e51b154 (grep: fix a \"path_list\" memory leak, 2021-10-22).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 47 ++++++++++++++++++-----------------------------\n 1 file changed, 18 insertions(+), 29 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 6989492bf5c..27851c6efb7 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -48,16 +48,9 @@ struct expire_reflog_policy_cb {\n \tstruct commit_list *tips;\n };\n \n-struct collected_reflog {\n-\tstruct object_id oid;\n-\tchar reflog[FLEX_ARRAY];\n-};\n-\n-struct collect_reflog_cb {\n-\tstruct collected_reflog **e;\n-\tint alloc;\n-\tint nr;\n-\tstruct worktree *wt;\n+struct worktree_reflogs {\n+\tstruct worktree *worktree;\n+\tstruct string_list reflogs;\n };\n \n /* Remember to update object flag allocation in object.h */\n@@ -403,24 +396,20 @@ static void reflog_expiry_cleanup(void *cb_data)\n \n static int collect_reflog(const char *ref, const struct object_id *oid, int unused, void *cb_data)\n {\n-\tstruct collected_reflog *e;\n-\tstruct collect_reflog_cb *cb = cb_data;\n+\tstruct worktree_reflogs *cb = cb_data;\n+\tstruct worktree *worktree = cb->worktree;\n \tstruct strbuf newref = STRBUF_INIT;\n \n \t/*\n \t * Avoid collecting the same shared ref multiple times because\n \t * they are available via all worktrees.\n \t */\n-\tif (!cb->wt->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n+\tif (!worktree->is_current && ref_type(ref) == REF_TYPE_NORMAL)\n \t\treturn 0;\n \n-\tstrbuf_worktree_ref(cb->wt, &newref, ref);\n-\tFLEX_ALLOC_STR(e, reflog, newref.buf);\n-\tstrbuf_release(&newref);\n+\tstrbuf_worktree_ref(worktree, &newref, ref);\n+\tstring_list_append_nodup(&cb->reflogs, strbuf_detach(&newref, NULL));\n \n-\toidcpy(&e->oid, oid);\n-\tALLOC_GROW(cb->e, cb->nr + 1, cb->alloc);\n-\tcb->e[cb->nr++] = e;\n \treturn 0;\n }\n \n@@ -609,33 +598,33 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t}\n \n \tif (do_all) {\n-\t\tstruct collect_reflog_cb collected;\n+\t\tstruct worktree_reflogs collected = {\n+\t\t\t.reflogs = STRING_LIST_INIT_DUP,\n+\t\t};\n+\t\tstruct string_list_item *item;\n \t\tstruct worktree **worktrees, **p;\n-\t\tint i;\n \n-\t\tmemset(&collected, 0, sizeof(collected));\n \t\tworktrees = get_worktrees();\n \t\tfor (p = worktrees; *p; p++) {\n \t\t\tif (!all_worktrees && !(*p)->is_current)\n \t\t\t\tcontinue;\n-\t\t\tcollected.wt = *p;\n+\t\t\tcollected.worktree = *p;\n \t\t\trefs_for_each_reflog(get_worktree_ref_store(*p),\n \t\t\t\t\t     collect_reflog, &collected);\n \t\t}\n \t\tfree_worktrees(worktrees);\n-\t\tfor (i = 0; i < collected.nr; i++) {\n-\t\t\tstruct collected_reflog *e = collected.e[i];\n+\n+\t\tfor_each_string_list_item(item, &collected.reflogs) {\n \t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n \n-\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, e->reflog);\n-\t\t\tstatus |= reflog_expire(e->reflog, flags,\n+\t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n+\t\t\tstatus |= reflog_expire(item->string, flags,\n \t\t\t\t\t\treflog_expiry_prepare,\n \t\t\t\t\t\tshould_expire_reflog_ent,\n \t\t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t\t&cb);\n-\t\t\tfree(e);\n \t\t}\n-\t\tfree(collected.e);\n+\t\tstring_list_clear(&collected.reflogs, 0);\n \t}\n \n \tfor (; i < argc; i++) {\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444728","messageId":"patch-v3-7.9-b3a62b9b177-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 7/9] reflog: reduce scope of \"struct rev_info\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:46Z","receivedAt":"2021-12-22T04:07:16Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cmd.stalefix\" handling added in 1389d9ddaa6 (reflog expire\n--fix-stale, 2007-01-06) to use a locally scoped \"struct\nrev_info\". This code relies on mark_reachable_objects() twiddling\nflags in the walked objects.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 13 +++++++------\n 1 file changed, 7 insertions(+), 6 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex fe0bd353829..4ff63846058 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -28,7 +28,6 @@ static timestamp_t default_reflog_expire;\n static timestamp_t default_reflog_expire_unreachable;\n \n struct cmd_reflog_expire_cb {\n-\tstruct rev_info revs;\n \tint stalefix;\n \ttimestamp_t expire_total;\n \ttimestamp_t expire_unreachable;\n@@ -594,13 +593,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t * from reflog if the repository was pruned with older git.\n \t */\n \tif (cmd.stalefix) {\n-\t\trepo_init_revisions(the_repository, &cmd.revs, prefix);\n-\t\tcmd.revs.do_not_die_on_missing_tree = 1;\n-\t\tcmd.revs.ignore_missing = 1;\n-\t\tcmd.revs.ignore_missing_links = 1;\n+\t\tstruct rev_info revs;\n+\n+\t\trepo_init_revisions(the_repository, &revs, prefix);\n+\t\trevs.do_not_die_on_missing_tree = 1;\n+\t\trevs.ignore_missing = 1;\n+\t\trevs.ignore_missing_links = 1;\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n-\t\tmark_reachable_objects(&cmd.revs, 0, 0, NULL);\n+\t\tmark_reachable_objects(&revs, 0, 0, NULL);\n \t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tputchar('\\n');\n \t}\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444729","messageId":"patch-v3-6.9-cfa80e84c6d-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 6/9] reflog expire: don't use lookup_commit_reference_gently()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:45Z","receivedAt":"2021-12-22T04:07:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In the initial implementation of \"git reflog\" in 4264dc15e19 (git\nreflog expire, 2006-12-19) we had this\nlookup_commit_reference_gently().\n\nI don't think we've ever found tags that we need to recursively\ndereference in reflogs, so this should at least be changed to a\n\"lookup commit\" as I'm doing here, although I can't think of a way\nwhere it mattered in practice.\n\nI also think we'd probably like to just die here if we have a NULL\nobject, but as this code needs to handle potentially broken\nrepositories let's just show an \"error\" but continue, the non-quiet\nlookup_commit() will do for us. None of our tests cover the case where\n\"commit\" is NULL after this lookup.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 3 +--\n 1 file changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex f18a63751f3..fe0bd353829 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -359,8 +359,7 @@ static void reflog_expiry_prepare(const char *refname,\n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n-\t\tcommit = lookup_commit_reference_gently(the_repository,\n-\t\t\t\t\t\t\toid, 1);\n+\t\tcommit = lookup_commit(the_repository, oid);\n \t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444730","messageId":"patch-v3-5.9-896ae9f73eb-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 5/9] reflog expire: refactor & use \"tip_commit\" only for UE_NORMAL","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:44Z","receivedAt":"2021-12-22T04:07:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add an intermediate variable for \"tip_commit\" in\nreflog_expiry_prepare(), and only add it to the struct if we're\nhandling the UE_NORMAL case.\n\nThe code behaves the same way as before, but this makes the control\nflow clearer, and the shorter name allows us to fold a 4-line i/else\ninto a one-line ternary instead.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c | 14 +++++++-------\n 1 file changed, 7 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 8d05660e644..f18a63751f3 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -354,16 +354,14 @@ static void reflog_expiry_prepare(const char *refname,\n {\n \tstruct expire_reflog_policy_cb *cb = cb_data;\n \tstruct commit_list *elem;\n+\tstruct commit *commit = NULL;\n \n \tif (!cb->cmd.expire_unreachable || is_head(refname)) {\n \t\tcb->unreachable_expire_kind = UE_HEAD;\n \t} else {\n-\t\tcb->tip_commit = lookup_commit_reference_gently(the_repository,\n-\t\t\t\t\t\t\t\toid, 1);\n-\t\tif (!cb->tip_commit)\n-\t\t\tcb->unreachable_expire_kind = UE_ALWAYS;\n-\t\telse\n-\t\t\tcb->unreachable_expire_kind = UE_NORMAL;\n+\t\tcommit = lookup_commit_reference_gently(the_repository,\n+\t\t\t\t\t\t\toid, 1);\n+\t\tcb->unreachable_expire_kind = commit ? UE_NORMAL : UE_ALWAYS;\n \t}\n \n \tif (cb->cmd.expire_unreachable <= cb->cmd.expire_total)\n@@ -378,7 +376,9 @@ static void reflog_expiry_prepare(const char *refname,\n \t\t\tcommit_list_insert(elem->item, &cb->mark_list);\n \t\tbreak;\n \tcase UE_NORMAL:\n-\t\tcommit_list_insert(cb->tip_commit, &cb->mark_list);\n+\t\tcommit_list_insert(commit, &cb->mark_list);\n+\t\t/* For reflog_expiry_cleanup() below */\n+\t\tcb->tip_commit = commit;\n \t}\n \tcb->mark_limit = cb->cmd.expire_total;\n \tmark_reachable(cb);\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444731","messageId":"patch-v3-8.9-6748298a782-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 8/9] refs files-backend: assume cb->newlog if !EXPIRE_REFLOGS_DRY_RUN","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:47Z","receivedAt":"2021-12-22T04:07:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"It's not possible for \"cb->newlog\" to be NULL if\n!EXPIRE_REFLOGS_DRY_RUN, since files_reflog_expire() would have\nerror()'d and taken the \"goto failure\" branch if it couldn't open the\nfile. By not using the \"newlog\" field private to \"file-backend.c\"'s\n\"struct expire_reflog_cb\", we can move this verbosity logging to\n\"builtin/reflog.c\" in a subsequent commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n refs/files-backend.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 90b671025a7..5f8586a36e3 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3105,12 +3105,12 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \n \tif ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,\n \t\t\t\t   message, policy_cb)) {\n-\t\tif (!cb->newlog)\n+\t\tif (cb->flags & EXPIRE_REFLOGS_DRY_RUN)\n \t\t\tprintf(\"would prune %s\", message);\n \t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n \t\t\tprintf(\"prune %s\", message);\n \t} else {\n-\t\tif (cb->newlog) {\n+\t\tif (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {\n \t\t\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\",\n \t\t\t\toid_to_hex(ooid), oid_to_hex(noid),\n \t\t\t\temail, timestamp, tz, message);\n-- \n2.34.1.1146.gb52885e7c44\n\n"},{"id":"444732","messageId":"patch-v3-9.9-2fb9de8ae51-20211222T040557Z-avarab@gmail.com","threadId":"57005","inReplyTo":"cover-v3-0.9-00000000000-20211222T040557Z-avarab@gmail.com","subject":"[PATCH v3 9/9] reflog + refs-backend: move \"verbose\" out of the backend","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-12-22T04:06:48Z","receivedAt":"2021-12-22T04:07:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the handling of the \"verbose\" flag entirely out of\n\"refs/files-backend.c\" and into \"builtin/reflog.c\". This allows the\nbackend to stop knowing about the EXPIRE_REFLOGS_VERBOSE flag.\n\nThe expire_reflog_ent() function shouldn't need to deal with the\nimplementation detail of whether or not we're emitting verbose output,\nby doing this the --verbose output becomes backend-agnostic, so\nreftable will get the same output.\n\nI think the output is rather bad currently, and should e.g. be\nimplemented with some better future mode of progress.[ch], but that's\na topic for another improvement.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/reflog.c     | 56 +++++++++++++++++++++++++++++++++++++-------\n refs.h               |  3 +--\n refs/files-backend.c | 44 ++++++++++++++++------------------\n 3 files changed, 68 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/reflog.c b/builtin/reflog.c\nindex 4ff63846058..a4b1dd27e13 100644\n--- a/builtin/reflog.c\n+++ b/builtin/reflog.c\n@@ -45,6 +45,7 @@ struct expire_reflog_policy_cb {\n \tstruct cmd_reflog_expire_cb cmd;\n \tstruct commit *tip_commit;\n \tstruct commit_list *tips;\n+\tunsigned int dry_run:1;\n };\n \n struct worktree_reflogs {\n@@ -319,6 +320,28 @@ static int should_expire_reflog_ent(struct object_id *ooid, struct object_id *no\n \treturn 0;\n }\n \n+static int should_expire_reflog_ent_verbose(struct object_id *ooid,\n+\t\t\t\t\t    struct object_id *noid,\n+\t\t\t\t\t    const char *email,\n+\t\t\t\t\t    timestamp_t timestamp, int tz,\n+\t\t\t\t\t    const char *message, void *cb_data)\n+{\n+\tstruct expire_reflog_policy_cb *cb = cb_data;\n+\tint expire;\n+\n+\texpire = should_expire_reflog_ent(ooid, noid, email, timestamp, tz,\n+\t\t\t\t\t  message, cb);\n+\n+\tif (!expire)\n+\t\tprintf(\"keep %s\", message);\n+\telse if (cb->dry_run)\n+\t\tprintf(\"would prune %s\", message);\n+\telse\n+\t\tprintf(\"prune %s\", message);\n+\n+\treturn expire;\n+}\n+\n static int push_tip_to_list(const char *refname, const struct object_id *oid,\n \t\t\t    int flags, void *cb_data)\n {\n@@ -539,6 +562,8 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \tint i, status, do_all, all_worktrees = 1;\n \tint explicit_expiry = 0;\n \tunsigned int flags = 0;\n+\tint verbose = 0;\n+\treflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;\n \n \tdefault_reflog_expire_unreachable = now - 30 * 24 * 3600;\n \tdefault_reflog_expire = now - 90 * 24 * 3600;\n@@ -576,7 +601,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--single-worktree\"))\n \t\t\tall_worktrees = 0;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n+\t\t\tverbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -587,6 +612,9 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\t\tbreak;\n \t}\n \n+\tif (verbose)\n+\t\tshould_prune_fn = should_expire_reflog_ent_verbose;\n+\n \t/*\n \t * We can trust the commits and objects reachable from refs\n \t * even in older repository.  We cannot trust what's reachable\n@@ -599,10 +627,10 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\trevs.do_not_die_on_missing_tree = 1;\n \t\trevs.ignore_missing = 1;\n \t\trevs.ignore_missing_links = 1;\n-\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\tif (verbose)\n \t\t\tprintf(_(\"Marking reachable objects...\"));\n \t\tmark_reachable_objects(&revs, 0, 0, NULL);\n-\t\tif (flags & EXPIRE_REFLOGS_VERBOSE)\n+\t\tif (verbose)\n \t\t\tputchar('\\n');\n \t}\n \n@@ -624,12 +652,15 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tfree_worktrees(worktrees);\n \n \t\tfor_each_string_list_item(item, &collected.reflogs) {\n-\t\t\tstruct expire_reflog_policy_cb cb = { .cmd = cmd };\n+\t\t\tstruct expire_reflog_policy_cb cb = {\n+\t\t\t\t.cmd = cmd,\n+\t\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n+\t\t\t};\n \n \t\t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, item->string);\n \t\t\tstatus |= reflog_expire(item->string, flags,\n \t\t\t\t\t\treflog_expiry_prepare,\n-\t\t\t\t\t\tshould_expire_reflog_ent,\n+\t\t\t\t\t\tshould_prune_fn,\n \t\t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t\t&cb);\n \t\t}\n@@ -647,7 +678,7 @@ static int cmd_reflog_expire(int argc, const char **argv, const char *prefix)\n \t\tset_reflog_expiry_param(&cb.cmd, explicit_expiry, ref);\n \t\tstatus |= reflog_expire(ref, flags,\n \t\t\t\t\treflog_expiry_prepare,\n-\t\t\t\t\tshould_expire_reflog_ent,\n+\t\t\t\t\tshould_prune_fn,\n \t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t&cb);\n \t\tfree(ref);\n@@ -670,6 +701,8 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \tstruct cmd_reflog_expire_cb cmd = { 0 };\n \tint i, status = 0;\n \tunsigned int flags = 0;\n+\tint verbose = 0;\n+\treflog_expiry_should_prune_fn *should_prune_fn = should_expire_reflog_ent;\n \n \tfor (i = 1; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n@@ -680,7 +713,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\telse if (!strcmp(arg, \"--updateref\"))\n \t\t\tflags |= EXPIRE_REFLOGS_UPDATE_REF;\n \t\telse if (!strcmp(arg, \"--verbose\"))\n-\t\t\tflags |= EXPIRE_REFLOGS_VERBOSE;\n+\t\t\tverbose = 1;\n \t\telse if (!strcmp(arg, \"--\")) {\n \t\t\ti++;\n \t\t\tbreak;\n@@ -691,6 +724,9 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\t\tbreak;\n \t}\n \n+\tif (verbose)\n+\t\tshould_prune_fn = should_expire_reflog_ent_verbose;\n+\n \tif (argc - i < 1)\n \t\treturn error(_(\"no reflog specified to delete\"));\n \n@@ -698,7 +734,9 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tconst char *spec = strstr(argv[i], \"@{\");\n \t\tchar *ep, *ref;\n \t\tint recno;\n-\t\tstruct expire_reflog_policy_cb cb = { 0 };\n+\t\tstruct expire_reflog_policy_cb cb = {\n+\t\t\t.dry_run = !!(flags & EXPIRE_REFLOGS_DRY_RUN),\n+\t\t};\n \n \t\tif (!spec) {\n \t\t\tstatus |= error(_(\"not a reflog: %s\"), argv[i]);\n@@ -723,7 +761,7 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix)\n \t\tcb.cmd = cmd;\n \t\tstatus |= reflog_expire(ref, flags,\n \t\t\t\t\treflog_expiry_prepare,\n-\t\t\t\t\tshould_expire_reflog_ent,\n+\t\t\t\t\tshould_prune_fn,\n \t\t\t\t\treflog_expiry_cleanup,\n \t\t\t\t\t&cb);\n \t\tfree(ref);\ndiff --git a/refs.h b/refs.h\nindex 92360e55a20..8f91a7f9ff2 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -820,8 +820,7 @@ enum ref_type ref_type(const char *refname);\n enum expire_reflog_flags {\n \tEXPIRE_REFLOGS_DRY_RUN = 1 << 0,\n \tEXPIRE_REFLOGS_UPDATE_REF = 1 << 1,\n-\tEXPIRE_REFLOGS_VERBOSE = 1 << 2,\n-\tEXPIRE_REFLOGS_REWRITE = 1 << 3\n+\tEXPIRE_REFLOGS_REWRITE = 1 << 2,\n };\n \n /*\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 5f8586a36e3..6178ad8c77c 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -3086,11 +3086,12 @@ static int files_initial_transaction_commit(struct ref_store *ref_store,\n }\n \n struct expire_reflog_cb {\n-\tunsigned int flags;\n \treflog_expiry_should_prune_fn *should_prune_fn;\n \tvoid *policy_cb;\n \tFILE *newlog;\n \tstruct object_id last_kept_oid;\n+\tunsigned int rewrite:1,\n+\t\t     dry_run:1;\n };\n \n static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n@@ -3098,33 +3099,27 @@ static int expire_reflog_ent(struct object_id *ooid, struct object_id *noid,\n \t\t\t     const char *message, void *cb_data)\n {\n \tstruct expire_reflog_cb *cb = cb_data;\n-\tstruct expire_reflog_policy_cb *policy_cb = cb->policy_cb;\n+\treflog_expiry_should_prune_fn *fn = cb->should_prune_fn;\n \n-\tif (cb->flags & EXPIRE_REFLOGS_REWRITE)\n+\tif (cb->rewrite)\n \t\tooid = &cb->last_kept_oid;\n \n-\tif ((*cb->should_prune_fn)(ooid, noid, email, timestamp, tz,\n-\t\t\t\t   message, policy_cb)) {\n-\t\tif (cb->flags & EXPIRE_REFLOGS_DRY_RUN)\n-\t\t\tprintf(\"would prune %s\", message);\n-\t\telse if (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"prune %s\", message);\n-\t} else {\n-\t\tif (!(cb->flags & EXPIRE_REFLOGS_DRY_RUN)) {\n-\t\t\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\",\n-\t\t\t\toid_to_hex(ooid), oid_to_hex(noid),\n-\t\t\t\temail, timestamp, tz, message);\n-\t\t\toidcpy(&cb->last_kept_oid, noid);\n-\t\t}\n-\t\tif (cb->flags & EXPIRE_REFLOGS_VERBOSE)\n-\t\t\tprintf(\"keep %s\", message);\n-\t}\n+\tif (fn(ooid, noid, email, timestamp, tz, message, cb->policy_cb))\n+\t\treturn 0;\n+\n+\tif (cb->dry_run)\n+\t\treturn 0; /* --dry-run */\n+\n+\tfprintf(cb->newlog, \"%s %s %s %\"PRItime\" %+05d\\t%s\", oid_to_hex(ooid),\n+\t\toid_to_hex(noid), email, timestamp, tz, message);\n+\toidcpy(&cb->last_kept_oid, noid);\n+\n \treturn 0;\n }\n \n static int files_reflog_expire(struct ref_store *ref_store,\n \t\t\t       const char *refname,\n-\t\t\t       unsigned int flags,\n+\t\t\t       unsigned int expire_flags,\n \t\t\t       reflog_expiry_prepare_fn prepare_fn,\n \t\t\t       reflog_expiry_should_prune_fn should_prune_fn,\n \t\t\t       reflog_expiry_cleanup_fn cleanup_fn,\n@@ -3142,7 +3137,8 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \tconst struct object_id *oid;\n \n \tmemset(&cb, 0, sizeof(cb));\n-\tcb.flags = flags;\n+\tcb.rewrite = !!(expire_flags & EXPIRE_REFLOGS_REWRITE);\n+\tcb.dry_run = !!(expire_flags & EXPIRE_REFLOGS_DRY_RUN);\n \tcb.policy_cb = policy_cb_data;\n \tcb.should_prune_fn = should_prune_fn;\n \n@@ -3178,7 +3174,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \n \tfiles_reflog_path(refs, &log_file_sb, refname);\n \tlog_file = strbuf_detach(&log_file_sb, NULL);\n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\tif (!cb.dry_run) {\n \t\t/*\n \t\t * Even though holding $GIT_DIR/logs/$reflog.lock has\n \t\t * no locking implications, we use the lock_file\n@@ -3205,7 +3201,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \trefs_for_each_reflog_ent(ref_store, refname, expire_reflog_ent, &cb);\n \t(*cleanup_fn)(cb.policy_cb);\n \n-\tif (!(flags & EXPIRE_REFLOGS_DRY_RUN)) {\n+\tif (!cb.dry_run) {\n \t\t/*\n \t\t * It doesn't make sense to adjust a reference pointed\n \t\t * to by a symbolic ref based on expiring entries in\n@@ -3215,7 +3211,7 @@ static int files_reflog_expire(struct ref_store *ref_store,\n \t\t */\n \t\tint update = 0;\n \n-\t\tif ((flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n+\t\tif ((expire_flags & EXPIRE_REFLOGS_UPDATE_REF) &&\n \t\t    !is_null_oid(&cb.last_kept_oid)) {\n \t\t\tint ignore_errno;\n \t\t\tint type;\n-- \n2.34.1.1146.gb52885e7c44\n\n"}]}