[RFC][PATCH 2/2] worktree: stop passing NULL as primary worktree

[Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>]

Some functions like repo_git_pathv() in path.c, wt_status_check_rebase()
and wt_status_check_bisect() in wt-status.c, get_worktree_git_dir() in
builtin/worktree.c are called with a NULL worktree. This makes it difficult
to access fields like `wt->repo` and add extra handling for checking wt is
defined.

Add a helper function of get_worktrees_internal() as get_worktrees_repo()
and pass struct repository down the callstack to get_main_worktree()
function so that the primary worktree of a specific repository can be
accessed instead of just the_repository.

Access the primary worktree by the help of get_worktrees_repo() and pass it
to the functions as struct worktree instead of passing NULL.

Further `worktree_git_path()` no longer needs a separate
`struct repository *` parameter. Use `wt->repo` instead and update its
callers accordingly.

while at, replace any unecessary checks for wt to be defined with
is_main_worktree(wt).

Signed-off-by: Shreyansh Paliwal <shreyanshpaliwalcmsmn@gmail.com>
---
 builtin/fsck.c     |  2 +-
 builtin/worktree.c | 10 +++++++---
 path.c             | 27 +++++++++++++--------------
 path.h             |  9 +++------
 refs.c             |  4 ++--
 revision.c         |  6 ++++--
 worktree.c         | 29 ++++++++++++++++-------------
 worktree.h         |  7 +++++++
 wt-status.c        | 22 ++++++++++++----------
 9 files changed, 65 insertions(+), 51 deletions(-)

diff --git a/builtin/fsck.c b/builtin/fsck.c
index 0512f78a87..42ba0afb91 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1137,7 +1137,7 @@ int cmd_fsck(int argc,
 			 * and may get overwritten by other calls
 			 * while we're examining the index.
 			 */
-			path = xstrdup(worktree_git_path(the_repository, wt, "index"));
+			path = xstrdup(worktree_git_path(wt, "index"));
 			wt_gitdir = get_worktree_git_dir(wt);
 
 			read_index_from(&istate, path, wt_gitdir);
diff --git a/builtin/worktree.c b/builtin/worktree.c
index fbdaf2eb2e..27c5889c89 100644
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -328,6 +328,8 @@ static void check_candidate_path(const char *path,
 	wt = find_worktree_by_path(worktrees, path);
 	if (!wt)
 		return;
+	if(is_main_worktree(wt))
+		die(_("'%s' is the main worktree"), path);
 
 	locked = !!worktree_lock_reason(wt);
 	if ((!locked && force) || (locked && force > 1)) {
@@ -660,7 +662,8 @@ static int can_use_local_refs(const struct add_opts *opts)
 		if (!opts->quiet) {
 			struct strbuf path = STRBUF_INIT;
 			struct strbuf contents = STRBUF_INIT;
-			char *wt_gitdir = get_worktree_git_dir(NULL);
+			struct worktree **worktrees = get_worktrees_repo(the_repository);
+			char *wt_gitdir = get_worktree_git_dir(worktrees[0]);
 
 			strbuf_add_real_path(&path, wt_gitdir);
 			strbuf_addstr(&path, "/HEAD");
@@ -675,6 +678,7 @@ static int can_use_local_refs(const struct add_opts *opts)
 			strbuf_release(&path);
 			strbuf_release(&contents);
 			free(wt_gitdir);
+			free_worktrees(worktrees);
 		}
 		return 1;
 	}
@@ -1191,14 +1195,14 @@ static void validate_no_submodules(const struct worktree *wt)
 
 	wt_gitdir = get_worktree_git_dir(wt);
 
-	if (is_directory(worktree_git_path(the_repository, wt, "modules"))) {
+	if (is_directory(worktree_git_path(wt, "modules"))) {
 		/*
 		 * There could be false positives, e.g. the "modules"
 		 * directory exists but is empty. But it's a rare case and
 		 * this simpler check is probably good enough for now.
 		 */
 		found_submodules = 1;
-	} else if (read_index_from(&istate, worktree_git_path(the_repository, wt, "index"),
+	} else if (read_index_from(&istate, worktree_git_path(wt, "index"),
 				   wt_gitdir) > 0) {
 		for (i = 0; i < istate.cache_nr; i++) {
 			struct cache_entry *ce = istate.cache[i];
diff --git a/path.c b/path.c
index d726537622..4ac86e1e58 100644
--- a/path.c
+++ b/path.c
@@ -408,9 +408,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,
 				   const struct repository *repo,
 				   const struct worktree *wt)
 {
-	if (!wt)
-		strbuf_addstr(buf, repo->gitdir);
-	else if (!wt->id)
+	if (is_main_worktree(wt))
 		strbuf_addstr(buf, repo->commondir);
 	else
 		repo_common_path_append(repo, buf, "worktrees/%s", wt->id);
@@ -426,7 +424,7 @@ static void repo_git_pathv(struct repository *repo,
 		strbuf_addch(buf, '/');
 	gitdir_len = buf->len;
 	strbuf_vaddf(buf, fmt, args);
-	if (!wt)
+	if (is_main_worktree(wt))
 		adjust_git_path(repo, buf, gitdir_len);
 	strbuf_cleanup_path(buf);
 }
@@ -437,8 +435,10 @@ char *repo_git_path(struct repository *repo,
 	struct strbuf path = STRBUF_INIT;
 	va_list args;
 	va_start(args, fmt);
-	repo_git_pathv(repo, NULL, &path, fmt, args);
+	struct worktree **worktrees = get_worktrees_repo(repo);
+	repo_git_pathv(repo, worktrees[0], &path, fmt, args);
 	va_end(args);
+	free_worktrees(worktrees);
 	return strbuf_detach(&path, NULL);
 }
 
@@ -448,8 +448,10 @@ const char *repo_git_path_append(struct repository *repo,
 {
 	va_list args;
 	va_start(args, fmt);
-	repo_git_pathv(repo, NULL, sb, fmt, args);
+	struct worktree **worktrees = get_worktrees_repo(repo);
+	repo_git_pathv(repo, worktrees[0], sb, fmt, args);
 	va_end(args);
+	free_worktrees(worktrees);
 	return sb->buf;
 }
 
@@ -460,8 +462,10 @@ const char *repo_git_path_replace(struct repository *repo,
 	va_list args;
 	strbuf_reset(sb);
 	va_start(args, fmt);
-	repo_git_pathv(repo, NULL, sb, fmt, args);
+	struct worktree **worktrees = get_worktrees_repo(repo);
+	repo_git_pathv(repo, worktrees[0], sb, fmt, args);
 	va_end(args);
+	free_worktrees(worktrees);
 	return sb->buf;
 }
 
@@ -486,17 +490,12 @@ const char *mkpath(const char *fmt, ...)
 	return cleanup_path(pathname->buf);
 }
 
-const char *worktree_git_path(struct repository *r,
-			      const struct worktree *wt, const char *fmt, ...)
+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)
 {
 	struct strbuf *pathname = get_pathname();
 	va_list args;
-
-	if (wt && wt->repo != r)
-		BUG("worktree not connected to expected repository");
-
 	va_start(args, fmt);
-	repo_git_pathv(r, wt, pathname, fmt, args);
+	repo_git_pathv(wt->repo, wt, pathname, fmt, args);
 	va_end(args);
 	return pathname->buf;
 }
diff --git a/path.h b/path.h
index 0ec95a0b07..54e09b2883 100644
--- a/path.h
+++ b/path.h
@@ -66,13 +66,10 @@ const char *repo_git_path_replace(struct repository *repo,
 
 /*
  * Similar to repo_git_path() but can produce paths for a specified
- * worktree instead of current one. When no worktree is given, then the path is
- * computed relative to main worktree of the given repository.
+ * worktree instead of current one.
  */
-const char *worktree_git_path(struct repository *r,
-			      const struct worktree *wt,
-			      const char *fmt, ...)
-	__attribute__((format (printf, 3, 4)));
+const char *worktree_git_path(const struct worktree *wt, const char *fmt, ...)
+	__attribute__((format (printf, 2, 3)));
 
 /*
  * The `repo_worktree_path` family of functions will construct a path into a
diff --git a/refs.c b/refs.c
index 627b7f8698..98df2235e7 100644
--- a/refs.c
+++ b/refs.c
@@ -2324,12 +2324,12 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)
 	if (wt->is_current)
 		return get_main_ref_store(wt->repo);
 
-	id = wt->id ? wt->id : "/";
+	id = wt->id;
 	refs = lookup_ref_store_map(&wt->repo->worktree_ref_stores, id);
 	if (refs)
 		return refs;
 
-	if (wt->id) {
+	if (!is_main_worktree(wt)) {
 		struct strbuf common_path = STRBUF_INIT;
 		repo_common_path_append(wt->repo, &common_path,
 					"worktrees/%s", wt->id);
diff --git a/revision.c b/revision.c
index 9b131670f7..a9d4f796ed 100644
--- a/revision.c
+++ b/revision.c
@@ -1730,12 +1730,14 @@ void add_reflogs_to_pending(struct rev_info *revs, unsigned flags)
 
 	cb.all_revs = revs;
 	cb.all_flags = flags;
-	cb.wt = NULL;
+	struct worktree **worktrees = get_worktrees_repo(the_repository);
+	cb.wt = worktrees[0];
 	refs_for_each_reflog(get_main_ref_store(the_repository),
 			     handle_one_reflog, &cb);
 
 	if (!revs->single_worktree)
 		add_other_reflogs_to_pending(&cb);
+	free_worktrees(worktrees);
 }
 
 static void add_cache_tree(struct cache_tree *it, struct rev_info *revs,
@@ -1847,7 +1849,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
 		wt_gitdir = get_worktree_git_dir(wt);
 
 		if (read_index_from(&istate,
-				    worktree_git_path(the_repository, wt, "index"),
+				    worktree_git_path(wt, "index"),
 				    wt_gitdir) > 0)
 			do_add_index_objects_to_pending(revs, &istate, flags);
 
diff --git a/worktree.c b/worktree.c
index b29934407f..1059c18115 100644
--- a/worktree.c
+++ b/worktree.c
@@ -91,16 +91,16 @@ static int is_main_worktree_bare(struct repository *repo)
 /**
  * get the main worktree
  */
-static struct worktree *get_main_worktree(int skip_reading_head)
+static struct worktree *get_main_worktree(struct repository *repo, int skip_reading_head)
 {
 	struct worktree *worktree = NULL;
 	struct strbuf worktree_path = STRBUF_INIT;
 
-	strbuf_add_real_path(&worktree_path, repo_get_common_dir(the_repository));
+	strbuf_add_real_path(&worktree_path, repo_get_common_dir(repo));
 	strbuf_strip_suffix(&worktree_path, "/.git");
 
 	CALLOC_ARRAY(worktree, 1);
-	worktree->repo = the_repository;
+	worktree->repo = repo;
 	worktree->id = xstrdup("/");
 	worktree->path = strbuf_detach(&worktree_path, NULL);
 	worktree->is_current = is_current_worktree(worktree);
@@ -112,7 +112,7 @@ static struct worktree *get_main_worktree(int skip_reading_head)
 		 * This check is unnecessary if we're currently in the main worktree,
 		 * as prior checks already consulted all configs of the current worktree.
 		 */
-		(!worktree->is_current && is_main_worktree_bare(the_repository));
+		(!worktree->is_current && is_main_worktree_bare(repo));
 
 	if (!skip_reading_head)
 		add_head_info(worktree);
@@ -165,7 +165,7 @@ struct worktree *get_linked_worktree(const char *id,
  * retrieving worktree metadata that could be used when the worktree is known
  * to not be in a healthy state, e.g. when creating or repairing it.
  */
-static struct worktree **get_worktrees_internal(int skip_reading_head)
+static struct worktree **get_worktrees_internal(struct repository *repo, int skip_reading_head)
 {
 	struct worktree **list = NULL;
 	struct strbuf path = STRBUF_INIT;
@@ -175,9 +175,9 @@ static struct worktree **get_worktrees_internal(int skip_reading_head)
 
 	ALLOC_ARRAY(list, alloc);
 
-	list[counter++] = get_main_worktree(skip_reading_head);
+	list[counter++] = get_main_worktree(repo, skip_reading_head);
 
-	strbuf_addf(&path, "%s/worktrees", repo_get_common_dir(the_repository));
+	strbuf_addf(&path, "%s/worktrees", repo_get_common_dir(repo));
 	dir = opendir(path.buf);
 	strbuf_release(&path);
 	if (dir) {
@@ -199,12 +199,15 @@ static struct worktree **get_worktrees_internal(int skip_reading_head)
 
 struct worktree **get_worktrees(void)
 {
-	return get_worktrees_internal(0);
+	return get_worktrees_repo(the_repository);
+}
+struct worktree **get_worktrees_repo(struct repository *repo)
+{
+	return get_worktrees_internal(repo, 0);
 }
-
 struct worktree **get_worktrees_without_reading_head(void)
 {
-	return get_worktrees_internal(1);
+	return get_worktrees_internal(the_repository, 1);
 }
 
 char *get_worktree_git_dir(const struct worktree *wt)
@@ -289,7 +292,7 @@ const char *worktree_lock_reason(struct worktree *wt)
 	if (!wt->lock_reason_valid) {
 		struct strbuf path = STRBUF_INIT;
 
-		strbuf_addstr(&path, worktree_git_path(the_repository, wt, "locked"));
+		strbuf_addstr(&path, worktree_git_path(wt, "locked"));
 		if (file_exists(path.buf)) {
 			struct strbuf lock_reason = STRBUF_INIT;
 			if (strbuf_read_file(&lock_reason, path.buf, 0) < 0)
@@ -690,7 +693,7 @@ static void repair_noop(int iserr UNUSED,
 
 void repair_worktrees(worktree_repair_fn fn, void *cb_data, int use_relative_paths)
 {
-	struct worktree **worktrees = get_worktrees_internal(1);
+	struct worktree **worktrees = get_worktrees_internal(the_repository, 1);
 	struct worktree **wt = worktrees + 1; /* +1 skips main worktree */
 
 	if (!fn)
@@ -735,7 +738,7 @@ void repair_worktree_after_gitdir_move(struct worktree *wt, const char *old_path
 
 void repair_worktrees_after_gitdir_move(const char *old_path)
 {
-	struct worktree **worktrees = get_worktrees_internal(1);
+	struct worktree **worktrees = get_worktrees_internal(the_repository, 1);
 	struct worktree **wt = worktrees + 1; /* +1 skips main worktree */
 
 	for (; *wt; wt++)
diff --git a/worktree.h b/worktree.h
index e4bcccdc0a..fc5f87b5b1 100644
--- a/worktree.h
+++ b/worktree.h
@@ -30,6 +30,13 @@ struct worktree {
  */
 struct worktree **get_worktrees(void);
 
+/*
+ * Like `get_worktrees`, but it returns the worktrees for the given repository.
+ * This is useful for cases where local repository context is used rather than
+ * the_repository global.
+ */
+struct worktree **get_worktrees_repo(struct repository *repo);
+
 /*
  * Like `get_worktrees`, but does not read HEAD. Skip reading HEAD allows to
  * get the worktree without worrying about failures pertaining to parsing
diff --git a/wt-status.c b/wt-status.c
index e12adb26b9..11aea61c81 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -1624,7 +1624,7 @@ static char *get_branch(const struct worktree *wt, const char *path)
 	struct object_id oid;
 	const char *branch_name;
 
-	if (strbuf_read_file(&sb, worktree_git_path(the_repository, wt, "%s", path), 0) <= 0)
+	if (strbuf_read_file(&sb, worktree_git_path(wt, "%s", path), 0) <= 0)
 		goto got_nothing;
 
 	while (sb.len && sb.buf[sb.len - 1] == '\n')
@@ -1723,18 +1723,18 @@ int wt_status_check_rebase(const struct worktree *wt,
 {
 	struct stat st;
 
-	if (!stat(worktree_git_path(the_repository, wt, "rebase-apply"), &st)) {
-		if (!stat(worktree_git_path(the_repository, wt, "rebase-apply/applying"), &st)) {
+	if (!stat(worktree_git_path(wt, "rebase-apply"), &st)) {
+		if (!stat(worktree_git_path(wt, "rebase-apply/applying"), &st)) {
 			state->am_in_progress = 1;
-			if (!stat(worktree_git_path(the_repository, wt, "rebase-apply/patch"), &st) && !st.st_size)
+			if (!stat(worktree_git_path(wt, "rebase-apply/patch"), &st) && !st.st_size)
 				state->am_empty_patch = 1;
 		} else {
 			state->rebase_in_progress = 1;
 			state->branch = get_branch(wt, "rebase-apply/head-name");
 			state->onto = get_branch(wt, "rebase-apply/onto");
 		}
-	} else if (!stat(worktree_git_path(the_repository, wt, "rebase-merge"), &st)) {
-		if (!stat(worktree_git_path(the_repository, wt, "rebase-merge/interactive"), &st))
+	} else if (!stat(worktree_git_path(wt, "rebase-merge"), &st)) {
+		if (!stat(worktree_git_path(wt, "rebase-merge/interactive"), &st))
 			state->rebase_interactive_in_progress = 1;
 		else
 			state->rebase_in_progress = 1;
@@ -1750,7 +1750,7 @@ int wt_status_check_bisect(const struct worktree *wt,
 {
 	struct stat st;
 
-	if (!stat(worktree_git_path(the_repository, wt, "BISECT_LOG"), &st)) {
+	if (!stat(worktree_git_path(wt, "BISECT_LOG"), &st)) {
 		state->bisect_in_progress = 1;
 		state->bisecting_from = get_branch(wt, "BISECT_START");
 		return 1;
@@ -1795,18 +1795,19 @@ void wt_status_get_state(struct repository *r,
 	struct stat st;
 	struct object_id oid;
 	enum replay_action action;
+	struct worktree **worktrees = get_worktrees_repo(r);
 
 	if (!stat(git_path_merge_head(r), &st)) {
-		wt_status_check_rebase(NULL, state);
+		wt_status_check_rebase(worktrees[0], state);
 		state->merge_in_progress = 1;
-	} else if (wt_status_check_rebase(NULL, state)) {
+	} else if (wt_status_check_rebase(worktrees[0], state)) {
 		;		/* all set */
 	} else if (refs_ref_exists(get_main_ref_store(r), "CHERRY_PICK_HEAD") &&
 		   !repo_get_oid(r, "CHERRY_PICK_HEAD", &oid)) {
 		state->cherry_pick_in_progress = 1;
 		oidcpy(&state->cherry_pick_head_oid, &oid);
 	}
-	wt_status_check_bisect(NULL, state);
+	wt_status_check_bisect(worktrees[0], state);
 	if (refs_ref_exists(get_main_ref_store(r), "REVERT_HEAD") &&
 	    !repo_get_oid(r, "REVERT_HEAD", &oid)) {
 		state->revert_in_progress = 1;
@@ -1824,6 +1825,7 @@ void wt_status_get_state(struct repository *r,
 	if (get_detached_from)
 		wt_status_get_detached_from(r, state);
 	wt_status_check_sparse_checkout(r, state);
+	free_worktrees(worktrees);
 }
 
 static void wt_longstatus_print_state(struct wt_status *s)
-- 
2.53.0