git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Karthik Nayak <karthik.188@gmail.com>
To: git@vger.kernel.org
Cc: Karthik Nayak <karthik.188@gmail.com>, ps@pks.im, jltobler@gmail.com
Subject: [PATCH v2 0/3] refs: cleanup code around optimizations
Date: Mon, 20 Oct 2025 10:18:28 +0200	[thread overview]
Message-ID: <20251020-refs-code-cleanup-v2-0-f5349ed0f6a5@gmail.com> (raw)

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



             reply	other threads:[~2025-10-20  8:18 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-10-20  8:18 Karthik Nayak [this message]
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20251020-refs-code-cleanup-v2-0-f5349ed0f6a5@gmail.com \
    --to=karthik.188@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).