* [PATCH v2 0/3] refs: cleanup code around optimizations
@ 2025-10-20 8:18 Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 1/3] refs: move to using the '.optimize' functions Karthik Nayak
` (3 more replies)
0 siblings, 4 replies; 6+ messages in thread
From: Karthik Nayak @ 2025-10-20 8:18 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, jltobler
This is extracted from a recent series I sent [1], which I've since
dropped to follow up with a different approach. I think these patches
hold value individually.
They mostly cleanup code around 'git refs optimize' which was added
recently in db0babf9b2 (Merge branch 'ms/refs-optimize', 2025-10-02).
The code in the refs subsystem contains both 'pack-refs' and 'optimize'
functions, which are one and the same.
This series unifies this to only retain the 'optimize' functions and
naming, since it backend generic.
This is based on top of master 143f58ef75 (Sync with Git 2.51.1,
2025-10-15) with 'ps/ref-peeled-tags' merged in.
[1]: 20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail•com
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
---
Changes in v2:
- Squash the second commit into the first.
- Change some variable names to also no longer refer to pack_refs.
- Fix commit messages.
- Link to v1: https://lore.kernel.org/r/20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com
---
pack-refs.c | 20 ++++++++++----------
refs.c | 8 +-------
refs.h | 24 +++++++++---------------
refs/debug.c | 8 ++++----
refs/files-backend.c | 22 ++++++----------------
refs/packed-backend.c | 6 +++---
refs/refs-internal.h | 5 +----
refs/reftable-backend.c | 15 ++++-----------
t/pack-refs-tests.sh | 2 --
t/t0601-reffiles-pack-refs.sh | 2 ++
t/t1463-refs-optimize.sh | 2 ++
11 files changed, 42 insertions(+), 72 deletions(-)
Karthik Nayak (3):
refs: move to using the '.optimize' functions
refs: rename 'pack_refs_opts' to 'refs_optimize_opts'
t/pack-refs-tests: move the 'test_done' to callees
Range-diff versus v1:
1: ded0c04d60 ! 1: fa153f2552 refs: move to using the '.optimize' functions
@@ Metadata
## Commit message ##
refs: move to using the '.optimize' functions
- The `struct ref_store` variable, exposes two ways to optimize a reftable
+ The `struct ref_store` variable exposes two ways to optimize a reftable
backend:
1. pack_refs
@@ Commit message
latter is more generic and covers all backends. While the naming is
different, both of these functions perform the same functionality.
- In the following commit, we will consolidate this code to only maintain
- the 'optimize' functions. In preparation, modify the backends so that
- they exclusively implement the `optimize` callback, only. All users of
- the refs subsystem already use the 'optimize' function so there is no
- changes needed on the callee side.
+ Consolidate this code to only maintain the 'optimize' functions. Do this
+ by modifying the backends so that they exclusively implement the
+ `optimize` callback, only. All users of the refs subsystem already use
+ the 'optimize' function so there is no changes needed on the callee
+ side. Finally, cleanup all references to the 'pack_refs' field of the
+ structure and code around it.
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
+ ## refs.c ##
+@@ refs.c: void base_ref_store_init(struct ref_store *refs, struct repository *repo,
+ refs->gitdir = xstrdup(path);
+ }
+
+-/* backend functions */
+-int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
+-{
+- return refs->be->pack_refs(refs, opts);
+-}
+-
+ int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)
+ {
+ return refs->be->optimize(refs, opts);
+
+ ## refs.h ##
+@@ refs.h: struct pack_refs_opts {
+ struct string_list *includes;
+ };
+
+-/*
+- * Write a packed-refs file for the current repository.
+- * flags: Combination of the above PACK_REFS_* flags.
+- */
+-int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
+-
+ /*
+ * Optimize the ref store. The exact behavior is up to the backend.
+ * For the files backend, this is equivalent to packing refs.
+
## refs/debug.c ##
@@ refs/debug.c: static int debug_transaction_abort(struct ref_store *refs,
return res;
@@ refs/packed-backend.c: struct ref_storage_be refs_be_packed = {
.copy_ref = NULL,
+ ## refs/refs-internal.h ##
+@@ refs/refs-internal.h: typedef int ref_transaction_commit_fn(struct ref_store *refs,
+ struct ref_transaction *transaction,
+ struct strbuf *err);
+
+-typedef int pack_refs_fn(struct ref_store *ref_store,
+- struct pack_refs_opts *opts);
+ typedef int optimize_fn(struct ref_store *ref_store,
+ struct pack_refs_opts *opts);
+ typedef int rename_ref_fn(struct ref_store *ref_store,
+@@ refs/refs-internal.h: struct ref_storage_be {
+ ref_transaction_finish_fn *transaction_finish;
+ ref_transaction_abort_fn *transaction_abort;
+
+- pack_refs_fn *pack_refs;
+ optimize_fn *optimize;
+ rename_ref_fn *rename_ref;
+ copy_ref_fn *copy_ref;
+
## refs/reftable-backend.c ##
@@ refs/reftable-backend.c: static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,
return ret;
2: 7670a662af < -: ---------- refs: cleanup code around optimization
3: ad2c55175e ! 2: 16e8ec5501 refs: rename 'pack_refs_opts' to 'refs_optimize_opts'
@@ pack-refs.c: int pack_refs_core(int argc,
struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
struct string_list included_refs = STRING_LIST_INIT_NODUP;
- struct pack_refs_opts pack_refs_opts = {
-+ struct refs_optimize_opts pack_refs_opts = {
++ struct refs_optimize_opts optimize_opts = {
.exclusions = &excludes,
.includes = &included_refs,
- .flags = PACK_REFS_PRUNE,
@@ pack-refs.c: int pack_refs_core(int argc,
OPT_BOOL(0, "all", &pack_all, N_("pack everything")),
- OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
- OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), PACK_REFS_AUTO),
-+ OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), REFS_OPTIMIZE_PRUNE),
-+ OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), REFS_OPTIMIZE_AUTO),
- OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
+- OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
++ OPT_BIT(0, "prune", &optimize_opts.flags, N_("prune loose refs (default)"), REFS_OPTIMIZE_PRUNE),
++ OPT_BIT(0, "auto", &optimize_opts.flags, N_("auto-pack refs as needed"), REFS_OPTIMIZE_AUTO),
++ OPT_STRING_LIST(0, "include", optimize_opts.includes, N_("pattern"),
N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
+ N_("references to exclude")),
+@@ pack-refs.c: int pack_refs_core(int argc,
+ usage_with_options(usage_opts, opts);
+
+ for_each_string_list_item(item, &option_excluded_refs)
+- add_ref_exclusion(pack_refs_opts.exclusions, item->string);
++ add_ref_exclusion(optimize_opts.exclusions, item->string);
+
+ if (pack_all)
+- string_list_append(pack_refs_opts.includes, "*");
++ string_list_append(optimize_opts.includes, "*");
+
+- if (!pack_refs_opts.includes->nr)
+- string_list_append(pack_refs_opts.includes, "refs/tags/*");
++ if (!optimize_opts.includes->nr)
++ string_list_append(optimize_opts.includes, "refs/tags/*");
+
+- ret = refs_optimize(get_main_ref_store(repo), &pack_refs_opts);
++ ret = refs_optimize(get_main_ref_store(repo), &optimize_opts);
+
+ clear_ref_exclusions(&excludes);
+ string_list_clear(&included_refs, 0);
## refs.c ##
@@ refs.c: void base_ref_store_init(struct ref_store *refs, struct repository *repo,
@@ refs.c: void base_ref_store_init(struct ref_store *refs, struct repository *repo
## refs.h ##
@@ refs.h: void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
+ const struct string_list *refnames);
/*
- * Flags for controlling behaviour of refs_optimize()
+- * Flags for controlling behaviour of pack_refs()
- * PACK_REFS_PRUNE: Prune loose refs after packing
- * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
- * result are decided by the ref backend. Backends may ignore
- * this flag and fall back to a normal repack.
++ * Flags for controlling behaviour of refs_optimize()
+ * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing
+ * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end
+ * result are decided by the ref backend. Backends may ignore
@@ refs/packed-backend.c: static int packed_transaction_finish(struct ref_store *re
static int packed_optimize(struct ref_store *ref_store UNUSED,
- struct pack_refs_opts *pack_opts UNUSED)
-+ struct refs_optimize_opts *pack_opts UNUSED)
++ struct refs_optimize_opts *opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
4: c065b61ffb = 3: 6cfaeb7207 t/pack-refs-tests: move the 'test_done' to callees
base-commit: 854a80fd48848f942c4a566c9880dc5f089887af
change-id: 20251015-refs-code-cleanup-871a7ce7c8dd
Thanks
- Karthik
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2 1/3] refs: move to using the '.optimize' functions
2025-10-20 8:18 [PATCH v2 0/3] refs: cleanup code around optimizations Karthik Nayak
@ 2025-10-20 8:18 ` Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 2/3] refs: rename 'pack_refs_opts' to 'refs_optimize_opts' Karthik Nayak
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Karthik Nayak @ 2025-10-20 8:18 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, jltobler
The `struct ref_store` variable exposes two ways to optimize a reftable
backend:
1. pack_refs
2. optimize
The former was specific to the 'files' + 'packed' refs backend. The
latter is more generic and covers all backends. While the naming is
different, both of these functions perform the same functionality.
Consolidate this code to only maintain the 'optimize' functions. Do this
by modifying the backends so that they exclusively implement the
`optimize` callback, only. All users of the refs subsystem already use
the 'optimize' function so there is no changes needed on the callee
side. Finally, cleanup all references to the 'pack_refs' field of the
structure and code around it.
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
---
refs.c | 6 ------
refs.h | 6 ------
refs/debug.c | 8 ++++----
refs/files-backend.c | 14 ++------------
refs/packed-backend.c | 6 +++---
refs/refs-internal.h | 3 ---
refs/reftable-backend.c | 13 +++----------
7 files changed, 12 insertions(+), 44 deletions(-)
diff --git a/refs.c b/refs.c
index a41a94ae55..b9a4a60646 100644
--- a/refs.c
+++ b/refs.c
@@ -2313,12 +2313,6 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
refs->gitdir = xstrdup(path);
}
-/* backend functions */
-int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts)
-{
- return refs->be->pack_refs(refs, opts);
-}
-
int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)
{
return refs->be->optimize(refs, opts);
diff --git a/refs.h b/refs.h
index 23437d1220..6edb633eb6 100644
--- a/refs.h
+++ b/refs.h
@@ -514,12 +514,6 @@ struct pack_refs_opts {
struct string_list *includes;
};
-/*
- * Write a packed-refs file for the current repository.
- * flags: Combination of the above PACK_REFS_* flags.
- */
-int refs_pack_refs(struct ref_store *refs, struct pack_refs_opts *opts);
-
/*
* Optimize the ref store. The exact behavior is up to the backend.
* For the files backend, this is equivalent to packing refs.
diff --git a/refs/debug.c b/refs/debug.c
index 01499b9033..40cd1d9c15 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -116,11 +116,11 @@ static int debug_transaction_abort(struct ref_store *refs,
return res;
}
-static int debug_pack_refs(struct ref_store *ref_store, struct pack_refs_opts *opts)
+static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
- int res = drefs->refs->be->pack_refs(drefs->refs, opts);
- trace_printf_key(&trace_refs, "pack_refs: %d\n", res);
+ int res = drefs->refs->be->optimize(drefs->refs, opts);
+ trace_printf_key(&trace_refs, "optimize: %d\n", res);
return res;
}
@@ -430,7 +430,7 @@ struct ref_storage_be refs_be_debug = {
.transaction_finish = debug_transaction_finish,
.transaction_abort = debug_transaction_abort,
- .pack_refs = debug_pack_refs,
+ .optimize = debug_optimize,
.rename_ref = debug_rename_ref,
.copy_ref = debug_copy_ref,
diff --git a/refs/files-backend.c b/refs/files-backend.c
index ed8a1729d6..92d90fc508 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1444,8 +1444,8 @@ static int should_pack_refs(struct files_ref_store *refs,
return 0;
}
-static int files_pack_refs(struct ref_store *ref_store,
- struct pack_refs_opts *opts)
+static int files_optimize(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ -1512,15 +1512,6 @@ static int files_pack_refs(struct ref_store *ref_store,
return 0;
}
-static int files_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)
-{
- /*
- * For the "files" backend, "optimizing" is the same as "packing".
- * So, we just call the existing worker function for packing.
- */
- return files_pack_refs(ref_store, opts);
-}
-
/*
* People using contrib's git-new-workdir have .git/logs/refs ->
* /some/other/path/.git/logs/refs, and that may live on another device.
@@ -3969,7 +3960,6 @@ struct ref_storage_be refs_be_files = {
.transaction_finish = files_transaction_finish,
.transaction_abort = files_transaction_abort,
- .pack_refs = files_pack_refs,
.optimize = files_optimize,
.rename_ref = files_rename_ref,
.copy_ref = files_copy_ref,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 1ab0c50393..20cf9fab18 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1773,8 +1773,8 @@ static int packed_transaction_finish(struct ref_store *ref_store,
return ret;
}
-static int packed_pack_refs(struct ref_store *ref_store UNUSED,
- struct pack_refs_opts *pack_opts UNUSED)
+static int packed_optimize(struct ref_store *ref_store UNUSED,
+ struct pack_refs_opts *pack_opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
@@ -2129,7 +2129,7 @@ struct ref_storage_be refs_be_packed = {
.transaction_finish = packed_transaction_finish,
.transaction_abort = packed_transaction_abort,
- .pack_refs = packed_pack_refs,
+ .optimize = packed_optimize,
.rename_ref = NULL,
.copy_ref = NULL,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 4671517dad..fc5149df5b 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -422,8 +422,6 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
struct ref_transaction *transaction,
struct strbuf *err);
-typedef int pack_refs_fn(struct ref_store *ref_store,
- struct pack_refs_opts *opts);
typedef int optimize_fn(struct ref_store *ref_store,
struct pack_refs_opts *opts);
typedef int rename_ref_fn(struct ref_store *ref_store,
@@ -550,7 +548,6 @@ struct ref_storage_be {
ref_transaction_finish_fn *transaction_finish;
ref_transaction_abort_fn *transaction_abort;
- pack_refs_fn *pack_refs;
optimize_fn *optimize;
rename_ref_fn *rename_ref;
copy_ref_fn *copy_ref;
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 6bbfd5618d..43cc66a48e 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1700,11 +1700,11 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,
return ret;
}
-static int reftable_be_pack_refs(struct ref_store *ref_store,
- struct pack_refs_opts *opts)
+static int reftable_be_optimize(struct ref_store *ref_store,
+ struct pack_refs_opts *opts)
{
struct reftable_ref_store *refs =
- reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "pack_refs");
+ reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "optimize_refs");
struct reftable_stack *stack;
int ret;
@@ -1733,12 +1733,6 @@ static int reftable_be_pack_refs(struct ref_store *ref_store,
return ret;
}
-static int reftable_be_optimize(struct ref_store *ref_store,
- struct pack_refs_opts *opts)
-{
- return reftable_be_pack_refs(ref_store, opts);
-}
-
struct write_create_symref_arg {
struct reftable_ref_store *refs;
struct reftable_stack *stack;
@@ -2761,7 +2755,6 @@ struct ref_storage_be refs_be_reftable = {
.transaction_finish = reftable_be_transaction_finish,
.transaction_abort = reftable_be_transaction_abort,
- .pack_refs = reftable_be_pack_refs,
.optimize = reftable_be_optimize,
.rename_ref = reftable_be_rename_ref,
.copy_ref = reftable_be_copy_ref,
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 2/3] refs: rename 'pack_refs_opts' to 'refs_optimize_opts'
2025-10-20 8:18 [PATCH v2 0/3] refs: cleanup code around optimizations Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 1/3] refs: move to using the '.optimize' functions Karthik Nayak
@ 2025-10-20 8:18 ` Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 3/3] t/pack-refs-tests: move the 'test_done' to callees Karthik Nayak
2025-10-21 5:16 ` [PATCH v2 0/3] refs: cleanup code around optimizations Patrick Steinhardt
3 siblings, 0 replies; 6+ messages in thread
From: Karthik Nayak @ 2025-10-20 8:18 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, jltobler
The previous commit removed all references to 'pack_refs()' within
the refs subsystem. Continue this cleanup by also renaming
'pack_refs_opts' to 'refs_optimize_opts' and the respective flags
accordingly. Keeping the naming consistent will make the code easier to
maintain.
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
---
pack-refs.c | 20 ++++++++++----------
refs.c | 2 +-
refs.h | 18 +++++++++---------
refs/debug.c | 2 +-
refs/files-backend.c | 10 +++++-----
refs/packed-backend.c | 2 +-
refs/refs-internal.h | 2 +-
refs/reftable-backend.c | 4 ++--
8 files changed, 30 insertions(+), 30 deletions(-)
diff --git a/pack-refs.c b/pack-refs.c
index 1a5e07d8b8..eb6b2ba2c2 100644
--- a/pack-refs.c
+++ b/pack-refs.c
@@ -14,10 +14,10 @@ int pack_refs_core(int argc,
{
struct ref_exclusions excludes = REF_EXCLUSIONS_INIT;
struct string_list included_refs = STRING_LIST_INIT_NODUP;
- struct pack_refs_opts pack_refs_opts = {
+ struct refs_optimize_opts optimize_opts = {
.exclusions = &excludes,
.includes = &included_refs,
- .flags = PACK_REFS_PRUNE,
+ .flags = REFS_OPTIMIZE_PRUNE,
};
struct string_list option_excluded_refs = STRING_LIST_INIT_NODUP;
struct string_list_item *item;
@@ -26,9 +26,9 @@ int pack_refs_core(int argc,
struct option opts[] = {
OPT_BOOL(0, "all", &pack_all, N_("pack everything")),
- OPT_BIT(0, "prune", &pack_refs_opts.flags, N_("prune loose refs (default)"), PACK_REFS_PRUNE),
- OPT_BIT(0, "auto", &pack_refs_opts.flags, N_("auto-pack refs as needed"), PACK_REFS_AUTO),
- OPT_STRING_LIST(0, "include", pack_refs_opts.includes, N_("pattern"),
+ OPT_BIT(0, "prune", &optimize_opts.flags, N_("prune loose refs (default)"), REFS_OPTIMIZE_PRUNE),
+ OPT_BIT(0, "auto", &optimize_opts.flags, N_("auto-pack refs as needed"), REFS_OPTIMIZE_AUTO),
+ OPT_STRING_LIST(0, "include", optimize_opts.includes, N_("pattern"),
N_("references to include")),
OPT_STRING_LIST(0, "exclude", &option_excluded_refs, N_("pattern"),
N_("references to exclude")),
@@ -39,15 +39,15 @@ int pack_refs_core(int argc,
usage_with_options(usage_opts, opts);
for_each_string_list_item(item, &option_excluded_refs)
- add_ref_exclusion(pack_refs_opts.exclusions, item->string);
+ add_ref_exclusion(optimize_opts.exclusions, item->string);
if (pack_all)
- string_list_append(pack_refs_opts.includes, "*");
+ string_list_append(optimize_opts.includes, "*");
- if (!pack_refs_opts.includes->nr)
- string_list_append(pack_refs_opts.includes, "refs/tags/*");
+ if (!optimize_opts.includes->nr)
+ string_list_append(optimize_opts.includes, "refs/tags/*");
- ret = refs_optimize(get_main_ref_store(repo), &pack_refs_opts);
+ ret = refs_optimize(get_main_ref_store(repo), &optimize_opts);
clear_ref_exclusions(&excludes);
string_list_clear(&included_refs, 0);
diff --git a/refs.c b/refs.c
index b9a4a60646..0d0831f29b 100644
--- a/refs.c
+++ b/refs.c
@@ -2313,7 +2313,7 @@ void base_ref_store_init(struct ref_store *refs, struct repository *repo,
refs->gitdir = xstrdup(path);
}
-int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts)
+int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts)
{
return refs->be->optimize(refs, opts);
}
diff --git a/refs.h b/refs.h
index 6edb633eb6..d2630af97f 100644
--- a/refs.h
+++ b/refs.h
@@ -499,16 +499,16 @@ void refs_warn_dangling_symrefs(struct ref_store *refs, FILE *fp,
const struct string_list *refnames);
/*
- * Flags for controlling behaviour of pack_refs()
- * PACK_REFS_PRUNE: Prune loose refs after packing
- * PACK_REFS_AUTO: Pack refs on a best effort basis. The heuristics and end
- * result are decided by the ref backend. Backends may ignore
- * this flag and fall back to a normal repack.
+ * Flags for controlling behaviour of refs_optimize()
+ * REFS_OPTIMIZE_PRUNE: Prune loose refs after packing
+ * REFS_OPTIMIZE_AUTO: Pack refs on a best effort basis. The heuristics and end
+ * result are decided by the ref backend. Backends may ignore
+ * this flag and fall back to a normal repack.
*/
-#define PACK_REFS_PRUNE (1 << 0)
-#define PACK_REFS_AUTO (1 << 1)
+#define REFS_OPTIMIZE_PRUNE (1 << 0)
+#define REFS_OPTIMIZE_AUTO (1 << 1)
-struct pack_refs_opts {
+struct refs_optimize_opts {
unsigned int flags;
struct ref_exclusions *exclusions;
struct string_list *includes;
@@ -518,7 +518,7 @@ struct pack_refs_opts {
* Optimize the ref store. The exact behavior is up to the backend.
* For the files backend, this is equivalent to packing refs.
*/
-int refs_optimize(struct ref_store *refs, struct pack_refs_opts *opts);
+int refs_optimize(struct ref_store *refs, struct refs_optimize_opts *opts);
/*
* Setup reflog before using. Fill in err and return -1 on failure.
diff --git a/refs/debug.c b/refs/debug.c
index 40cd1d9c15..2defd2d465 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -116,7 +116,7 @@ static int debug_transaction_abort(struct ref_store *refs,
return res;
}
-static int debug_optimize(struct ref_store *ref_store, struct pack_refs_opts *opts)
+static int debug_optimize(struct ref_store *ref_store, struct refs_optimize_opts *opts)
{
struct debug_ref_store *drefs = (struct debug_ref_store *)ref_store;
int res = drefs->refs->be->optimize(drefs->refs, opts);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 92d90fc508..2f60395cd2 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -1355,7 +1355,7 @@ static void prune_refs(struct files_ref_store *refs, struct ref_to_prune **refs_
*/
static int should_pack_ref(struct files_ref_store *refs,
const struct reference *ref,
- struct pack_refs_opts *opts)
+ struct refs_optimize_opts *opts)
{
struct string_list_item *item;
@@ -1383,7 +1383,7 @@ static int should_pack_ref(struct files_ref_store *refs,
}
static int should_pack_refs(struct files_ref_store *refs,
- struct pack_refs_opts *opts)
+ struct refs_optimize_opts *opts)
{
struct ref_iterator *iter;
size_t packed_size;
@@ -1391,7 +1391,7 @@ static int should_pack_refs(struct files_ref_store *refs,
size_t limit;
int ret;
- if (!(opts->flags & PACK_REFS_AUTO))
+ if (!(opts->flags & REFS_OPTIMIZE_AUTO))
return 1;
ret = packed_refs_size(refs->packed_ref_store, &packed_size);
@@ -1445,7 +1445,7 @@ static int should_pack_refs(struct files_ref_store *refs,
}
static int files_optimize(struct ref_store *ref_store,
- struct pack_refs_opts *opts)
+ struct refs_optimize_opts *opts)
{
struct files_ref_store *refs =
files_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB,
@@ -1488,7 +1488,7 @@ static int files_optimize(struct ref_store *ref_store,
iter->ref.name, err.buf);
/* Schedule the loose reference for pruning if requested. */
- if ((opts->flags & PACK_REFS_PRUNE)) {
+ if ((opts->flags & REFS_OPTIMIZE_PRUNE)) {
struct ref_to_prune *n;
FLEX_ALLOC_STR(n, name, iter->ref.name);
oidcpy(&n->oid, iter->ref.oid);
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index 20cf9fab18..10062fd8b6 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -1774,7 +1774,7 @@ static int packed_transaction_finish(struct ref_store *ref_store,
}
static int packed_optimize(struct ref_store *ref_store UNUSED,
- struct pack_refs_opts *pack_opts UNUSED)
+ struct refs_optimize_opts *opts UNUSED)
{
/*
* Packed refs are already packed. It might be that loose refs
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index fc5149df5b..dee42f231d 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -423,7 +423,7 @@ typedef int ref_transaction_commit_fn(struct ref_store *refs,
struct strbuf *err);
typedef int optimize_fn(struct ref_store *ref_store,
- struct pack_refs_opts *opts);
+ struct refs_optimize_opts *opts);
typedef int rename_ref_fn(struct ref_store *ref_store,
const char *oldref, const char *newref,
const char *logmsg);
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 43cc66a48e..c23c45f3bf 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -1701,7 +1701,7 @@ static int reftable_be_transaction_finish(struct ref_store *ref_store UNUSED,
}
static int reftable_be_optimize(struct ref_store *ref_store,
- struct pack_refs_opts *opts)
+ struct refs_optimize_opts *opts)
{
struct reftable_ref_store *refs =
reftable_be_downcast(ref_store, REF_STORE_WRITE | REF_STORE_ODB, "optimize_refs");
@@ -1715,7 +1715,7 @@ static int reftable_be_optimize(struct ref_store *ref_store,
if (!stack)
stack = refs->main_backend.stack;
- if (opts->flags & PACK_REFS_AUTO)
+ if (opts->flags & REFS_OPTIMIZE_AUTO)
ret = reftable_stack_auto_compact(stack);
else
ret = reftable_stack_compact_all(stack, NULL);
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v2 3/3] t/pack-refs-tests: move the 'test_done' to callees
2025-10-20 8:18 [PATCH v2 0/3] refs: cleanup code around optimizations Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 1/3] refs: move to using the '.optimize' functions Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 2/3] refs: rename 'pack_refs_opts' to 'refs_optimize_opts' Karthik Nayak
@ 2025-10-20 8:18 ` Karthik Nayak
2025-10-21 5:16 ` [PATCH v2 0/3] refs: cleanup code around optimizations Patrick Steinhardt
3 siblings, 0 replies; 6+ messages in thread
From: Karthik Nayak @ 2025-10-20 8:18 UTC (permalink / raw)
To: git; +Cc: Karthik Nayak, ps, jltobler
In ac0bad0af4 (t0601: refactor tests to be shareable, 2025-09-19), we
refactored 't/t0601-reffiles-pack-refs.sh' to move all of the tests to
't/pack-refs-tests.sh', which became a common test suite which was also
used by 't/t1463-refs-optimize.sh'.
This also moved the 'test_done' directive to 't/pack-refs-tests.sh'.
Which inhibits additional tests from being added to either of the tests.
Let's move the directive out to both the tests, so that we can add
additional specific tests to them. Also the test flow logic shouldn't be
part of tests which can be embedded in other test scripts.
Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
---
t/pack-refs-tests.sh | 2 --
t/t0601-reffiles-pack-refs.sh | 2 ++
t/t1463-refs-optimize.sh | 2 ++
3 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/t/pack-refs-tests.sh b/t/pack-refs-tests.sh
index 095823d915..81086c3690 100644
--- a/t/pack-refs-tests.sh
+++ b/t/pack-refs-tests.sh
@@ -459,5 +459,3 @@ test_expect_success 'pack-refs does not store invalid peeled tag value' '
test_grep ! "^\^" .git/packed-refs
)
'
-
-test_done
diff --git a/t/t0601-reffiles-pack-refs.sh b/t/t0601-reffiles-pack-refs.sh
index 12cf5d1dcb..3c706978ef 100755
--- a/t/t0601-reffiles-pack-refs.sh
+++ b/t/t0601-reffiles-pack-refs.sh
@@ -18,3 +18,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT
. ./test-lib.sh
. "$TEST_DIRECTORY"/pack-refs-tests.sh
+
+test_done
diff --git a/t/t1463-refs-optimize.sh b/t/t1463-refs-optimize.sh
index c11c905d79..9afe3c1ed7 100755
--- a/t/t1463-refs-optimize.sh
+++ b/t/t1463-refs-optimize.sh
@@ -15,3 +15,5 @@ export GIT_TEST_DEFAULT_REF_FORMAT
pack_refs='refs optimize'
. "$TEST_DIRECTORY"/pack-refs-tests.sh
+
+test_done
--
2.51.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/3] refs: cleanup code around optimizations
2025-10-20 8:18 [PATCH v2 0/3] refs: cleanup code around optimizations Karthik Nayak
` (2 preceding siblings ...)
2025-10-20 8:18 ` [PATCH v2 3/3] t/pack-refs-tests: move the 'test_done' to callees Karthik Nayak
@ 2025-10-21 5:16 ` Patrick Steinhardt
2025-10-22 11:59 ` Karthik Nayak
3 siblings, 1 reply; 6+ messages in thread
From: Patrick Steinhardt @ 2025-10-21 5:16 UTC (permalink / raw)
To: Karthik Nayak; +Cc: git, jltobler
On Mon, Oct 20, 2025 at 10:18:28AM +0200, Karthik Nayak wrote:
> This is extracted from a recent series I sent [1], which I've since
> dropped to follow up with a different approach. I think these patches
> hold value individually.
>
> They mostly cleanup code around 'git refs optimize' which was added
> recently in db0babf9b2 (Merge branch 'ms/refs-optimize', 2025-10-02).
> The code in the refs subsystem contains both 'pack-refs' and 'optimize'
> functions, which are one and the same.
>
> This series unifies this to only retain the 'optimize' functions and
> naming, since it backend generic.
>
> This is based on top of master 143f58ef75 (Sync with Git 2.51.1,
> 2025-10-15) with 'ps/ref-peeled-tags' merged in.
>
> [1]: 20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail•com
>
> Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
> ---
> Changes in v2:
> - Squash the second commit into the first.
> - Change some variable names to also no longer refer to pack_refs.
> - Fix commit messages.
> - Link to v1: https://lore.kernel.org/r/20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com
This version looks good to me. Thanks for all these cleanups!
Patrick
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 0/3] refs: cleanup code around optimizations
2025-10-21 5:16 ` [PATCH v2 0/3] refs: cleanup code around optimizations Patrick Steinhardt
@ 2025-10-22 11:59 ` Karthik Nayak
0 siblings, 0 replies; 6+ messages in thread
From: Karthik Nayak @ 2025-10-22 11:59 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, jltobler
[-- Attachment #1: Type: text/plain, Size: 1311 bytes --]
Patrick Steinhardt <ps@pks•im> writes:
> On Mon, Oct 20, 2025 at 10:18:28AM +0200, Karthik Nayak wrote:
>> This is extracted from a recent series I sent [1], which I've since
>> dropped to follow up with a different approach. I think these patches
>> hold value individually.
>>
>> They mostly cleanup code around 'git refs optimize' which was added
>> recently in db0babf9b2 (Merge branch 'ms/refs-optimize', 2025-10-02).
>> The code in the refs subsystem contains both 'pack-refs' and 'optimize'
>> functions, which are one and the same.
>>
>> This series unifies this to only retain the 'optimize' functions and
>> naming, since it backend generic.
>>
>> This is based on top of master 143f58ef75 (Sync with Git 2.51.1,
>> 2025-10-15) with 'ps/ref-peeled-tags' merged in.
>>
>> [1]: 20251010-562-add-option-to-check-if-reference-backend-needs-repacking-v1-0-c7962be584fa@gmail•com
>>
>> Signed-off-by: Karthik Nayak <karthik.188@gmail•com>
>> ---
>> Changes in v2:
>> - Squash the second commit into the first.
>> - Change some variable names to also no longer refer to pack_refs.
>> - Fix commit messages.
>> - Link to v1: https://lore.kernel.org/r/20251015-refs-code-cleanup-v1-0-550fdd8a3b41@gmail.com
>
> This version looks good to me. Thanks for all these cleanups!
>
> Patrick
Thanks for the review!
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-10-22 11:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-20 8:18 [PATCH v2 0/3] refs: cleanup code around optimizations Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 1/3] refs: move to using the '.optimize' functions Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 2/3] refs: rename 'pack_refs_opts' to 'refs_optimize_opts' Karthik Nayak
2025-10-20 8:18 ` [PATCH v2 3/3] t/pack-refs-tests: move the 'test_done' to callees Karthik Nayak
2025-10-21 5:16 ` [PATCH v2 0/3] refs: cleanup code around optimizations Patrick Steinhardt
2025-10-22 11:59 ` Karthik Nayak
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox