{"thread":{"id":"64412","subject":"[PATCH 1/5] reftable/stack: return stack segments directly","startedAt":"2025-10-31T14:22:26Z","lastAt":"2025-11-10T06:46:29Z","messageCount":57,"participants":["Karthik Nayak","Justin Tobler","Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":5},"messages":[{"id":"530029","messageId":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-1-a03d53e28d0e@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH 1/5] reftable/stack: return stack segments directly","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-31T14:22:21Z","receivedAt":"2025-10-31T14:22:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `stack_table_sizes_for_compaction()` function returns individual\nsizes of each reftable table. This function is only called by\n`reftable_stack_auto_compact()` to decide which tables need to be\ncompacted, if any.\n\nModify the function to directly return the segments, which avoids the\nextra step of receiving the sizes only to pass it to\n`suggest_compaction_segment()`.\n\nA future commit will also add functionality for checking whether\nauto-compaction is necessary without performing it. This change allows\ncode re-usability in that context.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/stack.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 65d89820bd..49387f9344 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,\n \treturn seg;\n }\n \n-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n+static int stack_segments_for_compaction(struct reftable_stack *st,\n+\t\t\t\t\t struct segment *seg)\n {\n \tint version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;\n \tint overhead = header_size(version) - 1;\n@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n \n \tREFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);\n \tif (!sizes)\n-\t\treturn NULL;\n+\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n \n \tfor (size_t i = 0; i < st->merged->tables_len; i++)\n \t\tsizes[i] = st->tables[i]->size - overhead;\n \n-\treturn sizes;\n+\t*seg = suggest_compaction_segment(sizes, st->merged->tables_len,\n+\t\t\t\t\t st->opts.auto_compaction_factor);\n+\treftable_free(sizes);\n+\n+\treturn 0;\n }\n \n int reftable_stack_auto_compact(struct reftable_stack *st)\n {\n \tstruct segment seg;\n-\tuint64_t *sizes;\n+\tint err;\n \n \tif (st->merged->tables_len < 2)\n \t\treturn 0;\n \n-\tsizes = stack_table_sizes_for_compaction(st);\n-\tif (!sizes)\n-\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\n-\tseg = suggest_compaction_segment(sizes, st->merged->tables_len,\n-\t\t\t\t\t st->opts.auto_compaction_factor);\n-\treftable_free(sizes);\n+\terr = stack_segments_for_compaction(st, &seg);\n+\tif (err)\n+\t\treturn err;\n \n \tif (segment_size(&seg) > 0)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n\n-- \n2.51.0\n\n"},{"id":"530032","messageId":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","threadId":"64412","inReplyTo":null,"subject":"[PATCH 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-31T14:22:20Z","receivedAt":"2025-10-31T14:22:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hello,\n\nI recently raised a patch series [1] to add 'git refs optimize --required'\nwhich checks if the reference backend can be optimized, without actually\nperforming the optimization.\n\nBack then, we had decided [2] that it would be a better to broaden the\napproach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This\nwould allow users to check if maintenance was required for the\nrepository and users could also provide a task via the '--task' to check\nif maintenance was needed for a particular task.\n\nIdeally the subcommand will be used with the '--auto' flag which can\ncheck the same heuristics as that used with 'git maintenance run\n--auto'. Future patches can also add support for the '--schedule' flag\nwhich can be used to check required schedule it met. However that flag\nisn't added as part of this series.\n\nThis series implements that.\n\nCommits 1-3 add the required functionality in the refs subsystem to\nexpose an 'optimize_required' field which can be used to check if\nbackends need to be optimized.\nCommit 4 utilizes this within the 'git-maintenance(1)' code.\nCommit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.\n\nThis is based on top of master a99f379adf (The 27th batch, 2025-10-30)\nand is dependent on the following series:\n\n - kn/refs-optim-cleanup\n - ps/ref-peeled-tags\n\nMerges cleanly with `next`. I think those two topics are close to being\nmerged to `next` so hopefully this dependency tree doesn't get too\ncomplicated. I'll rebase as needed to resolve conflicts.\n\n[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/\n[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/\n\n---\n Documentation/git-maintenance.adoc | 8 ++++\n builtin/gc.c | 86 +++++++++++++++++++++++++++++++++-----\n object.h | 1 -\n refs.c | 7 ++++\n refs.h | 7 ++++\n refs/debug.c | 13 ++++++\n refs/files-backend.c | 11 +++++\n refs/packed-backend.c | 13 ++++++\n refs/refs-internal.h | 6 +++\n refs/reftable-backend.c | 25 +++++++++++\n reftable/reftable-stack.h | 5 +++\n reftable/stack.c | 48 ++++++++++++++++-----\n t/t7900-maintenance.sh | 54 +++++++++++++++++-------\n t/unit-tests/u-reftable-stack.c | 12 +++++-\n 14 files changed, 256 insertions(+), 40 deletions(-)\n\nKarthik Nayak (5):\n reftable/stack: return stack segments directly\n reftable/stack: add function to check if optimization is required\n refs: add a `optimize_required` field to `struct ref_storage_be`\n maintenance: add checking logic in `pack_refs_condition()`\n maintenance: add 'is-needed' subcommand\n\n\n\nbase-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406\nchange-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606\n\nThanks\n- Karthik\n\n"},{"id":"530030","messageId":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-2-a03d53e28d0e@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-31T14:22:22Z","receivedAt":"2025-10-31T14:22:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The reftable backend, performs auto-compaction as part of its regular\nflow, which is required to keep the number of tables part of a stack at\nbay. This allows it to stay optimized.\n\nCompaction can also be triggered voluntarily by the user via the 'git\npack-refs' or the 'git refs optimize' command. However, currently there\nis no way for the user to check if optimization is required without\nactually performing it.\n\nAdd and expose `reftable_stack_compaction_required()` which will allow\nusers to check if the reftable backend can be optimized.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/reftable-stack.h | 5 +++++\n reftable/stack.c | 25 +++++++++++++++++++++++++\n t/unit-tests/u-reftable-stack.c | 12 ++++++++++--\n 3 files changed, 40 insertions(+), 2 deletions(-)\n\ndiff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\nindex d70fcb705d..a875149439 100644\n--- a/reftable/reftable-stack.h\n+++ b/reftable/reftable-stack.h\n@@ -123,6 +123,11 @@ struct reftable_log_expiry_config {\n int reftable_stack_compact_all(struct reftable_stack *st,\n \t\t\t struct reftable_log_expiry_config *config);\n \n+/* Check if compaction is required. */\n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required);\n+\n /* heuristically compact unbalanced table stack. */\n int reftable_stack_auto_compact(struct reftable_stack *st);\n \ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 49387f9344..18fa41cd5c 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n \treturn 0;\n }\n \n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required)\n+{\n+\tstruct segment seg;\n+\tint err = 0;\n+\n+\tif (st->merged->tables_len < 2) {\n+\t\t*required = false;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!use_heuristics) {\n+\t\t*required = true;\n+\t\treturn 0;\n+\t}\n+\n+\terr = stack_segments_for_compaction(st, &seg);\n+\tif (err)\n+\t\treturn err;\n+\n+\t*required = segment_size(&seg) > 0;\n+\treturn 0;\n+}\n+\n int reftable_stack_auto_compact(struct reftable_stack *st)\n {\n \tstruct segment seg;\ndiff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c\nindex a8b91812e8..b8110cdeee 100644\n--- a/t/unit-tests/u-reftable-stack.c\n+++ b/t/unit-tests/u-reftable-stack.c\n@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t\t.value_type = REFTABLE_REF_SYMREF,\n \t\t\t.value.symref = (char *) \"master\",\n \t\t};\n+\t\tbool required = false;\n \t\tchar buf[128];\n \n \t\t/*\n@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t * auto compaction is disabled. When enabled, we should merge\n \t\t * all tables in the stack.\n \t\t */\n-\t\tif (i != n)\n+\t\tcl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);\n+\t\tif (i != n) {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, i + 1);\n-\t\telse\n+\t\t\tif (i < 1)\n+\t\t\t\tcl_assert_equal_b(required, false);\n+\t\t\telse\n+\t\t\t\tcl_assert_equal_b(required, true);\n+\t\t} else {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, 1);\n+\t\t\tcl_assert_equal_b(required, false);\n+\t\t}\n \t}\n \n \treftable_stack_destroy(st);\n\n-- \n2.51.0\n\n"},{"id":"530031","messageId":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-3-a03d53e28d0e@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-31T14:22:23Z","receivedAt":"2025-10-31T14:22:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"To allow users of the refs namespace to check if the reference backend\nrequires optimization, add a new field `optimize_required` field to\n`struct ref_storage_be`. This field is of type `optimize_required_fn`\nwhich is also introduced in this commit.\n\nModify the debug, files, packed and reftable backend to implement this\nfield. A following commit will expose this via 'git pack-refs' and 'git\nrefs optimize'.\n\nSigned-off-by: Karthik Nayak \n---\n refs.c | 7 +++++++\n refs.h | 7 +++++++\n refs/debug.c | 13 +++++++++++++\n refs/files-backend.c | 11 +++++++++++\n refs/packed-backend.c | 13 +++++++++++++\n refs/refs-internal.h | 6 ++++++\n refs/reftable-backend.c | 25 +++++++++++++++++++++++++\n 7 files changed, 82 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0d0831f29b..5583f6e09d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n \treturn refs->be->optimize(refs, opts);\n }\n \n+int refs_optimize_required(struct ref_store *refs,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required)\n+{\n+\treturn refs->be->optimize_required(refs, opts, required);\n+}\n+\n int reference_get_peeled_oid(struct repository *repo,\n \t\t\t const struct reference *ref,\n \t\t\t struct object_id *peeled_oid)\ndiff --git a/refs.h b/refs.h\nindex 6b05bba527..d9051bbb04 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -520,6 +520,13 @@ struct refs_optimize_opts {\n */\n int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n \n+/*\n+ * Check if refs backend can be optimized by calling 'refs_optimize'.\n+ */\n+int refs_optimize_required(struct ref_store *ref_store,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required);\n+\n /*\n * Setup reflog before using. Fill in err and return -1 on failure.\n */\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 2defd2d465..36f8c58b6c 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts\n \treturn res;\n }\n \n+static int debug_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n+\tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n+\ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n+\t\t\t required ? \"yes\" : \"no\", res);\n+\treturn res;\n+}\n+\n static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n \t\t\t const char *newref, const char *logmsg)\n {\n@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {\n \t.transaction_abort = debug_transaction_abort,\n \n \t.optimize = debug_optimize,\n+\t.optimize_required = debug_optimize_required,\n+\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 a1e70b1c10..6e0c9b340a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,\n \treturn 0;\n }\n \n+static int files_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t \"optimize_required\");\n+\t*required = should_pack_refs(refs, opts);\n+\treturn 0;\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@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {\n \t.transaction_abort = files_transaction_abort,\n \n \t.optimize = files_optimize,\n+\t.optimize_required = files_optimize_required,\n \t.rename_ref = files_rename_ref,\n \t.copy_ref = files_copy_ref,\n \ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 10062fd8b6..19ce4d5872 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,\n \treturn 0;\n }\n \n+static int packed_optimize_required(struct ref_store *ref_store UNUSED,\n+\t\t\t\t struct refs_optimize_opts *opts UNUSED,\n+\t\t\t\t bool *required)\n+{\n+\t/*\n+\t * Packed refs are already optimized.\n+\t */\n+\t*required = false;\n+\treturn 0;\n+}\n+\n static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)\n {\n \treturn empty_ref_iterator_begin();\n@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {\n \t.transaction_abort = packed_transaction_abort,\n \n \t.optimize = packed_optimize,\n+\t.optimize_required = packed_optimize_required,\n+\n \t.rename_ref = NULL,\n \t.copy_ref = NULL,\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dee42f231d..c7d2a6e50b 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,\n \n typedef int optimize_fn(struct ref_store *ref_store,\n \t\t\tstruct refs_optimize_opts *opts);\n+\n+typedef int optimize_required_fn(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required);\n+\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);\n@@ -549,6 +554,7 @@ struct ref_storage_be {\n \tref_transaction_abort_fn *transaction_abort;\n \n \toptimize_fn *optimize;\n+\toptimize_required_fn *optimize_required;\n \trename_ref_fn *rename_ref;\n \tcopy_ref_fn *copy_ref;\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex c23c45f3bf..a3ae0cf74a 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,\n \treturn ret;\n }\n \n+static int reftable_be_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t\t bool *required)\n+{\n+\tstruct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t\t \"optimize_refs_required\");\n+\tstruct reftable_stack *stack;\n+\tbool use_heuristics = false;\n+\n+\tif (refs->err)\n+\t\treturn refs->err;\n+\n+\tstack = refs->worktree_backend.stack;\n+\tif (!stack)\n+\t\tstack = refs->main_backend.stack;\n+\n+\tif (opts->flags & REFS_OPTIMIZE_AUTO)\n+\t\tuse_heuristics = true;\n+\n+\treturn reftable_stack_compaction_required(stack, use_heuristics,\n+\t\t\t\t\t\t required);\n+}\n+\n struct write_create_symref_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_stack *stack;\n@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {\n \t.transaction_abort = reftable_be_transaction_abort,\n \n \t.optimize = reftable_be_optimize,\n+\t.optimize_required = reftable_be_optimize_required,\n+\n \t.rename_ref = reftable_be_rename_ref,\n \t.copy_ref = reftable_be_copy_ref,\n \n\n-- \n2.51.0\n\n"},{"id":"530033","messageId":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-4-a03d53e28d0e@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-31T14:22:24Z","receivedAt":"2025-10-31T14:22:28Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command support an '--auto' flag. Usage of the\nflag ensures to run maintenance tasks only if certain thresholds are\nmet. The heuristic is defined on a task level, wherein each task defines\na 'auto_condition', which states if the task should be run.\n\nThe 'pack-refs' task is hard-coded to return 1 as:\n1. There was never a way to check if the reference backend needs to be\noptimized without actually performing the optimization.\n2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would\noptimize based on heuristics.\n\nThe previous commit added a `refs_optimize_required()` function, which\ncan be used to check if a reference backend required optimization. Use\nthis within `pack_refs_condition()`.\n\nThis allows us to add a 'git maintenance is-needed' subcommand which can\nnotify the user if maintenance is needed without actually performing the\noptimization, without this change, the reference backend would always\nstate that optimization is needed.\n\nSince we import 'revision.h', we need to remove the definition for\n'SEEN' which is duplicated in the included header.\n\nSigned-off-by: Karthik Nayak \n---\n builtin/gc.c | 30 +++++++++++++++++++++---------\n object.h | 1 -\n 2 files changed, 21 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c6d62c74a7..72177305ff 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -35,6 +35,7 @@\n #include \"path.h\"\n #include \"reflog.h\"\n #include \"rerere.h\"\n+#include \"revision.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n #include \"promisor-remote.h\"\n@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n \n static int pack_refs_condition(UNUSED struct gc_config *cfg)\n {\n-\t/*\n-\t * The auto-repacking logic for refs is handled by the ref backends and\n-\t * exposed via `git pack-refs --auto`. We thus always return truish\n-\t * here and let the backend decide for us.\n-\t */\n-\treturn 1;\n+\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n+\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n+\tstruct refs_optimize_opts optimize_opts = {\n+\t\t.exclusions = &excludes,\n+\t\t.includes = &included_refs,\n+\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n+\t};\n+\tbool required;\n+\n+\t// Check for all refs, similar to 'git refs optimize --all'.\n+\tstring_list_append(optimize_opts.includes, \"*\");\n+\n+\tif (refs_optimize_required(get_main_ref_store(the_repository),\n+\t\t\t\t &optimize_opts, &required))\n+\t\treturn 0;\n+\n+\tclear_ref_exclusions(&excludes);\n+\tstring_list_clear(&included_refs, 0);\n+\n+\treturn required;\n }\n \n static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,\n@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-/* Remember to update object flag allocation in object.h */\n-#define SEEN\t\t(1u<<0)\n-\n struct cg_auto_data {\n \tint num_not_in_graph;\n \tint limit;\ndiff --git a/object.h b/object.h\nindex 1499f63d50..832299e763 100644\n--- a/object.h\n+++ b/object.h\n@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);\n * list-objects-filter.c: 21\n * bloom.c: 2122\n * builtin/fsck.c: 0--3\n- * builtin/gc.c: 0\n * builtin/index-pack.c: 2021\n * reflog.c: 10--12\n * builtin/show-branch.c: 0-------------------------------------------26\n\n-- \n2.51.0\n\n"},{"id":"530034","messageId":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-5-a03d53e28d0e@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-10-31T14:22:25Z","receivedAt":"2025-10-31T14:22:29Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command provides tooling to run maintenance\ntasks over Git repositories. The 'run' subcommand, as the name suggests,\nruns the maintenance tasks. When used with the '--auto' flag, it uses\nheuristics to determine if the required thresholds are met for running\nsaid maintenance tasks.\n\nThere is however a lack of insight into these heuristics. Meaning, the\nchecks are linked to the execution.\n\nAdd a new 'is-needed' subcommand to 'git-maintenance(1)' which allows\nusers to simply check if it is needed to run maintenance without\nperforming it.\n\nThis subcommand can check if it is needed to run maintenance without\nactually running it. Ideally it should be used with the '--auto' flag,\nwhich would allow users to check if the thresholds required are met. The\nsubcommand also supports the '--task' flag which can be used to check\nspecific maintenance tasks.\n\nWhile adding the respective tests in 't/t7900-maintenance.sh', remove a\nduplicate of the test: 'worktree-prune task with --auto honors\nmaintenance.worktree-prune.auto'.\n\nSigned-off-by: Karthik Nayak \n---\n Documentation/git-maintenance.adoc | 8 ++++++\n builtin/gc.c | 56 +++++++++++++++++++++++++++++++++++++-\n t/t7900-maintenance.sh | 54 +++++++++++++++++++++++++-----------\n 3 files changed, 101 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\nindex 540b5cf68b..edcc88f4d0 100644\n--- a/Documentation/git-maintenance.adoc\n+++ b/Documentation/git-maintenance.adoc\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git maintenance' run []\n 'git maintenance' start [--scheduler=]\n 'git maintenance' (stop|register|unregister) []\n+'git maintenance' is-needed []\n \n \n DESCRIPTION\n@@ -84,6 +85,11 @@ The `unregister` subcommand will report an error if the current repository\n is not already registered. Use the `--force` option to return success even\n when the current repository is not registered.\n \n+is-needed::\n+ Check whether maintenance needs to be run without actually running it.\n+ Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n+ Can be used along with `--task`. Ideally should be used with '--auto'.\n+\n TASKS\n -----\n \n@@ -183,6 +189,8 @@ OPTIONS\n \tin the `gc.auto` config setting, or when the number of pack-files\n \texceeds the `gc.autoPackLimit` config setting. Not compatible with\n \tthe `--schedule` option.\n+\tWhen combined with the `is-needed` subcommand, check if the required\n+\tthresholds are met without actually running maintenance.\n \n --schedule::\n \tWhen combined with the `run` subcommand, run maintenance tasks\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 72177305ff..4d20487ed6 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -3253,7 +3253,60 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n \treturn update_background_schedule(NULL, 0);\n }\n \n-static const char * const builtin_maintenance_usage[] = {\n+static const char *const builtin_maintenance_is_needed_usage[] = {\n+\t\"git maintenance is-needed [--task=] [--schedule]\",\n+\tNULL\n+};\n+\n+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n+\t\t\t\t struct repository *repo UNUSED)\n+{\n+\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n+\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n+\tstruct gc_config cfg = GC_CONFIG_INIT;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n+\t\t\t N_(\"run tasks based on the state of the repository\")),\n+\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n+\t\t\t N_(\"check a specific task\"),\n+\t\t\t PARSE_OPT_NONEG, task_option_parse),\n+\t\tOPT_END()\n+\t};\n+\tbool is_needed = false;\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t builtin_maintenance_is_needed_usage,\n+\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tgc_config(&cfg);\n+\tinitialize_task_config(&opts, &selected_tasks);\n+\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n+\n+\tif (opts.auto_flag) {\n+\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n+\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n+\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n+\t\t\t\tis_needed = true;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t} else {\n+\t\t/* When not using --auto, we should always require maintenance. */\n+\t\tis_needed = true;\n+\t}\n+\n+\tstring_list_clear(&selected_tasks, 0);\n+\tmaintenance_run_opts_release(&opts);\n+\tgc_config_release(&cfg);\n+\n+\tif (is_needed)\n+\t\treturn 0;\n+\treturn 1;\n+}\n+\n+static const char *const builtin_maintenance_usage[] = {\n \tN_(\"git maintenance []\"),\n \tNULL,\n };\n@@ -3270,6 +3323,7 @@ int cmd_maintenance(int argc,\n \t\tOPT_SUBCOMMAND(\"stop\", &fn, maintenance_stop),\n \t\tOPT_SUBCOMMAND(\"register\", &fn, maintenance_register),\n \t\tOPT_SUBCOMMAND(\"unregister\", &fn, maintenance_unregister),\n+\t\tOPT_SUBCOMMAND(\"is-needed\", &fn, maintenance_is_needed),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex ddd273d8dc..a17e2091c2 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '\n \t\tgit maintenance run --auto 2>/dev/null &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/run-no-quiet.txt\" \\\n \t\tgit maintenance run --no-quiet 2>/dev/null &&\n+\tgit maintenance is-needed &&\n \ttest_subcommand git gc --quiet --no-detach --skip-foreground-tasks /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand git prune-packed --quiet /dev/null &&\n@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {\n \ttest_commit A &&\n \tgit repack -adk &&\n \tgit multi-pack-index write &&\n+\t! git -c maintenance.incremental-repack.auto=1 \\\n+\t\tmaintenance is-needed --auto --task=incremental-repack &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/midx-init.txt\" git \\\n \t\t-c maintenance.incremental-repack.auto=1 \\\n \t\tmaintenance run --auto --task=incremental-repack 2>/dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n \tgit reflog expire --all --expire=now &&\n \ttest_commit reflog-one &&\n \ttest_commit reflog-two &&\n+\n+\t! git -c maintenance.reflog-expire.auto=3 \\\n+\t\tmaintenance is-needed --auto --task=reflog-expire &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/reflog-expire-auto.txt\" \\\n \t\tgit -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&\n \ttest_subcommand ! git reflog expire --all .git/worktrees/abc &&\n+\tgit maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git maintenance run --auto --task=worktree-prune\n '\n \n@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n \t# A positive value should require at least this many prunable worktrees.\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n-'\n-\n-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n-\t# A negative value should always prune.\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n-\n-\tmkdir .git/worktrees &&\n-\t: >.git/worktrees/first &&\n-\t: >.git/worktrees/second &&\n-\t: >.git/worktrees/third &&\n-\n-\t# Zero should never prune.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n-\t# A positive value should require at least this many prunable worktrees.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n+\tgit -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n '\n \n@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '\n \trm -rf worktree &&\n \n \trm -f worktree-prune.txt &&\n+\t! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&\n \ttest_subcommand ! git worktree prune --expire 1.week.ago .git/rr-cache/entry &&\n+\tgit maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n '\n \n@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n \n \t# A negative value should always prune.\n+\tgit -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n \n \t# A positive value prunes when there is at least one entry.\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \tmkdir .git/rr-cache &&\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \t: >.git/rr-cache/entry-1 &&\n+\tgit -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \n \t# Zero should never prune.\n \t: >.git/rr-cache/entry-1 &&\n+\t! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc\n '\n \n\n-- \n2.51.0\n\n"},{"id":"530040","messageId":"7gjrsjgi32akawqwcamzil2rblqelfvgmrxmgef5ssrslntmc6@43cra6zhledc","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-1-a03d53e28d0e@gmail.com","subject":"Re: [PATCH 1/5] reftable/stack: return stack segments directly","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-31T16:22:56Z","receivedAt":"2025-10-31T16:23:00Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/31 03:22PM, Karthik Nayak wrote:\n> The `stack_table_sizes_for_compaction()` function returns individual\n> sizes of each reftable table. This function is only called by\n> `reftable_stack_auto_compact()` to decide which tables need to be\n> compacted, if any.\n\n`stack_table_sizes_for_compaction()` provides the sizes of tables which\ngets used by `suggest_compaction_segment()` to figure out the range of\ntables that need to be compacted in order to restore the geometric\nsequence. `reftable_stack_auto_compact()` coordinates invoking these two\nfunctions and actually performs the compaction via\n`stack_compact_range()`.\n\n> Modify the function to directly return the segments, which avoids the\n> extra step of receiving the sizes only to pass it to\n> `suggest_compaction_segment()`.\n\nOk, so we want `suggest_compaction_segment()` to be invoked by\n`stack_table_sizes_for_compaction()` instead of\n`reftable_stack_auto_compact()`. So we are not really avoiding this\nstep, but just changing where it occurs.\n\n> A future commit will also add functionality for checking whether\n> auto-compaction is necessary without performing it. This change allows\n> code re-usability in that context.\n\nMakes sense.\n\n> Signed-off-by: Karthik Nayak \n> ---\n> reftable/stack.c | 23 ++++++++++++-----------\n> 1 file changed, 12 insertions(+), 11 deletions(-)\n> \n> diff --git a/reftable/stack.c b/reftable/stack.c\n> index 65d89820bd..49387f9344 100644\n> --- a/reftable/stack.c\n> +++ b/reftable/stack.c\n> @@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,\n> \treturn seg;\n> }\n> \n> -static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n> +static int stack_segments_for_compaction(struct reftable_stack *st,\n> +\t\t\t\t\t struct segment *seg)\n\n`stack_segements_for_compaction()` now handles both getting the table\nsizes and getting the segment range for compaction.\n\n> {\n> \tint version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;\n> \tint overhead = header_size(version) - 1;\n> @@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n> \n> \tREFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);\n> \tif (!sizes)\n> -\t\treturn NULL;\n> +\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n> \n> \tfor (size_t i = 0; i < st->merged->tables_len; i++)\n> \t\tsizes[i] = st->tables[i]->size - overhead;\n> \n> -\treturn sizes;\n> +\t*seg = suggest_compaction_segment(sizes, st->merged->tables_len,\n> +\t\t\t\t\t st->opts.auto_compaction_factor);\n> +\treftable_free(sizes);\n> +\n> +\treturn 0;\n> }\n> \n> int reftable_stack_auto_compact(struct reftable_stack *st)\n> {\n> \tstruct segment seg;\n> -\tuint64_t *sizes;\n> +\tint err;\n> \n> \tif (st->merged->tables_len < 2)\n> \t\treturn 0;\n> \n> -\tsizes = stack_table_sizes_for_compaction(st);\n> -\tif (!sizes)\n> -\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n> -\n> -\tseg = suggest_compaction_segment(sizes, st->merged->tables_len,\n> -\t\t\t\t\t st->opts.auto_compaction_factor);\n> -\treftable_free(sizes);\n> +\terr = stack_segments_for_compaction(st, &seg);\n> +\tif (err)\n> +\t\treturn err;\n\nLooks good.\n\n> \n> \tif (segment_size(&seg) > 0)\n> \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n\nDo we expect the errors returned by `stack_segments_for_compaction()` to\nalways be negative? If so, I wonder if we should also have it return the\nnumber of tables in the segment. That way it could also handle the\nfollowup `segment_size()`.\n\n-Justin\n"},{"id":"530041","messageId":"tdgxvocyp2armupgbti2wnbjphdvidooddbdyrynmdokjgqr3o@tzrbu5lcgipt","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-2-a03d53e28d0e@gmail.com","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-10-31T17:02:25Z","receivedAt":"2025-10-31T17:02:32Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/10/31 03:22PM, Karthik Nayak wrote:\n> The reftable backend, performs auto-compaction as part of its regular\n> flow, which is required to keep the number of tables part of a stack at\n> bay. This allows it to stay optimized.\n> \n> Compaction can also be triggered voluntarily by the user via the 'git\n> pack-refs' or the 'git refs optimize' command. However, currently there\n> is no way for the user to check if optimization is required without\n> actually performing it.\n> \n> Add and expose `reftable_stack_compaction_required()` which will allow\n> users to check if the reftable backend can be optimized.\n> \n> Signed-off-by: Karthik Nayak \n> ---\n> reftable/reftable-stack.h | 5 +++++\n> reftable/stack.c | 25 +++++++++++++++++++++++++\n> t/unit-tests/u-reftable-stack.c | 12 ++++++++++--\n> 3 files changed, 40 insertions(+), 2 deletions(-)\n> \n> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\n> index d70fcb705d..a875149439 100644\n> --- a/reftable/reftable-stack.h\n> +++ b/reftable/reftable-stack.h\n> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {\n> int reftable_stack_compact_all(struct reftable_stack *st,\n> \t\t\t struct reftable_log_expiry_config *config);\n> \n> +/* Check if compaction is required. */\n> +int reftable_stack_compaction_required(struct reftable_stack *st,\n> +\t\t\t\t bool use_heuristics,\n> +\t\t\t\t bool *required);\n> +\n> /* heuristically compact unbalanced table stack. */\n> int reftable_stack_auto_compact(struct reftable_stack *st);\n> \n> diff --git a/reftable/stack.c b/reftable/stack.c\n> index 49387f9344..18fa41cd5c 100644\n> --- a/reftable/stack.c\n> +++ b/reftable/stack.c\n> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n> \treturn 0;\n> }\n> \n> +int reftable_stack_compaction_required(struct reftable_stack *st,\n> +\t\t\t\t bool use_heuristics,\n> +\t\t\t\t bool *required)\n> +{\n> +\tstruct segment seg;\n> +\tint err = 0;\n> +\n> +\tif (st->merged->tables_len < 2) {\n> +\t\t*required = false;\n> +\t\treturn 0;\n> +\t}\n\nBoth `reftable_stack_auto_compact()` and `suggest_compaction_segement()`\nalready check if the stack has less than two tables. I wonder if we can\navoid having multiple of these checks by instead having a single one at\nthe start of `stack_segements_for_compaction()`?\n\n> +\tif (!use_heuristics) {\n> +\t\t*required = true;\n> +\t\treturn 0;\n> +\t}\n\nIs there a reason we would want to skip validating the geometric\nsequence and just assume it compaction is required?\n\n> +\n> +\terr = stack_segments_for_compaction(st, &seg);\n> +\tif (err)\n> +\t\treturn err;\n> +\n> +\t*required = segment_size(&seg) > 0;\n\nAs mentioned on the previous patch, I wonder if we could just return the\nnumber of tables in the compaction segment as part of\n`stack_segments_for_compaction()`. A negative value could indicate an\nerror. All other values would reflect the number of tables to be\ncompacted.\n\nThis way callers interested in whether compaction should be performed\ncould just do: stack_segments_for_compaction > 0. We could maybe avoid\nhaving a separate function like we do here and just expose\n`stack_segments_for_compaction()`.\n\n-Justin\n"},{"id":"530043","messageId":"xmqqseez0wcs.fsf@gitster.g","threadId":"64412","inReplyTo":"tdgxvocyp2armupgbti2wnbjphdvidooddbdyrynmdokjgqr3o@tzrbu5lcgipt","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-31T18:17:23Z","receivedAt":"2025-10-31T18:17:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Justin Tobler writes:\n\n>> +\terr = stack_segments_for_compaction(st, &seg);\n>> +\tif (err)\n>> +\t\treturn err;\n>> +\n>> +\t*required = segment_size(&seg) > 0;\n>\n> As mentioned on the previous patch, I wonder if we could just return the\n> number of tables in the compaction segment as part of\n> `stack_segments_for_compaction()`. A negative value could indicate an\n> error. All other values would reflect the number of tables to be\n> compacted.\n>\n> This way callers interested in whether compaction should be performed\n> could just do: stack_segments_for_compaction > 0. We could maybe avoid\n> having a separate function like we do here and just expose\n> `stack_segments_for_compaction()`.\n\nIs the cost of compacting a single table expected to be roughly the\nsame across tables? The number of tables to be compacted would not\nbe a useful information to help making a better decision otherwise,\nso I am guessing that it is the underlying assumption the above\nsuggestion comes from.\n\n"},{"id":"530118","messageId":"aQi1c6ZLM-1dqrCI@pks.im","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-2-a03d53e28d0e@gmail.com","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-03T14:00:19Z","receivedAt":"2025-11-03T14:00:26Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 31, 2025 at 03:22:22PM +0100, Karthik Nayak wrote:\n> The reftable backend, performs auto-compaction as part of its regular\n\ns/,//\n\n> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\n> index d70fcb705d..a875149439 100644\n> --- a/reftable/reftable-stack.h\n> +++ b/reftable/reftable-stack.h\n> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {\n> int reftable_stack_compact_all(struct reftable_stack *st,\n> \t\t\t struct reftable_log_expiry_config *config);\n> \n> +/* Check if compaction is required. */\n> +int reftable_stack_compaction_required(struct reftable_stack *st,\n> +\t\t\t\t bool use_heuristics,\n> +\t\t\t\t bool *required);\n> +\n\nI think the documentation here could be improved a bit. Somebody not\ndeeply familiar with reftables wouldn't know what `use_heuristics`\nreally is supposed to mean.\n\n> diff --git a/reftable/stack.c b/reftable/stack.c\n> index 49387f9344..18fa41cd5c 100644\n> --- a/reftable/stack.c\n> +++ b/reftable/stack.c\n> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n> \treturn 0;\n> }\n> \n> +int reftable_stack_compaction_required(struct reftable_stack *st,\n> +\t\t\t\t bool use_heuristics,\n> +\t\t\t\t bool *required)\n> +{\n> +\tstruct segment seg;\n> +\tint err = 0;\n> +\n> +\tif (st->merged->tables_len < 2) {\n> +\t\t*required = false;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (!use_heuristics) {\n> +\t\t*required = true;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\terr = stack_segments_for_compaction(st, &seg);\n> +\tif (err)\n> +\t\treturn err;\n> +\n> +\t*required = segment_size(&seg) > 0;\n> +\treturn 0;\n> +}\n> +\n\nAll of these conditions make sense.\n\nPatrick\n"},{"id":"530119","messageId":"aQi1e0zWfRaxSKtz@pks.im","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-4-a03d53e28d0e@gmail.com","subject":"Re: [PATCH 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-03T14:00:27Z","receivedAt":"2025-11-03T14:00:33Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 31, 2025 at 03:22:24PM +0100, Karthik Nayak wrote:\n> The 'git-maintenance(1)' command support an '--auto' flag. Usage of the\n\ns/support/&s/\n\n> flag ensures to run maintenance tasks only if certain thresholds are\n> met. The heuristic is defined on a task level, wherein each task defines\n> a 'auto_condition', which states if the task should be run.\n\ns/a/an/\n\n> The 'pack-refs' task is hard-coded to return 1 as:\n> 1. There was never a way to check if the reference backend needs to be\n> optimized without actually performing the optimization.\n> 2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would\n> optimize based on heuristics.\n> \n> The previous commit added a `refs_optimize_required()` function, which\n> can be used to check if a reference backend required optimization. Use\n> this within `pack_refs_condition()`.\n> \n> This allows us to add a 'git maintenance is-needed' subcommand which can\n> notify the user if maintenance is needed without actually performing the\n> optimization, without this change, the reference backend would always\n\ns/optimize, without/optimize. Without/\n\n> state that optimization is needed.\n> \n> Since we import 'revision.h', we need to remove the definition for\n> 'SEEN' which is duplicated in the included header.\n\nQuite weird that it was redefined in the first place. Feels like a nice\nside effect.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index c6d62c74a7..72177305ff 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n> \n> static int pack_refs_condition(UNUSED struct gc_config *cfg)\n> {\n> -\t/*\n> -\t * The auto-repacking logic for refs is handled by the ref backends and\n> -\t * exposed via `git pack-refs --auto`. We thus always return truish\n> -\t * here and let the backend decide for us.\n> -\t */\n> -\treturn 1;\n> +\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n> +\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n> +\tstruct refs_optimize_opts optimize_opts = {\n> +\t\t.exclusions = &excludes,\n> +\t\t.includes = &included_refs,\n\nA bit weird that we have to declare these two fields even though we\ndon't really care for either of them. But I don't mind that too much.\n\n> +\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n> +\t};\n> +\tbool required;\n> +\n> +\t// Check for all refs, similar to 'git refs optimize --all'.\n\nStyle: this should use `/* */` comments.\n\n> +\tstring_list_append(optimize_opts.includes, \"*\");\n> +\n> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n> +\t\t\t\t &optimize_opts, &required))\n> +\t\treturn 0;\n> +\n> +\tclear_ref_exclusions(&excludes);\n> +\tstring_list_clear(&included_refs, 0);\n> +\n> +\treturn required;\n\nYou return a boolean, but the function is declared to return an integer.\nThis works, but it feels wrong.\n\nPatrick\n"},{"id":"530120","messageId":"aQi1g9TX7FoDgo9n@pks.im","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-5-a03d53e28d0e@gmail.com","subject":"Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-03T14:00:35Z","receivedAt":"2025-11-03T14:00:40Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Fri, Oct 31, 2025 at 03:22:25PM +0100, Karthik Nayak wrote:\n> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\n> index 540b5cf68b..edcc88f4d0 100644\n> --- a/Documentation/git-maintenance.adoc\n> +++ b/Documentation/git-maintenance.adoc\n> @@ -84,6 +85,11 @@ The `unregister` subcommand will report an error if the current repository\n> is not already registered. Use the `--force` option to return success even\n> when the current repository is not registered.\n> \n> +is-needed::\n> + Check whether maintenance needs to be run without actually running it.\n> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n> + Can be used along with `--task`. Ideally should be used with '--auto'.\n\nOkay. I assume when `--task` is not given we'll check all tasks\nspecified by the configured strategy? Might make sense to document if\nso.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index 72177305ff..4d20487ed6 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -3253,7 +3253,60 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n> \treturn update_background_schedule(NULL, 0);\n> }\n> \n> -static const char * const builtin_maintenance_usage[] = {\n> +static const char *const builtin_maintenance_is_needed_usage[] = {\n> +\t\"git maintenance is-needed [--task=] [--schedule]\",\n> +\tNULL\n> +};\n> +\n> +static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n> +\t\t\t\t struct repository *repo UNUSED)\n> +{\n> +\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n> +\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n> +\tstruct gc_config cfg = GC_CONFIG_INIT;\n> +\tstruct option options[] = {\n> +\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n> +\t\t\t N_(\"run tasks based on the state of the repository\")),\n> +\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n> +\t\t\t N_(\"check a specific task\"),\n> +\t\t\t PARSE_OPT_NONEG, task_option_parse),\n> +\t\tOPT_END()\n> +\t};\n> +\tbool is_needed = false;\n> +\n> +\targc = parse_options(argc, argv, prefix, options,\n> +\t\t\t builtin_maintenance_is_needed_usage,\n> +\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> +\n> +\tgc_config(&cfg);\n> +\tinitialize_task_config(&opts, &selected_tasks);\n> +\n> +\tif (argc)\n> +\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n\nShouldn't this check be directly after the call to `parse_options()`?\n\n> +\tif (opts.auto_flag) {\n> +\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n> +\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n> +\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n> +\t\t\t\tis_needed = true;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n\nOkay, we need to guard against the auto-condition not existing indeed.\nThis is only due to the \"prefetch\" task though, all the others do have\nthe callback.\n\n> +\t} else {\n> +\t\t/* When not using --auto, we should always require maintenance. */\n> +\t\tis_needed = true;\n> +\t}\n\nI guess for now this is good enough, but it's not quite true. Some tasks\nwon't require maintenance even without `--auto`, like for example when\nthe reftable stack only has a single table.\n\nPatrick\n"},{"id":"530125","messageId":"CAOLa=ZQa21A+fF=ukZMmx3zu1DrMFU-EcZGrZConS-L16+ih1A@mail.gmail.com","threadId":"64412","inReplyTo":"7gjrsjgi32akawqwcamzil2rblqelfvgmrxmgef5ssrslntmc6@43cra6zhledc","subject":"Re: [PATCH 1/5] reftable/stack: return stack segments directly","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-03T15:05:51Z","receivedAt":"2025-11-03T15:05:55Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler writes:\n\n[snip]\n\n>>\n>> \tif (segment_size(&seg) > 0)\n>> \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n>\n> Do we expect the errors returned by `stack_segments_for_compaction()` to\n> always be negative? If so, I wonder if we should also have it return the\n> number of tables in the segment. That way it could also handle the\n> followup `segment_size()`.\n>\n\nCurrently yes, since all 'REFTABLE_' errors return negative\nvalue. But I must say I'm not a fan of combining errors and values\ntogether in a single return. This only creates confusion.\n\nI'm not sure removing `segment_size()` is also a good idea, because it\ndescribes what the check is. Otherwise we're looking at something like:\n\n@@ -1655,11 +1646,10 @@ int reftable_stack_auto_compact(struct\nreftable_stack *st)\n \tif (st->merged->tables_len < 2)\n \t\treturn 0;\n\n-\terr = stack_segments_for_compaction(st, &seg);\n-\tif (err)\n+\terr_or_stack_size = stack_segments_for_compaction(st, &seg);\n+\tif (err_or_stack_size < 0)\n \t\treturn err;\n-\n-\tif (segment_size(&seg) > 0)\n+\telse if (err_or_stack_size > 0)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n \t\t\t\t\t NULL, STACK_COMPACT_RANGE_BEST_EFFORT);\n\nI'm not sure that this would be better? Or am I missing something?\n"},{"id":"530128","messageId":"CAOLa=ZRzLviMkc8C8617L48NwJPvi7F1Qsozezm9gUQ0_dRU4A@mail.gmail.com","threadId":"64412","inReplyTo":"tdgxvocyp2armupgbti2wnbjphdvidooddbdyrynmdokjgqr3o@tzrbu5lcgipt","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-03T15:51:56Z","receivedAt":"2025-11-03T15:51:59Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Justin Tobler writes:\n\n>> +int reftable_stack_compaction_required(struct reftable_stack *st,\n>> +\t\t\t\t bool use_heuristics,\n>> +\t\t\t\t bool *required)\n>> +{\n>> +\tstruct segment seg;\n>> +\tint err = 0;\n>> +\n>> +\tif (st->merged->tables_len < 2) {\n>> +\t\t*required = false;\n>> +\t\treturn 0;\n>> +\t}\n>\n> Both `reftable_stack_auto_compact()` and `suggest_compaction_segement()`\n> already check if the stack has less than two tables. I wonder if we can\n> avoid having multiple of these checks by instead having a single one at\n> the start of `stack_segements_for_compaction()`?\n>\n\nWell we can't for two reasons:\n1. We want to perform this check independent of whether `use_heuristics`\n is set or not.\n2. Currently `stack_segements_for_compaction()` does one thing only,\n which is stack the segments. I wouldn't want to introduce another\n responsibility to it.\n\n>> +\tif (!use_heuristics) {\n>> +\t\t*required = true;\n>> +\t\treturn 0;\n>> +\t}\n>\n> Is there a reason we would want to skip validating the geometric\n> sequence and just assume it compaction is required?\n>\n\nThis is the difference between running 'git refs optimize' with and\nwithout '--auto'. With '--auto' we will use heuristics to do a geometric\nprogression. Without, we simply compact all tables into one.\n\nSo we need to support both modes here.\n>> +\n>> +\terr = stack_segments_for_compaction(st, &seg);\n>> +\tif (err)\n>> +\t\treturn err;\n>> +\n>> +\t*required = segment_size(&seg) > 0;\n>\n> As mentioned on the previous patch, I wonder if we could just return the\n> number of tables in the compaction segment as part of\n> `stack_segments_for_compaction()`. A negative value could indicate an\n> error. All other values would reflect the number of tables to be\n> compacted.\n>\n> This way callers interested in whether compaction should be performed\n> could just do: stack_segments_for_compaction > 0. We could maybe avoid\n> having a separate function like we do here and just expose\n> `stack_segments_for_compaction()`.\n>\n\nWe'd still need to expose a new function as\n`stack_segments_for_compaction()` is still internal details to the\nreftable backend, which we wouldn't want to expose externally. Users of\nthis function, should only need to know a boolean value wether the\nbackend needs to be optimized or not.\n"},{"id":"530130","messageId":"CAOLa=ZRywiKbsdAZYzVqGUKO1NBPz3iBhcjch9KFq72eZvUoPw@mail.gmail.com","threadId":"64412","inReplyTo":"xmqqseez0wcs.fsf@gitster.g","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-03T16:20:10Z","receivedAt":"2025-11-03T16:20:12Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Justin Tobler writes:\n>\n>>> +\terr = stack_segments_for_compaction(st, &seg);\n>>> +\tif (err)\n>>> +\t\treturn err;\n>>> +\n>>> +\t*required = segment_size(&seg) > 0;\n>>\n>> As mentioned on the previous patch, I wonder if we could just return the\n>> number of tables in the compaction segment as part of\n>> `stack_segments_for_compaction()`. A negative value could indicate an\n>> error. All other values would reflect the number of tables to be\n>> compacted.\n>>\n>> This way callers interested in whether compaction should be performed\n>> could just do: stack_segments_for_compaction > 0. We could maybe avoid\n>> having a separate function like we do here and just expose\n>> `stack_segments_for_compaction()`.\n>\n> Is the cost of compacting a single table expected to be roughly the\n> same across tables? The number of tables to be compacted would not\n> be a useful information to help making a better decision otherwise,\n> so I am guessing that it is the underlying assumption the above\n> suggestion comes from.\n\nIt would be sufficient information to know whether or not we can\ncompact. But only when in 'git refs optimize --auto' mode.\n"},{"id":"530133","messageId":"CAOLa=ZQf_YC4-z8eOa=VaMdHFynfK-aWBC80sV5N4T5gzweq=Q@mail.gmail.com","threadId":"64412","inReplyTo":"aQi1c6ZLM-1dqrCI@pks.im","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-03T16:35:31Z","receivedAt":"2025-11-03T16:35:33Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Fri, Oct 31, 2025 at 03:22:22PM +0100, Karthik Nayak wrote:\n>> The reftable backend, performs auto-compaction as part of its regular\n>\n> s/,//\n>\n>> diff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\n>> index d70fcb705d..a875149439 100644\n>> --- a/reftable/reftable-stack.h\n>> +++ b/reftable/reftable-stack.h\n>> @@ -123,6 +123,11 @@ struct reftable_log_expiry_config {\n>> int reftable_stack_compact_all(struct reftable_stack *st,\n>> \t\t\t struct reftable_log_expiry_config *config);\n>>\n>> +/* Check if compaction is required. */\n>> +int reftable_stack_compaction_required(struct reftable_stack *st,\n>> +\t\t\t\t bool use_heuristics,\n>> +\t\t\t\t bool *required);\n>> +\n>\n> I think the documentation here could be improved a bit. Somebody not\n> deeply familiar with reftables wouldn't know what `use_heuristics`\n> really is supposed to mean.\n>\n\nI agree. I'll add in something.\n\n>> diff --git a/reftable/stack.c b/reftable/stack.c\n>> index 49387f9344..18fa41cd5c 100644\n>> --- a/reftable/stack.c\n>> +++ b/reftable/stack.c\n>> @@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n>> \treturn 0;\n>> }\n>>\n>> +int reftable_stack_compaction_required(struct reftable_stack *st,\n>> +\t\t\t\t bool use_heuristics,\n>> +\t\t\t\t bool *required)\n>> +{\n>> +\tstruct segment seg;\n>> +\tint err = 0;\n>> +\n>> +\tif (st->merged->tables_len < 2) {\n>> +\t\t*required = false;\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\tif (!use_heuristics) {\n>> +\t\t*required = true;\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\terr = stack_segments_for_compaction(st, &seg);\n>> +\tif (err)\n>> +\t\treturn err;\n>> +\n>> +\t*required = segment_size(&seg) > 0;\n>> +\treturn 0;\n>> +}\n>> +\n>\n> All of these conditions make sense.\n>\n> Patrick\n\nThanks!\n"},{"id":"530135","messageId":"CAOLa=ZQSEETU_AzKdr2ugH9982bgPFazAR_jHFoX6px7Txy=Yw@mail.gmail.com","threadId":"64412","inReplyTo":"aQi1e0zWfRaxSKtz@pks.im","subject":"Re: [PATCH 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-03T17:04:26Z","receivedAt":"2025-11-03T17:04:29Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Fri, Oct 31, 2025 at 03:22:24PM +0100, Karthik Nayak wrote:\n>> The 'git-maintenance(1)' command support an '--auto' flag. Usage of the\n>\n> s/support/&s/\n>\n\nAh, will change.\n\n>> flag ensures to run maintenance tasks only if certain thresholds are\n>> met. The heuristic is defined on a task level, wherein each task defines\n>> a 'auto_condition', which states if the task should be run.\n>\n> s/a/an/\n\nYup, thanks!\n\n>\n>> The 'pack-refs' task is hard-coded to return 1 as:\n>> 1. There was never a way to check if the reference backend needs to be\n>> optimized without actually performing the optimization.\n>> 2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would\n>> optimize based on heuristics.\n>>\n>> The previous commit added a `refs_optimize_required()` function, which\n>> can be used to check if a reference backend required optimization. Use\n>> this within `pack_refs_condition()`.\n>>\n>> This allows us to add a 'git maintenance is-needed' subcommand which can\n>> notify the user if maintenance is needed without actually performing the\n>> optimization, without this change, the reference backend would always\n>\n> s/optimize, without/optimize. Without/\n>\n\nThanks, this is better.\n\n>> state that optimization is needed.\n>>\n>> Since we import 'revision.h', we need to remove the definition for\n>> 'SEEN' which is duplicated in the included header.\n>\n> Quite weird that it was redefined in the first place. Feels like a nice\n> side effect.\n>\n\nIndeed.\n\n>> diff --git a/builtin/gc.c b/builtin/gc.c\n>> index c6d62c74a7..72177305ff 100644\n>> --- a/builtin/gc.c\n>> +++ b/builtin/gc.c\n>> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n>>\n>> static int pack_refs_condition(UNUSED struct gc_config *cfg)\n>> {\n>> -\t/*\n>> -\t * The auto-repacking logic for refs is handled by the ref backends and\n>> -\t * exposed via `git pack-refs --auto`. We thus always return truish\n>> -\t * here and let the backend decide for us.\n>> -\t */\n>> -\treturn 1;\n>> +\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n>> +\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n>> +\tstruct refs_optimize_opts optimize_opts = {\n>> +\t\t.exclusions = &excludes,\n>> +\t\t.includes = &included_refs,\n>\n> A bit weird that we have to declare these two fields even though we\n> don't really care for either of them. But I don't mind that too much.\n>\n\nYeah, I think there is some cleanup to be done in the files backend. But\nI don't think it should be part of this series. If we don't add these,\nwe crash with a SIGSEGV.\n\n>> +\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n>> +\t};\n>> +\tbool required;\n>> +\n>> +\t// Check for all refs, similar to 'git refs optimize --all'.\n>\n> Style: this should use `/* */` comments.\n>\n\nThanks, will fix.\n\n>> +\tstring_list_append(optimize_opts.includes, \"*\");\n>> +\n>> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n>> +\t\t\t\t &optimize_opts, &required))\n>> +\t\treturn 0;\n>> +\n>> +\tclear_ref_exclusions(&excludes);\n>> +\tstring_list_clear(&included_refs, 0);\n>> +\n>> +\treturn required;\n>\n> You return a boolean, but the function is declared to return an integer.\n> This works, but it feels wrong.\n>\n> Patrick\n\nI get what you're saying but returning `required == true` also feel like\na bool return to me (even though it is an int in C).\n\nAnyways, I'll make the change. I don't care much for either.\n"},{"id":"530137","messageId":"CAOLa=ZSsEygvz1_aj4KomfF0Jo0vJi3yVLtJbhLX=RLgW6_GzQ@mail.gmail.com","threadId":"64412","inReplyTo":"aQi1g9TX7FoDgo9n@pks.im","subject":"Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-03T17:18:35Z","receivedAt":"2025-11-03T17:18:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Fri, Oct 31, 2025 at 03:22:25PM +0100, Karthik Nayak wrote:\n>> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\n>> index 540b5cf68b..edcc88f4d0 100644\n>> --- a/Documentation/git-maintenance.adoc\n>> +++ b/Documentation/git-maintenance.adoc\n>> @@ -84,6 +85,11 @@ The `unregister` subcommand will report an error if the current repository\n>> is not already registered. Use the `--force` option to return success even\n>> when the current repository is not registered.\n>>\n>> +is-needed::\n>> + Check whether maintenance needs to be run without actually running it.\n>> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n>> + Can be used along with `--task`. Ideally should be used with '--auto'.\n>\n> Okay. I assume when `--task` is not given we'll check all tasks\n> specified by the configured strategy? Might make sense to document if\n> so.\n>\n\nActually no. It's similar to the 'run' command, if nothing is specified,\nwe check `maintenance..enabled`. By default it is only enabled for\n'gc'. This is important information, I will add it in.\n\n>> diff --git a/builtin/gc.c b/builtin/gc.c\n>> index 72177305ff..4d20487ed6 100644\n>> --- a/builtin/gc.c\n>> +++ b/builtin/gc.c\n>> @@ -3253,7 +3253,60 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n>> \treturn update_background_schedule(NULL, 0);\n>> }\n>>\n>> -static const char * const builtin_maintenance_usage[] = {\n>> +static const char *const builtin_maintenance_is_needed_usage[] = {\n>> +\t\"git maintenance is-needed [--task=] [--schedule]\",\n>> +\tNULL\n>> +};\n>> +\n>> +static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n>> +\t\t\t\t struct repository *repo UNUSED)\n>> +{\n>> +\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n>> +\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n>> +\tstruct gc_config cfg = GC_CONFIG_INIT;\n>> +\tstruct option options[] = {\n>> +\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n>> +\t\t\t N_(\"run tasks based on the state of the repository\")),\n>> +\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n>> +\t\t\t N_(\"check a specific task\"),\n>> +\t\t\t PARSE_OPT_NONEG, task_option_parse),\n>> +\t\tOPT_END()\n>> +\t};\n>> +\tbool is_needed = false;\n>> +\n>> +\targc = parse_options(argc, argv, prefix, options,\n>> +\t\t\t builtin_maintenance_is_needed_usage,\n>> +\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n>> +\n>> +\tgc_config(&cfg);\n>> +\tinitialize_task_config(&opts, &selected_tasks);\n>> +\n>> +\tif (argc)\n>> +\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n>\n> Shouldn't this check be directly after the call to `parse_options()`?\n>\n\nYes, I moved it around, will fix it.\n\n>> +\tif (opts.auto_flag) {\n>> +\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n>> +\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n>> +\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n>> +\t\t\t\tis_needed = true;\n>> +\t\t\t\tbreak;\n>> +\t\t\t}\n>> +\t\t}\n>\n> Okay, we need to guard against the auto-condition not existing indeed.\n> This is only due to the \"prefetch\" task though, all the others do have\n> the callback.\n>\n\nYup, that's correct.\n\n>> +\t} else {\n>> +\t\t/* When not using --auto, we should always require maintenance. */\n>> +\t\tis_needed = true;\n>> +\t}\n>\n> I guess for now this is good enough, but it's not quite true. Some tasks\n> won't require maintenance even without `--auto`, like for example when\n> the reftable stack only has a single table.\n>\n> Patrick\n\nGood point. Thought I'm not sure how we'd go about it. Initially I\nwanted to not have an `--auto` flag and simply make it the default\nbehavior. But that would restrict us from introducing the `schedule`\nflag in the future. Which I think might be a worthwhile addition.\n"},{"id":"530141","messageId":"6b45z4xnzwzfi4ll5bintxqsrdwpaeb2mhozlujufalgrgfys7@6bw4z2ukplkn","threadId":"64412","inReplyTo":"CAOLa=ZRzLviMkc8C8617L48NwJPvi7F1Qsozezm9gUQ0_dRU4A@mail.gmail.com","subject":"Re: [PATCH 2/5] reftable/stack: add function to check if optimization is required","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-11-03T17:59:47Z","receivedAt":"2025-11-03T17:59:51Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/11/03 07:51AM, Karthik Nayak wrote:\n> Justin Tobler writes:\n> \n> >> +int reftable_stack_compaction_required(struct reftable_stack *st,\n> >> +\t\t\t\t bool use_heuristics,\n> >> +\t\t\t\t bool *required)\n> >> +{\n> >> +\tstruct segment seg;\n> >> +\tint err = 0;\n> >> +\n> >> +\tif (st->merged->tables_len < 2) {\n> >> +\t\t*required = false;\n> >> +\t\treturn 0;\n> >> +\t}\n> >\n> > Both `reftable_stack_auto_compact()` and `suggest_compaction_segement()`\n> > already check if the stack has less than two tables. I wonder if we can\n> > avoid having multiple of these checks by instead having a single one at\n> > the start of `stack_segements_for_compaction()`?\n> >\n> \n> Well we can't for two reasons:\n> 1. We want to perform this check independent of whether `use_heuristics`\n> is set or not.\n> 2. Currently `stack_segements_for_compaction()` does one thing only,\n> which is stack the segments. I wouldn't want to introduce another\n> responsibility to it.\n\nThat's fair. From my understanding, `stack_segements_for_compaction()`\npopulates a segment which defines the range of tables that should be\ncompacted to restore the geometric sequence. Since we want to ultimately\nknow whether compaction needs to occur, my thought process was we could\nmaybe have a single function (\"check_compaction_needed()\") that\neffectively returns a boolean and maybe be able to reuse that. I don't\nthink it matters much though and as you mention we also want to consider\n`use_heuristics`.\n\n> >> +\tif (!use_heuristics) {\n> >> +\t\t*required = true;\n> >> +\t\treturn 0;\n> >> +\t}\n> >\n> > Is there a reason we would want to skip validating the geometric\n> > sequence and just assume it compaction is required?\n> >\n> \n> This is the difference between running 'git refs optimize' with and\n> without '--auto'. With '--auto' we will use heuristics to do a geometric\n> progression. Without, we simply compact all tables into one.\n\nThat's for the clarification. So without --auto, instead of following a\ngeometric sequence, a different maintenance strategy is used and we\ncompact all the tables into one. Makes sense.\n\n-Justin\n"},{"id":"530146","messageId":"2nr5ig2cg5bc2zvtinm4p2fxssuim5kb4bsflrx3xnos2pwkk3@tya7zuj4pgg6","threadId":"64412","inReplyTo":"CAOLa=ZQa21A+fF=ukZMmx3zu1DrMFU-EcZGrZConS-L16+ih1A@mail.gmail.com","subject":"Re: [PATCH 1/5] reftable/stack: return stack segments directly","fromName":"Justin Tobler","fromEmail":"jltobler@gmail.com","sentAt":"2025-11-03T18:03:27Z","receivedAt":"2025-11-03T18:03:29Z","isPatch":true,"sender":{"key":"jltobler@gmail.com","avatar":"https://avatars.githubusercontent.com/u/53454972?v=4"},"body":"On 25/11/03 07:05AM, Karthik Nayak wrote:\n> Justin Tobler writes:\n> \n> [snip]\n> \n> >>\n> >> \tif (segment_size(&seg) > 0)\n> >> \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n> >\n> > Do we expect the errors returned by `stack_segments_for_compaction()` to\n> > always be negative? If so, I wonder if we should also have it return the\n> > number of tables in the segment. That way it could also handle the\n> > followup `segment_size()`.\n> >\n> \n> Currently yes, since all 'REFTABLE_' errors return negative\n> value. But I must say I'm not a fan of combining errors and values\n> together in a single return. This only creates confusion.\n> \n> I'm not sure removing `segment_size()` is also a good idea, because it\n> describes what the check is. Otherwise we're looking at something like:\n> \n> @@ -1655,11 +1646,10 @@ int reftable_stack_auto_compact(struct\n> reftable_stack *st)\n> \tif (st->merged->tables_len < 2)\n> \t\treturn 0;\n> \n> -\terr = stack_segments_for_compaction(st, &seg);\n> -\tif (err)\n> +\terr_or_stack_size = stack_segments_for_compaction(st, &seg);\n> +\tif (err_or_stack_size < 0)\n> \t\treturn err;\n> -\n> -\tif (segment_size(&seg) > 0)\n> +\telse if (err_or_stack_size > 0)\n> \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n> \t\t\t\t\t NULL, STACK_COMPACT_RANGE_BEST_EFFORT);\n> \n> I'm not sure that this would be better? Or am I missing something?\n\nThat's fair. I was thinking that we could just make\n`stack_segments_for_compaction()` responsible for the boolean check of\nwhether compaction is required or not. It's probably not worth\noverloading the return value though.\n\n-Justin\n"},{"id":"530174","messageId":"aQmU_hOPO55_ojw2@pks.im","threadId":"64412","inReplyTo":"CAOLa=ZSsEygvz1_aj4KomfF0Jo0vJi3yVLtJbhLX=RLgW6_GzQ@mail.gmail.com","subject":"Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-04T05:54:06Z","receivedAt":"2025-11-04T05:54:13Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Nov 03, 2025 at 09:18:35AM -0800, Karthik Nayak wrote:\n> Patrick Steinhardt writes:\n> \n> > On Fri, Oct 31, 2025 at 03:22:25PM +0100, Karthik Nayak wrote:\n> >> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\n> >> index 540b5cf68b..edcc88f4d0 100644\n> >> --- a/Documentation/git-maintenance.adoc\n> >> +++ b/Documentation/git-maintenance.adoc\n> >> @@ -84,6 +85,11 @@ The `unregister` subcommand will report an error if the current repository\n> >> is not already registered. Use the `--force` option to return success even\n> >> when the current repository is not registered.\n> >>\n> >> +is-needed::\n> >> + Check whether maintenance needs to be run without actually running it.\n> >> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n> >> + Can be used along with `--task`. Ideally should be used with '--auto'.\n> >\n> > Okay. I assume when `--task` is not given we'll check all tasks\n> > specified by the configured strategy? Might make sense to document if\n> > so.\n> >\n> \n> Actually no. It's similar to the 'run' command, if nothing is specified,\n> we check `maintenance..enabled`. By default it is only enabled for\n> 'gc'. This is important information, I will add it in.\n\nBut we use `initialize_task_config()`, and that function knows to use\nthe configured strategy unless it's given an explicit list of tasks. So\nwe do use the maintenance strategy.\n\n> >> diff --git a/builtin/gc.c b/builtin/gc.c\n> >> index 72177305ff..4d20487ed6 100644\n> >> --- a/builtin/gc.c\n> >> +++ b/builtin/gc.c\n[snip]\n> >> +\t} else {\n> >> +\t\t/* When not using --auto, we should always require maintenance. */\n> >> +\t\tis_needed = true;\n> >> +\t}\n> >\n> > I guess for now this is good enough, but it's not quite true. Some tasks\n> > won't require maintenance even without `--auto`, like for example when\n> > the reftable stack only has a single table.\n> >\n> > Patrick\n> \n> Good point. Thought I'm not sure how we'd go about it. Initially I\n> wanted to not have an `--auto` flag and simply make it the default\n> behavior. But that would restrict us from introducing the `schedule`\n> flag in the future. Which I think might be a worthwhile addition.\n\nYeah, agreed.\n\nI guess eventually we could extend `auto_condition()` to honor the\n\"--auto\" flag:\n\n - If it's set the task verifies that it needs to trigger housekeeping\n tasks with heuristics.\n\n - Otherwise it checks whether there even is anything that could be\n cleaned up.\n\nBut that's certainly out of scope of this patch series, I think it's\ngood enough to bail on that specific part for now.\n\nPatrick\n"},{"id":"530176","messageId":"CAOLa=ZRA33ro1-9jbh71QpAa3Sj-NZY5fOL_T4Shyn8jPYQi_A@mail.gmail.com","threadId":"64412","inReplyTo":"aQmU_hOPO55_ojw2@pks.im","subject":"Re: [PATCH 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:28:08Z","receivedAt":"2025-11-04T08:28:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Mon, Nov 03, 2025 at 09:18:35AM -0800, Karthik Nayak wrote:\n>> Patrick Steinhardt writes:\n>>\n>> > On Fri, Oct 31, 2025 at 03:22:25PM +0100, Karthik Nayak wrote:\n>> >> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\n>> >> index 540b5cf68b..edcc88f4d0 100644\n>> >> --- a/Documentation/git-maintenance.adoc\n>> >> +++ b/Documentation/git-maintenance.adoc\n>> >> @@ -84,6 +85,11 @@ The `unregister` subcommand will report an error if the current repository\n>> >> is not already registered. Use the `--force` option to return success even\n>> >> when the current repository is not registered.\n>> >>\n>> >> +is-needed::\n>> >> + Check whether maintenance needs to be run without actually running it.\n>> >> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n>> >> + Can be used along with `--task`. Ideally should be used with '--auto'.\n>> >\n>> > Okay. I assume when `--task` is not given we'll check all tasks\n>> > specified by the configured strategy? Might make sense to document if\n>> > so.\n>> >\n>>\n>> Actually no. It's similar to the 'run' command, if nothing is specified,\n>> we check `maintenance..enabled`. By default it is only enabled for\n>> 'gc'. This is important information, I will add it in.\n>\n> But we use `initialize_task_config()`, and that function knows to use\n> the configured strategy unless it's given an explicit list of tasks. So\n> we do use the maintenance strategy.\n\nYes, and the default strategy is to run 'gc'.\n\nI miss-read your earlier comment, you were talking about the configured\nstrategy. I thought you were asking if no '--task' is given, we'd run\nall available tasks.\n\n>\n>> >> diff --git a/builtin/gc.c b/builtin/gc.c\n>> >> index 72177305ff..4d20487ed6 100644\n>> >> --- a/builtin/gc.c\n>> >> +++ b/builtin/gc.c\n> [snip]\n>> >> +\t} else {\n>> >> +\t\t/* When not using --auto, we should always require maintenance. */\n>> >> +\t\tis_needed = true;\n>> >> +\t}\n>> >\n>> > I guess for now this is good enough, but it's not quite true. Some tasks\n>> > won't require maintenance even without `--auto`, like for example when\n>> > the reftable stack only has a single table.\n>> >\n>> > Patrick\n>>\n>> Good point. Thought I'm not sure how we'd go about it. Initially I\n>> wanted to not have an `--auto` flag and simply make it the default\n>> behavior. But that would restrict us from introducing the `schedule`\n>> flag in the future. Which I think might be a worthwhile addition.\n>\n> Yeah, agreed.\n>\n> I guess eventually we could extend `auto_condition()` to honor the\n> \"--auto\" flag:\n>\n> - If it's set the task verifies that it needs to trigger housekeeping\n> tasks with heuristics.\n>\n> - Otherwise it checks whether there even is anything that could be\n> cleaned up.\n>\n> But that's certainly out of scope of this patch series, I think it's\n> good enough to bail on that specific part for now.\n>\n> Patrick\n\nI think that's a fair conclusion. I'll leave this as is for now then.\n\nThanks for the review.\n"},{"id":"530177","messageId":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH v2 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:43:55Z","receivedAt":"2025-11-04T08:44:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hello,\n\nI recently raised a patch series [1] to add 'git refs optimize --required'\nwhich checks if the reference backend can be optimized, without actually\nperforming the optimization.\n\nBack then, we had decided [2] that it would be a better to broaden the\napproach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This\nwould allow users to check if maintenance was required for the\nrepository and users could also provide a task via the '--task' to check\nif maintenance was needed for a particular task.\n\nIdeally the subcommand will be used with the '--auto' flag which can\ncheck the same heuristics as that used with 'git maintenance run\n--auto'. Future patches can also add support for the '--schedule' flag\nwhich can be used to check required schedule it met. However that flag\nisn't added as part of this series.\n\nThis series implements that.\n\nCommits 1-3 add the required functionality in the refs subsystem to\nexpose an 'optimize_required' field which can be used to check if\nbackends need to be optimized.\nCommit 4 utilizes this within the 'git-maintenance(1)' code.\nCommit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.\n\nThis is based on top of master a99f379adf (The 27th batch, 2025-10-30)\nand is dependent on the following series:\n\n - kn/refs-optim-cleanup\n - ps/ref-peeled-tags\n\nMerges cleanly with `next`. I think those two topics are close to being\nmerged to `next` so hopefully this dependency tree doesn't get too\ncomplicated. I'll rebase as needed to resolve conflicts.\n\n[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/\n[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/\n\n---\nChanges in v2:\n- Added more documentation for `reftable_stack_compaction_required()`.\n- Fixed some typos and grammar mistakes in commit messages.\n- Clarify which tasks will be run when '--task' is not used.\n- Move the call to 'usage_with_options()' to be with 'parse_options()'.\n- Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com\n\n---\n Documentation/git-maintenance.adoc | 13 ++++++\n builtin/gc.c | 85 +++++++++++++++++++++++++++++++++-----\n object.h | 1 -\n refs.c | 7 ++++\n refs.h | 7 ++++\n refs/debug.c | 13 ++++++\n refs/files-backend.c | 11 +++++\n refs/packed-backend.c | 13 ++++++\n refs/refs-internal.h | 6 +++\n refs/reftable-backend.c | 25 +++++++++++\n reftable/reftable-stack.h | 11 +++++\n reftable/stack.c | 48 ++++++++++++++++-----\n t/t7900-maintenance.sh | 54 +++++++++++++++++-------\n t/unit-tests/u-reftable-stack.c | 12 +++++-\n 14 files changed, 266 insertions(+), 40 deletions(-)\n\nKarthik Nayak (5):\n reftable/stack: return stack segments directly\n reftable/stack: add function to check if optimization is required\n refs: add a `optimize_required` field to `struct ref_storage_be`\n maintenance: add checking logic in `pack_refs_condition()`\n maintenance: add 'is-needed' subcommand\n\nRange-diff versus v1:\n\n1: e5e6eedfbe = 1: 5431fe40ef reftable/stack: return stack segments directly\n2: 6fac9e5eb5 ! 2: 248882aad6 reftable/stack: add function to check if optimization is required\n @@ Metadata\n ## Commit message ##\n reftable/stack: add function to check if optimization is required\n \n - The reftable backend, performs auto-compaction as part of its regular\n + The reftable backend performs auto-compaction as part of its regular\n flow, which is required to keep the number of tables part of a stack at\n bay. This allows it to stay optimized.\n \n @@ reftable/reftable-stack.h: struct reftable_log_expiry_config {\n int reftable_stack_compact_all(struct reftable_stack *st,\n \t\t\t struct reftable_log_expiry_config *config);\n \n -+/* Check if compaction is required. */\n ++/*\n ++ * Check if compaction is required.\n ++ *\n ++ * When `use_heuristics` is false, check if all tables can be compacted to a\n ++ * single table. If true, use heuristics to determine if the tables need to be\n ++ * compacted to maintain geometric progression.\n ++ */\n +int reftable_stack_compaction_required(struct reftable_stack *st,\n +\t\t\t\t bool use_heuristics,\n +\t\t\t\t bool *required);\n3: 2ec39102a5 = 3: e98c2c2e3e refs: add a `optimize_required` field to `struct ref_storage_be`\n4: c246efdc4a ! 4: 9b7fa79c8c maintenance: add checking logic in `pack_refs_condition()`\n @@ Metadata\n ## Commit message ##\n maintenance: add checking logic in `pack_refs_condition()`\n \n - The 'git-maintenance(1)' command support an '--auto' flag. Usage of the\n + The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the\n flag ensures to run maintenance tasks only if certain thresholds are\n met. The heuristic is defined on a task level, wherein each task defines\n - a 'auto_condition', which states if the task should be run.\n + an 'auto_condition', which states if the task should be run.\n \n The 'pack-refs' task is hard-coded to return 1 as:\n 1. There was never a way to check if the reference backend needs to be\n @@ Commit message\n \n This allows us to add a 'git maintenance is-needed' subcommand which can\n notify the user if maintenance is needed without actually performing the\n - optimization, without this change, the reference backend would always\n + optimization. Without this change, the reference backend would always\n state that optimization is needed.\n \n Since we import 'revision.h', we need to remove the definition for\n @@ builtin/gc.c: static void maintenance_run_opts_release(struct maintenance_run_op\n +\t};\n +\tbool required;\n +\n -+\t// Check for all refs, similar to 'git refs optimize --all'.\n ++\t/* Check for all refs, similar to 'git refs optimize --all'. */\n +\tstring_list_append(optimize_opts.includes, \"*\");\n +\n +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n @@ builtin/gc.c: static void maintenance_run_opts_release(struct maintenance_run_op\n +\tclear_ref_exclusions(&excludes);\n +\tstring_list_clear(&included_refs, 0);\n +\n -+\treturn required;\n ++\treturn required == true;\n }\n \n static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,\n5: 64c15c1319 ! 5: ed3658528b maintenance: add 'is-needed' subcommand\n @@ Documentation/git-maintenance.adoc: The `unregister` subcommand will report an e\n +is-needed::\n + Check whether maintenance needs to be run without actually running it.\n + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n -+ Can be used along with `--task`. Ideally should be used with '--auto'.\n ++ Ideally used with the '--auto' flag.\n +++\n ++If one or more `--task` options\tare specified, then those tasks are checked\n ++in that order. Otherwise, the tasks are determined by which\n ++`maintenance..enabled` config options are true. By default, only\n ++`maintenance.gc.enabled` is true.\n +\n TASKS\n -----\n @@ builtin/gc.c: static int maintenance_stop(int argc, const char **argv, const cha\n +\targc = parse_options(argc, argv, prefix, options,\n +\t\t\t builtin_maintenance_is_needed_usage,\n +\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n ++\tif (argc)\n ++\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n +\n +\tgc_config(&cfg);\n +\tinitialize_task_config(&opts, &selected_tasks);\n +\n -+\tif (argc)\n -+\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n -+\n +\tif (opts.auto_flag) {\n +\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n +\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n\n\nbase-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406\nchange-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606\n\nThanks\n- Karthik\n\n"},{"id":"530178","messageId":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-1-303462a9e4ed@gmail.com","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","subject":"[PATCH v2 1/5] reftable/stack: return stack segments directly","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:43:56Z","receivedAt":"2025-11-04T08:44:08Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `stack_table_sizes_for_compaction()` function returns individual\nsizes of each reftable table. This function is only called by\n`reftable_stack_auto_compact()` to decide which tables need to be\ncompacted, if any.\n\nModify the function to directly return the segments, which avoids the\nextra step of receiving the sizes only to pass it to\n`suggest_compaction_segment()`.\n\nA future commit will also add functionality for checking whether\nauto-compaction is necessary without performing it. This change allows\ncode re-usability in that context.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/stack.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 65d89820bd..49387f9344 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,\n \treturn seg;\n }\n \n-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n+static int stack_segments_for_compaction(struct reftable_stack *st,\n+\t\t\t\t\t struct segment *seg)\n {\n \tint version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;\n \tint overhead = header_size(version) - 1;\n@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n \n \tREFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);\n \tif (!sizes)\n-\t\treturn NULL;\n+\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n \n \tfor (size_t i = 0; i < st->merged->tables_len; i++)\n \t\tsizes[i] = st->tables[i]->size - overhead;\n \n-\treturn sizes;\n+\t*seg = suggest_compaction_segment(sizes, st->merged->tables_len,\n+\t\t\t\t\t st->opts.auto_compaction_factor);\n+\treftable_free(sizes);\n+\n+\treturn 0;\n }\n \n int reftable_stack_auto_compact(struct reftable_stack *st)\n {\n \tstruct segment seg;\n-\tuint64_t *sizes;\n+\tint err;\n \n \tif (st->merged->tables_len < 2)\n \t\treturn 0;\n \n-\tsizes = stack_table_sizes_for_compaction(st);\n-\tif (!sizes)\n-\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\n-\tseg = suggest_compaction_segment(sizes, st->merged->tables_len,\n-\t\t\t\t\t st->opts.auto_compaction_factor);\n-\treftable_free(sizes);\n+\terr = stack_segments_for_compaction(st, &seg);\n+\tif (err)\n+\t\treturn err;\n \n \tif (segment_size(&seg) > 0)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n\n-- \n2.51.0\n\n"},{"id":"530179","messageId":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-3-303462a9e4ed@gmail.com","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","subject":"[PATCH v2 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:43:58Z","receivedAt":"2025-11-04T08:44:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"To allow users of the refs namespace to check if the reference backend\nrequires optimization, add a new field `optimize_required` field to\n`struct ref_storage_be`. This field is of type `optimize_required_fn`\nwhich is also introduced in this commit.\n\nModify the debug, files, packed and reftable backend to implement this\nfield. A following commit will expose this via 'git pack-refs' and 'git\nrefs optimize'.\n\nSigned-off-by: Karthik Nayak \n---\n refs.c | 7 +++++++\n refs.h | 7 +++++++\n refs/debug.c | 13 +++++++++++++\n refs/files-backend.c | 11 +++++++++++\n refs/packed-backend.c | 13 +++++++++++++\n refs/refs-internal.h | 6 ++++++\n refs/reftable-backend.c | 25 +++++++++++++++++++++++++\n 7 files changed, 82 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0d0831f29b..5583f6e09d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n \treturn refs->be->optimize(refs, opts);\n }\n \n+int refs_optimize_required(struct ref_store *refs,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required)\n+{\n+\treturn refs->be->optimize_required(refs, opts, required);\n+}\n+\n int reference_get_peeled_oid(struct repository *repo,\n \t\t\t const struct reference *ref,\n \t\t\t struct object_id *peeled_oid)\ndiff --git a/refs.h b/refs.h\nindex 6b05bba527..d9051bbb04 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -520,6 +520,13 @@ struct refs_optimize_opts {\n */\n int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n \n+/*\n+ * Check if refs backend can be optimized by calling 'refs_optimize'.\n+ */\n+int refs_optimize_required(struct ref_store *ref_store,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required);\n+\n /*\n * Setup reflog before using. Fill in err and return -1 on failure.\n */\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 2defd2d465..36f8c58b6c 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts\n \treturn res;\n }\n \n+static int debug_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n+\tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n+\ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n+\t\t\t required ? \"yes\" : \"no\", res);\n+\treturn res;\n+}\n+\n static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n \t\t\t const char *newref, const char *logmsg)\n {\n@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {\n \t.transaction_abort = debug_transaction_abort,\n \n \t.optimize = debug_optimize,\n+\t.optimize_required = debug_optimize_required,\n+\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 a1e70b1c10..6e0c9b340a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,\n \treturn 0;\n }\n \n+static int files_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t \"optimize_required\");\n+\t*required = should_pack_refs(refs, opts);\n+\treturn 0;\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@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {\n \t.transaction_abort = files_transaction_abort,\n \n \t.optimize = files_optimize,\n+\t.optimize_required = files_optimize_required,\n \t.rename_ref = files_rename_ref,\n \t.copy_ref = files_copy_ref,\n \ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 10062fd8b6..19ce4d5872 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,\n \treturn 0;\n }\n \n+static int packed_optimize_required(struct ref_store *ref_store UNUSED,\n+\t\t\t\t struct refs_optimize_opts *opts UNUSED,\n+\t\t\t\t bool *required)\n+{\n+\t/*\n+\t * Packed refs are already optimized.\n+\t */\n+\t*required = false;\n+\treturn 0;\n+}\n+\n static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)\n {\n \treturn empty_ref_iterator_begin();\n@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {\n \t.transaction_abort = packed_transaction_abort,\n \n \t.optimize = packed_optimize,\n+\t.optimize_required = packed_optimize_required,\n+\n \t.rename_ref = NULL,\n \t.copy_ref = NULL,\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dee42f231d..c7d2a6e50b 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,\n \n typedef int optimize_fn(struct ref_store *ref_store,\n \t\t\tstruct refs_optimize_opts *opts);\n+\n+typedef int optimize_required_fn(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required);\n+\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);\n@@ -549,6 +554,7 @@ struct ref_storage_be {\n \tref_transaction_abort_fn *transaction_abort;\n \n \toptimize_fn *optimize;\n+\toptimize_required_fn *optimize_required;\n \trename_ref_fn *rename_ref;\n \tcopy_ref_fn *copy_ref;\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex c23c45f3bf..a3ae0cf74a 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,\n \treturn ret;\n }\n \n+static int reftable_be_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t\t bool *required)\n+{\n+\tstruct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t\t \"optimize_refs_required\");\n+\tstruct reftable_stack *stack;\n+\tbool use_heuristics = false;\n+\n+\tif (refs->err)\n+\t\treturn refs->err;\n+\n+\tstack = refs->worktree_backend.stack;\n+\tif (!stack)\n+\t\tstack = refs->main_backend.stack;\n+\n+\tif (opts->flags & REFS_OPTIMIZE_AUTO)\n+\t\tuse_heuristics = true;\n+\n+\treturn reftable_stack_compaction_required(stack, use_heuristics,\n+\t\t\t\t\t\t required);\n+}\n+\n struct write_create_symref_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_stack *stack;\n@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {\n \t.transaction_abort = reftable_be_transaction_abort,\n \n \t.optimize = reftable_be_optimize,\n+\t.optimize_required = reftable_be_optimize_required,\n+\n \t.rename_ref = reftable_be_rename_ref,\n \t.copy_ref = reftable_be_copy_ref,\n \n\n-- \n2.51.0\n\n"},{"id":"530181","messageId":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-2-303462a9e4ed@gmail.com","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","subject":"[PATCH v2 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:43:57Z","receivedAt":"2025-11-04T08:44:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The reftable backend performs auto-compaction as part of its regular\nflow, which is required to keep the number of tables part of a stack at\nbay. This allows it to stay optimized.\n\nCompaction can also be triggered voluntarily by the user via the 'git\npack-refs' or the 'git refs optimize' command. However, currently there\nis no way for the user to check if optimization is required without\nactually performing it.\n\nAdd and expose `reftable_stack_compaction_required()` which will allow\nusers to check if the reftable backend can be optimized.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/reftable-stack.h | 11 +++++++++++\n reftable/stack.c | 25 +++++++++++++++++++++++++\n t/unit-tests/u-reftable-stack.c | 12 ++++++++++--\n 3 files changed, 46 insertions(+), 2 deletions(-)\n\ndiff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\nindex d70fcb705d..c2415cbc6e 100644\n--- a/reftable/reftable-stack.h\n+++ b/reftable/reftable-stack.h\n@@ -123,6 +123,17 @@ struct reftable_log_expiry_config {\n int reftable_stack_compact_all(struct reftable_stack *st,\n \t\t\t struct reftable_log_expiry_config *config);\n \n+/*\n+ * Check if compaction is required.\n+ *\n+ * When `use_heuristics` is false, check if all tables can be compacted to a\n+ * single table. If true, use heuristics to determine if the tables need to be\n+ * compacted to maintain geometric progression.\n+ */\n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required);\n+\n /* heuristically compact unbalanced table stack. */\n int reftable_stack_auto_compact(struct reftable_stack *st);\n \ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 49387f9344..18fa41cd5c 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1647,6 +1647,31 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n \treturn 0;\n }\n \n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required)\n+{\n+\tstruct segment seg;\n+\tint err = 0;\n+\n+\tif (st->merged->tables_len < 2) {\n+\t\t*required = false;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!use_heuristics) {\n+\t\t*required = true;\n+\t\treturn 0;\n+\t}\n+\n+\terr = stack_segments_for_compaction(st, &seg);\n+\tif (err)\n+\t\treturn err;\n+\n+\t*required = segment_size(&seg) > 0;\n+\treturn 0;\n+}\n+\n int reftable_stack_auto_compact(struct reftable_stack *st)\n {\n \tstruct segment seg;\ndiff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c\nindex a8b91812e8..b8110cdeee 100644\n--- a/t/unit-tests/u-reftable-stack.c\n+++ b/t/unit-tests/u-reftable-stack.c\n@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t\t.value_type = REFTABLE_REF_SYMREF,\n \t\t\t.value.symref = (char *) \"master\",\n \t\t};\n+\t\tbool required = false;\n \t\tchar buf[128];\n \n \t\t/*\n@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t * auto compaction is disabled. When enabled, we should merge\n \t\t * all tables in the stack.\n \t\t */\n-\t\tif (i != n)\n+\t\tcl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);\n+\t\tif (i != n) {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, i + 1);\n-\t\telse\n+\t\t\tif (i < 1)\n+\t\t\t\tcl_assert_equal_b(required, false);\n+\t\t\telse\n+\t\t\t\tcl_assert_equal_b(required, true);\n+\t\t} else {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, 1);\n+\t\t\tcl_assert_equal_b(required, false);\n+\t\t}\n \t}\n \n \treftable_stack_destroy(st);\n\n-- \n2.51.0\n\n"},{"id":"530180","messageId":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-4-303462a9e4ed@gmail.com","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","subject":"[PATCH v2 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:43:59Z","receivedAt":"2025-11-04T08:44:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the\nflag ensures to run maintenance tasks only if certain thresholds are\nmet. The heuristic is defined on a task level, wherein each task defines\nan 'auto_condition', which states if the task should be run.\n\nThe 'pack-refs' task is hard-coded to return 1 as:\n1. There was never a way to check if the reference backend needs to be\noptimized without actually performing the optimization.\n2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would\noptimize based on heuristics.\n\nThe previous commit added a `refs_optimize_required()` function, which\ncan be used to check if a reference backend required optimization. Use\nthis within `pack_refs_condition()`.\n\nThis allows us to add a 'git maintenance is-needed' subcommand which can\nnotify the user if maintenance is needed without actually performing the\noptimization. Without this change, the reference backend would always\nstate that optimization is needed.\n\nSince we import 'revision.h', we need to remove the definition for\n'SEEN' which is duplicated in the included header.\n\nSigned-off-by: Karthik Nayak \n---\n builtin/gc.c | 30 +++++++++++++++++++++---------\n object.h | 1 -\n 2 files changed, 21 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c6d62c74a7..c3e7a84ec2 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -35,6 +35,7 @@\n #include \"path.h\"\n #include \"reflog.h\"\n #include \"rerere.h\"\n+#include \"revision.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n #include \"promisor-remote.h\"\n@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n \n static int pack_refs_condition(UNUSED struct gc_config *cfg)\n {\n-\t/*\n-\t * The auto-repacking logic for refs is handled by the ref backends and\n-\t * exposed via `git pack-refs --auto`. We thus always return truish\n-\t * here and let the backend decide for us.\n-\t */\n-\treturn 1;\n+\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n+\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n+\tstruct refs_optimize_opts optimize_opts = {\n+\t\t.exclusions = &excludes,\n+\t\t.includes = &included_refs,\n+\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n+\t};\n+\tbool required;\n+\n+\t/* Check for all refs, similar to 'git refs optimize --all'. */\n+\tstring_list_append(optimize_opts.includes, \"*\");\n+\n+\tif (refs_optimize_required(get_main_ref_store(the_repository),\n+\t\t\t\t &optimize_opts, &required))\n+\t\treturn 0;\n+\n+\tclear_ref_exclusions(&excludes);\n+\tstring_list_clear(&included_refs, 0);\n+\n+\treturn required == true;\n }\n \n static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,\n@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-/* Remember to update object flag allocation in object.h */\n-#define SEEN\t\t(1u<<0)\n-\n struct cg_auto_data {\n \tint num_not_in_graph;\n \tint limit;\ndiff --git a/object.h b/object.h\nindex 1499f63d50..832299e763 100644\n--- a/object.h\n+++ b/object.h\n@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);\n * list-objects-filter.c: 21\n * bloom.c: 2122\n * builtin/fsck.c: 0--3\n- * builtin/gc.c: 0\n * builtin/index-pack.c: 2021\n * reflog.c: 10--12\n * builtin/show-branch.c: 0-------------------------------------------26\n\n-- \n2.51.0\n\n"},{"id":"530182","messageId":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-5-303462a9e4ed@gmail.com","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","subject":"[PATCH v2 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-04T08:44:00Z","receivedAt":"2025-11-04T08:44:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command provides tooling to run maintenance\ntasks over Git repositories. The 'run' subcommand, as the name suggests,\nruns the maintenance tasks. When used with the '--auto' flag, it uses\nheuristics to determine if the required thresholds are met for running\nsaid maintenance tasks.\n\nThere is however a lack of insight into these heuristics. Meaning, the\nchecks are linked to the execution.\n\nAdd a new 'is-needed' subcommand to 'git-maintenance(1)' which allows\nusers to simply check if it is needed to run maintenance without\nperforming it.\n\nThis subcommand can check if it is needed to run maintenance without\nactually running it. Ideally it should be used with the '--auto' flag,\nwhich would allow users to check if the thresholds required are met. The\nsubcommand also supports the '--task' flag which can be used to check\nspecific maintenance tasks.\n\nWhile adding the respective tests in 't/t7900-maintenance.sh', remove a\nduplicate of the test: 'worktree-prune task with --auto honors\nmaintenance.worktree-prune.auto'.\n\nSigned-off-by: Karthik Nayak \n---\n Documentation/git-maintenance.adoc | 13 +++++++++\n builtin/gc.c | 55 +++++++++++++++++++++++++++++++++++++-\n t/t7900-maintenance.sh | 54 ++++++++++++++++++++++++++-----------\n 3 files changed, 105 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\nindex 540b5cf68b..37939510d4 100644\n--- a/Documentation/git-maintenance.adoc\n+++ b/Documentation/git-maintenance.adoc\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git maintenance' run []\n 'git maintenance' start [--scheduler=]\n 'git maintenance' (stop|register|unregister) []\n+'git maintenance' is-needed []\n \n \n DESCRIPTION\n@@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository\n is not already registered. Use the `--force` option to return success even\n when the current repository is not registered.\n \n+is-needed::\n+ Check whether maintenance needs to be run without actually running it.\n+ Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n+ Ideally used with the '--auto' flag.\n++\n+If one or more `--task` options\tare specified, then those tasks are checked\n+in that order. Otherwise, the tasks are determined by which\n+`maintenance..enabled` config options are true. By default, only\n+`maintenance.gc.enabled` is true.\n+\n TASKS\n -----\n \n@@ -183,6 +194,8 @@ OPTIONS\n \tin the `gc.auto` config setting, or when the number of pack-files\n \texceeds the `gc.autoPackLimit` config setting. Not compatible with\n \tthe `--schedule` option.\n+\tWhen combined with the `is-needed` subcommand, check if the required\n+\tthresholds are met without actually running maintenance.\n \n --schedule::\n \tWhen combined with the `run` subcommand, run maintenance tasks\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c3e7a84ec2..e5ba2a2e72 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -3253,7 +3253,59 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n \treturn update_background_schedule(NULL, 0);\n }\n \n-static const char * const builtin_maintenance_usage[] = {\n+static const char *const builtin_maintenance_is_needed_usage[] = {\n+\t\"git maintenance is-needed [--task=] [--schedule]\",\n+\tNULL\n+};\n+\n+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n+\t\t\t\t struct repository *repo UNUSED)\n+{\n+\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n+\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n+\tstruct gc_config cfg = GC_CONFIG_INIT;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n+\t\t\t N_(\"run tasks based on the state of the repository\")),\n+\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n+\t\t\t N_(\"check a specific task\"),\n+\t\t\t PARSE_OPT_NONEG, task_option_parse),\n+\t\tOPT_END()\n+\t};\n+\tbool is_needed = false;\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t builtin_maintenance_is_needed_usage,\n+\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n+\n+\tgc_config(&cfg);\n+\tinitialize_task_config(&opts, &selected_tasks);\n+\n+\tif (opts.auto_flag) {\n+\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n+\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n+\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n+\t\t\t\tis_needed = true;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t} else {\n+\t\t/* When not using --auto, we should always require maintenance. */\n+\t\tis_needed = true;\n+\t}\n+\n+\tstring_list_clear(&selected_tasks, 0);\n+\tmaintenance_run_opts_release(&opts);\n+\tgc_config_release(&cfg);\n+\n+\tif (is_needed)\n+\t\treturn 0;\n+\treturn 1;\n+}\n+\n+static const char *const builtin_maintenance_usage[] = {\n \tN_(\"git maintenance []\"),\n \tNULL,\n };\n@@ -3270,6 +3322,7 @@ int cmd_maintenance(int argc,\n \t\tOPT_SUBCOMMAND(\"stop\", &fn, maintenance_stop),\n \t\tOPT_SUBCOMMAND(\"register\", &fn, maintenance_register),\n \t\tOPT_SUBCOMMAND(\"unregister\", &fn, maintenance_unregister),\n+\t\tOPT_SUBCOMMAND(\"is-needed\", &fn, maintenance_is_needed),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex ddd273d8dc..a17e2091c2 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '\n \t\tgit maintenance run --auto 2>/dev/null &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/run-no-quiet.txt\" \\\n \t\tgit maintenance run --no-quiet 2>/dev/null &&\n+\tgit maintenance is-needed &&\n \ttest_subcommand git gc --quiet --no-detach --skip-foreground-tasks /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand git prune-packed --quiet /dev/null &&\n@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {\n \ttest_commit A &&\n \tgit repack -adk &&\n \tgit multi-pack-index write &&\n+\t! git -c maintenance.incremental-repack.auto=1 \\\n+\t\tmaintenance is-needed --auto --task=incremental-repack &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/midx-init.txt\" git \\\n \t\t-c maintenance.incremental-repack.auto=1 \\\n \t\tmaintenance run --auto --task=incremental-repack 2>/dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n \tgit reflog expire --all --expire=now &&\n \ttest_commit reflog-one &&\n \ttest_commit reflog-two &&\n+\n+\t! git -c maintenance.reflog-expire.auto=3 \\\n+\t\tmaintenance is-needed --auto --task=reflog-expire &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/reflog-expire-auto.txt\" \\\n \t\tgit -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&\n \ttest_subcommand ! git reflog expire --all .git/worktrees/abc &&\n+\tgit maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git maintenance run --auto --task=worktree-prune\n '\n \n@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n \t# A positive value should require at least this many prunable worktrees.\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n-'\n-\n-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n-\t# A negative value should always prune.\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n-\n-\tmkdir .git/worktrees &&\n-\t: >.git/worktrees/first &&\n-\t: >.git/worktrees/second &&\n-\t: >.git/worktrees/third &&\n-\n-\t# Zero should never prune.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n-\t# A positive value should require at least this many prunable worktrees.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n+\tgit -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n '\n \n@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '\n \trm -rf worktree &&\n \n \trm -f worktree-prune.txt &&\n+\t! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&\n \ttest_subcommand ! git worktree prune --expire 1.week.ago .git/rr-cache/entry &&\n+\tgit maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n '\n \n@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n \n \t# A negative value should always prune.\n+\tgit -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n \n \t# A positive value prunes when there is at least one entry.\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \tmkdir .git/rr-cache &&\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \t: >.git/rr-cache/entry-1 &&\n+\tgit -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \n \t# Zero should never prune.\n \t: >.git/rr-cache/entry-1 &&\n+\t! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc\n '\n \n\n-- \n2.51.0\n\n"},{"id":"530198","messageId":"xmqqa511reg0.fsf@gitster.g","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com","subject":"Re: [PATCH v2 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-04T15:43:27Z","receivedAt":"2025-11-04T15:43:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> This is based on top of master a99f379adf (The 27th batch, 2025-10-30)\n> and is dependent on the following series:\n>\n> - kn/refs-optim-cleanup\n> - ps/ref-peeled-tags\n\nYuck. ps/ref-peeled-tags needed an update so kn/refs-optim-cleanup\nthat depends on it needs rebuilding on top (no action needed from\nyour side, but somebody is doing the necessary rebasing somewhere),\nand then these five patches need to be queued on top, which will\nrequire further shuffling when any of these two series need to be\nupdated again.\n\nI expect that during the pre-release freeze things will be slowing\ndown, so I'll manage and survive ;-)\n\nWill queue. Thanks.\n"},{"id":"530216","messageId":"xmqqcy5xpmrw.fsf@gitster.g","threadId":"64412","inReplyTo":"20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-2-303462a9e4ed@gmail.com","subject":"Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-04T20:26:27Z","receivedAt":"2025-11-04T20:26:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> The reftable backend performs auto-compaction as part of its regular\n> flow, which is required to keep the number of tables part of a stack at\n> bay. This allows it to stay optimized.\n\nSounds very sensible.\n\n> Compaction can also be triggered voluntarily by the user via the 'git\n> pack-refs' or the 'git refs optimize' command. However, currently there\n> is no way for the user to check if optimization is required without\n> actually performing it.\n\nSounds very sensible goal.\n\nBut where is the existing logic to decide when it needs to\nauto-compact, performed as part of its regular flow?\n\nAfter reading \"the reftable machinery already decides when it needs\nto compact and does so\" plus \"but the logic to decide is not made\navailable to users\", I would have expected for this patch to extract\nsuch an existing logic or otherwise make it available to new callers\nso that things like \"gc --auto\" can call it, but the diffstat shows\nmostly additions, which does not give readers any confidence in the\nnew function that answers \"do we need compaction?\". It would give\n_an_ answer, but there is no clue if the answer it gives is the same\nanswer as the existing logic that decides when to compact as part of\nthe regular operation.\n\nI am puzzled.\n\n> +int reftable_stack_compaction_required(struct reftable_stack *st,\n> +\t\t\t\t bool use_heuristics,\n> +\t\t\t\t bool *required)\n> +{\n> +\tstruct segment seg;\n> +\tint err = 0;\n> +\n> +\tif (st->merged->tables_len < 2) {\n> +\t\t*required = false;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tif (!use_heuristics) {\n> +\t\t*required = true;\n> +\t\treturn 0;\n> +\t}\n> +\n> +\terr = stack_segments_for_compaction(st, &seg);\n> +\tif (err)\n> +\t\treturn err;\n> +\n> +\t*required = segment_size(&seg) > 0;\n> +\treturn 0;\n> +}\n\nSpecifically, where is the above logic come from? Is it duplicating\nan existing logic but that code is hard to separate out into this\nhelper?\n\n> int reftable_stack_auto_compact(struct reftable_stack *st)\n> {\n> \tstruct segment seg;\n> diff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c\n> index a8b91812e8..b8110cdeee 100644\n> --- a/t/unit-tests/u-reftable-stack.c\n> +++ b/t/unit-tests/u-reftable-stack.c\n> @@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n> \t\t\t.value_type = REFTABLE_REF_SYMREF,\n> \t\t\t.value.symref = (char *) \"master\",\n> \t\t};\n> +\t\tbool required = false;\n> \t\tchar buf[128];\n> \n> \t\t/*\n> @@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n> \t\t * auto compaction is disabled. When enabled, we should merge\n> \t\t * all tables in the stack.\n> \t\t */\n> -\t\tif (i != n)\n> +\t\tcl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);\n> +\t\tif (i != n) {\n> \t\t\tcl_assert_equal_i(st->merged->tables_len, i + 1);\n> -\t\telse\n> +\t\t\tif (i < 1)\n> +\t\t\t\tcl_assert_equal_b(required, false);\n> +\t\t\telse\n> +\t\t\t\tcl_assert_equal_b(required, true);\n> +\t\t} else {\n> \t\t\tcl_assert_equal_i(st->merged->tables_len, 1);\n> +\t\t\tcl_assert_equal_b(required, false);\n> +\t\t}\n> \t}\n> \n> \treftable_stack_destroy(st);\n"},{"id":"530241","messageId":"CAOLa=ZTqqenfKETuvssJ-8KbaVAp5gG1n_jypkm-uBuH6vAO0A@mail.gmail.com","threadId":"64412","inReplyTo":"xmqqa511reg0.fsf@gitster.g","subject":"Re: [PATCH v2 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-05T14:00:13Z","receivedAt":"2025-11-05T14:00:17Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> This is based on top of master a99f379adf (The 27th batch, 2025-10-30)\n>> and is dependent on the following series:\n>>\n>> - kn/refs-optim-cleanup\n>> - ps/ref-peeled-tags\n>\n> Yuck. ps/ref-peeled-tags needed an update so kn/refs-optim-cleanup\n> that depends on it needs rebuilding on top (no action needed from\n> your side, but somebody is doing the necessary rebasing somewhere),\n> and then these five patches need to be queued on top, which will\n> require further shuffling when any of these two series need to be\n> updated again.\n>\n\nI know, and I must thank you in this regard for putting up with this.\nThis dependency chain is certainly not pleasant.\n\n> I expect that during the pre-release freeze things will be slowing\n> down, so I'll manage and survive ;-)\n>\n> Will queue. Thanks.\n\nThanks!\n"},{"id":"530242","messageId":"CAOLa=ZRD_zNCnGf3ibU=X04vC8WjxzRVAyg+OwPr1Hf12kSGgA@mail.gmail.com","threadId":"64412","inReplyTo":"xmqqcy5xpmrw.fsf@gitster.g","subject":"Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-05T14:11:53Z","receivedAt":"2025-11-05T14:11:56Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> The reftable backend performs auto-compaction as part of its regular\n>> flow, which is required to keep the number of tables part of a stack at\n>> bay. This allows it to stay optimized.\n>\n> Sounds very sensible.\n>\n>> Compaction can also be triggered voluntarily by the user via the 'git\n>> pack-refs' or the 'git refs optimize' command. However, currently there\n>> is no way for the user to check if optimization is required without\n>> actually performing it.\n>\n> Sounds very sensible goal.\n>\n> But where is the existing logic to decide when it needs to\n> auto-compact, performed as part of its regular flow?\n>\n> After reading \"the reftable machinery already decides when it needs\n> to compact and does so\" plus \"but the logic to decide is not made\n> available to users\", I would have expected for this patch to extract\n> such an existing logic or otherwise make it available to new callers\n> so that things like \"gc --auto\" can call it, but the diffstat shows\n> mostly additions, which does not give readers any confidence in the\n> new function that answers \"do we need compaction?\". It would give\n> _an_ answer, but there is no clue if the answer it gives is the same\n> answer as the existing logic that decides when to compact as part of\n> the regular operation.\n>\n> I am puzzled.\n>\n>> +int reftable_stack_compaction_required(struct reftable_stack *st,\n>> +\t\t\t\t bool use_heuristics,\n>> +\t\t\t\t bool *required)\n>> +{\n>> +\tstruct segment seg;\n>> +\tint err = 0;\n>> +\n>> +\tif (st->merged->tables_len < 2) {\n>> +\t\t*required = false;\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\tif (!use_heuristics) {\n>> +\t\t*required = true;\n>> +\t\treturn 0;\n>> +\t}\n>> +\n>> +\terr = stack_segments_for_compaction(st, &seg);\n>> +\tif (err)\n>> +\t\treturn err;\n>> +\n>> +\t*required = segment_size(&seg) > 0;\n>> +\treturn 0;\n>> +}\n>\n> Specifically, where is the above logic come from? Is it duplicating\n> an existing logic but that code is hard to separate out into this\n> helper?\n>\n\nGood question.\n\nMost of this logic is already part of 'reftable_stack_auto_compact()'.\nWe also have another similar function 'reftable_stack_compact_all()'.\nThe former is used for compaction based on heuristics and the latter is\nfor compacting all tables into one. In the refs subsystem usage of\nheuristics is denoted by the usage of the 'REFS_OPTIMIZE_AUTO' flag.\n\nThe function we're introducing allows users to explicitly mention if\nthey want to use heuristics or not. This allows us to differentiate\nbetween the two modes. The result of which is that this uses intertwined\nlogic of the two existing functions. Hence we can't extract any code out.\n\nI'll add this information in the commit message.\n\nKarthik\n"},{"id":"530259","messageId":"xmqqms50l594.fsf@gitster.g","threadId":"64412","inReplyTo":"CAOLa=ZRD_zNCnGf3ibU=X04vC8WjxzRVAyg+OwPr1Hf12kSGgA@mail.gmail.com","subject":"Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-05T18:10:47Z","receivedAt":"2025-11-05T18:10:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Junio C Hamano writes:\n>\n>> Karthik Nayak writes:\n>>\n>>> The reftable backend performs auto-compaction as part of its regular\n>>> flow, which is required to keep the number of tables part of a stack at\n>>> bay. This allows it to stay optimized.\n>>\n>> Sounds very sensible.\n>>\n>>> Compaction can also be triggered voluntarily by the user via the 'git\n>>> pack-refs' or the 'git refs optimize' command. However, currently there\n>>> is no way for the user to check if optimization is required without\n>>> actually performing it.\n>>\n>> Sounds very sensible goal.\n>>\n>> But where is the existing logic to decide when it needs to\n>> auto-compact, performed as part of its regular flow?\n>>\n>> After reading \"the reftable machinery already decides when it needs\n>> to compact and does so\" plus \"but the logic to decide is not made\n>> available to users\", I would have expected for this patch to extract\n>> such an existing logic or otherwise make it available to new callers\n>> so that things like \"gc --auto\" can call it, but the diffstat shows\n>> mostly additions, which does not give readers any confidence in the\n>> new function that answers \"do we need compaction?\". It would give\n>> _an_ answer, but there is no clue if the answer it gives is the same\n>> answer as the existing logic that decides when to compact as part of\n>> the regular operation.\n>>\n>> I am puzzled.\n>>\n>>> +int reftable_stack_compaction_required(struct reftable_stack *st,\n>>> +\t\t\t\t bool use_heuristics,\n>>> +\t\t\t\t bool *required)\n>>> +{\n>>> +\tstruct segment seg;\n>>> +\tint err = 0;\n>>> +\n>>> +\tif (st->merged->tables_len < 2) {\n>>> +\t\t*required = false;\n>>> +\t\treturn 0;\n>>> +\t}\n>>> +\n>>> +\tif (!use_heuristics) {\n>>> +\t\t*required = true;\n>>> +\t\treturn 0;\n>>> +\t}\n>>> +\n>>> +\terr = stack_segments_for_compaction(st, &seg);\n>>> +\tif (err)\n>>> +\t\treturn err;\n>>> +\n>>> +\t*required = segment_size(&seg) > 0;\n>>> +\treturn 0;\n>>> +}\n>>\n>> Specifically, where is the above logic come from? Is it duplicating\n>> an existing logic but that code is hard to separate out into this\n>> helper?\n>>\n>\n> Good question.\n>\n> Most of this logic is already part of 'reftable_stack_auto_compact()'.\n> We also have another similar function 'reftable_stack_compact_all()'.\n> The former is used for compaction based on heuristics and the latter is\n> for compacting all tables into one. In the refs subsystem usage of\n> heuristics is denoted by the usage of the 'REFS_OPTIMIZE_AUTO' flag.\n>\n> The function we're introducing allows users to explicitly mention if\n> they want to use heuristics or not. This allows us to differentiate\n> between the two modes. The result of which is that this uses intertwined\n> logic of the two existing functions. Hence we can't extract any code out.\n>\n> I'll add this information in the commit message.\n\nYou mean you already have two duplicate implementations whose\ndefinition of \"when should we compact?\" can drift apart over time\n(worse, they may already be subtly different), and you are adding\nyet another one?\n\nInstead of describing such an insanity in the commit message, can we\nrefactor to have a single central logic that is used from three\nplaces?\n\nThanks.\n"},{"id":"530294","messageId":"CAOLa=ZTTfaGsQrK-e5c9h4nMk656GM0Mue6Ytie4GOes4n_-5g@mail.gmail.com","threadId":"64412","inReplyTo":"xmqqms50l594.fsf@gitster.g","subject":"Re: [PATCH v2 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:18:51Z","receivedAt":"2025-11-06T08:18:53Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Karthik Nayak writes:\n>\n>> Junio C Hamano writes:\n>>\n>>> Karthik Nayak writes:\n>>>\n>>>> The reftable backend performs auto-compaction as part of its regular\n>>>> flow, which is required to keep the number of tables part of a stack at\n>>>> bay. This allows it to stay optimized.\n>>>\n>>> Sounds very sensible.\n>>>\n>>>> Compaction can also be triggered voluntarily by the user via the 'git\n>>>> pack-refs' or the 'git refs optimize' command. However, currently there\n>>>> is no way for the user to check if optimization is required without\n>>>> actually performing it.\n>>>\n>>> Sounds very sensible goal.\n>>>\n>>> But where is the existing logic to decide when it needs to\n>>> auto-compact, performed as part of its regular flow?\n>>>\n>>> After reading \"the reftable machinery already decides when it needs\n>>> to compact and does so\" plus \"but the logic to decide is not made\n>>> available to users\", I would have expected for this patch to extract\n>>> such an existing logic or otherwise make it available to new callers\n>>> so that things like \"gc --auto\" can call it, but the diffstat shows\n>>> mostly additions, which does not give readers any confidence in the\n>>> new function that answers \"do we need compaction?\". It would give\n>>> _an_ answer, but there is no clue if the answer it gives is the same\n>>> answer as the existing logic that decides when to compact as part of\n>>> the regular operation.\n>>>\n>>> I am puzzled.\n>>>\n>>>> +int reftable_stack_compaction_required(struct reftable_stack *st,\n>>>> +\t\t\t\t bool use_heuristics,\n>>>> +\t\t\t\t bool *required)\n>>>> +{\n>>>> +\tstruct segment seg;\n>>>> +\tint err = 0;\n>>>> +\n>>>> +\tif (st->merged->tables_len < 2) {\n>>>> +\t\t*required = false;\n>>>> +\t\treturn 0;\n>>>> +\t}\n>>>> +\n>>>> +\tif (!use_heuristics) {\n>>>> +\t\t*required = true;\n>>>> +\t\treturn 0;\n>>>> +\t}\n>>>> +\n>>>> +\terr = stack_segments_for_compaction(st, &seg);\n>>>> +\tif (err)\n>>>> +\t\treturn err;\n>>>> +\n>>>> +\t*required = segment_size(&seg) > 0;\n>>>> +\treturn 0;\n>>>> +}\n>>>\n>>> Specifically, where is the above logic come from? Is it duplicating\n>>> an existing logic but that code is hard to separate out into this\n>>> helper?\n>>>\n>>\n>> Good question.\n>>\n>> Most of this logic is already part of 'reftable_stack_auto_compact()'.\n>> We also have another similar function 'reftable_stack_compact_all()'.\n>> The former is used for compaction based on heuristics and the latter is\n>> for compacting all tables into one. In the refs subsystem usage of\n>> heuristics is denoted by the usage of the 'REFS_OPTIMIZE_AUTO' flag.\n>>\n>> The function we're introducing allows users to explicitly mention if\n>> they want to use heuristics or not. This allows us to differentiate\n>> between the two modes. The result of which is that this uses intertwined\n>> logic of the two existing functions. Hence we can't extract any code out.\n>>\n>> I'll add this information in the commit message.\n>\n> You mean you already have two duplicate implementations whose\n> definition of \"when should we compact?\" can drift apart over time\n> (worse, they may already be subtly different), and you are adding\n> yet another one?\n\nMore like we have two functions:\n1. compact all tables into one\n2. compact based on heuristics\n\nThis function oversees logic from both.\n\n> Instead of describing such an insanity in the commit message, can we\n> refactor to have a single central logic that is used from three\n> places?\n>\n> Thanks.\n\nYou're right though, I did manage to extract out the common code and\nwill send in a new version. Thanks for the push.\n\n- Karthik\n"},{"id":"530295","messageId":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH v3 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:22:29Z","receivedAt":"2025-11-06T08:22:35Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hello,\n\nI recently raised a patch series [1] to add 'git refs optimize --required'\nwhich checks if the reference backend can be optimized, without actually\nperforming the optimization.\n\nBack then, we had decided [2] that it would be a better to broaden the\napproach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This\nwould allow users to check if maintenance was required for the\nrepository and users could also provide a task via the '--task' to check\nif maintenance was needed for a particular task.\n\nIdeally the subcommand will be used with the '--auto' flag which can\ncheck the same heuristics as that used with 'git maintenance run\n--auto'. Future patches can also add support for the '--schedule' flag\nwhich can be used to check required schedule it met. However that flag\nisn't added as part of this series.\n\nThis series implements that.\n\nCommits 1-3 add the required functionality in the refs subsystem to\nexpose an 'optimize_required' field which can be used to check if\nbackends need to be optimized.\nCommit 4 utilizes this within the 'git-maintenance(1)' code.\nCommit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.\n\nThis is based on top of master a99f379adf (The 27th batch, 2025-10-30)\nand is dependent on the following series:\n\n - kn/refs-optim-cleanup\n - ps/ref-peeled-tags\n\nMerges cleanly with `next`. I think those two topics are close to being\nmerged to `next` so hopefully this dependency tree doesn't get too\ncomplicated. I'll rebase as needed to resolve conflicts.\n\n[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/\n[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/\n\n---\nChanges in v3:\n- In patch 2/5 extract out code for deciding if compaction is required\n into a static function. This removes duplication of logic for deciding\n if compaction is needed.\n- Link to v2: https://patch.msgid.link/20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com\n\nChanges in v2:\n- Added more documentation for `reftable_stack_compaction_required()`.\n- Fixed some typos and grammar mistakes in commit messages.\n- Clarify which tasks will be run when '--task' is not used.\n- Move the call to 'usage_with_options()' to be with 'parse_options()'.\n- Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com\n\n---\n Documentation/git-maintenance.adoc | 13 ++++++\n builtin/gc.c | 85 +++++++++++++++++++++++++++++++++-----\n object.h | 1 -\n refs.c | 7 ++++\n refs.h | 7 ++++\n refs/debug.c | 13 ++++++\n refs/files-backend.c | 11 +++++\n refs/packed-backend.c | 13 ++++++\n refs/refs-internal.h | 6 +++\n refs/reftable-backend.c | 25 +++++++++++\n reftable/reftable-stack.h | 11 +++++\n reftable/stack.c | 61 ++++++++++++++++++++-------\n t/t7900-maintenance.sh | 54 +++++++++++++++++-------\n t/unit-tests/u-reftable-stack.c | 12 +++++-\n 14 files changed, 276 insertions(+), 43 deletions(-)\n\nKarthik Nayak (5):\n reftable/stack: return stack segments directly\n reftable/stack: add function to check if optimization is required\n refs: add a `optimize_required` field to `struct ref_storage_be`\n maintenance: add checking logic in `pack_refs_condition()`\n maintenance: add 'is-needed' subcommand\n\nRange-diff versus v2:\n\n1: 16b447b66c = 1: fe8977aefc reftable/stack: return stack segments directly\n2: b463d5c69d ! 2: e73f672566 reftable/stack: add function to check if optimization is required\n @@ Commit message\n is no way for the user to check if optimization is required without\n actually performing it.\n \n - Add and expose `reftable_stack_compaction_required()` which will allow\n - users to check if the reftable backend can be optimized.\n + Extract out the heuristics logic from 'reftable_stack_auto_compact()'\n + into an internal function 'update_segment_if_compaction_required()'.\n + Then use this to add and expose `reftable_stack_compaction_required()`\n + which will allow users to check if the reftable backend can be\n + optimized.\n \n Signed-off-by: Karthik Nayak \n \n @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack\n \treturn 0;\n }\n \n -+int reftable_stack_compaction_required(struct reftable_stack *st,\n -+\t\t\t\t bool use_heuristics,\n -+\t\t\t\t bool *required)\n -+{\n -+\tstruct segment seg;\n -+\tint err = 0;\n -+\n +-int reftable_stack_auto_compact(struct reftable_stack *st)\n ++static int update_segment_if_compaction_required(struct reftable_stack *st,\n ++\t\t\t\t\t\t struct segment *seg,\n ++\t\t\t\t\t\t bool use_heuristics,\n ++\t\t\t\t\t\t bool *required)\n + {\n +-\tstruct segment seg;\n + \tint err;\n + \n +-\tif (st->merged->tables_len < 2)\n +\tif (st->merged->tables_len < 2) {\n +\t\t*required = false;\n +\t\treturn 0;\n @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack\n +\n +\tif (!use_heuristics) {\n +\t\t*required = true;\n -+\t\treturn 0;\n + \t\treturn 0;\n +\t}\n +\n -+\terr = stack_segments_for_compaction(st, &seg);\n ++\terr = stack_segments_for_compaction(st, seg);\n +\tif (err)\n +\t\treturn err;\n +\n -+\t*required = segment_size(&seg) > 0;\n ++\t*required = segment_size(seg) > 0;\n +\treturn 0;\n +}\n +\n - int reftable_stack_auto_compact(struct reftable_stack *st)\n - {\n - \tstruct segment seg;\n ++int reftable_stack_compaction_required(struct reftable_stack *st,\n ++\t\t\t\t bool use_heuristics,\n ++\t\t\t\t bool *required)\n ++{\n ++\tstruct segment seg;\n ++\treturn update_segment_if_compaction_required(st, &seg, use_heuristics,\n ++\t\t\t\t\t\t required);\n ++}\n ++\n ++int reftable_stack_auto_compact(struct reftable_stack *st)\n ++{\n ++\tstruct segment seg;\n ++\tbool required;\n ++\tint err;\n + \n +-\terr = stack_segments_for_compaction(st, &seg);\n ++\terr = update_segment_if_compaction_required(st, &seg, true, &required);\n + \tif (err)\n + \t\treturn err;\n + \n +-\tif (segment_size(&seg) > 0)\n ++\tif (required)\n + \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n + \t\t\t\t\t NULL, STACK_COMPACT_RANGE_BEST_EFFORT);\n + \n \n ## t/unit-tests/u-reftable-stack.c ##\n @@ t/unit-tests/u-reftable-stack.c: void test_reftable_stack__add_performs_auto_compaction(void)\n3: b9f44f61f2 = 3: 7d2428b2f9 refs: add a `optimize_required` field to `struct ref_storage_be`\n4: f94ddfdba3 = 4: 3c5f969d62 maintenance: add checking logic in `pack_refs_condition()`\n5: bc56849266 = 5: bc2d3c26e2 maintenance: add 'is-needed' subcommand\n\n\nbase-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406\nchange-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606\n\nThanks\n- Karthik\n\n"},{"id":"530296","messageId":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-1-d611a2a95cf5@gmail.com","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com","subject":"[PATCH v3 1/5] reftable/stack: return stack segments directly","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:22:30Z","receivedAt":"2025-11-06T08:22:37Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `stack_table_sizes_for_compaction()` function returns individual\nsizes of each reftable table. This function is only called by\n`reftable_stack_auto_compact()` to decide which tables need to be\ncompacted, if any.\n\nModify the function to directly return the segments, which avoids the\nextra step of receiving the sizes only to pass it to\n`suggest_compaction_segment()`.\n\nA future commit will also add functionality for checking whether\nauto-compaction is necessary without performing it. This change allows\ncode re-usability in that context.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/stack.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 65d89820bd..49387f9344 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,\n \treturn seg;\n }\n \n-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n+static int stack_segments_for_compaction(struct reftable_stack *st,\n+\t\t\t\t\t struct segment *seg)\n {\n \tint version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;\n \tint overhead = header_size(version) - 1;\n@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n \n \tREFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);\n \tif (!sizes)\n-\t\treturn NULL;\n+\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n \n \tfor (size_t i = 0; i < st->merged->tables_len; i++)\n \t\tsizes[i] = st->tables[i]->size - overhead;\n \n-\treturn sizes;\n+\t*seg = suggest_compaction_segment(sizes, st->merged->tables_len,\n+\t\t\t\t\t st->opts.auto_compaction_factor);\n+\treftable_free(sizes);\n+\n+\treturn 0;\n }\n \n int reftable_stack_auto_compact(struct reftable_stack *st)\n {\n \tstruct segment seg;\n-\tuint64_t *sizes;\n+\tint err;\n \n \tif (st->merged->tables_len < 2)\n \t\treturn 0;\n \n-\tsizes = stack_table_sizes_for_compaction(st);\n-\tif (!sizes)\n-\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\n-\tseg = suggest_compaction_segment(sizes, st->merged->tables_len,\n-\t\t\t\t\t st->opts.auto_compaction_factor);\n-\treftable_free(sizes);\n+\terr = stack_segments_for_compaction(st, &seg);\n+\tif (err)\n+\t\treturn err;\n \n \tif (segment_size(&seg) > 0)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n\n-- \n2.51.0\n\n"},{"id":"530297","messageId":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-2-d611a2a95cf5@gmail.com","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com","subject":"[PATCH v3 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:22:31Z","receivedAt":"2025-11-06T08:22:39Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The reftable backend performs auto-compaction as part of its regular\nflow, which is required to keep the number of tables part of a stack at\nbay. This allows it to stay optimized.\n\nCompaction can also be triggered voluntarily by the user via the 'git\npack-refs' or the 'git refs optimize' command. However, currently there\nis no way for the user to check if optimization is required without\nactually performing it.\n\nExtract out the heuristics logic from 'reftable_stack_auto_compact()'\ninto an internal function 'update_segment_if_compaction_required()'.\nThen use this to add and expose `reftable_stack_compaction_required()`\nwhich will allow users to check if the reftable backend can be\noptimized.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/reftable-stack.h | 11 +++++++++++\n reftable/stack.c | 42 ++++++++++++++++++++++++++++++++++++-----\n t/unit-tests/u-reftable-stack.c | 12 ++++++++++--\n 3 files changed, 58 insertions(+), 7 deletions(-)\n\ndiff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\nindex d70fcb705d..c2415cbc6e 100644\n--- a/reftable/reftable-stack.h\n+++ b/reftable/reftable-stack.h\n@@ -123,6 +123,17 @@ struct reftable_log_expiry_config {\n int reftable_stack_compact_all(struct reftable_stack *st,\n \t\t\t struct reftable_log_expiry_config *config);\n \n+/*\n+ * Check if compaction is required.\n+ *\n+ * When `use_heuristics` is false, check if all tables can be compacted to a\n+ * single table. If true, use heuristics to determine if the tables need to be\n+ * compacted to maintain geometric progression.\n+ */\n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required);\n+\n /* heuristically compact unbalanced table stack. */\n int reftable_stack_auto_compact(struct reftable_stack *st);\n \ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 49387f9344..826500abed 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n \treturn 0;\n }\n \n-int reftable_stack_auto_compact(struct reftable_stack *st)\n+static int update_segment_if_compaction_required(struct reftable_stack *st,\n+\t\t\t\t\t\t struct segment *seg,\n+\t\t\t\t\t\t bool use_heuristics,\n+\t\t\t\t\t\t bool *required)\n {\n-\tstruct segment seg;\n \tint err;\n \n-\tif (st->merged->tables_len < 2)\n+\tif (st->merged->tables_len < 2) {\n+\t\t*required = false;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!use_heuristics) {\n+\t\t*required = true;\n \t\treturn 0;\n+\t}\n+\n+\terr = stack_segments_for_compaction(st, seg);\n+\tif (err)\n+\t\treturn err;\n+\n+\t*required = segment_size(seg) > 0;\n+\treturn 0;\n+}\n+\n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required)\n+{\n+\tstruct segment seg;\n+\treturn update_segment_if_compaction_required(st, &seg, use_heuristics,\n+\t\t\t\t\t\t required);\n+}\n+\n+int reftable_stack_auto_compact(struct reftable_stack *st)\n+{\n+\tstruct segment seg;\n+\tbool required;\n+\tint err;\n \n-\terr = stack_segments_for_compaction(st, &seg);\n+\terr = update_segment_if_compaction_required(st, &seg, true, &required);\n \tif (err)\n \t\treturn err;\n \n-\tif (segment_size(&seg) > 0)\n+\tif (required)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n \t\t\t\t\t NULL, STACK_COMPACT_RANGE_BEST_EFFORT);\n \ndiff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c\nindex a8b91812e8..b8110cdeee 100644\n--- a/t/unit-tests/u-reftable-stack.c\n+++ b/t/unit-tests/u-reftable-stack.c\n@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t\t.value_type = REFTABLE_REF_SYMREF,\n \t\t\t.value.symref = (char *) \"master\",\n \t\t};\n+\t\tbool required = false;\n \t\tchar buf[128];\n \n \t\t/*\n@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t * auto compaction is disabled. When enabled, we should merge\n \t\t * all tables in the stack.\n \t\t */\n-\t\tif (i != n)\n+\t\tcl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);\n+\t\tif (i != n) {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, i + 1);\n-\t\telse\n+\t\t\tif (i < 1)\n+\t\t\t\tcl_assert_equal_b(required, false);\n+\t\t\telse\n+\t\t\t\tcl_assert_equal_b(required, true);\n+\t\t} else {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, 1);\n+\t\t\tcl_assert_equal_b(required, false);\n+\t\t}\n \t}\n \n \treftable_stack_destroy(st);\n\n-- \n2.51.0\n\n"},{"id":"530298","messageId":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-3-d611a2a95cf5@gmail.com","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com","subject":"[PATCH v3 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:22:32Z","receivedAt":"2025-11-06T08:22:40Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"To allow users of the refs namespace to check if the reference backend\nrequires optimization, add a new field `optimize_required` field to\n`struct ref_storage_be`. This field is of type `optimize_required_fn`\nwhich is also introduced in this commit.\n\nModify the debug, files, packed and reftable backend to implement this\nfield. A following commit will expose this via 'git pack-refs' and 'git\nrefs optimize'.\n\nSigned-off-by: Karthik Nayak \n---\n refs.c | 7 +++++++\n refs.h | 7 +++++++\n refs/debug.c | 13 +++++++++++++\n refs/files-backend.c | 11 +++++++++++\n refs/packed-backend.c | 13 +++++++++++++\n refs/refs-internal.h | 6 ++++++\n refs/reftable-backend.c | 25 +++++++++++++++++++++++++\n 7 files changed, 82 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0d0831f29b..5583f6e09d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n \treturn refs->be->optimize(refs, opts);\n }\n \n+int refs_optimize_required(struct ref_store *refs,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required)\n+{\n+\treturn refs->be->optimize_required(refs, opts, required);\n+}\n+\n int reference_get_peeled_oid(struct repository *repo,\n \t\t\t const struct reference *ref,\n \t\t\t struct object_id *peeled_oid)\ndiff --git a/refs.h b/refs.h\nindex 6b05bba527..d9051bbb04 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -520,6 +520,13 @@ struct refs_optimize_opts {\n */\n int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n \n+/*\n+ * Check if refs backend can be optimized by calling 'refs_optimize'.\n+ */\n+int refs_optimize_required(struct ref_store *ref_store,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required);\n+\n /*\n * Setup reflog before using. Fill in err and return -1 on failure.\n */\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 2defd2d465..36f8c58b6c 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts\n \treturn res;\n }\n \n+static int debug_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n+\tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n+\ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n+\t\t\t required ? \"yes\" : \"no\", res);\n+\treturn res;\n+}\n+\n static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n \t\t\t const char *newref, const char *logmsg)\n {\n@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {\n \t.transaction_abort = debug_transaction_abort,\n \n \t.optimize = debug_optimize,\n+\t.optimize_required = debug_optimize_required,\n+\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 a1e70b1c10..6e0c9b340a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,\n \treturn 0;\n }\n \n+static int files_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t \"optimize_required\");\n+\t*required = should_pack_refs(refs, opts);\n+\treturn 0;\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@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {\n \t.transaction_abort = files_transaction_abort,\n \n \t.optimize = files_optimize,\n+\t.optimize_required = files_optimize_required,\n \t.rename_ref = files_rename_ref,\n \t.copy_ref = files_copy_ref,\n \ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 10062fd8b6..19ce4d5872 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,\n \treturn 0;\n }\n \n+static int packed_optimize_required(struct ref_store *ref_store UNUSED,\n+\t\t\t\t struct refs_optimize_opts *opts UNUSED,\n+\t\t\t\t bool *required)\n+{\n+\t/*\n+\t * Packed refs are already optimized.\n+\t */\n+\t*required = false;\n+\treturn 0;\n+}\n+\n static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)\n {\n \treturn empty_ref_iterator_begin();\n@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {\n \t.transaction_abort = packed_transaction_abort,\n \n \t.optimize = packed_optimize,\n+\t.optimize_required = packed_optimize_required,\n+\n \t.rename_ref = NULL,\n \t.copy_ref = NULL,\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dee42f231d..c7d2a6e50b 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,\n \n typedef int optimize_fn(struct ref_store *ref_store,\n \t\t\tstruct refs_optimize_opts *opts);\n+\n+typedef int optimize_required_fn(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required);\n+\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);\n@@ -549,6 +554,7 @@ struct ref_storage_be {\n \tref_transaction_abort_fn *transaction_abort;\n \n \toptimize_fn *optimize;\n+\toptimize_required_fn *optimize_required;\n \trename_ref_fn *rename_ref;\n \tcopy_ref_fn *copy_ref;\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex c23c45f3bf..a3ae0cf74a 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,\n \treturn ret;\n }\n \n+static int reftable_be_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t\t bool *required)\n+{\n+\tstruct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t\t \"optimize_refs_required\");\n+\tstruct reftable_stack *stack;\n+\tbool use_heuristics = false;\n+\n+\tif (refs->err)\n+\t\treturn refs->err;\n+\n+\tstack = refs->worktree_backend.stack;\n+\tif (!stack)\n+\t\tstack = refs->main_backend.stack;\n+\n+\tif (opts->flags & REFS_OPTIMIZE_AUTO)\n+\t\tuse_heuristics = true;\n+\n+\treturn reftable_stack_compaction_required(stack, use_heuristics,\n+\t\t\t\t\t\t required);\n+}\n+\n struct write_create_symref_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_stack *stack;\n@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {\n \t.transaction_abort = reftable_be_transaction_abort,\n \n \t.optimize = reftable_be_optimize,\n+\t.optimize_required = reftable_be_optimize_required,\n+\n \t.rename_ref = reftable_be_rename_ref,\n \t.copy_ref = reftable_be_copy_ref,\n \n\n-- \n2.51.0\n\n"},{"id":"530299","messageId":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-4-d611a2a95cf5@gmail.com","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com","subject":"[PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:22:33Z","receivedAt":"2025-11-06T08:22:42Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the\nflag ensures to run maintenance tasks only if certain thresholds are\nmet. The heuristic is defined on a task level, wherein each task defines\nan 'auto_condition', which states if the task should be run.\n\nThe 'pack-refs' task is hard-coded to return 1 as:\n1. There was never a way to check if the reference backend needs to be\noptimized without actually performing the optimization.\n2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would\noptimize based on heuristics.\n\nThe previous commit added a `refs_optimize_required()` function, which\ncan be used to check if a reference backend required optimization. Use\nthis within `pack_refs_condition()`.\n\nThis allows us to add a 'git maintenance is-needed' subcommand which can\nnotify the user if maintenance is needed without actually performing the\noptimization. Without this change, the reference backend would always\nstate that optimization is needed.\n\nSince we import 'revision.h', we need to remove the definition for\n'SEEN' which is duplicated in the included header.\n\nSigned-off-by: Karthik Nayak \n---\n builtin/gc.c | 30 +++++++++++++++++++++---------\n object.h | 1 -\n 2 files changed, 21 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c6d62c74a7..c3e7a84ec2 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -35,6 +35,7 @@\n #include \"path.h\"\n #include \"reflog.h\"\n #include \"rerere.h\"\n+#include \"revision.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n #include \"promisor-remote.h\"\n@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n \n static int pack_refs_condition(UNUSED struct gc_config *cfg)\n {\n-\t/*\n-\t * The auto-repacking logic for refs is handled by the ref backends and\n-\t * exposed via `git pack-refs --auto`. We thus always return truish\n-\t * here and let the backend decide for us.\n-\t */\n-\treturn 1;\n+\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n+\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n+\tstruct refs_optimize_opts optimize_opts = {\n+\t\t.exclusions = &excludes,\n+\t\t.includes = &included_refs,\n+\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n+\t};\n+\tbool required;\n+\n+\t/* Check for all refs, similar to 'git refs optimize --all'. */\n+\tstring_list_append(optimize_opts.includes, \"*\");\n+\n+\tif (refs_optimize_required(get_main_ref_store(the_repository),\n+\t\t\t\t &optimize_opts, &required))\n+\t\treturn 0;\n+\n+\tclear_ref_exclusions(&excludes);\n+\tstring_list_clear(&included_refs, 0);\n+\n+\treturn required == true;\n }\n \n static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,\n@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-/* Remember to update object flag allocation in object.h */\n-#define SEEN\t\t(1u<<0)\n-\n struct cg_auto_data {\n \tint num_not_in_graph;\n \tint limit;\ndiff --git a/object.h b/object.h\nindex 1499f63d50..832299e763 100644\n--- a/object.h\n+++ b/object.h\n@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);\n * list-objects-filter.c: 21\n * bloom.c: 2122\n * builtin/fsck.c: 0--3\n- * builtin/gc.c: 0\n * builtin/index-pack.c: 2021\n * reflog.c: 10--12\n * builtin/show-branch.c: 0-------------------------------------------26\n\n-- \n2.51.0\n\n"},{"id":"530300","messageId":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-5-d611a2a95cf5@gmail.com","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com","subject":"[PATCH v3 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T08:22:34Z","receivedAt":"2025-11-06T08:22:44Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command provides tooling to run maintenance\ntasks over Git repositories. The 'run' subcommand, as the name suggests,\nruns the maintenance tasks. When used with the '--auto' flag, it uses\nheuristics to determine if the required thresholds are met for running\nsaid maintenance tasks.\n\nThere is however a lack of insight into these heuristics. Meaning, the\nchecks are linked to the execution.\n\nAdd a new 'is-needed' subcommand to 'git-maintenance(1)' which allows\nusers to simply check if it is needed to run maintenance without\nperforming it.\n\nThis subcommand can check if it is needed to run maintenance without\nactually running it. Ideally it should be used with the '--auto' flag,\nwhich would allow users to check if the thresholds required are met. The\nsubcommand also supports the '--task' flag which can be used to check\nspecific maintenance tasks.\n\nWhile adding the respective tests in 't/t7900-maintenance.sh', remove a\nduplicate of the test: 'worktree-prune task with --auto honors\nmaintenance.worktree-prune.auto'.\n\nSigned-off-by: Karthik Nayak \n---\n Documentation/git-maintenance.adoc | 13 +++++++++\n builtin/gc.c | 55 +++++++++++++++++++++++++++++++++++++-\n t/t7900-maintenance.sh | 54 ++++++++++++++++++++++++++-----------\n 3 files changed, 105 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\nindex 540b5cf68b..37939510d4 100644\n--- a/Documentation/git-maintenance.adoc\n+++ b/Documentation/git-maintenance.adoc\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git maintenance' run []\n 'git maintenance' start [--scheduler=]\n 'git maintenance' (stop|register|unregister) []\n+'git maintenance' is-needed []\n \n \n DESCRIPTION\n@@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository\n is not already registered. Use the `--force` option to return success even\n when the current repository is not registered.\n \n+is-needed::\n+ Check whether maintenance needs to be run without actually running it.\n+ Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n+ Ideally used with the '--auto' flag.\n++\n+If one or more `--task` options\tare specified, then those tasks are checked\n+in that order. Otherwise, the tasks are determined by which\n+`maintenance..enabled` config options are true. By default, only\n+`maintenance.gc.enabled` is true.\n+\n TASKS\n -----\n \n@@ -183,6 +194,8 @@ OPTIONS\n \tin the `gc.auto` config setting, or when the number of pack-files\n \texceeds the `gc.autoPackLimit` config setting. Not compatible with\n \tthe `--schedule` option.\n+\tWhen combined with the `is-needed` subcommand, check if the required\n+\tthresholds are met without actually running maintenance.\n \n --schedule::\n \tWhen combined with the `run` subcommand, run maintenance tasks\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c3e7a84ec2..e5ba2a2e72 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -3253,7 +3253,59 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n \treturn update_background_schedule(NULL, 0);\n }\n \n-static const char * const builtin_maintenance_usage[] = {\n+static const char *const builtin_maintenance_is_needed_usage[] = {\n+\t\"git maintenance is-needed [--task=] [--schedule]\",\n+\tNULL\n+};\n+\n+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n+\t\t\t\t struct repository *repo UNUSED)\n+{\n+\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n+\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n+\tstruct gc_config cfg = GC_CONFIG_INIT;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n+\t\t\t N_(\"run tasks based on the state of the repository\")),\n+\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n+\t\t\t N_(\"check a specific task\"),\n+\t\t\t PARSE_OPT_NONEG, task_option_parse),\n+\t\tOPT_END()\n+\t};\n+\tbool is_needed = false;\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t builtin_maintenance_is_needed_usage,\n+\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n+\n+\tgc_config(&cfg);\n+\tinitialize_task_config(&opts, &selected_tasks);\n+\n+\tif (opts.auto_flag) {\n+\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n+\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n+\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n+\t\t\t\tis_needed = true;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t} else {\n+\t\t/* When not using --auto, we should always require maintenance. */\n+\t\tis_needed = true;\n+\t}\n+\n+\tstring_list_clear(&selected_tasks, 0);\n+\tmaintenance_run_opts_release(&opts);\n+\tgc_config_release(&cfg);\n+\n+\tif (is_needed)\n+\t\treturn 0;\n+\treturn 1;\n+}\n+\n+static const char *const builtin_maintenance_usage[] = {\n \tN_(\"git maintenance []\"),\n \tNULL,\n };\n@@ -3270,6 +3322,7 @@ int cmd_maintenance(int argc,\n \t\tOPT_SUBCOMMAND(\"stop\", &fn, maintenance_stop),\n \t\tOPT_SUBCOMMAND(\"register\", &fn, maintenance_register),\n \t\tOPT_SUBCOMMAND(\"unregister\", &fn, maintenance_unregister),\n+\t\tOPT_SUBCOMMAND(\"is-needed\", &fn, maintenance_is_needed),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex ddd273d8dc..a17e2091c2 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '\n \t\tgit maintenance run --auto 2>/dev/null &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/run-no-quiet.txt\" \\\n \t\tgit maintenance run --no-quiet 2>/dev/null &&\n+\tgit maintenance is-needed &&\n \ttest_subcommand git gc --quiet --no-detach --skip-foreground-tasks /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand git prune-packed --quiet /dev/null &&\n@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {\n \ttest_commit A &&\n \tgit repack -adk &&\n \tgit multi-pack-index write &&\n+\t! git -c maintenance.incremental-repack.auto=1 \\\n+\t\tmaintenance is-needed --auto --task=incremental-repack &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/midx-init.txt\" git \\\n \t\t-c maintenance.incremental-repack.auto=1 \\\n \t\tmaintenance run --auto --task=incremental-repack 2>/dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n \tgit reflog expire --all --expire=now &&\n \ttest_commit reflog-one &&\n \ttest_commit reflog-two &&\n+\n+\t! git -c maintenance.reflog-expire.auto=3 \\\n+\t\tmaintenance is-needed --auto --task=reflog-expire &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/reflog-expire-auto.txt\" \\\n \t\tgit -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&\n \ttest_subcommand ! git reflog expire --all .git/worktrees/abc &&\n+\tgit maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git maintenance run --auto --task=worktree-prune\n '\n \n@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n \t# A positive value should require at least this many prunable worktrees.\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n-'\n-\n-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n-\t# A negative value should always prune.\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n-\n-\tmkdir .git/worktrees &&\n-\t: >.git/worktrees/first &&\n-\t: >.git/worktrees/second &&\n-\t: >.git/worktrees/third &&\n-\n-\t# Zero should never prune.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n-\t# A positive value should require at least this many prunable worktrees.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n+\tgit -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n '\n \n@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '\n \trm -rf worktree &&\n \n \trm -f worktree-prune.txt &&\n+\t! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&\n \ttest_subcommand ! git worktree prune --expire 1.week.ago .git/rr-cache/entry &&\n+\tgit maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n '\n \n@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n \n \t# A negative value should always prune.\n+\tgit -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n \n \t# A positive value prunes when there is at least one entry.\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \tmkdir .git/rr-cache &&\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \t: >.git/rr-cache/entry-1 &&\n+\tgit -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \n \t# Zero should never prune.\n \t: >.git/rr-cache/entry-1 &&\n+\t! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc\n '\n \n\n-- \n2.51.0\n\n"},{"id":"530310","messageId":"aQyNSOdPWAxm15U3@pks.im","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-4-d611a2a95cf5@gmail.com","subject":"Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-06T11:58:00Z","receivedAt":"2025-11-06T11:58:08Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 06, 2025 at 09:22:33AM +0100, Karthik Nayak wrote:\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index c6d62c74a7..c3e7a84ec2 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n> \n> static int pack_refs_condition(UNUSED struct gc_config *cfg)\n> {\n> -\t/*\n> -\t * The auto-repacking logic for refs is handled by the ref backends and\n> -\t * exposed via `git pack-refs --auto`. We thus always return truish\n> -\t * here and let the backend decide for us.\n> -\t */\n> -\treturn 1;\n> +\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n> +\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n> +\tstruct refs_optimize_opts optimize_opts = {\n> +\t\t.exclusions = &excludes,\n> +\t\t.includes = &included_refs,\n> +\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n> +\t};\n> +\tbool required;\n> +\n> +\t/* Check for all refs, similar to 'git refs optimize --all'. */\n> +\tstring_list_append(optimize_opts.includes, \"*\");\n> +\n> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n> +\t\t\t\t &optimize_opts, &required))\n> +\t\treturn 0;\n> +\n> +\tclear_ref_exclusions(&excludes);\n> +\tstring_list_clear(&included_refs, 0);\n> +\n> +\treturn required == true;\n\nTiny nit: I think in our codebase this can be written in a more\nidiomatic way by saying `!!required`.\n\nOther than that I don't have anything more to add to this series.\nThanks!\n\nPatrick\n"},{"id":"530311","messageId":"aQyOZ0e6HO0_77Au@pks.im","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-5-d611a2a95cf5@gmail.com","subject":"Re: [PATCH v3 5/5] maintenance: add 'is-needed' subcommand","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-06T12:02:47Z","receivedAt":"2025-11-06T12:02:54Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 06, 2025 at 09:22:34AM +0100, Karthik Nayak wrote:\n> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\n> index 540b5cf68b..37939510d4 100644\n> --- a/Documentation/git-maintenance.adoc\n> +++ b/Documentation/git-maintenance.adoc\n> @@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository\n> is not already registered. Use the `--force` option to return success even\n> when the current repository is not registered.\n> \n> +is-needed::\n> + Check whether maintenance needs to be run without actually running it.\n> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n> + Ideally used with the '--auto' flag.\n> ++\n> +If one or more `--task` options\tare specified, then those tasks are checked\n\nI spoke too soon, forgot that there's one more patch :) s/\\t/ /\n\n> +in that order. Otherwise, the tasks are determined by which\n> +`maintenance..enabled` config options are true. By default, only\n> +`maintenance.gc.enabled` is true.\n\nThis could use a pointer to \"maintenance.strategy\", but I see that you\ntook this explanation from the \"run\" subcommand. I think this is good\nenough for now.\n\n> diff --git a/builtin/gc.c b/builtin/gc.c\n> index c3e7a84ec2..e5ba2a2e72 100644\n> --- a/builtin/gc.c\n> +++ b/builtin/gc.c\n> @@ -3253,7 +3253,59 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n> \treturn update_background_schedule(NULL, 0);\n> }\n> \n> -static const char * const builtin_maintenance_usage[] = {\n> +static const char *const builtin_maintenance_is_needed_usage[] = {\n> +\t\"git maintenance is-needed [--task=] [--schedule]\",\n> +\tNULL\n> +};\n> +\n> +static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n> +\t\t\t\t struct repository *repo UNUSED)\n> +{\n> +\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n> +\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n> +\tstruct gc_config cfg = GC_CONFIG_INIT;\n> +\tstruct option options[] = {\n> +\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n> +\t\t\t N_(\"run tasks based on the state of the repository\")),\n> +\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n> +\t\t\t N_(\"check a specific task\"),\n> +\t\t\t PARSE_OPT_NONEG, task_option_parse),\n> +\t\tOPT_END()\n> +\t};\n> +\tbool is_needed = false;\n> +\n> +\targc = parse_options(argc, argv, prefix, options,\n> +\t\t\t builtin_maintenance_is_needed_usage,\n> +\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n> +\tif (argc)\n> +\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n> +\n> +\tgc_config(&cfg);\n> +\tinitialize_task_config(&opts, &selected_tasks);\n> +\n> +\tif (opts.auto_flag) {\n> +\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n> +\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n> +\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n> +\t\t\t\tis_needed = true;\n> +\t\t\t\tbreak;\n> +\t\t\t}\n> +\t\t}\n> +\t} else {\n> +\t\t/* When not using --auto, we should always require maintenance. */\n\nNit: we might add a TODO comment here.\n\n /*\n * When not using --auto we always require maintenance right now.\n *\n * TODO: this certainly is too eager, as some maintenance tasks may\n * decide to not do anything because the data structures are already\n * fully optimized. We may eventually want to extend the auto\n * condition to also cover non-auto runs so that we can detect such\n * cases.\n /\n\nPatrick\n"},{"id":"530313","messageId":"CAOLa=ZQ18H8WCp_m=7rzWt1HRRiWv5Ag63ceV2r=_BwyttyK5w@mail.gmail.com","threadId":"64412","inReplyTo":"aQyNSOdPWAxm15U3@pks.im","subject":"Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T13:04:25Z","receivedAt":"2025-11-06T13:04:27Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Thu, Nov 06, 2025 at 09:22:33AM +0100, Karthik Nayak wrote:\n>> diff --git a/builtin/gc.c b/builtin/gc.c\n>> index c6d62c74a7..c3e7a84ec2 100644\n>> --- a/builtin/gc.c\n>> +++ b/builtin/gc.c\n>> @@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n>>\n>> static int pack_refs_condition(UNUSED struct gc_config *cfg)\n>> {\n>> -\t/*\n>> -\t * The auto-repacking logic for refs is handled by the ref backends and\n>> -\t * exposed via `git pack-refs --auto`. We thus always return truish\n>> -\t * here and let the backend decide for us.\n>> -\t */\n>> -\treturn 1;\n>> +\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n>> +\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n>> +\tstruct refs_optimize_opts optimize_opts = {\n>> +\t\t.exclusions = &excludes,\n>> +\t\t.includes = &included_refs,\n>> +\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n>> +\t};\n>> +\tbool required;\n>> +\n>> +\t/* Check for all refs, similar to 'git refs optimize --all'. */\n>> +\tstring_list_append(optimize_opts.includes, \"*\");\n>> +\n>> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n>> +\t\t\t\t &optimize_opts, &required))\n>> +\t\treturn 0;\n>> +\n>> +\tclear_ref_exclusions(&excludes);\n>> +\tstring_list_clear(&included_refs, 0);\n>> +\n>> +\treturn required == true;\n>\n> Tiny nit: I think in our codebase this can be written in a more\n> idiomatic way by saying `!!required`.\n>\n\nFair. Will change.\n\n> Other than that I don't have anything more to add to this series.\n> Thanks!\n>\n> Patrick\n\nThanks for your review!\n"},{"id":"530314","messageId":"CAOLa=ZS9J9SfMFp7+dmue=isJrpFSbTU7z8TCShOb36XdB8Y_Q@mail.gmail.com","threadId":"64412","inReplyTo":"aQyOZ0e6HO0_77Au@pks.im","subject":"Re: [PATCH v3 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-06T13:07:23Z","receivedAt":"2025-11-06T13:07:26Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Patrick Steinhardt writes:\n\n> On Thu, Nov 06, 2025 at 09:22:34AM +0100, Karthik Nayak wrote:\n>> diff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\n>> index 540b5cf68b..37939510d4 100644\n>> --- a/Documentation/git-maintenance.adoc\n>> +++ b/Documentation/git-maintenance.adoc\n>> @@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository\n>> is not already registered. Use the `--force` option to return success even\n>> when the current repository is not registered.\n>>\n>> +is-needed::\n>> + Check whether maintenance needs to be run without actually running it.\n>> + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n>> + Ideally used with the '--auto' flag.\n>> ++\n>> +If one or more `--task` options\tare specified, then those tasks are checked\n>\n> I spoke too soon, forgot that there's one more patch :) s/\\t/ /\n\nWeird, not sure how that happened, good catch.\n\n>\n>> +in that order. Otherwise, the tasks are determined by which\n>> +`maintenance..enabled` config options are true. By default, only\n>> +`maintenance.gc.enabled` is true.\n>\n> This could use a pointer to \"maintenance.strategy\", but I see that you\n> took this explanation from the \"run\" subcommand. I think this is good\n> enough for now.\n\nYeah, that's what I went with, so I'll leave it as is :)\n\n[snip]\n\n>> +\tif (opts.auto_flag) {\n>> +\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n>> +\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n>> +\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n>> +\t\t\t\tis_needed = true;\n>> +\t\t\t\tbreak;\n>> +\t\t\t}\n>> +\t\t}\n>> +\t} else {\n>> +\t\t/* When not using --auto, we should always require maintenance. */\n>\n> Nit: we might add a TODO comment here.\n>\n> /*\n> * When not using --auto we always require maintenance right now.\n> *\n> * TODO: this certainly is too eager, as some maintenance tasks may\n> * decide to not do anything because the data structures are already\n> * fully optimized. We may eventually want to extend the auto\n> * condition to also cover non-auto runs so that we can detect such\n> * cases.\n> /\n>\n> Patrick\n\nSure this makes sense, will add it in.\n\nThanks\nKarthik\n"},{"id":"530321","messageId":"xmqqpl9vjiaj.fsf@gitster.g","threadId":"64412","inReplyTo":"aQyNSOdPWAxm15U3@pks.im","subject":"Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-06T15:24:20Z","receivedAt":"2025-11-06T15:24:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt writes:\n\n>> +\t/* Check for all refs, similar to 'git refs optimize --all'. */\n>> +\tstring_list_append(optimize_opts.includes, \"*\");\n>> +\n>> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n>> +\t\t\t\t &optimize_opts, &required))\n>> +\t\treturn 0;\n>> +\n>> +\tclear_ref_exclusions(&excludes);\n>> +\tstring_list_clear(&included_refs, 0);\n>> +\n>> +\treturn required == true;\n>\n> Tiny nit: I think in our codebase this can be written in a more\n> idiomatic way by saying `!!required`.\n\nComparing for equality with Boolean in general is stupid, as\nBooleans are designed to be usable as-is. If it is \"true\", it is\ntrue, and you do not have to compare it with \"true\" to ascertain\nthat it is true.\n\nI do 100% prefer \"!!required\" over \"required == true\" or \"required\n!= false\" all the time, since it is more idiomatic, but I vaguely\nrecall we had something that contradicts it in the CodingGuidelines\ndocument. Perhaps we'd want to fix that.\n\nThanks.\n\n\n[Footnote]\n\nBut doesn't your suggested rewrite potentially change the meaning?\n\nThe original allows required to be \"true\" and nothing else, while\n\"!!required\" allows it to be any form of true (and in C, things that\nare not zero, even a pointer that is not NULL, are all true).\n"},{"id":"530332","messageId":"xmqq8qgjhvnm.fsf@gitster.g","threadId":"64412","inReplyTo":"20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-2-d611a2a95cf5@gmail.com","subject":"Re: [PATCH v3 2/5] reftable/stack: add function to check if optimization is required","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-06T18:18:37Z","receivedAt":"2025-11-06T18:18:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> The reftable backend performs auto-compaction as part of its regular\n> flow, which is required to keep the number of tables part of a stack at\n> bay. This allows it to stay optimized.\n>\n> Compaction can also be triggered voluntarily by the user via the 'git\n> pack-refs' or the 'git refs optimize' command. However, currently there\n> is no way for the user to check if optimization is required without\n> actually performing it.\n>\n> Extract out the heuristics logic from 'reftable_stack_auto_compact()'\n> into an internal function 'update_segment_if_compaction_required()'.\n> Then use this to add and expose `reftable_stack_compaction_required()`\n> which will allow users to check if the reftable backend can be\n> optimized.\n>\n> Signed-off-by: Karthik Nayak \n> ---\n> reftable/reftable-stack.h | 11 +++++++++++\n> reftable/stack.c | 42 ++++++++++++++++++++++++++++++++++++-----\n> t/unit-tests/u-reftable-stack.c | 12 ++++++++++--\n> 3 files changed, 58 insertions(+), 7 deletions(-)\n\nThe required change is surprisingly small, which is a good sign.\n\n> diff --git a/reftable/stack.c b/reftable/stack.c\n> index 49387f9344..826500abed 100644\n> --- a/reftable/stack.c\n> +++ b/reftable/stack.c\n> @@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n> \treturn 0;\n> }\n> \n> -int reftable_stack_auto_compact(struct reftable_stack *st)\n> +static int update_segment_if_compaction_required(struct reftable_stack *st,\n> +\t\t\t\t\t\t struct segment *seg,\n> +\t\t\t\t\t\t bool use_heuristics,\n> +\t\t\t\t\t\t bool *required)\n> {\n\nAm I correct to understand that \"use_heuristics\" is almost a synonym\nto \"maintain geometric progression\" in the context of this patch?\nAre we expecting other heuristics in the future, in which case, this\nmay not be a single \"bool\" but a set of flag bits, and until then\ns/heuristics/geometric/ might make it a better name for the\nparameter?\n\nThanks.\n\n\n"},{"id":"530355","messageId":"aQ2MYbQKHUVoqDG1@pks.im","threadId":"64412","inReplyTo":"xmqq8qgjhvnm.fsf@gitster.g","subject":"Re: [PATCH v3 2/5] reftable/stack: add function to check if optimization is required","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-07T06:06:25Z","receivedAt":"2025-11-07T06:06:32Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Thu, Nov 06, 2025 at 10:18:37AM -0800, Junio C Hamano wrote:\n> Karthik Nayak writes:\n> > diff --git a/reftable/stack.c b/reftable/stack.c\n> > index 49387f9344..826500abed 100644\n> > --- a/reftable/stack.c\n> > +++ b/reftable/stack.c\n> > @@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n> > \treturn 0;\n> > }\n> > \n> > -int reftable_stack_auto_compact(struct reftable_stack *st)\n> > +static int update_segment_if_compaction_required(struct reftable_stack *st,\n> > +\t\t\t\t\t\t struct segment *seg,\n> > +\t\t\t\t\t\t bool use_heuristics,\n> > +\t\t\t\t\t\t bool *required)\n> > {\n> \n> Am I correct to understand that \"use_heuristics\" is almost a synonym\n> to \"maintain geometric progression\" in the context of this patch?\n> Are we expecting other heuristics in the future, in which case, this\n> may not be a single \"bool\" but a set of flag bits, and until then\n> s/heuristics/geometric/ might make it a better name for the\n> parameter?\n\nI don't expect that this will change anytime soon. So renaming it\naccordingly feels like the right direction to me indeed.\n\nPatrick\n"},{"id":"530381","messageId":"CAOLa=ZT6CnTRz5bX+Vv7pb_3oqV0XNSMEzh=57sF6O5bFYxWhQ@mail.gmail.com","threadId":"64412","inReplyTo":"xmqqpl9vjiaj.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-07T15:58:21Z","receivedAt":"2025-11-07T15:58:23Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Patrick Steinhardt writes:\n>\n>>> +\t/* Check for all refs, similar to 'git refs optimize --all'. */\n>>> +\tstring_list_append(optimize_opts.includes, \"*\");\n>>> +\n>>> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n>>> +\t\t\t\t &optimize_opts, &required))\n>>> +\t\treturn 0;\n>>> +\n>>> +\tclear_ref_exclusions(&excludes);\n>>> +\tstring_list_clear(&included_refs, 0);\n>>> +\n>>> +\treturn required == true;\n>>\n>> Tiny nit: I think in our codebase this can be written in a more\n>> idiomatic way by saying `!!required`.\n>\n> Comparing for equality with Boolean in general is stupid, as\n> Booleans are designed to be usable as-is. If it is \"true\", it is\n> true, and you do not have to compare it with \"true\" to ascertain\n> that it is true.\n>\n> I do 100% prefer \"!!required\" over \"required == true\" or \"required\n> != false\" all the time, since it is more idiomatic, but I vaguely\n> recall we had something that contradicts it in the CodingGuidelines\n> document. Perhaps we'd want to fix that.\n>\n\nI could only find\n\n - Some clever tricks, like using the !! operator with arithmetic\n constructs, can be extremely confusing to others. Avoid them,\n unless there is a compelling reason to use them.\n\nI think its okay? This is more of a suggestion than a rule.\n\n> Thanks.\n>\n>\n> [Footnote]\n>\n> But doesn't your suggested rewrite potentially change the meaning?\n>\n> The original allows required to be \"true\" and nothing else, while\n> \"!!required\" allows it to be any form of true (and in C, things that\n> are not zero, even a pointer that is not NULL, are all true).\n\nI get what you mean, but with the context that required is of type\n'bool', this would mean that we simply convert it to '0'/'1' here.\n\nWith all this, perhaps `return required` as used in the v1 was the best\napproach. I'm happy to go either ways.\n"},{"id":"530382","messageId":"CAOLa=ZT9_d5yKPP9g_pWZL23RkbMLN+SXgPCuSmto0mF6XCy4g@mail.gmail.com","threadId":"64412","inReplyTo":"xmqqpl9vjiaj.fsf@gitster.g","subject":"Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-07T15:58:51Z","receivedAt":"2025-11-07T15:58:54Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Junio C Hamano writes:\n\n> Patrick Steinhardt writes:\n>\n>>> +\t/* Check for all refs, similar to 'git refs optimize --all'. */\n>>> +\tstring_list_append(optimize_opts.includes, \"*\");\n>>> +\n>>> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n>>> +\t\t\t\t &optimize_opts, &required))\n>>> +\t\treturn 0;\n>>> +\n>>> +\tclear_ref_exclusions(&excludes);\n>>> +\tstring_list_clear(&included_refs, 0);\n>>> +\n>>> +\treturn required == true;\n>>\n>> Tiny nit: I think in our codebase this can be written in a more\n>> idiomatic way by saying `!!required`.\n>\n> Comparing for equality with Boolean in general is stupid, as\n> Booleans are designed to be usable as-is. If it is \"true\", it is\n> true, and you do not have to compare it with \"true\" to ascertain\n> that it is true.\n>\n> I do 100% prefer \"!!required\" over \"required == true\" or \"required\n> != false\" all the time, since it is more idiomatic, but I vaguely\n> recall we had something that contradicts it in the CodingGuidelines\n> document. Perhaps we'd want to fix that.\n>\n\nI could only find\n\n - Some clever tricks, like using the !! operator with arithmetic\n constructs, can be extremely confusing to others. Avoid them,\n unless there is a compelling reason to use them.\n\nI think its okay? This is more of a suggestion than a rule.\n\n> Thanks.\n>\n>\n> [Footnote]\n>\n> But doesn't your suggested rewrite potentially change the meaning?\n>\n> The original allows required to be \"true\" and nothing else, while\n> \"!!required\" allows it to be any form of true (and in C, things that\n> are not zero, even a pointer that is not NULL, are all true).\n\nI get what you mean, but with the context that required is of type\n'bool', this would mean that we simply convert it to '0'/'1' here.\n\nWith all this, perhaps `return required` as used in the v1 was the best\napproach. I'm happy to go either ways.\n"},{"id":"530385","messageId":"xmqqo6pdg5hs.fsf@gitster.g","threadId":"64412","inReplyTo":"CAOLa=ZT6CnTRz5bX+Vv7pb_3oqV0XNSMEzh=57sF6O5bFYxWhQ@mail.gmail.com","subject":"Re: [PATCH v3 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-11-07T16:41:19Z","receivedAt":"2025-11-07T16:41:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Karthik Nayak writes:\n\n> Junio C Hamano writes:\n>\n>> Patrick Steinhardt writes:\n>>\n>>>> +\t/* Check for all refs, similar to 'git refs optimize --all'. */\n>>>> +\tstring_list_append(optimize_opts.includes, \"*\");\n>>>> +\n>>>> +\tif (refs_optimize_required(get_main_ref_store(the_repository),\n>>>> +\t\t\t\t &optimize_opts, &required))\n>>>> +\t\treturn 0;\n>>>> +\n>>>> +\tclear_ref_exclusions(&excludes);\n>>>> +\tstring_list_clear(&included_refs, 0);\n>>>> +\n>>>> +\treturn required == true;\n>>>\n>>> Tiny nit: I think in our codebase this can be written in a more\n>>> idiomatic way by saying `!!required`.\n>>\n>> Comparing for equality with Boolean in general is stupid, as\n>> Booleans are designed to be usable as-is. If it is \"true\", it is\n>> true, and you do not have to compare it with \"true\" to ascertain\n>> that it is true.\n>>\n>> I do 100% prefer \"!!required\" over \"required == true\" or \"required\n>> != false\" all the time, since it is more idiomatic, but I vaguely\n>> recall we had something that contradicts it in the CodingGuidelines\n>> document. Perhaps we'd want to fix that.\n>>\n>\n> I could only find\n>\n> - Some clever tricks, like using the !! operator with arithmetic\n> constructs, can be extremely confusing to others. Avoid them,\n> unless there is a compelling reason to use them.\n>\n> I think its okay? This is more of a suggestion than a rule.\n\n\"Unless there is a reason to use\" sounds like an outright\nprohibition to me, though.\n\nBy the way, in the on-topic part of the discussion, \"required\" is a\nbool, the helper function that takes &required takes a pointer to a\nbool, and the function in question returns a bool. So I should\nupdate my preference above. \"return required\" is the most natural\nway to write, and it uses \"bool\" as it was designed to be used.\nWhen the reader knows that required is a bool already, \"return\n!!required\" is just as pointless as \"return required == true\".\n\nIf required and the helper that takes a pointer to it were \"int\",\nand this function returns a bool, then my original preference would\napply; even if an \"int required\" has 3 in it, we probably can still\nsay \"return required\" and the function would coerce that 3 into\n\"true\", but manually coercing it to 0/1 with !!required is more\nexplicit and less confusing.\n\nThanks.\n\n"},{"id":"530412","messageId":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","threadId":"64412","inReplyTo":"20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com","subject":"[PATCH v4 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-08T21:51:52Z","receivedAt":"2025-11-08T21:52:05Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"Hello,\n\nI recently raised a patch series [1] to add 'git refs optimize --required'\nwhich checks if the reference backend can be optimized, without actually\nperforming the optimization.\n\nBack then, we had decided [2] that it would be a better to broaden the\napproach and add a 'is-needed' subcommand to 'git-maintenance(1)'. This\nwould allow users to check if maintenance was required for the\nrepository and users could also provide a task via the '--task' to check\nif maintenance was needed for a particular task.\n\nIdeally the subcommand will be used with the '--auto' flag which can\ncheck the same heuristics as that used with 'git maintenance run\n--auto'. Future patches can also add support for the '--schedule' flag\nwhich can be used to check required schedule it met. However that flag\nisn't added as part of this series.\n\nThis series implements that.\n\nCommits 1-3 add the required functionality in the refs subsystem to\nexpose an 'optimize_required' field which can be used to check if\nbackends need to be optimized.\nCommit 4 utilizes this within the 'git-maintenance(1)' code.\nCommit 5 adds the 'is-needed' subcommand to 'git-maintenance(1)'.\n\nThis is based on top of master a99f379adf (The 27th batch, 2025-10-30)\nand is dependent on the following series:\n\n - kn/refs-optim-cleanup\n - ps/ref-peeled-tags\n\nMerges cleanly with `next`. I think those two topics are close to being\nmerged to `next` so hopefully this dependency tree doesn't get too\ncomplicated. I'll rebase as needed to resolve conflicts.\n\n[1]: https://lore.kernel.org/git/20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail.com/\n[2]: https://lore.kernel.org/git/CAOLa=ZRdxm787nE4FSr2VUHDB+hW06Ggc6yUcKmeTKAb6B7YOA@mail.gmail.com/\n\n---\nChanges in v4:\n- In `update_segment_if_compaction_required()` change the argument name\n from `use_heuristics` to `use_geometric` since we only have one\n heuristic currently and this is much clearer to understand.\n- There were a lot of discussion on how to return a bool variable when\n the function has a return type of int. We discussed both '!!required',\n and 'required != true'. I'm going to punt this discussion keeping it\n simple as 'return required' as in my first version, since even Junio\n expressed his thoughts in favor of it.\n- Add a TODO for improvements to the flow when running `git maintenance\n is-needed` without the `--auto` flag.\n- Link to v3: https://patch.msgid.link/20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com\n\nChanges in v3:\n- In patch 2/5 extract out code for deciding if compaction is required\n into a static function. This removes duplication of logic for deciding\n if compaction is needed.\n- Link to v2: https://patch.msgid.link/20251104-562-add-sub-command-to-check-if-maintenance-is-needed-v2-0-303462a9e4ed@gmail.com\n\nChanges in v2:\n- Added more documentation for `reftable_stack_compaction_required()`.\n- Fixed some typos and grammar mistakes in commit messages.\n- Clarify which tasks will be run when '--task' is not used.\n- Move the call to 'usage_with_options()' to be with 'parse_options()'.\n- Link to v1: https://patch.msgid.link/20251031-562-add-sub-command-to-check-if-maintenance-is-needed-v1-0-a03d53e28d0e@gmail.com\n\n---\n Documentation/git-maintenance.adoc | 13 ++++++\n builtin/gc.c | 93 ++++++++++++++++++++++++++++++++++----\n object.h | 1 -\n refs.c | 7 +++\n refs.h | 7 +++\n refs/debug.c | 13 ++++++\n refs/files-backend.c | 11 +++++\n refs/packed-backend.c | 13 ++++++\n refs/refs-internal.h | 6 +++\n refs/reftable-backend.c | 25 ++++++++++\n reftable/reftable-stack.h | 11 +++++\n reftable/stack.c | 61 +++++++++++++++++++------\n t/t7900-maintenance.sh | 54 +++++++++++++++-------\n t/unit-tests/u-reftable-stack.c | 12 ++++-\n 14 files changed, 284 insertions(+), 43 deletions(-)\n\nKarthik Nayak (5):\n reftable/stack: return stack segments directly\n reftable/stack: add function to check if optimization is required\n refs: add a `optimize_required` field to `struct ref_storage_be`\n maintenance: add checking logic in `pack_refs_condition()`\n maintenance: add 'is-needed' subcommand\n\nRange-diff versus v3:\n\n1: 48c877bbc4 = 1: 4026aad0e2 reftable/stack: return stack segments directly\n2: b2c0da304b ! 2: ed2b307572 reftable/stack: add function to check if optimization is required\n @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack\n -int reftable_stack_auto_compact(struct reftable_stack *st)\n +static int update_segment_if_compaction_required(struct reftable_stack *st,\n +\t\t\t\t\t\t struct segment *seg,\n -+\t\t\t\t\t\t bool use_heuristics,\n ++\t\t\t\t\t\t bool use_geometric,\n +\t\t\t\t\t\t bool *required)\n {\n -\tstruct segment seg;\n @@ reftable/stack.c: static int stack_segments_for_compaction(struct reftable_stack\n +\t\treturn 0;\n +\t}\n +\n -+\tif (!use_heuristics) {\n ++\tif (!use_geometric) {\n +\t\t*required = true;\n \t\treturn 0;\n +\t}\n3: aec4861598 = 3: 4ac6fe6346 refs: add a `optimize_required` field to `struct ref_storage_be`\n4: 36d9bfbfe0 ! 4: e239f9d1d1 maintenance: add checking logic in `pack_refs_condition()`\n @@ builtin/gc.c: static void maintenance_run_opts_release(struct maintenance_run_op\n +\tclear_ref_exclusions(&excludes);\n +\tstring_list_clear(&included_refs, 0);\n +\n -+\treturn required == true;\n ++\treturn required;\n }\n \n static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,\n5: 2fc7dcb38c ! 5: 2457e5ce0e maintenance: add 'is-needed' subcommand\n @@ Documentation/git-maintenance.adoc: The `unregister` subcommand will report an e\n + Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n + Ideally used with the '--auto' flag.\n ++\n -+If one or more `--task` options\tare specified, then those tasks are checked\n ++If one or more `--task` options are specified, then those tasks are checked\n +in that order. Otherwise, the tasks are determined by which\n +`maintenance..enabled` config options are true. By default, only\n +`maintenance.gc.enabled` is true.\n @@ builtin/gc.c: static int maintenance_stop(int argc, const char **argv, const cha\n +\t\t\t}\n +\t\t}\n +\t} else {\n -+\t\t/* When not using --auto, we should always require maintenance. */\n ++\t\t/*\n ++\t\t * When not using --auto we always require maintenance right now.\n ++\t\t *\n ++\t\t * TODO: this certainly is too eager, as some maintenance tasks may\n ++\t\t * decide to not do anything because the data structures are already\n ++\t\t * fully optimized. We may eventually want to extend the auto\n ++\t\t * condition to also cover non-auto runs so that we can detect such\n ++\t\t * cases.\n ++\t\t */\n +\t\tis_needed = true;\n +\t}\n +\n\n\nbase-commit: edd2018f5db39d68d55a7a4af42375b1a06b9406\nchange-id: 20251021-562-add-sub-command-to-check-if-maintenance-is-needed-01cae01b4606\n\nThanks\n- Karthik\n\n"},{"id":"530413","messageId":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-1-a90f229b6023@gmail.com","threadId":"64412","inReplyTo":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","subject":"[PATCH v4 1/5] reftable/stack: return stack segments directly","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-08T21:51:53Z","receivedAt":"2025-11-08T21:52:07Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The `stack_table_sizes_for_compaction()` function returns individual\nsizes of each reftable table. This function is only called by\n`reftable_stack_auto_compact()` to decide which tables need to be\ncompacted, if any.\n\nModify the function to directly return the segments, which avoids the\nextra step of receiving the sizes only to pass it to\n`suggest_compaction_segment()`.\n\nA future commit will also add functionality for checking whether\nauto-compaction is necessary without performing it. This change allows\ncode re-usability in that context.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/stack.c | 23 ++++++++++++-----------\n 1 file changed, 12 insertions(+), 11 deletions(-)\n\ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 65d89820bd..49387f9344 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1626,7 +1626,8 @@ struct segment suggest_compaction_segment(uint64_t *sizes, size_t n,\n \treturn seg;\n }\n \n-static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n+static int stack_segments_for_compaction(struct reftable_stack *st,\n+\t\t\t\t\t struct segment *seg)\n {\n \tint version = (st->opts.hash_id == REFTABLE_HASH_SHA1) ? 1 : 2;\n \tint overhead = header_size(version) - 1;\n@@ -1634,29 +1635,29 @@ static uint64_t *stack_table_sizes_for_compaction(struct reftable_stack *st)\n \n \tREFTABLE_CALLOC_ARRAY(sizes, st->merged->tables_len);\n \tif (!sizes)\n-\t\treturn NULL;\n+\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n \n \tfor (size_t i = 0; i < st->merged->tables_len; i++)\n \t\tsizes[i] = st->tables[i]->size - overhead;\n \n-\treturn sizes;\n+\t*seg = suggest_compaction_segment(sizes, st->merged->tables_len,\n+\t\t\t\t\t st->opts.auto_compaction_factor);\n+\treftable_free(sizes);\n+\n+\treturn 0;\n }\n \n int reftable_stack_auto_compact(struct reftable_stack *st)\n {\n \tstruct segment seg;\n-\tuint64_t *sizes;\n+\tint err;\n \n \tif (st->merged->tables_len < 2)\n \t\treturn 0;\n \n-\tsizes = stack_table_sizes_for_compaction(st);\n-\tif (!sizes)\n-\t\treturn REFTABLE_OUT_OF_MEMORY_ERROR;\n-\n-\tseg = suggest_compaction_segment(sizes, st->merged->tables_len,\n-\t\t\t\t\t st->opts.auto_compaction_factor);\n-\treftable_free(sizes);\n+\terr = stack_segments_for_compaction(st, &seg);\n+\tif (err)\n+\t\treturn err;\n \n \tif (segment_size(&seg) > 0)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n\n-- \n2.51.0\n\n"},{"id":"530414","messageId":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-2-a90f229b6023@gmail.com","threadId":"64412","inReplyTo":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","subject":"[PATCH v4 2/5] reftable/stack: add function to check if optimization is required","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-08T21:51:54Z","receivedAt":"2025-11-08T21:52:10Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The reftable backend performs auto-compaction as part of its regular\nflow, which is required to keep the number of tables part of a stack at\nbay. This allows it to stay optimized.\n\nCompaction can also be triggered voluntarily by the user via the 'git\npack-refs' or the 'git refs optimize' command. However, currently there\nis no way for the user to check if optimization is required without\nactually performing it.\n\nExtract out the heuristics logic from 'reftable_stack_auto_compact()'\ninto an internal function 'update_segment_if_compaction_required()'.\nThen use this to add and expose `reftable_stack_compaction_required()`\nwhich will allow users to check if the reftable backend can be\noptimized.\n\nSigned-off-by: Karthik Nayak \n---\n reftable/reftable-stack.h | 11 +++++++++++\n reftable/stack.c | 42 ++++++++++++++++++++++++++++++++++++-----\n t/unit-tests/u-reftable-stack.c | 12 ++++++++++--\n 3 files changed, 58 insertions(+), 7 deletions(-)\n\ndiff --git a/reftable/reftable-stack.h b/reftable/reftable-stack.h\nindex d70fcb705d..c2415cbc6e 100644\n--- a/reftable/reftable-stack.h\n+++ b/reftable/reftable-stack.h\n@@ -123,6 +123,17 @@ struct reftable_log_expiry_config {\n int reftable_stack_compact_all(struct reftable_stack *st,\n \t\t\t struct reftable_log_expiry_config *config);\n \n+/*\n+ * Check if compaction is required.\n+ *\n+ * When `use_heuristics` is false, check if all tables can be compacted to a\n+ * single table. If true, use heuristics to determine if the tables need to be\n+ * compacted to maintain geometric progression.\n+ */\n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required);\n+\n /* heuristically compact unbalanced table stack. */\n int reftable_stack_auto_compact(struct reftable_stack *st);\n \ndiff --git a/reftable/stack.c b/reftable/stack.c\nindex 49387f9344..1c9f21dfe1 100644\n--- a/reftable/stack.c\n+++ b/reftable/stack.c\n@@ -1647,19 +1647,51 @@ static int stack_segments_for_compaction(struct reftable_stack *st,\n \treturn 0;\n }\n \n-int reftable_stack_auto_compact(struct reftable_stack *st)\n+static int update_segment_if_compaction_required(struct reftable_stack *st,\n+\t\t\t\t\t\t struct segment *seg,\n+\t\t\t\t\t\t bool use_geometric,\n+\t\t\t\t\t\t bool *required)\n {\n-\tstruct segment seg;\n \tint err;\n \n-\tif (st->merged->tables_len < 2)\n+\tif (st->merged->tables_len < 2) {\n+\t\t*required = false;\n+\t\treturn 0;\n+\t}\n+\n+\tif (!use_geometric) {\n+\t\t*required = true;\n \t\treturn 0;\n+\t}\n+\n+\terr = stack_segments_for_compaction(st, seg);\n+\tif (err)\n+\t\treturn err;\n+\n+\t*required = segment_size(seg) > 0;\n+\treturn 0;\n+}\n+\n+int reftable_stack_compaction_required(struct reftable_stack *st,\n+\t\t\t\t bool use_heuristics,\n+\t\t\t\t bool *required)\n+{\n+\tstruct segment seg;\n+\treturn update_segment_if_compaction_required(st, &seg, use_heuristics,\n+\t\t\t\t\t\t required);\n+}\n+\n+int reftable_stack_auto_compact(struct reftable_stack *st)\n+{\n+\tstruct segment seg;\n+\tbool required;\n+\tint err;\n \n-\terr = stack_segments_for_compaction(st, &seg);\n+\terr = update_segment_if_compaction_required(st, &seg, true, &required);\n \tif (err)\n \t\treturn err;\n \n-\tif (segment_size(&seg) > 0)\n+\tif (required)\n \t\treturn stack_compact_range(st, seg.start, seg.end - 1,\n \t\t\t\t\t NULL, STACK_COMPACT_RANGE_BEST_EFFORT);\n \ndiff --git a/t/unit-tests/u-reftable-stack.c b/t/unit-tests/u-reftable-stack.c\nindex a8b91812e8..b8110cdeee 100644\n--- a/t/unit-tests/u-reftable-stack.c\n+++ b/t/unit-tests/u-reftable-stack.c\n@@ -1067,6 +1067,7 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t\t.value_type = REFTABLE_REF_SYMREF,\n \t\t\t.value.symref = (char *) \"master\",\n \t\t};\n+\t\tbool required = false;\n \t\tchar buf[128];\n \n \t\t/*\n@@ -1087,10 +1088,17 @@ void test_reftable_stack__add_performs_auto_compaction(void)\n \t\t * auto compaction is disabled. When enabled, we should merge\n \t\t * all tables in the stack.\n \t\t */\n-\t\tif (i != n)\n+\t\tcl_assert_equal_i(reftable_stack_compaction_required(st, true, &required), 0);\n+\t\tif (i != n) {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, i + 1);\n-\t\telse\n+\t\t\tif (i < 1)\n+\t\t\t\tcl_assert_equal_b(required, false);\n+\t\t\telse\n+\t\t\t\tcl_assert_equal_b(required, true);\n+\t\t} else {\n \t\t\tcl_assert_equal_i(st->merged->tables_len, 1);\n+\t\t\tcl_assert_equal_b(required, false);\n+\t\t}\n \t}\n \n \treftable_stack_destroy(st);\n\n-- \n2.51.0\n\n"},{"id":"530415","messageId":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-3-a90f229b6023@gmail.com","threadId":"64412","inReplyTo":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","subject":"[PATCH v4 3/5] refs: add a `optimize_required` field to `struct ref_storage_be`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-08T21:51:55Z","receivedAt":"2025-11-08T21:52:11Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"To allow users of the refs namespace to check if the reference backend\nrequires optimization, add a new field `optimize_required` field to\n`struct ref_storage_be`. This field is of type `optimize_required_fn`\nwhich is also introduced in this commit.\n\nModify the debug, files, packed and reftable backend to implement this\nfield. A following commit will expose this via 'git pack-refs' and 'git\nrefs optimize'.\n\nSigned-off-by: Karthik Nayak \n---\n refs.c | 7 +++++++\n refs.h | 7 +++++++\n refs/debug.c | 13 +++++++++++++\n refs/files-backend.c | 11 +++++++++++\n refs/packed-backend.c | 13 +++++++++++++\n refs/refs-internal.h | 6 ++++++\n refs/reftable-backend.c | 25 +++++++++++++++++++++++++\n 7 files changed, 82 insertions(+)\n\ndiff --git a/refs.c b/refs.c\nindex 0d0831f29b..5583f6e09d 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -2318,6 +2318,13 @@ int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)\n \treturn refs->be->optimize(refs, opts);\n }\n \n+int refs_optimize_required(struct ref_store *refs,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required)\n+{\n+\treturn refs->be->optimize_required(refs, opts, required);\n+}\n+\n int reference_get_peeled_oid(struct repository *repo,\n \t\t\t const struct reference *ref,\n \t\t\t struct object_id *peeled_oid)\ndiff --git a/refs.h b/refs.h\nindex 6b05bba527..d9051bbb04 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -520,6 +520,13 @@ struct refs_optimize_opts {\n */\n int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);\n \n+/*\n+ * Check if refs backend can be optimized by calling 'refs_optimize'.\n+ */\n+int refs_optimize_required(struct ref_store *ref_store,\n+\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t bool *required);\n+\n /*\n * Setup reflog before using. Fill in err and return -1 on failure.\n */\ndiff --git a/refs/debug.c b/refs/debug.c\nindex 2defd2d465..36f8c58b6c 100644\n--- a/refs/debug.c\n+++ b/refs/debug.c\n@@ -124,6 +124,17 @@ static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts\n \treturn res;\n }\n \n+static int debug_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;\n+\tint res = drefs->refs->be->optimize_required(drefs->refs, opts, required);\n+\ttrace_printf_key(&trace_refs, \"optimize_required: %s, res: %d\\n\",\n+\t\t\t required ? \"yes\" : \"no\", res);\n+\treturn res;\n+}\n+\n static int debug_rename_ref(struct ref_store *ref_store, const char *oldref,\n \t\t\t const char *newref, const char *logmsg)\n {\n@@ -431,6 +442,8 @@ struct ref_storage_be refs_be_debug = {\n \t.transaction_abort = debug_transaction_abort,\n \n \t.optimize = debug_optimize,\n+\t.optimize_required = debug_optimize_required,\n+\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 a1e70b1c10..6e0c9b340a 100644\n--- a/refs/files-backend.c\n+++ b/refs/files-backend.c\n@@ -1512,6 +1512,16 @@ static int files_optimize(struct ref_store *ref_store,\n \treturn 0;\n }\n \n+static int files_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required)\n+{\n+\tstruct files_ref_store *refs = files_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t \"optimize_required\");\n+\t*required = should_pack_refs(refs, opts);\n+\treturn 0;\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@@ -3982,6 +3992,7 @@ struct ref_storage_be refs_be_files = {\n \t.transaction_abort = files_transaction_abort,\n \n \t.optimize = files_optimize,\n+\t.optimize_required = files_optimize_required,\n \t.rename_ref = files_rename_ref,\n \t.copy_ref = files_copy_ref,\n \ndiff --git a/refs/packed-backend.c b/refs/packed-backend.c\nindex 10062fd8b6..19ce4d5872 100644\n--- a/refs/packed-backend.c\n+++ b/refs/packed-backend.c\n@@ -1784,6 +1784,17 @@ static int packed_optimize(struct ref_store *ref_store UNUSED,\n \treturn 0;\n }\n \n+static int packed_optimize_required(struct ref_store *ref_store UNUSED,\n+\t\t\t\t struct refs_optimize_opts *opts UNUSED,\n+\t\t\t\t bool *required)\n+{\n+\t/*\n+\t * Packed refs are already optimized.\n+\t */\n+\t*required = false;\n+\treturn 0;\n+}\n+\n static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_store UNUSED)\n {\n \treturn empty_ref_iterator_begin();\n@@ -2130,6 +2141,8 @@ struct ref_storage_be refs_be_packed = {\n \t.transaction_abort = packed_transaction_abort,\n \n \t.optimize = packed_optimize,\n+\t.optimize_required = packed_optimize_required,\n+\n \t.rename_ref = NULL,\n \t.copy_ref = NULL,\n \ndiff --git a/refs/refs-internal.h b/refs/refs-internal.h\nindex dee42f231d..c7d2a6e50b 100644\n--- a/refs/refs-internal.h\n+++ b/refs/refs-internal.h\n@@ -424,6 +424,11 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,\n \n typedef int optimize_fn(struct ref_store *ref_store,\n \t\t\tstruct refs_optimize_opts *opts);\n+\n+typedef int optimize_required_fn(struct ref_store *ref_store,\n+\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t bool *required);\n+\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);\n@@ -549,6 +554,7 @@ struct ref_storage_be {\n \tref_transaction_abort_fn *transaction_abort;\n \n \toptimize_fn *optimize;\n+\toptimize_required_fn *optimize_required;\n \trename_ref_fn *rename_ref;\n \tcopy_ref_fn *copy_ref;\n \ndiff --git a/refs/reftable-backend.c b/refs/reftable-backend.c\nindex c23c45f3bf..a3ae0cf74a 100644\n--- a/refs/reftable-backend.c\n+++ b/refs/reftable-backend.c\n@@ -1733,6 +1733,29 @@ static int reftable_be_optimize(struct ref_store *ref_store,\n \treturn ret;\n }\n \n+static int reftable_be_optimize_required(struct ref_store *ref_store,\n+\t\t\t\t\t struct refs_optimize_opts *opts,\n+\t\t\t\t\t bool *required)\n+{\n+\tstruct reftable_ref_store *refs = reftable_be_downcast(ref_store, REF_STORE_READ,\n+\t\t\t\t\t\t\t \"optimize_refs_required\");\n+\tstruct reftable_stack *stack;\n+\tbool use_heuristics = false;\n+\n+\tif (refs->err)\n+\t\treturn refs->err;\n+\n+\tstack = refs->worktree_backend.stack;\n+\tif (!stack)\n+\t\tstack = refs->main_backend.stack;\n+\n+\tif (opts->flags & REFS_OPTIMIZE_AUTO)\n+\t\tuse_heuristics = true;\n+\n+\treturn reftable_stack_compaction_required(stack, use_heuristics,\n+\t\t\t\t\t\t required);\n+}\n+\n struct write_create_symref_arg {\n \tstruct reftable_ref_store *refs;\n \tstruct reftable_stack *stack;\n@@ -2756,6 +2779,8 @@ struct ref_storage_be refs_be_reftable = {\n \t.transaction_abort = reftable_be_transaction_abort,\n \n \t.optimize = reftable_be_optimize,\n+\t.optimize_required = reftable_be_optimize_required,\n+\n \t.rename_ref = reftable_be_rename_ref,\n \t.copy_ref = reftable_be_copy_ref,\n \n\n-- \n2.51.0\n\n"},{"id":"530416","messageId":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-4-a90f229b6023@gmail.com","threadId":"64412","inReplyTo":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","subject":"[PATCH v4 4/5] maintenance: add checking logic in `pack_refs_condition()`","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-08T21:51:56Z","receivedAt":"2025-11-08T21:52:13Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command supports an '--auto' flag. Usage of the\nflag ensures to run maintenance tasks only if certain thresholds are\nmet. The heuristic is defined on a task level, wherein each task defines\nan 'auto_condition', which states if the task should be run.\n\nThe 'pack-refs' task is hard-coded to return 1 as:\n1. There was never a way to check if the reference backend needs to be\noptimized without actually performing the optimization.\n2. We can pass in the '--auto' flag to 'git-pack-refs(1)' which would\noptimize based on heuristics.\n\nThe previous commit added a `refs_optimize_required()` function, which\ncan be used to check if a reference backend required optimization. Use\nthis within `pack_refs_condition()`.\n\nThis allows us to add a 'git maintenance is-needed' subcommand which can\nnotify the user if maintenance is needed without actually performing the\noptimization. Without this change, the reference backend would always\nstate that optimization is needed.\n\nSince we import 'revision.h', we need to remove the definition for\n'SEEN' which is duplicated in the included header.\n\nSigned-off-by: Karthik Nayak \n---\n builtin/gc.c | 30 +++++++++++++++++++++---------\n object.h | 1 -\n 2 files changed, 21 insertions(+), 10 deletions(-)\n\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex c6d62c74a7..85e9a38d10 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -35,6 +35,7 @@\n #include \"path.h\"\n #include \"reflog.h\"\n #include \"rerere.h\"\n+#include \"revision.h\"\n #include \"blob.h\"\n #include \"tree.h\"\n #include \"promisor-remote.h\"\n@@ -285,12 +286,26 @@ static void maintenance_run_opts_release(struct maintenance_run_opts *opts)\n \n static int pack_refs_condition(UNUSED struct gc_config *cfg)\n {\n-\t/*\n-\t * The auto-repacking logic for refs is handled by the ref backends and\n-\t * exposed via `git pack-refs --auto`. We thus always return truish\n-\t * here and let the backend decide for us.\n-\t */\n-\treturn 1;\n+\tstruct string_list included_refs = STRING_LIST_INIT_NODUP;\n+\tstruct ref_exclusions excludes = REF_EXCLUSIONS_INIT;\n+\tstruct refs_optimize_opts optimize_opts = {\n+\t\t.exclusions = &excludes,\n+\t\t.includes = &included_refs,\n+\t\t.flags = REFS_OPTIMIZE_PRUNE | REFS_OPTIMIZE_AUTO,\n+\t};\n+\tbool required;\n+\n+\t/* Check for all refs, similar to 'git refs optimize --all'. */\n+\tstring_list_append(optimize_opts.includes, \"*\");\n+\n+\tif (refs_optimize_required(get_main_ref_store(the_repository),\n+\t\t\t\t &optimize_opts, &required))\n+\t\treturn 0;\n+\n+\tclear_ref_exclusions(&excludes);\n+\tstring_list_clear(&included_refs, 0);\n+\n+\treturn required;\n }\n \n static int maintenance_task_pack_refs(struct maintenance_run_opts *opts,\n@@ -1090,9 +1105,6 @@ static int maintenance_opt_schedule(const struct option *opt, const char *arg,\n \treturn 0;\n }\n \n-/* Remember to update object flag allocation in object.h */\n-#define SEEN\t\t(1u<<0)\n-\n struct cg_auto_data {\n \tint num_not_in_graph;\n \tint limit;\ndiff --git a/object.h b/object.h\nindex 1499f63d50..832299e763 100644\n--- a/object.h\n+++ b/object.h\n@@ -79,7 +79,6 @@ void object_array_init(struct object_array *array);\n * list-objects-filter.c: 21\n * bloom.c: 2122\n * builtin/fsck.c: 0--3\n- * builtin/gc.c: 0\n * builtin/index-pack.c: 2021\n * reflog.c: 10--12\n * builtin/show-branch.c: 0-------------------------------------------26\n\n-- \n2.51.0\n\n"},{"id":"530417","messageId":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-5-a90f229b6023@gmail.com","threadId":"64412","inReplyTo":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","subject":"[PATCH v4 5/5] maintenance: add 'is-needed' subcommand","fromName":"Karthik Nayak","fromEmail":"karthik.188@gmail.com","sentAt":"2025-11-08T21:51:57Z","receivedAt":"2025-11-08T21:52:15Z","isPatch":true,"sender":{"key":"karthik.188@gmail.com","avatar":"https://avatars.githubusercontent.com/u/1786334?v=4"},"body":"The 'git-maintenance(1)' command provides tooling to run maintenance\ntasks over Git repositories. The 'run' subcommand, as the name suggests,\nruns the maintenance tasks. When used with the '--auto' flag, it uses\nheuristics to determine if the required thresholds are met for running\nsaid maintenance tasks.\n\nThere is however a lack of insight into these heuristics. Meaning, the\nchecks are linked to the execution.\n\nAdd a new 'is-needed' subcommand to 'git-maintenance(1)' which allows\nusers to simply check if it is needed to run maintenance without\nperforming it.\n\nThis subcommand can check if it is needed to run maintenance without\nactually running it. Ideally it should be used with the '--auto' flag,\nwhich would allow users to check if the thresholds required are met. The\nsubcommand also supports the '--task' flag which can be used to check\nspecific maintenance tasks.\n\nWhile adding the respective tests in 't/t7900-maintenance.sh', remove a\nduplicate of the test: 'worktree-prune task with --auto honors\nmaintenance.worktree-prune.auto'.\n\nSigned-off-by: Karthik Nayak \n---\n Documentation/git-maintenance.adoc | 13 ++++++++\n builtin/gc.c | 63 +++++++++++++++++++++++++++++++++++++-\n t/t7900-maintenance.sh | 54 ++++++++++++++++++++++----------\n 3 files changed, 113 insertions(+), 17 deletions(-)\n\ndiff --git a/Documentation/git-maintenance.adoc b/Documentation/git-maintenance.adoc\nindex 540b5cf68b..bda616f14c 100644\n--- a/Documentation/git-maintenance.adoc\n+++ b/Documentation/git-maintenance.adoc\n@@ -12,6 +12,7 @@ SYNOPSIS\n 'git maintenance' run []\n 'git maintenance' start [--scheduler=]\n 'git maintenance' (stop|register|unregister) []\n+'git maintenance' is-needed []\n \n \n DESCRIPTION\n@@ -84,6 +85,16 @@ The `unregister` subcommand will report an error if the current repository\n is not already registered. Use the `--force` option to return success even\n when the current repository is not registered.\n \n+is-needed::\n+ Check whether maintenance needs to be run without actually running it.\n+ Exits with a 0 status code if maintenance needs to be run, 1 otherwise.\n+ Ideally used with the '--auto' flag.\n++\n+If one or more `--task` options are specified, then those tasks are checked\n+in that order. Otherwise, the tasks are determined by which\n+`maintenance..enabled` config options are true. By default, only\n+`maintenance.gc.enabled` is true.\n+\n TASKS\n -----\n \n@@ -183,6 +194,8 @@ OPTIONS\n \tin the `gc.auto` config setting, or when the number of pack-files\n \texceeds the `gc.autoPackLimit` config setting. Not compatible with\n \tthe `--schedule` option.\n+\tWhen combined with the `is-needed` subcommand, check if the required\n+\tthresholds are met without actually running maintenance.\n \n --schedule::\n \tWhen combined with the `run` subcommand, run maintenance tasks\ndiff --git a/builtin/gc.c b/builtin/gc.c\nindex 85e9a38d10..928c805f02 100644\n--- a/builtin/gc.c\n+++ b/builtin/gc.c\n@@ -3253,7 +3253,67 @@ static int maintenance_stop(int argc, const char **argv, const char *prefix,\n \treturn update_background_schedule(NULL, 0);\n }\n \n-static const char * const builtin_maintenance_usage[] = {\n+static const char *const builtin_maintenance_is_needed_usage[] = {\n+\t\"git maintenance is-needed [--task=] [--schedule]\",\n+\tNULL\n+};\n+\n+static int maintenance_is_needed(int argc, const char **argv, const char *prefix,\n+\t\t\t\t struct repository *repo UNUSED)\n+{\n+\tstruct maintenance_run_opts opts = MAINTENANCE_RUN_OPTS_INIT;\n+\tstruct string_list selected_tasks = STRING_LIST_INIT_DUP;\n+\tstruct gc_config cfg = GC_CONFIG_INIT;\n+\tstruct option options[] = {\n+\t\tOPT_BOOL(0, \"auto\", &opts.auto_flag,\n+\t\t\t N_(\"run tasks based on the state of the repository\")),\n+\t\tOPT_CALLBACK_F(0, \"task\", &selected_tasks, N_(\"task\"),\n+\t\t\t N_(\"check a specific task\"),\n+\t\t\t PARSE_OPT_NONEG, task_option_parse),\n+\t\tOPT_END()\n+\t};\n+\tbool is_needed = false;\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\t builtin_maintenance_is_needed_usage,\n+\t\t\t PARSE_OPT_STOP_AT_NON_OPTION);\n+\tif (argc)\n+\t\tusage_with_options(builtin_maintenance_is_needed_usage, options);\n+\n+\tgc_config(&cfg);\n+\tinitialize_task_config(&opts, &selected_tasks);\n+\n+\tif (opts.auto_flag) {\n+\t\tfor (size_t i = 0; i < opts.tasks_nr; i++) {\n+\t\t\tif (tasks[opts.tasks[i]].auto_condition &&\n+\t\t\t tasks[opts.tasks[i]].auto_condition(&cfg)) {\n+\t\t\t\tis_needed = true;\n+\t\t\t\tbreak;\n+\t\t\t}\n+\t\t}\n+\t} else {\n+\t\t/*\n+\t\t * When not using --auto we always require maintenance right now.\n+\t\t *\n+\t\t * TODO: this certainly is too eager, as some maintenance tasks may\n+\t\t * decide to not do anything because the data structures are already\n+\t\t * fully optimized. We may eventually want to extend the auto\n+\t\t * condition to also cover non-auto runs so that we can detect such\n+\t\t * cases.\n+\t\t */\n+\t\tis_needed = true;\n+\t}\n+\n+\tstring_list_clear(&selected_tasks, 0);\n+\tmaintenance_run_opts_release(&opts);\n+\tgc_config_release(&cfg);\n+\n+\tif (is_needed)\n+\t\treturn 0;\n+\treturn 1;\n+}\n+\n+static const char *const builtin_maintenance_usage[] = {\n \tN_(\"git maintenance []\"),\n \tNULL,\n };\n@@ -3270,6 +3330,7 @@ int cmd_maintenance(int argc,\n \t\tOPT_SUBCOMMAND(\"stop\", &fn, maintenance_stop),\n \t\tOPT_SUBCOMMAND(\"register\", &fn, maintenance_register),\n \t\tOPT_SUBCOMMAND(\"unregister\", &fn, maintenance_unregister),\n+\t\tOPT_SUBCOMMAND(\"is-needed\", &fn, maintenance_is_needed),\n \t\tOPT_END(),\n \t};\n \ndiff --git a/t/t7900-maintenance.sh b/t/t7900-maintenance.sh\nindex ddd273d8dc..a17e2091c2 100755\n--- a/t/t7900-maintenance.sh\n+++ b/t/t7900-maintenance.sh\n@@ -49,7 +49,9 @@ test_expect_success 'run [--auto|--quiet]' '\n \t\tgit maintenance run --auto 2>/dev/null &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/run-no-quiet.txt\" \\\n \t\tgit maintenance run --no-quiet 2>/dev/null &&\n+\tgit maintenance is-needed &&\n \ttest_subcommand git gc --quiet --no-detach --skip-foreground-tasks /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand ! git prune-packed --quiet /dev/null &&\n \ttest_subcommand git prune-packed --quiet /dev/null &&\n@@ -421,10 +435,13 @@ run_incremental_repack_and_verify () {\n \ttest_commit A &&\n \tgit repack -adk &&\n \tgit multi-pack-index write &&\n+\t! git -c maintenance.incremental-repack.auto=1 \\\n+\t\tmaintenance is-needed --auto --task=incremental-repack &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/midx-init.txt\" git \\\n \t\t-c maintenance.incremental-repack.auto=1 \\\n \t\tmaintenance run --auto --task=incremental-repack 2>/dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n \ttest_subcommand ! git multi-pack-index write --no-progress /dev/null &&\n@@ -485,9 +505,15 @@ test_expect_success 'reflog-expire task --auto only packs when exceeding limits'\n \tgit reflog expire --all --expire=now &&\n \ttest_commit reflog-one &&\n \ttest_commit reflog-two &&\n+\n+\t! git -c maintenance.reflog-expire.auto=3 \\\n+\t\tmaintenance is-needed --auto --task=reflog-expire &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/reflog-expire-auto.txt\" \\\n \t\tgit -c maintenance.reflog-expire.auto=3 maintenance run --auto --task=reflog-expire &&\n \ttest_subcommand ! git reflog expire --all .git/worktrees/abc &&\n+\tgit maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git maintenance run --auto --task=worktree-prune\n '\n \n@@ -530,22 +557,7 @@ test_expect_success 'worktree-prune task with --auto honors maintenance.worktree\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n \t# A positive value should require at least this many prunable worktrees.\n \ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n-'\n-\n-test_expect_success 'worktree-prune task with --auto honors maintenance.worktree-prune.auto' '\n-\t# A negative value should always prune.\n-\ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=-1 maintenance run --auto --task=worktree-prune &&\n-\n-\tmkdir .git/worktrees &&\n-\t: >.git/worktrees/first &&\n-\t: >.git/worktrees/second &&\n-\t: >.git/worktrees/third &&\n-\n-\t# Zero should never prune.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=0 maintenance run --auto --task=worktree-prune &&\n-\t# A positive value should require at least this many prunable worktrees.\n-\ttest_expect_worktree_prune ! git -c maintenance.worktree-prune.auto=4 maintenance run --auto --task=worktree-prune &&\n+\tgit -c maintenance.worktree-prune.auto=3 maintenance is-needed --auto --task=worktree-prune &&\n \ttest_expect_worktree_prune git -c maintenance.worktree-prune.auto=3 maintenance run --auto --task=worktree-prune\n '\n \n@@ -554,11 +566,13 @@ test_expect_success 'worktree-prune task honors gc.worktreePruneExpire' '\n \trm -rf worktree &&\n \n \trm -f worktree-prune.txt &&\n+\t! git -c gc.worktreePruneExpire=1.week.ago maintenance is-needed --auto --task=worktree-prune &&\n \tGIT_TRACE2_EVENT=\"$(pwd)/worktree-prune.txt\" git -c gc.worktreePruneExpire=1.week.ago maintenance run --auto --task=worktree-prune &&\n \ttest_subcommand ! git worktree prune --expire 1.week.ago .git/rr-cache/entry &&\n+\tgit maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git maintenance run --auto --task=rerere-gc\n '\n \n@@ -594,17 +611,22 @@ test_expect_success 'rerere-gc task with --auto honors maintenance.rerere-gc.aut\n \ttest_when_finished \"rm -rf .git/rr-cache\" &&\n \n \t# A negative value should always prune.\n+\tgit -c maintenance.rerere-gc.auto=-1 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=-1 maintenance run --auto --task=rerere-gc &&\n \n \t# A positive value prunes when there is at least one entry.\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \tmkdir .git/rr-cache &&\n+\t! git -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \t: >.git/rr-cache/entry-1 &&\n+\tgit -c maintenance.rerere-gc.auto=9000 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc git -c maintenance.rerere-gc.auto=9000 maintenance run --auto --task=rerere-gc &&\n \n \t# Zero should never prune.\n \t: >.git/rr-cache/entry-1 &&\n+\t! git -c maintenance.rerere-gc.auto=0 maintenance is-needed --auto --task=rerere-gc &&\n \ttest_expect_rerere_gc ! git -c maintenance.rerere-gc.auto=0 maintenance run --auto --task=rerere-gc\n '\n \n\n-- \n2.51.0\n\n"},{"id":"530431","messageId":"aRGKMH4Wc7PJ6Z5z@pks.im","threadId":"64412","inReplyTo":"20251108-562-add-sub-command-to-check-if-maintenance-is-needed-v4-0-a90f229b6023@gmail.com","subject":"Re: [PATCH v4 0/5] maintenance: add an 'is-needed' subcommand","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-11-10T06:46:17Z","receivedAt":"2025-11-10T06:46:29Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Sat, Nov 08, 2025 at 10:51:52PM +0100, Karthik Nayak wrote:\n> Changes in v4:\n> - In `update_segment_if_compaction_required()` change the argument name\n> from `use_heuristics` to `use_geometric` since we only have one\n> heuristic currently and this is much clearer to understand.\n> - There were a lot of discussion on how to return a bool variable when\n> the function has a return type of int. We discussed both '!!required',\n> and 'required != true'. I'm going to punt this discussion keeping it\n> simple as 'return required' as in my first version, since even Junio\n> expressed his thoughts in favor of it.\n> - Add a TODO for improvements to the flow when running `git maintenance\n> is-needed` without the `--auto` flag.\n> - Link to v3: https://patch.msgid.link/20251106-562-add-sub-command-to-check-if-maintenance-is-needed-v3-0-d611a2a95cf5@gmail.com\n\nI'm happy with this version. Thanks!\n\nPatrick\n"}]}