{"thread":{"id":"64327","subject":"[PATCH 0/4] refs: cleanup code around optimizations","startedAt":"2025-10-15T21:25:18Z","lastAt":"2025-10-16T12:15:11Z","messageCount":14,"participants":["Karthik Nayak","Justin Tobler","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"528854","messageId":"20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com","threadId":"64327","inReplyTo":null,"subject":"[PATCH 0/4] refs: cleanup code around optimizations","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-15T21:25:05Z","receivedAt":"2025-10-15T21:25:18Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"This is extracted from a recent series I sent [1], which I've since\ndropped to follow up with a different approach. I think these patches\nhold value individually.\n\nThey mostly cleanup code around 'git refs optimize' which was added\nrecently in db0babf9b2 (Merge branch 'ms/refs-optimize', 2025-10-02).\nThe code in the refs subsystem contains both 'pack-refs' and 'optimize'\nfunctions, which are one and the same.\n\nThis series unifies this to only retain the 'optimize' functions and\nnaming, since it backend generic.\n\nThis is based on top of master 143f58ef75 (Sync with Git 2.51.1,\n2025-10-15) with 'ps/ref-peeled-tags' merged in.\n\n[1]: 20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n pack-refs.c                   |  8 ++++----\n refs.c                        |  8 +-------\n refs.h                        | 24 +++++++++---------------\n refs/debug.c                  |  8 ++++----\n refs/files-backend.c          | 22 ++++++----------------\n refs/packed-backend.c         |  6 +++---\n refs/refs-internal.h          |  5 +----\n refs/reftable-backend.c       | 15 ++++-----------\n t/pack-refs-tests.sh          |  2 --\n t/t0601-reffiles-pack-refs.sh |  2 ++\n t/t1463-refs-optimize.sh      |  2 ++\n 11 files changed, 36 insertions(+), 66 deletions(-)\n\nKarthik Nayak (4):\n      refs: move to using the '.optimize' functions\n      refs: cleanup code around optimization\n      refs: rename 'pack_refs_opts' to 'refs_optimize_opts'\n      t/pack-refs-tests: move the 'test_done' to callees\n\n\n\nbase-commit: 854a80fd48848f942c4a566c9880dc5f089887af\nchange-id: 20251015-refs-code-cleanup-871a7ce7c8dd\n\nThanks\n- Karthik\n\n"},{"id":"528855","messageId":"20251015-refs-code-cleanup-v1-1-550fdd8a3b41@gmail.com","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com","subject":"[PATCH 1/4] refs: move to using the '.optimize' functions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-15T21:25:06Z","receivedAt":"2025-10-15T21:25:19Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `struct ref_store` variable, exposes two ways to optimize a reftable\nbackend:\n\n  1. pack_refs\n  2. optimize\n\nThe former was specific to the 'files' + 'packed' refs backend. The\nlatter is more generic and covers all backends. While the naming is\ndifferent, both of these functions perform the same functionality.\n\nIn the following commit, we will consolidate this code to only maintain\nthe 'optimize' functions. In preparation, modify the backends so that\nthey exclusively implement the `optimize` callback, only. All users of\nthe refs subsystem already use the 'optimize' function so there is no\nchanges needed on the callee side.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n refs/debug.c            |  8 ++++----\n refs/files-backend.c    | 14 ++------------\n refs/packed-backend.c   |  6 +++---\n refs/reftable-backend.c | 13 +++----------\n 4 files changed, 12 insertions(+), 29 deletions(-)\n\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 01499b9033..40cd1d9c15 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,\n \treturn res;\n }\n \n-static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)\n+static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n {\n \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n-\tint res = drefs->refs->be->pack_refs(drefs->refs, opts);\n-\ttrace_printf_key(&trace_refs, \"pack_refs: %d\\n\", res);\n+\tint res = drefs->refs->be->optimize(drefs->refs, opts);\n+\ttrace_printf_key(&trace_refs, \"optimize: %d\\n\", res);\n \treturn res;\n }\n \n@@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {\n \t.transaction_finish = debug_transaction_finish,\n \t.transaction_abort = debug_transaction_abort,\n \n-\t.pack_refs = debug_pack_refs,\n+\t.optimize = debug_optimize,\n \t.rename_ref = debug_rename_ref,\n \t.copy_ref = debug_copy_ref,\n \ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex ed8a1729d6..92d90fc508 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1444,8 +1444,8 @@ static int should_pack_refs(struct files_ref_store *refs,\n \treturn 0;\n }\n \n-static int files_pack_refs(struct ref_store *ref_store,\n-\t\t\t   struct pack_refs_opts *opts)\n+static int files_optimize(struct ref_store *ref_store,\n+\t\t\t  struct pack_refs_opts *opts)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,\n@@ -1512,15 +1512,6 @@ static int files_pack_refs(struct ref_store *ref_store,\n \treturn 0;\n }\n \n-static int files_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n-{\n-\t/*\n-\t * For the \"files\" backend, \"optimizing\" is the same as \"packing\".\n-\t * So, we just call the existing worker function for packing.\n-\t */\n-\treturn files_pack_refs(ref_store, opts);\n-}\n-\n /*\n  * People using contrib's git-new-workdir have .git/logs/refs ->\n  * /some/other/path/.git/logs/refs, and that may live on another device.\n@@ -3969,7 +3960,6 @@ struct ref_storage_be refs_be_files = {\n \t.transaction_finish = files_transaction_finish,\n \t.transaction_abort = files_transaction_abort,\n \n-\t.pack_refs = files_pack_refs,\n \t.optimize = files_optimize,\n \t.rename_ref = files_rename_ref,\n \t.copy_ref = files_copy_ref,\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 1ab0c50393..20cf9fab18 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1773,8 +1773,8 @@ static int packed_transaction_finish(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-static int packed_pack_refs(struct ref_store *ref_store UNUSED,\n-\t\t\t    struct pack_refs_opts *pack_opts UNUSED)\n+static int packed_optimize(struct ref_store *ref_store UNUSED,\n+\t\t\t   struct pack_refs_opts *pack_opts UNUSED)\n {\n \t/*\n \t * Packed refs are already packed. It might be that loose refs\n@@ -2129,7 +2129,7 @@ struct ref_storage_be refs_be_packed = {\n \t.transaction_finish = packed_transaction_finish,\n \t.transaction_abort = packed_transaction_abort,\n \n-\t.pack_refs = packed_pack_refs,\n+\t.optimize = packed_optimize,\n \t.rename_ref = NULL,\n \t.copy_ref = NULL,\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 6bbfd5618d..43cc66a48e 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1700,11 +1700,11 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,\n \treturn ret;\n }\n \n-static int reftable_be_pack_refs(struct ref_store *ref_store,\n-\t\t\t\t struct pack_refs_opts *opts)\n+static int reftable_be_optimize(struct ref_store *ref_store,\n+\t\t\t\tstruct pack_refs_opts *opts)\n {\n \tstruct reftable_ref_store *refs =\n-\t\treftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, \"pack_refs\");\n+\t\treftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, \"optimize_refs\");\n \tstruct reftable_stack *stack;\n \tint ret;\n \n@@ -1733,12 +1733,6 @@ static int reftable_be_pack_refs(struct ref_store *ref_store,\n \treturn ret;\n }\n \n-static int reftable_be_optimize(struct ref_store *ref_store,\n-\t\t\t\tstruct pack_refs_opts *opts)\n-{\n-\treturn reftable_be_pack_refs(ref_store, opts);\n-}\n-\n struct write_create_symref_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_stack *stack;\n@@ -2761,7 +2755,6 @@ struct ref_storage_be refs_be_reftable = {\n \t.transaction_finish = reftable_be_transaction_finish,\n \t.transaction_abort = reftable_be_transaction_abort,\n \n-\t.pack_refs = reftable_be_pack_refs,\n \t.optimize = reftable_be_optimize,\n \t.rename_ref = reftable_be_rename_ref,\n \t.copy_ref = reftable_be_copy_ref,\n\n-- \n2.51.0\n\n"},{"id":"528856","messageId":"20251015-refs-code-cleanup-v1-2-550fdd8a3b41@gmail.com","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com","subject":"[PATCH 2/4] refs: cleanup code around optimization","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-15T21:25:07Z","receivedAt":"2025-10-15T21:25:20Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The previous commit, moved all backends to only use/support the\n'optimize' function within the `ref_store` structure. With this, cleanup\nall references to the 'pack_refs' field of the structure and code around\nit.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n refs.c               | 6 ------\n refs.h               | 8 +-------\n refs/refs-internal.h | 3 ---\n 3 files changed, 1 insertion(+), 16 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex a41a94ae55..b9a4a60646 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2313,12 +2313,6 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,\n \trefs->gitdir = xstrdup(path);\n }\n \n-/* backend functions */\n-int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)\n-{\n-\treturn refs->be->pack_refs(refs, opts);\n-}\n-\n int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)\n {\n \treturn refs->be->optimize(refs, opts);\ndiff --git a/refs.h b/refs.h\nindex 23437d1220..04e917fec0 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -499,7 +499,7 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,\n \t\t\t\tconst struct string_list *refnames);\n \n /*\n- * Flags for controlling behaviour of pack_refs()\n+ * Flags for controlling behaviour of refs_optimize()\n  * PACK_REFS_PRUNE: Prune loose refs after packing\n  * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end\n  *                 result are decided by the ref backend. Backends may ignore\n@@ -514,12 +514,6 @@ struct pack_refs_opts {\n \tstruct string_list *includes;\n };\n \n-/*\n- * Write a packed-refs file for the current repository.\n- * flags: Combination of the above PACK_REFS_* flags.\n- */\n-int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);\n-\n /*\n  * Optimize the ref store. The exact behavior is up to the backend.\n  * For the files backend, this is equivalent to packing refs.\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex 4671517dad..fc5149df5b 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -422,8 +422,6 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,\n \t\t\t\t      struct ref_transaction *transaction,\n \t\t\t\t      struct strbuf *err);\n \n-typedef int pack_refs_fn(struct ref_store *ref_store,\n-\t\t\t struct pack_refs_opts *opts);\n typedef int optimize_fn(struct ref_store *ref_store,\n \t\t\tstruct pack_refs_opts *opts);\n typedef int rename_ref_fn(struct ref_store *ref_store,\n@@ -550,7 +548,6 @@ struct ref_storage_be {\n \tref_transaction_finish_fn *transaction_finish;\n \tref_transaction_abort_fn *transaction_abort;\n \n-\tpack_refs_fn *pack_refs;\n \toptimize_fn *optimize;\n \trename_ref_fn *rename_ref;\n \tcopy_ref_fn *copy_ref;\n\n-- \n2.51.0\n\n"},{"id":"528857","messageId":"20251015-refs-code-cleanup-v1-3-550fdd8a3b41@gmail.com","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com","subject":"[PATCH 3/4] refs: rename 'pack_refs_opts' to 'refs_optimize_opts'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-15T21:25:08Z","receivedAt":"2025-10-15T21:25:21Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The previous commit removed all references to 'pack_refs()' within\nthe refs subsystem. Continue this cleanup by also renaming\n'pack_refs_opts' to 'refs_optimize_opts' and the respective flags\naccordingly. Keeping the naming consistent will make the code easier to\nmaintain.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n pack-refs.c             |  8 ++++----\n refs.c                  |  2 +-\n refs.h                  | 16 ++++++++--------\n refs/debug.c            |  2 +-\n refs/files-backend.c    | 10 +++++-----\n refs/packed-backend.c   |  2 +-\n refs/refs-internal.h    |  2 +-\n refs/reftable-backend.c |  4 ++--\n 8 files changed, 23 insertions(+), 23 deletions(-)\n\ndiff --git a/pack-refs.c b/pack-refs.c\nindex 1a5e07d8b8..d0ffed93c1 100644\n--- a/pack-refs.c\n+++ b/pack-refs.c\n@@ -14,10 +14,10 @@ int pack_refs_core(int argc,\n {\n \tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n \tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n-\tstruct pack_refs_opts pack_refs_opts = {\n+\tstruct refs_optimize_opts pack_refs_opts = {\n \t\t.exclusions = &excludes,\n \t\t.includes = &included_refs,\n-\t\t.flags = PACK_REFS_PRUNE,\n+\t\t.flags = REFS_OPTIMIZE_PRUNE,\n \t};\n \tstruct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;\n \tstruct string_list_item *item;\n@@ -26,8 +26,8 @@ int pack_refs_core(int argc,\n \n \tstruct option opts[] = {\n \t\tOPT_BOOL(0, \"all\",   &pack_all, N_(\"pack everything\")),\n-\t\tOPT_BIT(0, \"prune\", &pack_refs_opts.flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n-\t\tOPT_BIT(0, \"auto\", &pack_refs_opts.flags, N_(\"auto-pack refs as needed\"), PACK_REFS_AUTO),\n+\t\tOPT_BIT(0, \"prune\", &pack_refs_opts.flags, N_(\"prune loose refs (default)\"), REFS_OPTIMIZE_PRUNE),\n+\t\tOPT_BIT(0, \"auto\", &pack_refs_opts.flags, N_(\"auto-pack refs as needed\"), REFS_OPTIMIZE_AUTO),\n \t\tOPT_STRING_LIST(0, \"include\", pack_refs_opts.includes, N_(\"pattern\"),\n \t\t\tN_(\"references to include\")),\n \t\tOPT_STRING_LIST(0, \"exclude\", &option_excluded_refs, N_(\"pattern\"),\ndiff --git a/refs.c b/refs.c\nindex b9a4a60646..0d0831f29b 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,\n \trefs->gitdir = xstrdup(path);\n }\n \n-int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)\n+int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n {\n \treturn refs->be->optimize(refs, opts);\n }\ndiff --git a/refs.h b/refs.h\nindex 04e917fec0..d2630af97f 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,\n \n /*\n  * Flags for controlling behaviour of refs_optimize()\n- * PACK_REFS_PRUNE: Prune loose refs after packing\n- * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end\n- *                 result are decided by the ref backend. Backends may ignore\n- *                 this flag and fall back to a normal repack.\n+ * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing\n+ * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end\n+ *                     result are decided by the ref backend. Backends may ignore\n+ *                     this flag and fall back to a normal repack.\n  */\n-#define PACK_REFS_PRUNE (1 << 0)\n-#define PACK_REFS_AUTO  (1 << 1)\n+#define REFS_OPTIMIZE_PRUNE (1 << 0)\n+#define REFS_OPTIMIZE_AUTO  (1 << 1)\n \n-struct pack_refs_opts {\n+struct refs_optimize_opts {\n \tunsigned int flags;\n \tstruct ref_exclusions *exclusions;\n \tstruct string_list *includes;\n@@ -518,7 +518,7 @@ struct pack_refs_opts {\n  * Optimize the ref store. The exact behavior is up to the backend.\n  * For the files backend, this is equivalent to packing refs.\n  */\n-int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts);\n+int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n \n /*\n  * Setup reflog before using. Fill in err and return -1 on failure.\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 40cd1d9c15..2defd2d465 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -116,7 +116,7 @@ static int debug_transaction_abort(struct ref_store *refs,\n \treturn res;\n }\n \n-static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n+static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts *opts)\n {\n \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n \tint res = drefs->refs->be->optimize(drefs->refs, opts);\ndiff --git a/refs/files-backend.c b/refs/files-backend.c\nindex 92d90fc508..2f60395cd2 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1355,7 +1355,7 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_\n  */\n static int should_pack_ref(struct files_ref_store *refs,\n \t\t\t   const struct reference *ref,\n-\t\t\t   struct pack_refs_opts *opts)\n+\t\t\t   struct refs_optimize_opts *opts)\n {\n \tstruct string_list_item *item;\n \n@@ -1383,7 +1383,7 @@ static int should_pack_ref(struct files_ref_store *refs,\n }\n \n static int should_pack_refs(struct files_ref_store *refs,\n-\t\t\t    struct pack_refs_opts *opts)\n+\t\t\t    struct refs_optimize_opts *opts)\n {\n \tstruct ref_iterator *iter;\n \tsize_t packed_size;\n@@ -1391,7 +1391,7 @@ static int should_pack_refs(struct files_ref_store *refs,\n \tsize_t limit;\n \tint ret;\n \n-\tif (!(opts->flags & PACK_REFS_AUTO))\n+\tif (!(opts->flags & REFS_OPTIMIZE_AUTO))\n \t\treturn 1;\n \n \tret = packed_refs_size(refs->packed_ref_store, &packed_size);\n@@ -1445,7 +1445,7 @@ static int should_pack_refs(struct files_ref_store *refs,\n }\n \n static int files_optimize(struct ref_store *ref_store,\n-\t\t\t  struct pack_refs_opts *opts)\n+\t\t\t  struct refs_optimize_opts *opts)\n {\n \tstruct files_ref_store *refs =\n \t\tfiles_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,\n@@ -1488,7 +1488,7 @@ static int files_optimize(struct ref_store *ref_store,\n \t\t\t    iter->ref.name, err.buf);\n \n \t\t/* Schedule the loose reference for pruning if requested. */\n-\t\tif ((opts->flags & PACK_REFS_PRUNE)) {\n+\t\tif ((opts->flags & REFS_OPTIMIZE_PRUNE)) {\n \t\t\tstruct ref_to_prune *n;\n \t\t\tFLEX_ALLOC_STR(n, name, iter->ref.name);\n \t\t\toidcpy(&n->oid, iter->ref.oid);\ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 20cf9fab18..0aa0ff6701 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store,\n }\n \n static int packed_optimize(struct ref_store *ref_store UNUSED,\n-\t\t\t   struct pack_refs_opts *pack_opts UNUSED)\n+\t\t\t   struct refs_optimize_opts *pack_opts UNUSED)\n {\n \t/*\n \t * Packed refs are already packed. It might be that loose refs\ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex fc5149df5b..dee42f231d 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -423,7 +423,7 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,\n \t\t\t\t      struct strbuf *err);\n \n typedef int optimize_fn(struct ref_store *ref_store,\n-\t\t\tstruct pack_refs_opts *opts);\n+\t\t\tstruct refs_optimize_opts *opts);\n typedef int rename_ref_fn(struct ref_store *ref_store,\n \t\t\t  const char *oldref, const char *newref,\n \t\t\t  const char *logmsg);\ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex 43cc66a48e..c23c45f3bf 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1701,7 +1701,7 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,\n }\n \n static int reftable_be_optimize(struct ref_store *ref_store,\n-\t\t\t\tstruct pack_refs_opts *opts)\n+\t\t\t\tstruct refs_optimize_opts *opts)\n {\n \tstruct reftable_ref_store *refs =\n \t\treftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, \"optimize_refs\");\n@@ -1715,7 +1715,7 @@ static int reftable_be_optimize(struct ref_store *ref_store,\n \tif (!stack)\n \t\tstack = refs->main_backend.stack;\n \n-\tif (opts->flags & PACK_REFS_AUTO)\n+\tif (opts->flags & REFS_OPTIMIZE_AUTO)\n \t\tret = reftable_stack_auto_compact(stack);\n \telse\n \t\tret = reftable_stack_compact_all(stack, NULL);\n\n-- \n2.51.0\n\n"},{"id":"528858","messageId":"20251015-refs-code-cleanup-v1-4-550fdd8a3b41@gmail.com","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com","subject":"[PATCH 4/4] t/pack-refs-tests: move the 'test_done' to callees","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-15T21:25:09Z","receivedAt":"2025-10-15T21:25:22Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"In ac0bad0af4 (t0601: refactor tests to be shareable, 2025-09-19), we\nrefactored 't/t0601-reffiles-pack-refs.sh' to move all of the tests to\n't/pack-refs-tests.sh', which became a common test suite which was also\nused by 't/t1463-refs-optimize.sh'.\n\nThis also moved the 'test_done' directive to 't/pack-refs-tests.sh'.\nWhich inhibits additional tests from being added to either of the tests.\nLet's move the directive out to both the tests, so that we can add\nadditional specific tests to them. Also the test flow logic shouldn't be\npart of tests which can be embedded in other test scripts.\n\nSigned-off-by: Karthik Nayak <karthik.188@gmail.com>\n---\n t/pack-refs-tests.sh          | 2 --\n t/t0601-reffiles-pack-refs.sh | 2 ++\n t/t1463-refs-optimize.sh      | 2 ++\n 3 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh\nindex 095823d915..81086c3690 100644\n--- a/t/pack-refs-tests.sh\n+++ b/t/pack-refs-tests.sh\n@@ -459,5 +459,3 @@ test_expect_success 'pack-refs does not store invalid peeled tag value' '\n \t\ttest_grep ! \"^\\^\" .git/packed-refs\n \t)\n '\n-\n-test_done\ndiff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh\nindex 12cf5d1dcb..3c706978ef 100755\n--- a/t/t0601-reffiles-pack-refs.sh\n+++ b/t/t0601-reffiles-pack-refs.sh\n@@ -18,3 +18,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT\n . ./test-lib.sh\n \n . \"$TEST_DIRECTORY\"/pack-refs-tests.sh\n+\n+test_done\ndiff --git a/t/t1463-refs-optimize.sh b/t/t1463-refs-optimize.sh\nindex c11c905d79..9afe3c1ed7 100755\n--- a/t/t1463-refs-optimize.sh\n+++ b/t/t1463-refs-optimize.sh\n@@ -15,3 +15,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT\n \n pack_refs='refs optimize'\n . \"$TEST_DIRECTORY\"/pack-refs-tests.sh\n+\n+test_done\n\n-- \n2.51.0\n\n"},{"id":"528865","messageId":"zah4lafziwkqty2qw6erkbq3wfilfneom5vm6vcfqk5b644iff@rcebhmsrgmmd","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-1-550fdd8a3b41@gmail.com","subject":"Re: [PATCH 1/4] refs: move to using the '.optimize' functions","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-15T22:05:46Z","receivedAt":"2025-10-15T22:05:48Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/15 11:25PM, Karthik Nayak wrote:\n> The `struct ref_store` variable, exposes two ways to optimize a reftable\n\ns/variable,/variable/\n\n> backend:\n> \n>   1. pack_refs\n>   2. optimize\n> \n> The former was specific to the 'files' + 'packed' refs backend. The\n> latter is more generic and covers all backends. While the naming is\n> different, both of these functions perform the same functionality.\n> \n> In the following commit, we will consolidate this code to only maintain\n> the 'optimize' functions. In preparation, modify the backends so that\n> they exclusively implement the `optimize` callback, only. All users of\n> the refs subsystem already use the 'optimize' function so there is no\n> changes needed on the callee side.\n> \n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  refs/debug.c            |  8 ++++----\n>  refs/files-backend.c    | 14 ++------------\n>  refs/packed-backend.c   |  6 +++---\n>  refs/reftable-backend.c | 13 +++----------\n>  4 files changed, 12 insertions(+), 29 deletions(-)\n> \n> diff --git a/refs/debug.c b/refs/debug.c\n> index 01499b9033..40cd1d9c15 100644\n> --- a/refs/debug.c\n> +++ b/refs/debug.c\n> @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,\n>  \treturn res;\n>  }\n>  \n> -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)\n> +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n>  {\n>  \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n> -\tint res = drefs->refs->be->pack_refs(drefs->refs, opts);\n> -\ttrace_printf_key(&trace_refs, \"pack_refs: %d\\n\", res);\n> +\tint res = drefs->refs->be->optimize(drefs->refs, opts);\n> +\ttrace_printf_key(&trace_refs, \"optimize: %d\\n\", res);\n>  \treturn res;\n>  }\n>  \n> @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {\n>  \t.transaction_finish = debug_transaction_finish,\n>  \t.transaction_abort = debug_transaction_abort,\n>  \n> -\t.pack_refs = debug_pack_refs,\n> +\t.optimize = debug_optimize,\n\nquestion: Was the debug backend not using either of these callbacks?\nFrom the commit message, it sounds like all the backends were using the\noptimize callback.\n\n>  \t.rename_ref = debug_rename_ref,\n>  \t.copy_ref = debug_copy_ref,\n>  \n> diff --git a/refs/files-backend.c b/refs/files-backend.c\n> index ed8a1729d6..92d90fc508 100644\n> --- a/refs/files-backend.c\n> +++ b/refs/files-backend.c\n> @@ -1444,8 +1444,8 @@ static int should_pack_refs(struct files_ref_store *refs,\n>  \treturn 0;\n>  }\n>  \n> -static int files_pack_refs(struct ref_store *ref_store,\n> -\t\t\t   struct pack_refs_opts *opts)\n> +static int files_optimize(struct ref_store *ref_store,\n> +\t\t\t  struct pack_refs_opts *opts)\n>  {\n>  \tstruct files_ref_store *refs =\n>  \t\tfiles_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,\n> @@ -1512,15 +1512,6 @@ static int files_pack_refs(struct ref_store *ref_store,\n>  \treturn 0;\n>  }\n>  \n> -static int files_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n> -{\n> -\t/*\n> -\t * For the \"files\" backend, \"optimizing\" is the same as \"packing\".\n> -\t * So, we just call the existing worker function for packing.\n> -\t */\n> -\treturn files_pack_refs(ref_store, opts);\n> -}\n> -\n>  /*\n>   * People using contrib's git-new-workdir have .git/logs/refs ->\n>   * /some/other/path/.git/logs/refs, and that may live on another device.\n> @@ -3969,7 +3960,6 @@ struct ref_storage_be refs_be_files = {\n>  \t.transaction_finish = files_transaction_finish,\n>  \t.transaction_abort = files_transaction_abort,\n>  \n> -\t.pack_refs = files_pack_refs,\n>  \t.optimize = files_optimize,\n\nOk, we are removing the \"pack_refs\" callback and its implementations\nfrom all the backends in favor of using the just using the more generic\n\"optimize\" callback. Make sense.\n\nIt does look like we still have `refs_pack_refs()` which references the\n\"optimize\" callback. It looks like there are no users, but should we\nalso remove it as part of this patch?\n\n-Justin\n"},{"id":"528867","messageId":"wlq4kncpjgbsnc6bjhusgu32d3apagcykax2tquotdanyfejcy@2k4tlhv35fxy","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-2-550fdd8a3b41@gmail.com","subject":"Re: [PATCH 2/4] refs: cleanup code around optimization","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-15T22:09:16Z","receivedAt":"2025-10-15T22:09:17Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/15 11:25PM, Karthik Nayak wrote:\n> The previous commit moved all backends to only use/support the\n\ns/commit,/commit/\n\n> 'optimize' function within the `ref_store` structure. With this, cleanup\n> all references to the 'pack_refs' field of the structure and code around\n> it.\n> \n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  refs.c               | 6 ------\n>  refs.h               | 8 +-------\n>  refs/refs-internal.h | 3 ---\n>  3 files changed, 1 insertion(+), 16 deletions(-)\n> \n> diff --git a/refs.c b/refs.c\n> index a41a94ae55..b9a4a60646 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2313,12 +2313,6 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,\n>  \trefs->gitdir = xstrdup(path);\n>  }\n>  \n> -/* backend functions */\n> -int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)\n> -{\n> -\treturn refs->be->pack_refs(refs, opts);\n> -}\n\nAh ok, so we are removing `refs_pack_refs()` here in this patch. Maybe\nwe can just squash this change into the previous patch. Probably not a\nbig deal either way.\n\nThe other cleanups in this patch look good.\n\n-Justin\n"},{"id":"528868","messageId":"j4kd7andvwa77uf7yaxvfkrb5bpxyda4awhnyce5fypr3qijsp@sw4bzfd43byg","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-3-550fdd8a3b41@gmail.com","subject":"Re: [PATCH 3/4] refs: rename 'pack_refs_opts' to 'refs_optimize_opts'","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-15T22:18:51Z","receivedAt":"2025-10-15T22:18:52Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/15 11:25PM, Karthik Nayak wrote:\n> The previous commit removed all references to 'pack_refs()' within\n> the refs subsystem. Continue this cleanup by also renaming\n> 'pack_refs_opts' to 'refs_optimize_opts' and the respective flags\n> accordingly. Keeping the naming consistent will make the code easier to\n> maintain.\n> \n> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n> ---\n>  pack-refs.c             |  8 ++++----\n>  refs.c                  |  2 +-\n>  refs.h                  | 16 ++++++++--------\n>  refs/debug.c            |  2 +-\n>  refs/files-backend.c    | 10 +++++-----\n>  refs/packed-backend.c   |  2 +-\n>  refs/refs-internal.h    |  2 +-\n>  refs/reftable-backend.c |  4 ++--\n>  8 files changed, 23 insertions(+), 23 deletions(-)\n> \n> diff --git a/pack-refs.c b/pack-refs.c\n> index 1a5e07d8b8..d0ffed93c1 100644\n> --- a/pack-refs.c\n> +++ b/pack-refs.c\n> @@ -14,10 +14,10 @@ int pack_refs_core(int argc,\n>  {\n>  \tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n>  \tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n> -\tstruct pack_refs_opts pack_refs_opts = {\n> +\tstruct refs_optimize_opts pack_refs_opts = {\n\nWe could rename the variable name here to, but probably not a big deal\neither way.\n\n>  \t\t.exclusions = &excludes,\n>  \t\t.includes = &included_refs,\n> -\t\t.flags = PACK_REFS_PRUNE,\n> +\t\t.flags = REFS_OPTIMIZE_PRUNE,\n>  \t};\n>  \tstruct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;\n>  \tstruct string_list_item *item;\n> @@ -26,8 +26,8 @@ int pack_refs_core(int argc,\n>  \n>  \tstruct option opts[] = {\n>  \t\tOPT_BOOL(0, \"all\",   &pack_all, N_(\"pack everything\")),\n> -\t\tOPT_BIT(0, \"prune\", &pack_refs_opts.flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n> -\t\tOPT_BIT(0, \"auto\", &pack_refs_opts.flags, N_(\"auto-pack refs as needed\"), PACK_REFS_AUTO),\n> +\t\tOPT_BIT(0, \"prune\", &pack_refs_opts.flags, N_(\"prune loose refs (default)\"), REFS_OPTIMIZE_PRUNE),\n> +\t\tOPT_BIT(0, \"auto\", &pack_refs_opts.flags, N_(\"auto-pack refs as needed\"), REFS_OPTIMIZE_AUTO),\n>  \t\tOPT_STRING_LIST(0, \"include\", pack_refs_opts.includes, N_(\"pattern\"),\n>  \t\t\tN_(\"references to include\")),\n>  \t\tOPT_STRING_LIST(0, \"exclude\", &option_excluded_refs, N_(\"pattern\"),\n> diff --git a/refs.c b/refs.c\n> index b9a4a60646..0d0831f29b 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,\n>  \trefs->gitdir = xstrdup(path);\n>  }\n>  \n> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)\n> +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n>  {\n>  \treturn refs->be->optimize(refs, opts);\n>  }\n> diff --git a/refs.h b/refs.h\n> index 04e917fec0..d2630af97f 100644\n> --- a/refs.h\n> +++ b/refs.h\n> @@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,\n>  \n>  /*\n>   * Flags for controlling behaviour of refs_optimize()\n> - * PACK_REFS_PRUNE: Prune loose refs after packing\n> - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end\n> - *                 result are decided by the ref backend. Backends may ignore\n> - *                 this flag and fall back to a normal repack.\n> + * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing\n> + * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end\n> + *                     result are decided by the ref backend. Backends may ignore\n> + *                     this flag and fall back to a normal repack.\n>   */\n> -#define PACK_REFS_PRUNE (1 << 0)\n> -#define PACK_REFS_AUTO  (1 << 1)\n> +#define REFS_OPTIMIZE_PRUNE (1 << 0)\n> +#define REFS_OPTIMIZE_AUTO  (1 << 1)\n>  \n> -struct pack_refs_opts {\n> +struct refs_optimize_opts {\n>  \tunsigned int flags;\n>  \tstruct ref_exclusions *exclusions;\n>  \tstruct string_list *includes;\n> @@ -518,7 +518,7 @@ struct pack_refs_opts {\n>   * Optimize the ref store. The exact behavior is up to the backend.\n>   * For the files backend, this is equivalent to packing refs.\n>   */\n> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts);\n> +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n\nI noticed when poking around the code that the optimize callback was\nstill using the `pack_refs_opts`. Nice to see this clean up. :)\n\n>  /*\n>   * Setup reflog before using. Fill in err and return -1 on failure.\n[snip]\n> diff --git a/refs/packed-backend.c b/refs/packed-backend.c\n> index 20cf9fab18..0aa0ff6701 100644\n> --- a/refs/packed-backend.c\n> +++ b/refs/packed-backend.c\n> @@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store,\n>  }\n>  \n>  static int packed_optimize(struct ref_store *ref_store UNUSED,\n> -\t\t\t   struct pack_refs_opts *pack_opts UNUSED)\n> +\t\t\t   struct refs_optimize_opts *pack_opts UNUSED)\n\nAlso not a big deal, but we could rename `pack_opts` here to just\n`opts`. It's not even used anyways.\n\nAll the other trivial renames in this patch look good.\n\n-Justin\n"},{"id":"528946","messageId":"aPDKzqT2YaG2NmEu@pks.im","threadId":"64327","inReplyTo":"zah4lafziwkqty2qw6erkbq3wfilfneom5vm6vcfqk5b644iff@rcebhmsrgmmd","subject":"Re: [PATCH 1/4] refs: move to using the '.optimize' functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-16T10:37:02Z","receivedAt":"2025-10-16T10:37:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 15, 2025 at 05:05:46PM -0500, Justin Tobler wrote:\n> On 25/10/15 11:25PM, Karthik Nayak wrote:\n> > diff --git a/refs/debug.c b/refs/debug.c\n> > index 01499b9033..40cd1d9c15 100644\n> > --- a/refs/debug.c\n> > +++ b/refs/debug.c\n> > @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,\n> >  \treturn res;\n> >  }\n> >  \n> > -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)\n> > +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n> >  {\n> >  \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n> > -\tint res = drefs->refs->be->pack_refs(drefs->refs, opts);\n> > -\ttrace_printf_key(&trace_refs, \"pack_refs: %d\\n\", res);\n> > +\tint res = drefs->refs->be->optimize(drefs->refs, opts);\n> > +\ttrace_printf_key(&trace_refs, \"optimize: %d\\n\", res);\n> >  \treturn res;\n> >  }\n> >  \n> > @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {\n> >  \t.transaction_finish = debug_transaction_finish,\n> >  \t.transaction_abort = debug_transaction_abort,\n> >  \n> > -\t.pack_refs = debug_pack_refs,\n> > +\t.optimize = debug_optimize,\n> \n> question: Was the debug backend not using either of these callbacks?\n> From the commit message, it sounds like all the backends were using the\n> optimize callback.\n\nDoesn't look like it. Overall I kind of doubt the value that this\nbackend has. I have never had even a single use case for it, and I have\nbeen working with references extensively over the last two or three\nyears by now.\n\nMaybe we should just drop it eventually?\n\nPatrick\n"},{"id":"528947","messageId":"aPDLCqDd7Sri-0xb@pks.im","threadId":"64327","inReplyTo":"20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com","subject":"Re: [PATCH 0/4] refs: cleanup code around optimizations","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-16T10:38:02Z","receivedAt":"2025-10-16T10:38:14Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Wed, Oct 15, 2025 at 11:25:05PM +0200, Karthik Nayak wrote:\n> This is extracted from a recent series I sent [1], which I've since\n> dropped to follow up with a different approach. I think these patches\n> hold value individually.\n> \n> They mostly cleanup code around 'git refs optimize' which was added\n> recently in db0babf9b2 (Merge branch 'ms/refs-optimize', 2025-10-02).\n> The code in the refs subsystem contains both 'pack-refs' and 'optimize'\n> functions, which are one and the same.\n> \n> This series unifies this to only retain the 'optimize' functions and\n> naming, since it backend generic.\n> \n> This is based on top of master 143f58ef75 (Sync with Git 2.51.1,\n> 2025-10-15) with 'ps/ref-peeled-tags' merged in.\n\nThanks, these patches all look good to me, except for the small nits\nthat Justin has spotted. I guess this can use one more tiny reroll\nbefore it's ready to be merged.\n\nPatrick\n"},{"id":"528949","messageId":"CAOLa=ZQ-Ri7_SgPttqphRNNB7-AryNcSbcRsjWF_ua9jQxM92g@mail.gmail.com","threadId":"64327","inReplyTo":"zah4lafziwkqty2qw6erkbq3wfilfneom5vm6vcfqk5b644iff@rcebhmsrgmmd","subject":"Re: [PATCH 1/4] refs: move to using the '.optimize' functions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-16T12:03:25Z","receivedAt":"2025-10-16T12:03:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 25/10/15 11:25PM, Karthik Nayak wrote:\n>> The `struct ref_store` variable, exposes two ways to optimize a reftable\n>\n> s/variable,/variable/\n>\n\nWill fix.\n\n>> diff --git a/refs/debug.c b/refs/debug.c\n>> index 01499b9033..40cd1d9c15 100644\n>> --- a/refs/debug.c\n>> +++ b/refs/debug.c\n>> @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,\n>>  \treturn res;\n>>  }\n>>\n>> -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)\n>> +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n>>  {\n>>  \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n>> -\tint res = drefs->refs->be->pack_refs(drefs->refs, opts);\n>> -\ttrace_printf_key(&trace_refs, \"pack_refs: %d\\n\", res);\n>> +\tint res = drefs->refs->be->optimize(drefs->refs, opts);\n>> +\ttrace_printf_key(&trace_refs, \"optimize: %d\\n\", res);\n>>  \treturn res;\n>>  }\n>>\n>> @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {\n>>  \t.transaction_finish = debug_transaction_finish,\n>>  \t.transaction_abort = debug_transaction_abort,\n>>\n>> -\t.pack_refs = debug_pack_refs,\n>> +\t.optimize = debug_optimize,\n>\n> question: Was the debug backend not using either of these callbacks?\n> From the commit message, it sounds like all the backends were using the\n> optimize callback.\n>\n\nSince there are no users of `refs_pack_refs()` which uses the\n'.pack_refs' field, there are no users of 'debug_pack_refs()'.\n\nBut if tracing is enabled then the debug backend would wrap around the\nother backends and any calls to `refs_optimize()` would call\n`debug_optimize()`.\n\n>>  \t.rename_ref = debug_rename_ref,\n>>  \t.copy_ref = debug_copy_ref,\n>>\n>> diff --git a/refs/files-backend.c b/refs/files-backend.c\n>> index ed8a1729d6..92d90fc508 100644\n>> --- a/refs/files-backend.c\n>> +++ b/refs/files-backend.c\n>> @@ -1444,8 +1444,8 @@ static int should_pack_refs(struct files_ref_store *refs,\n>>  \treturn 0;\n>>  }\n>>\n>> -static int files_pack_refs(struct ref_store *ref_store,\n>> -\t\t\t   struct pack_refs_opts *opts)\n>> +static int files_optimize(struct ref_store *ref_store,\n>> +\t\t\t  struct pack_refs_opts *opts)\n>>  {\n>>  \tstruct files_ref_store *refs =\n>>  \t\tfiles_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,\n>> @@ -1512,15 +1512,6 @@ static int files_pack_refs(struct ref_store *ref_store,\n>>  \treturn 0;\n>>  }\n>>\n>> -static int files_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n>> -{\n>> -\t/*\n>> -\t * For the \"files\" backend, \"optimizing\" is the same as \"packing\".\n>> -\t * So, we just call the existing worker function for packing.\n>> -\t */\n>> -\treturn files_pack_refs(ref_store, opts);\n>> -}\n>> -\n>>  /*\n>>   * People using contrib's git-new-workdir have .git/logs/refs ->\n>>   * /some/other/path/.git/logs/refs, and that may live on another device.\n>> @@ -3969,7 +3960,6 @@ struct ref_storage_be refs_be_files = {\n>>  \t.transaction_finish = files_transaction_finish,\n>>  \t.transaction_abort = files_transaction_abort,\n>>\n>> -\t.pack_refs = files_pack_refs,\n>>  \t.optimize = files_optimize,\n>\n> Ok, we are removing the \"pack_refs\" callback and its implementations\n> from all the backends in favor of using the just using the more generic\n> \"optimize\" callback. Make sense.\n>\n> It does look like we still have `refs_pack_refs()` which references the\n> \"optimize\" callback. It looks like there are no users, but should we\n> also remove it as part of this patch?\n>\n> -Justin\n\nIt's done in the next commit, let me just squash that in to make it\neasier.\n"},{"id":"528950","messageId":"CAOLa=ZQGQ_Qan9PtE-akUpWwGg8j6y5xcKpyOpztN7bZp+rMvw@mail.gmail.com","threadId":"64327","inReplyTo":"aPDKzqT2YaG2NmEu@pks.im","subject":"Re: [PATCH 1/4] refs: move to using the '.optimize' functions","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-16T12:07:33Z","receivedAt":"2025-10-16T12:07:35Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Wed, Oct 15, 2025 at 05:05:46PM -0500, Justin Tobler wrote:\n>> On 25/10/15 11:25PM, Karthik Nayak wrote:\n>> > diff --git a/refs/debug.c b/refs/debug.c\n>> > index 01499b9033..40cd1d9c15 100644\n>> > --- a/refs/debug.c\n>> > +++ b/refs/debug.c\n>> > @@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,\n>> >  \treturn res;\n>> >  }\n>> >\n>> > -static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)\n>> > +static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)\n>> >  {\n>> >  \tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n>> > -\tint res = drefs->refs->be->pack_refs(drefs->refs, opts);\n>> > -\ttrace_printf_key(&trace_refs, \"pack_refs: %d\\n\", res);\n>> > +\tint res = drefs->refs->be->optimize(drefs->refs, opts);\n>> > +\ttrace_printf_key(&trace_refs, \"optimize: %d\\n\", res);\n>> >  \treturn res;\n>> >  }\n>> >\n>> > @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {\n>> >  \t.transaction_finish = debug_transaction_finish,\n>> >  \t.transaction_abort = debug_transaction_abort,\n>> >\n>> > -\t.pack_refs = debug_pack_refs,\n>> > +\t.optimize = debug_optimize,\n>>\n>> question: Was the debug backend not using either of these callbacks?\n>> From the commit message, it sounds like all the backends were using the\n>> optimize callback.\n>\n> Doesn't look like it. Overall I kind of doubt the value that this\n> backend has. I have never had even a single use case for it, and I have\n> been working with references extensively over the last two or three\n> years by now.\n>\n> Maybe we should just drop it eventually?\n>\n> Patrick\n\nI'd be happy to drop it too as a Git developer. But I can merit in\nkeeping it. It does wrap around all reference subsystem calls and if\ntracing is enabled it would log these reference calls. This can be used\nin bug reports.\n\nHowever, a counter argument would be that these only track the surface\nlevel APIs to each backend, but not really the internal details.\n\nConsidering this and also the fact that we need to update the backend\nfor every change made, I'm for dropping it too.\n\nKarthik\n"},{"id":"528951","messageId":"aPDhDG1kfja8dWXv@pks.im","threadId":"64327","inReplyTo":"CAOLa=ZQGQ_Qan9PtE-akUpWwGg8j6y5xcKpyOpztN7bZp+rMvw@mail.gmail.com","subject":"Re: [PATCH 1/4] refs: move to using the '.optimize' functions","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-16T12:11:56Z","receivedAt":"2025-10-16T12:12:07Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Oct 16, 2025 at 07:07:33AM -0500, Karthik Nayak wrote:\n> Patrick Steinhardt <ps@pks.im> writes:\n> > On Wed, Oct 15, 2025 at 05:05:46PM -0500, Justin Tobler wrote:\n> >> On 25/10/15 11:25PM, Karthik Nayak wrote:\n> >> > @@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {\n> >> >  \t.transaction_finish = debug_transaction_finish,\n> >> >  \t.transaction_abort = debug_transaction_abort,\n> >> >\n> >> > -\t.pack_refs = debug_pack_refs,\n> >> > +\t.optimize = debug_optimize,\n> >>\n> >> question: Was the debug backend not using either of these callbacks?\n> >> From the commit message, it sounds like all the backends were using the\n> >> optimize callback.\n> >\n> > Doesn't look like it. Overall I kind of doubt the value that this\n> > backend has. I have never had even a single use case for it, and I have\n> > been working with references extensively over the last two or three\n> > years by now.\n> >\n> > Maybe we should just drop it eventually?\n> >\n> > Patrick\n> \n> I'd be happy to drop it too as a Git developer. But I can merit in\n> keeping it. It does wrap around all reference subsystem calls and if\n> tracing is enabled it would log these reference calls. This can be used\n> in bug reports.\n> \n> However, a counter argument would be that these only track the surface\n> level APIs to each backend, but not really the internal details.\n> \n> Considering this and also the fact that we need to update the backend\n> for every change made, I'm for dropping it too.\n\nDigging a bit deeper: I think this actually _is_ exposed via our tracing\nAPI. So if you run Git with GIT_TRACE_REFS= set it will indeed use that\ndebug backend. TIL.\n\nSo with that I'll reverse my stance and say that this probably makes\nsense to keep.\n\nPatrick\n"},{"id":"528952","messageId":"CAOLa=ZT1Ug-UckYZ6B8KrrmzBLxVqAGJPiQfre6otoanWCs9OQ@mail.gmail.com","threadId":"64327","inReplyTo":"j4kd7andvwa77uf7yaxvfkrb5bpxyda4awhnyce5fypr3qijsp@sw4bzfd43byg","subject":"Re: [PATCH 3/4] refs: rename 'pack_refs_opts' to 'refs_optimize_opts'","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-16T12:15:08Z","receivedAt":"2025-10-16T12:15:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler <jltobler@gmail.com> writes:\n\n> On 25/10/15 11:25PM, Karthik Nayak wrote:\n>> The previous commit removed all references to 'pack_refs()' within\n>> the refs subsystem. Continue this cleanup by also renaming\n>> 'pack_refs_opts' to 'refs_optimize_opts' and the respective flags\n>> accordingly. Keeping the naming consistent will make the code easier to\n>> maintain.\n>>\n>> Signed-off-by: Karthik Nayak <karthik.188@gmail.com>\n>> ---\n>>  pack-refs.c             |  8 ++++----\n>>  refs.c                  |  2 +-\n>>  refs.h                  | 16 ++++++++--------\n>>  refs/debug.c            |  2 +-\n>>  refs/files-backend.c    | 10 +++++-----\n>>  refs/packed-backend.c   |  2 +-\n>>  refs/refs-internal.h    |  2 +-\n>>  refs/reftable-backend.c |  4 ++--\n>>  8 files changed, 23 insertions(+), 23 deletions(-)\n>>\n>> diff --git a/pack-refs.c b/pack-refs.c\n>> index 1a5e07d8b8..d0ffed93c1 100644\n>> --- a/pack-refs.c\n>> +++ b/pack-refs.c\n>> @@ -14,10 +14,10 @@ int pack_refs_core(int argc,\n>>  {\n>>  \tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n>>  \tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n>> -\tstruct pack_refs_opts pack_refs_opts = {\n>> +\tstruct refs_optimize_opts pack_refs_opts = {\n>\n> We could rename the variable name here to, but probably not a big deal\n> either way.\n>\n\nSince I'm re-rolling, let me do that :)\n\n>>  \t\t.exclusions = &excludes,\n>>  \t\t.includes = &included_refs,\n>> -\t\t.flags = PACK_REFS_PRUNE,\n>> +\t\t.flags = REFS_OPTIMIZE_PRUNE,\n>>  \t};\n>>  \tstruct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;\n>>  \tstruct string_list_item *item;\n>> @@ -26,8 +26,8 @@ int pack_refs_core(int argc,\n>>\n>>  \tstruct option opts[] = {\n>>  \t\tOPT_BOOL(0, \"all\",   &pack_all, N_(\"pack everything\")),\n>> -\t\tOPT_BIT(0, \"prune\", &pack_refs_opts.flags, N_(\"prune loose refs (default)\"), PACK_REFS_PRUNE),\n>> -\t\tOPT_BIT(0, \"auto\", &pack_refs_opts.flags, N_(\"auto-pack refs as needed\"), PACK_REFS_AUTO),\n>> +\t\tOPT_BIT(0, \"prune\", &pack_refs_opts.flags, N_(\"prune loose refs (default)\"), REFS_OPTIMIZE_PRUNE),\n>> +\t\tOPT_BIT(0, \"auto\", &pack_refs_opts.flags, N_(\"auto-pack refs as needed\"), REFS_OPTIMIZE_AUTO),\n>>  \t\tOPT_STRING_LIST(0, \"include\", pack_refs_opts.includes, N_(\"pattern\"),\n>>  \t\t\tN_(\"references to include\")),\n>>  \t\tOPT_STRING_LIST(0, \"exclude\", &option_excluded_refs, N_(\"pattern\"),\n>> diff --git a/refs.c b/refs.c\n>> index b9a4a60646..0d0831f29b 100644\n>> --- a/refs.c\n>> +++ b/refs.c\n>> @@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,\n>>  \trefs->gitdir = xstrdup(path);\n>>  }\n>>\n>> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)\n>> +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n>>  {\n>>  \treturn refs->be->optimize(refs, opts);\n>>  }\n>> diff --git a/refs.h b/refs.h\n>> index 04e917fec0..d2630af97f 100644\n>> --- a/refs.h\n>> +++ b/refs.h\n>> @@ -500,15 +500,15 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,\n>>\n>>  /*\n>>   * Flags for controlling behaviour of refs_optimize()\n>> - * PACK_REFS_PRUNE: Prune loose refs after packing\n>> - * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end\n>> - *                 result are decided by the ref backend. Backends may ignore\n>> - *                 this flag and fall back to a normal repack.\n>> + * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing\n>> + * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end\n>> + *                     result are decided by the ref backend. Backends may ignore\n>> + *                     this flag and fall back to a normal repack.\n>>   */\n>> -#define PACK_REFS_PRUNE (1 << 0)\n>> -#define PACK_REFS_AUTO  (1 << 1)\n>> +#define REFS_OPTIMIZE_PRUNE (1 << 0)\n>> +#define REFS_OPTIMIZE_AUTO  (1 << 1)\n>>\n>> -struct pack_refs_opts {\n>> +struct refs_optimize_opts {\n>>  \tunsigned int flags;\n>>  \tstruct ref_exclusions *exclusions;\n>>  \tstruct string_list *includes;\n>> @@ -518,7 +518,7 @@ struct pack_refs_opts {\n>>   * Optimize the ref store. The exact behavior is up to the backend.\n>>   * For the files backend, this is equivalent to packing refs.\n>>   */\n>> -int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts);\n>> +int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n>\n> I noticed when poking around the code that the optimize callback was\n> still using the `pack_refs_opts`. Nice to see this clean up. :)\n>\n>>  /*\n>>   * Setup reflog before using. Fill in err and return -1 on failure.\n> [snip]\n>> diff --git a/refs/packed-backend.c b/refs/packed-backend.c\n>> index 20cf9fab18..0aa0ff6701 100644\n>> --- a/refs/packed-backend.c\n>> +++ b/refs/packed-backend.c\n>> @@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store,\n>>  }\n>>\n>>  static int packed_optimize(struct ref_store *ref_store UNUSED,\n>> -\t\t\t   struct pack_refs_opts *pack_opts UNUSED)\n>> +\t\t\t   struct refs_optimize_opts *pack_opts UNUSED)\n>\n> Also not a big deal, but we could rename `pack_opts` here to just\n> `opts`. It's not even used anyways.\n>\n> All the other trivial renames in this patch look good.\n>\n> -Justin\n>\n\nWill do,\n\nThanks for the review.\n"}]}