git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v2 05/16] path: refactor `repo_submodule_path()` family of functions

From
Patrick Steinhardt <ps@pks.im>
Date
Feb 7, 2025, 11:03 UTC
Message-ID
<20250207-b4-pks-path-drop-the-repository-v2-5-13cad3c11b8a@pks.im>
In-Reply-To
<20250207-b4-pks-path-drop-the-repository-v2-0-13cad3c11b8a@pks.im>

As explained in an earlier commit, we're refactoring path-related functions to provide a consistent interface for computing paths into the commondir, gitdir and worktree. Refactor the "submodule" family of functions accordingly.

Note that in contrast to the other `repo_*_path()` families, we have to pass in the repository as a non-constant pointer. This is because we end up calling `repo_read_gitmodules()` deep down in the callstack, which may end up modifying the repository.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
---
 builtin/submodule--helper.c |  2 +-
 path.c                      | 37 +++++++++++++++++++++++++++++--------
 path.h                      | 30 ++++++++++++++++++------------
 t/helper/test-ref-store.c   |  7 +++----
 worktree.c                  |  3 ++-
 5 files changed, 53 insertions(+), 26 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 3a64f7e605..c1a8029714 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1826,7 +1826,7 @@ static int clone_submodule(const struct module_clone_data *clone_data,
 
 	connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
 
-	p = git_pathdup_submodule(clone_data_path, "config");
+	p = repo_submodule_path(the_repository, clone_data_path, "config");
 	if (!p)
 		die(_("could not get submodule directory for '%s'"), clone_data_path);
 
diff --git a/path.c b/path.c
index a7fa42162e..0d81e9fc32 100644
--- a/path.c
+++ b/path.c
@@ -560,14 +560,15 @@ const char *repo_worktree_path_replace(const struct repository *repo,
 }
 
 /* Returns 0 on success, negative on failure. */
-static int do_submodule_path(struct strbuf *buf, const char *path,
+static int do_submodule_path(struct repository *repo,
+			     struct strbuf *buf, const char *path,
 			     const char *fmt, va_list args)
 {
 	struct strbuf git_submodule_common_dir = STRBUF_INIT;
 	struct strbuf git_submodule_dir = STRBUF_INIT;
 	int ret;
 
-	ret = submodule_to_gitdir(the_repository, &git_submodule_dir, path);
+	ret = submodule_to_gitdir(repo, &git_submodule_dir, path);
 	if (ret)
 		goto cleanup;
 
@@ -586,13 +587,14 @@ static int do_submodule_path(struct strbuf *buf, const char *path,
 	return ret;
 }
 
-char *git_pathdup_submodule(const char *path, const char *fmt, ...)
+char *repo_submodule_path(struct repository *repo,
+			  const char *path, const char *fmt, ...)
 {
 	int err;
 	va_list args;
 	struct strbuf buf = STRBUF_INIT;
 	va_start(args, fmt);
-	err = do_submodule_path(&buf, path, fmt, args);
+	err = do_submodule_path(repo, &buf, path, fmt, args);
 	va_end(args);
 	if (err) {
 		strbuf_release(&buf);
@@ -601,16 +603,35 @@ char *git_pathdup_submodule(const char *path, const char *fmt, ...)
 	return strbuf_detach(&buf, NULL);
 }
 
-int strbuf_git_path_submodule(struct strbuf *buf, const char *path,
-			      const char *fmt, ...)
+const char *repo_submodule_path_append(struct repository *repo,
+				       struct strbuf *buf,
+				       const char *path,
+				       const char *fmt, ...)
 {
 	int err;
 	va_list args;
 	va_start(args, fmt);
-	err = do_submodule_path(buf, path, fmt, args);
+	err = do_submodule_path(repo, buf, path, fmt, args);
 	va_end(args);
+	if (err)
+		return NULL;
+	return buf->buf;
+}
 
-	return err;
+const char *repo_submodule_path_replace(struct repository *repo,
+					struct strbuf *buf,
+					const char *path,
+					const char *fmt, ...)
+{
+	int err;
+	va_list args;
+	strbuf_reset(buf);
+	va_start(args, fmt);
+	err = do_submodule_path(repo, buf, path, fmt, args);
+	va_end(args);
+	if (err)
+		return NULL;
+	return buf->buf;
 }
 
 void repo_common_pathv(const struct repository *repo,
diff --git a/path.h b/path.h
index d3f85f0676..4fe523626c 100644
--- a/path.h
+++ b/path.h
@@ -93,20 +93,26 @@ const char *repo_worktree_path_replace(const struct repository *repo,
 	__attribute__((format (printf, 3, 4)));
 
 /*
- * Return a path into a submodule's git directory located at `path`.  `path`
- * must only reference a submodule of the main repository (the_repository).
- */
-char *git_pathdup_submodule(const char *path, const char *fmt, ...)
-	__attribute__((format (printf, 2, 3)));
-
-/*
- * Construct a path into a submodule's git directory located at `path` and
- * append it to the provided buffer `sb`.  `path` must only reference a
- * submodule of the main repository (the_repository).
+ * The `repo_submodule_path` family of functions will construct a path into a
+ * submodule's git directory located at `path`. `path` must be a submodule path
+ * as found in the index and must be part of the given repository.
+ *
+ * Returns a `NULL` pointer in case the submodule cannot be found.
  */
-int strbuf_git_path_submodule(struct strbuf *sb, const char *path,
-				     const char *fmt, ...)
+char *repo_submodule_path(struct repository *repo,
+			  const char *path,
+			  const char *fmt, ...)
 	__attribute__((format (printf, 3, 4)));
+const char *repo_submodule_path_append(struct repository *repo,
+				       struct strbuf *sb,
+				       const char *path,
+				       const char *fmt, ...)
+	__attribute__((format (printf, 4, 5)));
+const char *repo_submodule_path_replace(struct repository *repo,
+					struct strbuf *sb,
+					const char *path,
+					const char *fmt, ...)
+	__attribute__((format (printf, 4, 5)));
 
 void report_linked_checkout_garbage(struct repository *r);
 
diff --git a/t/helper/test-ref-store.c b/t/helper/test-ref-store.c
index 1cc05f043a..e00fce592b 100644
--- a/t/helper/test-ref-store.c
+++ b/t/helper/test-ref-store.c
@@ -75,11 +75,10 @@ static const char **get_store(const char **argv, struct ref_store **refs)
 		*refs = get_main_ref_store(the_repository);
 	} else if (skip_prefix(argv[0], "submodule:", &gitdir)) {
 		struct strbuf sb = STRBUF_INIT;
-		int ret;
 
-		ret = strbuf_git_path_submodule(&sb, gitdir, "objects/");
-		if (ret)
-			die("strbuf_git_path_submodule failed: %d", ret);
+		if (!repo_submodule_path_append(the_repository,
+						&sb, gitdir, "objects/"))
+			die("computing submodule path failed");
 		add_to_alternates_memory(sb.buf);
 		strbuf_release(&sb);
 
diff --git a/worktree.c b/worktree.c
index f8d6e7127f..8f4fc10c44 100644
--- a/worktree.c
+++ b/worktree.c
@@ -487,7 +487,8 @@ int submodule_uses_worktrees(const char *path)
 	int ret = 0;
 	struct repository_format format = REPOSITORY_FORMAT_INIT;
 
-	submodule_gitdir = git_pathdup_submodule(path, "%s", "");
+	submodule_gitdir = repo_submodule_path(the_repository,
+					       path, "%s", "");
 	if (!submodule_gitdir)
 		return 0;
 

-- 
2.48.1.538.gc4cfc42d60.dirty
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 6 of 28 in “path: remove dependency on `the_repository`”
  1. 00/16 path: remove dependency on `the_repository`Patrick Steinhardt, Feb 7, 2025
  2. 01/16 path: refactor `repo_common_path()` family of functionsPatrick Steinhardt, Feb 7, 2025
  3. 02/16 path: refactor `repo_git_path()` family of functionsPatrick Steinhardt, Feb 7, 2025
  4. 03/16 path: refactor `repo_worktree_path()` family of functionsPatrick Steinhardt, Feb 7, 2025
  5. 04/16 submodule: refactor `submodule_to_gitdir()` to accept a repoPatrick Steinhardt, Feb 7, 2025
  6. 05/16 path: refactor `repo_submodule_path()` family of functionsPatrick Steinhardt, Feb 7, 2025
  7. 06/16 path: drop unused `strbuf_git_path()` functionPatrick Steinhardt, Feb 7, 2025
  8. 07/16 path: drop `git_pathdup()` in favor of `repo_git_path()`Patrick Steinhardt, Feb 7, 2025
  9. 08/16 path: drop `git_path_buf()` in favor of `repo_git_path_replace()`Patrick Steinhardt, Feb 7, 2025
  10. 09/16 worktree: return allocated string from `get_worktree_git_dir()`Patrick Steinhardt, Feb 7, 2025
  11. 10/16 path: drop `git_common_path()` in favor of `repo_common_path()`Patrick Steinhardt, Feb 7, 2025
  12. 11/16 rerere: let `rerere_path()` write paths into a caller-provided bufferPatrick Steinhardt, Feb 7, 2025
  13. Jeff KingFeb 22, 2025
  14. Junio C HamanoFeb 24, 2025
  15. Jeff KingFeb 24, 2025
  16. Junio C HamanoFeb 24, 2025
  17. Jeff KingFeb 24, 2025
  18. Junio C HamanoFeb 24, 2025
  19. Patrick SteinhardtFeb 25, 2025
  20. 12/16 path: drop `git_path()` in favor of `repo_git_path()`Patrick Steinhardt, Feb 7, 2025
  21. 13/16 repo-settings: introduce function to clear structPatrick Steinhardt, Feb 7, 2025
  22. 14/16 environment: move access to "core.hooksPath" into repo settingsPatrick Steinhardt, Feb 7, 2025
  23. 15/16 environment: move access to "core.sharedRepository" into repo settingsPatrick Steinhardt, Feb 7, 2025
  24. 16/16 path: adjust last remaining users of `the_repository`Patrick Steinhardt, Feb 7, 2025
  25. Karthik NayakFeb 7, 2025
  26. shejialuoFeb 8, 2025
  27. Junio C HamanoFeb 10, 2025
  28. shejialuoFeb 11, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.