From ed93ea16025decb60eb91308d682884e263e6f85 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 17 May 2024 10:18:19 +0200
Subject: refs: rename `init_db` callback to avoid confusion

Reference backends have two callbacks `init` and `init_db`. The
similarity of these two callbacks has repeatedly confused me whenever I
was looking at them, where I always had to look up which of them does
what.

Rename the `init_db` callback to `create_on_disk`, which should
hopefully be clearer.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs/refs-internal.h | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

(limited to 'refs/refs-internal.h')

diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 56641aa57a..c3d5f0a6cd 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -530,9 +530,9 @@ typedef struct ref_store *ref_store_init_fn(struct repository *repo,
 					    const char *gitdir,
 					    unsigned int flags);
 
-typedef int ref_init_db_fn(struct ref_store *refs,
-			   int flags,
-			   struct strbuf *err);
+typedef int ref_store_create_on_disk_fn(struct ref_store *refs,
+					int flags,
+					struct strbuf *err);
 
 typedef int ref_transaction_prepare_fn(struct ref_store *refs,
 				       struct ref_transaction *transaction,
@@ -668,7 +668,7 @@ typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refnam
 struct ref_storage_be {
 	const char *name;
 	ref_store_init_fn *init;
-	ref_init_db_fn *init_db;
+	ref_store_create_on_disk_fn *create_on_disk;
 
 	ref_transaction_prepare_fn *transaction_prepare;
 	ref_transaction_finish_fn *transaction_finish;
-- 
cgit v1.3-5-g9baa


From 71c871b48dfaf300ca20e205917db72ab9c6d7b3 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 17 May 2024 10:18:24 +0200
Subject: refs: implement releasing ref storages

Ref storages are typically only initialized once for `the_repository`
and then never released. Until now we got away with that without causing
memory leaks because `the_repository` stays reachable, and because the
ref backend is reachable via `the_repository` its memory basically never
leaks.

This is about to change though because of the upcoming migration logic,
which will create a secondary ref storage. In that case, we will either
have to release the old or new ref storage to avoid leaks.

Implement a new `release` callback and expose it via a new
`ref_storage_release()` function.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 refs.c                  |  6 ++++++
 refs.h                  |  5 +++++
 refs/debug.c            |  8 ++++++++
 refs/files-backend.c    | 10 ++++++++++
 refs/packed-backend.c   | 11 +++++++++++
 refs/refs-internal.h    |  5 +++++
 refs/reftable-backend.c | 23 +++++++++++++++++++++++
 7 files changed, 68 insertions(+)

(limited to 'refs/refs-internal.h')

diff --git a/refs.c b/refs.c
index ebc6de81e9..0f4d327c47 100644
--- a/refs.c
+++ b/refs.c
@@ -2041,6 +2041,12 @@ static struct ref_store *ref_store_init(struct repository *repo,
 	return refs;
 }
 
+void ref_store_release(struct ref_store *ref_store)
+{
+	ref_store->be->release(ref_store);
+	free(ref_store->gitdir);
+}
+
 struct ref_store *get_main_ref_store(struct repository *r)
 {
 	if (r->refs_private)
diff --git a/refs.h b/refs.h
index 421ff9fdb7..b11c250e8a 100644
--- a/refs.h
+++ b/refs.h
@@ -118,6 +118,11 @@ int is_branch(const char *refname);
 
 int ref_store_create_on_disk(struct ref_store *refs, int flags, struct strbuf *err);
 
+/*
+ * Release all memory and resources associated with the ref store.
+ */
+void ref_store_release(struct ref_store *ref_store);
+
 /*
  * Return the peeled value of the oid currently being iterated via
  * for_each_ref(), etc. This is equivalent to calling:
diff --git a/refs/debug.c b/refs/debug.c
index 4cc4910974..3a063077ba 100644
--- a/refs/debug.c
+++ b/refs/debug.c
@@ -33,6 +33,13 @@ struct ref_store *maybe_debug_wrap_ref_store(const char *gitdir, struct ref_stor
 	return (struct ref_store *)res;
 }
 
+static void debug_release(struct ref_store *refs)
+{
+	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
+	drefs->refs->be->release(drefs->refs);
+	trace_printf_key(&trace_refs, "release\n");
+}
+
 static int debug_create_on_disk(struct ref_store *refs, int flags, struct strbuf *err)
 {
 	struct debug_ref_store *drefs = (struct debug_ref_store *)refs;
@@ -427,6 +434,7 @@ static int debug_reflog_expire(struct ref_store *ref_store, const char *refname,
 struct ref_storage_be refs_be_debug = {
 	.name = "debug",
 	.init = NULL,
+	.release = debug_release,
 	.create_on_disk = debug_create_on_disk,
 
 	/*
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f9f15d1f76..62acd2721d 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -149,6 +149,14 @@ static struct files_ref_store *files_downcast(struct ref_store *ref_store,
 	return refs;
 }
 
+static void files_ref_store_release(struct ref_store *ref_store)
+{
+	struct files_ref_store *refs = files_downcast(ref_store, 0, "release");
+	free_ref_cache(refs->loose);
+	free(refs->gitcommondir);
+	ref_store_release(refs->packed_ref_store);
+}
+
 static void files_reflog_path(struct files_ref_store *refs,
 			      struct strbuf *sb,
 			      const char *refname)
@@ -3284,7 +3292,9 @@ static int files_ref_store_create_on_disk(struct ref_store *ref_store,
 struct ref_storage_be refs_be_files = {
 	.name = "files",
 	.init = files_ref_store_init,
+	.release = files_ref_store_release,
 	.create_on_disk = files_ref_store_create_on_disk,
+
 	.transaction_prepare = files_transaction_prepare,
 	.transaction_finish = files_transaction_finish,
 	.transaction_abort = files_transaction_abort,
diff --git a/refs/packed-backend.c b/refs/packed-backend.c
index b94183034e..9c98e6295f 100644
--- a/refs/packed-backend.c
+++ b/refs/packed-backend.c
@@ -252,6 +252,15 @@ static void clear_snapshot(struct packed_ref_store *refs)
 	}
 }
 
+static void packed_ref_store_release(struct ref_store *ref_store)
+{
+	struct packed_ref_store *refs = packed_downcast(ref_store, 0, "release");
+	clear_snapshot(refs);
+	rollback_lock_file(&refs->lock);
+	delete_tempfile(&refs->tempfile);
+	free(refs->path);
+}
+
 static NORETURN void die_unterminated_line(const char *path,
 					   const char *p, size_t len)
 {
@@ -1707,7 +1716,9 @@ static struct ref_iterator *packed_reflog_iterator_begin(struct ref_store *ref_s
 struct ref_storage_be refs_be_packed = {
 	.name = "packed",
 	.init = packed_ref_store_init,
+	.release = packed_ref_store_release,
 	.create_on_disk = packed_ref_store_create_on_disk,
+
 	.transaction_prepare = packed_transaction_prepare,
 	.transaction_finish = packed_transaction_finish,
 	.transaction_abort = packed_transaction_abort,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index c3d5f0a6cd..8624477e19 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -529,6 +529,10 @@ struct ref_store;
 typedef struct ref_store *ref_store_init_fn(struct repository *repo,
 					    const char *gitdir,
 					    unsigned int flags);
+/*
+ * Release all memory and resources associated with the ref store.
+ */
+typedef void ref_store_release_fn(struct ref_store *refs);
 
 typedef int ref_store_create_on_disk_fn(struct ref_store *refs,
 					int flags,
@@ -668,6 +672,7 @@ typedef int read_symbolic_ref_fn(struct ref_store *ref_store, const char *refnam
 struct ref_storage_be {
 	const char *name;
 	ref_store_init_fn *init;
+	ref_store_release_fn *release;
 	ref_store_create_on_disk_fn *create_on_disk;
 
 	ref_transaction_prepare_fn *transaction_prepare;
diff --git a/refs/reftable-backend.c b/refs/reftable-backend.c
index 8583a0cdba..7b73f73f59 100644
--- a/refs/reftable-backend.c
+++ b/refs/reftable-backend.c
@@ -293,6 +293,27 @@ done:
 	return &refs->base;
 }
 
+static void reftable_be_release(struct ref_store *ref_store)
+{
+	struct reftable_ref_store *refs = reftable_be_downcast(ref_store, 0, "release");
+	struct strmap_entry *entry;
+	struct hashmap_iter iter;
+
+	if (refs->main_stack) {
+		reftable_stack_destroy(refs->main_stack);
+		refs->main_stack = NULL;
+	}
+
+	if (refs->worktree_stack) {
+		reftable_stack_destroy(refs->worktree_stack);
+		refs->worktree_stack = NULL;
+	}
+
+	strmap_for_each_entry(&refs->worktree_stacks, &iter, entry)
+		reftable_stack_destroy(entry->value);
+	strmap_clear(&refs->worktree_stacks, 0);
+}
+
 static int reftable_be_create_on_disk(struct ref_store *ref_store,
 				      int flags UNUSED,
 				      struct strbuf *err UNUSED)
@@ -2248,7 +2269,9 @@ done:
 struct ref_storage_be refs_be_reftable = {
 	.name = "reftable",
 	.init = reftable_be_init,
+	.release = reftable_be_release,
 	.create_on_disk = reftable_be_create_on_disk,
+
 	.transaction_prepare = reftable_be_transaction_prepare,
 	.transaction_finish = reftable_be_transaction_finish,
 	.transaction_abort = reftable_be_transaction_abort,
-- 
cgit v1.3-5-g9baa


From 965f8991e59d84ba1b86e528f9c27852e746fa90 Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 17 May 2024 10:18:34 +0200
Subject: refs: pass repo when retrieving submodule ref store

Looking up submodule ref stores has two deficiencies:

  - The initialized subrepo will be attributed to `the_repository`.

  - The submodule ref store will be tracked in a global map.

This makes it impossible to have submodule ref stores for a repository
other than `the_repository`.

Modify the function to accept the parent repository as parameter and
move the global map into `struct repository`. Like this it becomes
possible to look up submodule ref stores for arbitrary repositories.

Note that this also adds a new reference to `the_repository` in
`resolve_gitlink_ref()`, which is part of the refs interfaces. This will
get adjusted in the next patch.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/submodule--helper.c |  6 ++++--
 refs.c                      | 22 +++++++---------------
 refs.h                      |  3 ++-
 refs/refs-internal.h        |  2 +-
 repository.c                |  8 ++++++++
 repository.h                |  8 ++++++++
 submodule.c                 |  3 ++-
 t/helper/test-ref-store.c   |  2 +-
 8 files changed, 33 insertions(+), 21 deletions(-)

(limited to 'refs/refs-internal.h')

diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index e604cb5ddb..5076a33500 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -679,7 +679,8 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
 			     displaypath);
 	} else if (!(flags & OPT_CACHED)) {
 		struct object_id oid;
-		struct ref_store *refs = get_submodule_ref_store(path);
+		struct ref_store *refs = repo_get_submodule_ref_store(the_repository,
+								      path);
 
 		if (!refs) {
 			print_status(flags, '-', path, ce_oid, displaypath);
@@ -903,7 +904,8 @@ static void generate_submodule_summary(struct summary_cb *info,
 
 	if (!info->cached && oideq(&p->oid_dst, null_oid())) {
 		if (S_ISGITLINK(p->mod_dst)) {
-			struct ref_store *refs = get_submodule_ref_store(p->sm_path);
+			struct ref_store *refs = repo_get_submodule_ref_store(the_repository,
+									      p->sm_path);
 
 			if (refs)
 				refs_head_ref(refs, handle_submodule_head_ref, &p->oid_dst);
diff --git a/refs.c b/refs.c
index 01e8453930..1d660d5ace 100644
--- a/refs.c
+++ b/refs.c
@@ -1949,8 +1949,7 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
 	struct ref_store *refs;
 	int flags;
 
-	refs = get_submodule_ref_store(submodule);
-
+	refs = repo_get_submodule_ref_store(the_repository, submodule);
 	if (!refs)
 		return -1;
 
@@ -1960,9 +1959,6 @@ int resolve_gitlink_ref(const char *submodule, const char *refname,
 	return 0;
 }
 
-/* A strmap of ref_stores, stored by submodule name: */
-static struct strmap submodule_ref_stores;
-
 /* A strmap of ref_stores, stored by worktree id: */
 static struct strmap worktree_ref_stores;
 
@@ -2036,7 +2032,8 @@ static void register_ref_store_map(struct strmap *map,
 		BUG("%s ref_store '%s' initialized twice", type, name);
 }
 
-struct ref_store *get_submodule_ref_store(const char *submodule)
+struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
+					       const char *submodule)
 {
 	struct strbuf submodule_sb = STRBUF_INIT;
 	struct ref_store *refs;
@@ -2057,7 +2054,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 		/* We need to strip off one or more trailing slashes */
 		submodule = to_free = xmemdupz(submodule, len);
 
-	refs = lookup_ref_store_map(&submodule_ref_stores, submodule);
+	refs = lookup_ref_store_map(&repo->submodule_ref_stores, submodule);
 	if (refs)
 		goto done;
 
@@ -2069,20 +2066,15 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
 		goto done;
 
 	subrepo = xmalloc(sizeof(*subrepo));
-	/*
-	 * NEEDSWORK: Make get_submodule_ref_store() work with arbitrary
-	 * superprojects other than the_repository. This probably should be
-	 * done by making it take a struct repository * parameter instead of a
-	 * submodule path.
-	 */
-	if (repo_submodule_init(subrepo, the_repository, submodule,
+
+	if (repo_submodule_init(subrepo, repo, submodule,
 				null_oid())) {
 		free(subrepo);
 		goto done;
 	}
 	refs = ref_store_init(subrepo, submodule_sb.buf,
 			      REF_STORE_READ | REF_STORE_ODB);
-	register_ref_store_map(&submodule_ref_stores, "submodule",
+	register_ref_store_map(&repo->submodule_ref_stores, "submodule",
 			       refs, submodule);
 
 done:
diff --git a/refs.h b/refs.h
index b11c250e8a..346a81e9e3 100644
--- a/refs.h
+++ b/refs.h
@@ -954,7 +954,8 @@ struct ref_store *get_main_ref_store(struct repository *r);
  * For backwards compatibility, submodule=="" is treated the same as
  * submodule==NULL.
  */
-struct ref_store *get_submodule_ref_store(const char *submodule);
+struct ref_store *repo_get_submodule_ref_store(struct repository *repo,
+					       const char *submodule);
 struct ref_store *get_worktree_ref_store(const struct worktree *wt);
 
 /*
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 8624477e19..178a355eb0 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -705,7 +705,7 @@ extern struct ref_storage_be refs_be_packed;
 /*
  * A representation of the reference store for the main repository or
  * a submodule. The ref_store instances for submodules are kept in a
- * hash map; see get_submodule_ref_store() for more info.
+ * hash map; see repo_get_submodule_ref_store() for more info.
  */
 struct ref_store {
 	/* The backend describing this ref_store's storage scheme: */
diff --git a/repository.c b/repository.c
index c50849fd83..bb9b9e2b52 100644
--- a/repository.c
+++ b/repository.c
@@ -14,6 +14,7 @@
 #include "sparse-index.h"
 #include "trace2.h"
 #include "promisor-remote.h"
+#include "refs.h"
 
 /* The main repository */
 static struct repository the_repo;
@@ -289,6 +290,9 @@ static void repo_clear_path_cache(struct repo_path_cache *cache)
 
 void repo_clear(struct repository *repo)
 {
+	struct hashmap_iter iter;
+	struct strmap_entry *e;
+
 	FREE_AND_NULL(repo->gitdir);
 	FREE_AND_NULL(repo->commondir);
 	FREE_AND_NULL(repo->graft_file);
@@ -329,6 +333,10 @@ void repo_clear(struct repository *repo)
 		FREE_AND_NULL(repo->remote_state);
 	}
 
+	strmap_for_each_entry(&repo->submodule_ref_stores, &iter, e)
+		ref_store_release(e->value);
+	strmap_clear(&repo->submodule_ref_stores, 1);
+
 	repo_clear_path_cache(&repo->cached_paths);
 }
 
diff --git a/repository.h b/repository.h
index 41ed22543a..0389df0461 100644
--- a/repository.h
+++ b/repository.h
@@ -1,6 +1,8 @@
 #ifndef REPOSITORY_H
 #define REPOSITORY_H
 
+#include "strmap.h"
+
 struct config_set;
 struct fsmonitor_settings;
 struct git_hash_algo;
@@ -108,6 +110,12 @@ struct repository {
 	 */
 	struct ref_store *refs_private;
 
+	/*
+	 * A strmap of ref_stores, stored by submodule name, accessible via
+	 * `repo_get_submodule_ref_store()`.
+	 */
+	struct strmap submodule_ref_stores;
+
 	/*
 	 * Contains path to often used file names.
 	 */
diff --git a/submodule.c b/submodule.c
index f6313cd99f..759cf1e1cd 100644
--- a/submodule.c
+++ b/submodule.c
@@ -99,7 +99,8 @@ int is_staging_gitmodules_ok(struct index_state *istate)
 static int for_each_remote_ref_submodule(const char *submodule,
 					 each_ref_fn fn, void *cb_data)
 {
-	return refs_for_each_remote_ref(get_submodule_ref_store(submodule),
+	return refs_for_each_remote_ref(repo_get_submodule_ref_store(the_repository,
+								     submodule),
 					fn, cb_data);
 }
 
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 82bbf6e2e6..a5a7609811 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -82,7 +82,7 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 		add_to_alternates_memory(sb.buf);
 		strbuf_release(&sb);
 
-		*refs = get_submodule_ref_store(gitdir);
+		*refs = repo_get_submodule_ref_store(the_repository, gitdir);
 	} else if (skip_prefix(argv[0], "worktree:", &gitdir)) {
 		struct worktree **p, **worktrees = get_worktrees();
 
-- 
cgit v1.3-5-g9baa


From 8378c9d27bace61d3fb238259604045a8997387b Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 17 May 2024 10:18:49 +0200
Subject: refs: convert iteration over replace refs to accept ref store

The function `for_each_replace_ref()` is a bit of an oddball across the
refs interfaces as it accepts a pointer to the repository instead of a
pointer to the ref store. The only reason for us to accept a repository
is so that we can eventually pass it back to the callback function that
the caller has provided. This is somewhat arbitrary though, as callers
that need the repository can instead make it accessible via the callback
payload.

Refactor the function to instead accept the ref store and adjust callers
accordingly. This allows us to get rid of some of the boilerplate that
we had to carry to pass along the repository and brings us in line with
the other functions that iterate through refs.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin/replace.c    | 13 +++++++-----
 refs.c               | 58 +++++++---------------------------------------------
 refs.h               | 17 ++-------------
 refs/iterator.c      |  6 +++---
 refs/refs-internal.h |  5 ++---
 replace-object.c     | 10 +++++----
 6 files changed, 28 insertions(+), 81 deletions(-)

(limited to 'refs/refs-internal.h')

diff --git a/builtin/replace.c b/builtin/replace.c
index bc2a948c80..b09c78b77d 100644
--- a/builtin/replace.c
+++ b/builtin/replace.c
@@ -43,11 +43,12 @@ enum replace_format {
 };
 
 struct show_data {
+	struct repository *repo;
 	const char *pattern;
 	enum replace_format format;
 };
 
-static int show_reference(struct repository *r, const char *refname,
+static int show_reference(const char *refname,
 			  const struct object_id *oid,
 			  int flag UNUSED, void *cb_data)
 {
@@ -62,11 +63,11 @@ static int show_reference(struct repository *r, const char *refname,
 			struct object_id object;
 			enum object_type obj_type, repl_type;
 
-			if (repo_get_oid(r, refname, &object))
+			if (repo_get_oid(data->repo, refname, &object))
 				return error(_("failed to resolve '%s' as a valid ref"), refname);
 
-			obj_type = oid_object_info(r, &object, NULL);
-			repl_type = oid_object_info(r, oid, NULL);
+			obj_type = oid_object_info(data->repo, &object, NULL);
+			repl_type = oid_object_info(data->repo, oid, NULL);
 
 			printf("%s (%s) -> %s (%s)\n", refname, type_name(obj_type),
 			       oid_to_hex(oid), type_name(repl_type));
@@ -80,6 +81,7 @@ static int list_replace_refs(const char *pattern, const char *format)
 {
 	struct show_data data;
 
+	data.repo = the_repository;
 	if (!pattern)
 		pattern = "*";
 	data.pattern = pattern;
@@ -99,7 +101,8 @@ static int list_replace_refs(const char *pattern, const char *format)
 			       "valid formats are 'short', 'medium' and 'long'"),
 			     format);
 
-	for_each_replace_ref(the_repository, show_reference, (void *)&data);
+	refs_for_each_replace_ref(get_main_ref_store(the_repository),
+				  show_reference, (void *)&data);
 
 	return 0;
 }
diff --git a/refs.c b/refs.c
index 70e712fcef..d705add3b8 100644
--- a/refs.c
+++ b/refs.c
@@ -1576,53 +1576,12 @@ struct ref_iterator *refs_ref_iterator_begin(
 	return iter;
 }
 
-/*
- * Call fn for each reference in the specified submodule for which the
- * refname begins with prefix. If trim is non-zero, then trim that
- * many characters off the beginning of each refname before passing
- * the refname to fn. flags can be DO_FOR_EACH_INCLUDE_BROKEN to
- * include broken references in the iteration. If fn ever returns a
- * non-zero value, stop the iteration and return that value;
- * otherwise, return 0.
- */
-static int do_for_each_repo_ref(struct repository *r, const char *prefix,
-				each_repo_ref_fn fn, int trim, int flags,
-				void *cb_data)
-{
-	struct ref_iterator *iter;
-	struct ref_store *refs = get_main_ref_store(r);
-
-	if (!refs)
-		return 0;
-
-	iter = refs_ref_iterator_begin(refs, prefix, NULL, trim, flags);
-
-	return do_for_each_repo_ref_iterator(r, iter, fn, cb_data);
-}
-
-struct do_for_each_ref_help {
-	each_ref_fn *fn;
-	void *cb_data;
-};
-
-static int do_for_each_ref_helper(struct repository *r UNUSED,
-				  const char *refname,
-				  const struct object_id *oid,
-				  int flags,
-				  void *cb_data)
-{
-	struct do_for_each_ref_help *hp = cb_data;
-
-	return hp->fn(refname, oid, flags, hp->cb_data);
-}
-
 static int do_for_each_ref(struct ref_store *refs, const char *prefix,
 			   const char **exclude_patterns,
 			   each_ref_fn fn, int trim,
 			   enum do_for_each_ref_flags flags, void *cb_data)
 {
 	struct ref_iterator *iter;
-	struct do_for_each_ref_help hp = { fn, cb_data };
 
 	if (!refs)
 		return 0;
@@ -1630,8 +1589,7 @@ static int do_for_each_ref(struct ref_store *refs, const char *prefix,
 	iter = refs_ref_iterator_begin(refs, prefix, exclude_patterns, trim,
 				       flags);
 
-	return do_for_each_repo_ref_iterator(the_repository, iter,
-					do_for_each_ref_helper, &hp);
+	return do_for_each_ref_iterator(iter, fn, cb_data);
 }
 
 int refs_for_each_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
@@ -1652,12 +1610,12 @@ int refs_for_each_fullref_in(struct ref_store *refs, const char *prefix,
 	return do_for_each_ref(refs, prefix, exclude_patterns, fn, 0, 0, cb_data);
 }
 
-int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data)
+int refs_for_each_replace_ref(struct ref_store *refs, each_ref_fn fn, void *cb_data)
 {
 	const char *git_replace_ref_base = ref_namespace[NAMESPACE_REPLACE].ref;
-	return do_for_each_repo_ref(r, git_replace_ref_base, fn,
-				    strlen(git_replace_ref_base),
-				    DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
+	return do_for_each_ref(refs, git_replace_ref_base, NULL, fn,
+			       strlen(git_replace_ref_base),
+			       DO_FOR_EACH_INCLUDE_BROKEN, cb_data);
 }
 
 int refs_for_each_namespaced_ref(struct ref_store *refs,
@@ -2425,8 +2383,7 @@ struct do_for_each_reflog_help {
 	void *cb_data;
 };
 
-static int do_for_each_reflog_helper(struct repository *r UNUSED,
-				     const char *refname,
+static int do_for_each_reflog_helper(const char *refname,
 				     const struct object_id *oid UNUSED,
 				     int flags,
 				     void *cb_data)
@@ -2442,8 +2399,7 @@ int refs_for_each_reflog(struct ref_store *refs, each_reflog_fn fn, void *cb_dat
 
 	iter = refs->be->reflog_iterator_begin(refs);
 
-	return do_for_each_repo_ref_iterator(the_repository, iter,
-					     do_for_each_reflog_helper, &hp);
+	return do_for_each_ref_iterator(iter, do_for_each_reflog_helper, &hp);
 }
 
 int refs_for_each_reflog_ent_reverse(struct ref_store *refs,
diff --git a/refs.h b/refs.h
index 90a7e2dde3..fa1f19464e 100644
--- a/refs.h
+++ b/refs.h
@@ -298,16 +298,6 @@ struct ref_transaction;
 typedef int each_ref_fn(const char *refname,
 			const struct object_id *oid, int flags, void *cb_data);
 
-/*
- * The same as each_ref_fn, but also with a repository argument that
- * contains the repository associated with the callback.
- */
-typedef int each_repo_ref_fn(struct repository *r,
-			     const char *refname,
-			     const struct object_id *oid,
-			     int flags,
-			     void *cb_data);
-
 /*
  * The following functions invoke the specified callback function for
  * each reference indicated.  If the function ever returns a nonzero
@@ -329,6 +319,8 @@ int refs_for_each_branch_ref(struct ref_store *refs,
 			     each_ref_fn fn, void *cb_data);
 int refs_for_each_remote_ref(struct ref_store *refs,
 			     each_ref_fn fn, void *cb_data);
+int refs_for_each_replace_ref(struct ref_store *refs,
+			      each_ref_fn fn, void *cb_data);
 
 /*
  * references matching any pattern in "exclude_patterns" are omitted from the
@@ -353,11 +345,6 @@ int refs_for_each_fullref_in_prefixes(struct ref_store *refs,
 				      const char **exclude_patterns,
 				      each_ref_fn fn, void *cb_data);
 
-/**
- * iterate refs from the respective area.
- */
-int for_each_replace_ref(struct repository *r, each_repo_ref_fn fn, void *cb_data);
-
 /* iterates all refs that match the specified glob pattern. */
 int refs_for_each_glob_ref(struct ref_store *refs, each_ref_fn fn,
 			   const char *pattern, void *cb_data);
diff --git a/refs/iterator.c b/refs/iterator.c
index 9db8b056d5..d355ebf0d5 100644
--- a/refs/iterator.c
+++ b/refs/iterator.c
@@ -440,15 +440,15 @@ struct ref_iterator *prefix_ref_iterator_begin(struct ref_iterator *iter0,
 
 struct ref_iterator *current_ref_iter = NULL;
 
-int do_for_each_repo_ref_iterator(struct repository *r, struct ref_iterator *iter,
-				  each_repo_ref_fn fn, void *cb_data)
+int do_for_each_ref_iterator(struct ref_iterator *iter,
+			     each_ref_fn fn, void *cb_data)
 {
 	int retval = 0, ok;
 	struct ref_iterator *old_ref_iter = current_ref_iter;
 
 	current_ref_iter = iter;
 	while ((ok = ref_iterator_advance(iter)) == ITER_OK) {
-		retval = fn(r, iter->refname, iter->oid, iter->flags, cb_data);
+		retval = fn(iter->refname, iter->oid, iter->flags, cb_data);
 		if (retval) {
 			/*
 			 * If ref_iterator_abort() returns ITER_ERROR,
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 178a355eb0..07e24c8481 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -503,9 +503,8 @@ extern struct ref_iterator *current_ref_iter;
  * adapter between the callback style of reference iteration and the
  * iterator style.
  */
-int do_for_each_repo_ref_iterator(struct repository *r,
-				  struct ref_iterator *iter,
-				  each_repo_ref_fn fn, void *cb_data);
+int do_for_each_ref_iterator(struct ref_iterator *iter,
+			     each_ref_fn fn, void *cb_data);
 
 struct ref_store;
 
diff --git a/replace-object.c b/replace-object.c
index 523215589d..73f5acbcd9 100644
--- a/replace-object.c
+++ b/replace-object.c
@@ -8,12 +8,13 @@
 #include "repository.h"
 #include "commit.h"
 
-static int register_replace_ref(struct repository *r,
-				const char *refname,
+static int register_replace_ref(const char *refname,
 				const struct object_id *oid,
 				int flag UNUSED,
-				void *cb_data UNUSED)
+				void *cb_data)
 {
+	struct repository *r = cb_data;
+
 	/* Get sha1 from refname */
 	const char *slash = strrchr(refname, '/');
 	const char *hash = slash ? slash + 1 : refname;
@@ -50,7 +51,8 @@ void prepare_replace_object(struct repository *r)
 		xmalloc(sizeof(*r->objects->replace_map));
 	oidmap_init(r->objects->replace_map, 0);
 
-	for_each_replace_ref(r, register_replace_ref, NULL);
+	refs_for_each_replace_ref(get_main_ref_store(r),
+				  register_replace_ref, r);
 	r->objects->replace_map_initialized = 1;
 
 	pthread_mutex_unlock(&r->objects->replace_mutex);
-- 
cgit v1.3-5-g9baa


From 19c76e8235747a61a703fe3bf7a5e40caf6fa3fa Mon Sep 17 00:00:00 2001
From: Patrick Steinhardt <ps@pks.im>
Date: Fri, 17 May 2024 10:18:59 +0200
Subject: refs: move object peeling into "object.c"

Peeling an object has nothing to do with refs, but we still have the
code in "refs.c". Move it over into "object.c", which is a more natural
place to put it.

Ideally, we'd also move `peel_iterated_oid()` over into "object.c". But
this function is tied to the refs interfaces because it uses a global
ref iterator variable to optimize peeling when the iterator already has
the peeled object ID readily available.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 object.c             | 21 +++++++++++++++++++++
 object.h             | 34 ++++++++++++++++++++++++++++++++++
 refs.c               | 22 ----------------------
 refs/refs-internal.h | 34 ----------------------------------
 4 files changed, 55 insertions(+), 56 deletions(-)

(limited to 'refs/refs-internal.h')

diff --git a/object.c b/object.c
index 51e384828e..995041926a 100644
--- a/object.c
+++ b/object.c
@@ -207,6 +207,27 @@ struct object *lookup_object_by_type(struct repository *r,
 	}
 }
 
+enum peel_status peel_object(const struct object_id *name, struct object_id *oid)
+{
+	struct object *o = lookup_unknown_object(the_repository, name);
+
+	if (o->type == OBJ_NONE) {
+		int type = oid_object_info(the_repository, name, NULL);
+		if (type < 0 || !object_as_type(o, type, 0))
+			return PEEL_INVALID;
+	}
+
+	if (o->type != OBJ_TAG)
+		return PEEL_NON_TAG;
+
+	o = deref_tag_noverify(o);
+	if (!o)
+		return PEEL_INVALID;
+
+	oidcpy(oid, &o->oid);
+	return PEEL_PEELED;
+}
+
 struct object *parse_object_buffer(struct repository *r, const struct object_id *oid, enum object_type type, unsigned long size, void *buffer, int *eaten_p)
 {
 	struct object *obj;
diff --git a/object.h b/object.h
index 9293e703cc..31ccd1bb10 100644
--- a/object.h
+++ b/object.h
@@ -256,6 +256,40 @@ struct object *lookup_unknown_object(struct repository *r, const struct object_i
 struct object *lookup_object_by_type(struct repository *r, const struct object_id *oid,
 				     enum object_type type);
 
+enum peel_status {
+	/* object was peeled successfully: */
+	PEEL_PEELED = 0,
+
+	/*
+	 * object cannot be peeled because the named object (or an
+	 * object referred to by a tag in the peel chain), does not
+	 * exist.
+	 */
+	PEEL_INVALID = -1,
+
+	/* object cannot be peeled because it is not a tag: */
+	PEEL_NON_TAG = -2,
+
+	/* ref_entry contains no peeled value because it is a symref: */
+	PEEL_IS_SYMREF = -3,
+
+	/*
+	 * ref_entry cannot be peeled because it is broken (i.e., the
+	 * symbolic reference cannot even be resolved to an object
+	 * name):
+	 */
+	PEEL_BROKEN = -4
+};
+
+/*
+ * Peel the named object; i.e., if the object is a tag, resolve the
+ * tag recursively until a non-tag is found.  If successful, store the
+ * result to oid and return PEEL_PEELED.  If the object is not a tag
+ * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
+ * and leave oid unchanged.
+ */
+enum peel_status peel_object(const struct object_id *name, struct object_id *oid);
+
 struct object_list *object_list_insert(struct object *item,
 				       struct object_list **list_p);
 
diff --git a/refs.c b/refs.c
index 48323dd28d..d1b530679f 100644
--- a/refs.c
+++ b/refs.c
@@ -19,7 +19,6 @@
 #include "object-store-ll.h"
 #include "object.h"
 #include "path.h"
-#include "tag.h"
 #include "submodule.h"
 #include "worktree.h"
 #include "strvec.h"
@@ -425,27 +424,6 @@ static int for_each_filter_refs(const char *refname,
 	return filter->fn(refname, oid, flags, filter->cb_data);
 }
 
-enum peel_status peel_object(const struct object_id *name, struct object_id *oid)
-{
-	struct object *o = lookup_unknown_object(the_repository, name);
-
-	if (o->type == OBJ_NONE) {
-		int type = oid_object_info(the_repository, name, NULL);
-		if (type < 0 || !object_as_type(o, type, 0))
-			return PEEL_INVALID;
-	}
-
-	if (o->type != OBJ_TAG)
-		return PEEL_NON_TAG;
-
-	o = deref_tag_noverify(o);
-	if (!o)
-		return PEEL_INVALID;
-
-	oidcpy(oid, &o->oid);
-	return PEEL_PEELED;
-}
-
 struct warn_if_dangling_data {
 	struct ref_store *refs;
 	FILE *fp;
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 07e24c8481..8d85589081 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -69,40 +69,6 @@ int ref_resolves_to_object(const char *refname,
 			   const struct object_id *oid,
 			   unsigned int flags);
 
-enum peel_status {
-	/* object was peeled successfully: */
-	PEEL_PEELED = 0,
-
-	/*
-	 * object cannot be peeled because the named object (or an
-	 * object referred to by a tag in the peel chain), does not
-	 * exist.
-	 */
-	PEEL_INVALID = -1,
-
-	/* object cannot be peeled because it is not a tag: */
-	PEEL_NON_TAG = -2,
-
-	/* ref_entry contains no peeled value because it is a symref: */
-	PEEL_IS_SYMREF = -3,
-
-	/*
-	 * ref_entry cannot be peeled because it is broken (i.e., the
-	 * symbolic reference cannot even be resolved to an object
-	 * name):
-	 */
-	PEEL_BROKEN = -4
-};
-
-/*
- * Peel the named object; i.e., if the object is a tag, resolve the
- * tag recursively until a non-tag is found.  If successful, store the
- * result to oid and return PEEL_PEELED.  If the object is not a tag
- * or is not valid, return PEEL_NON_TAG or PEEL_INVALID, respectively,
- * and leave oid unchanged.
- */
-enum peel_status peel_object(const struct object_id *name, struct object_id *oid);
-
 /**
  * Information needed for a single ref update. Set new_oid to the new
  * value or to null_oid to delete the ref. To check the old value
-- 
cgit v1.3-5-g9baa