diff mbox series

[05/16] refs: pass repo when retrieving submodule ref store

Message ID 1d48289809d5bff3d168b42b2b7f150e3ee953e2.1715836916.git.ps@pks.im (mailing list archive)
State Superseded
Headers show
Series refs: drop all references to `the_repository` | expand

Commit Message

Patrick Steinhardt May 16, 2024, 8:04 a.m. UTC
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.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 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(-)

Comments

Junio C Hamano May 16, 2024, 6:44 p.m. UTC | #1
Patrick Steinhardt <ps@pks.im> writes:

> 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.

Hmph.

> diff --git a/refs.c b/refs.c
> index 542acb25ff..86008ce7b4 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;

This still wants to work on the_repository, which means at this 5th
step of 16-patch series, we cannot do a submodule of a submodule
that hangs below the top-level superproject yet?

> @@ -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:

Nice.
Patrick Steinhardt May 17, 2024, 7:09 a.m. UTC | #2
On Thu, May 16, 2024 at 11:44:17AM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > 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.
> 
> Hmph.
> 
> > diff --git a/refs.c b/refs.c
> > index 542acb25ff..86008ce7b4 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;
> 
> This still wants to work on the_repository, which means at this 5th
> step of 16-patch series, we cannot do a submodule of a submodule
> that hangs below the top-level superproject yet?

Yeah, this gets fixed in the next patch.

Patrick
diff mbox series

Patch

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 542acb25ff..86008ce7b4 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 5ecdfb16dc..fc1a873e8a 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 cc1fe6e633..3c3f9a3555 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -706,7 +706,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();