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

Re: [PATCH][RFC] read-cache: read_index_from() accepts repo as arg

From
Duy Nguyen <pclouds@gmail.com>
Date
Apr 7, 2019, 10:00 UTC
Message-ID
<20190407100010.GA23427@ash>
In-Reply-To
<20190407073712.1642-1-jkapil.cs@gmail.com>
On Sun, Apr 07, 2019 at 01:07:12PM +0530, Kapil Jain wrote:
Show 5 quoted lines
> Signed-off-by: Kapil Jain <jkapil.cs@gmail.com>
> ---
> 
> In read-cache, the read_index_from() function had a TODO task, this
> patch completes that.

This line at least should be above the "---" line (i.e. part of the commit message).

Show 10 quoted lines
> diff --git a/read-cache.c b/read-cache.c
> index 4dc6de1b55..0444703284 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -2256,7 +2256,7 @@ static void freshen_shared_index(const char *shared_index, int warn)
>  }
>  
>  int read_index_from(struct index_state *istate, const char *path,
> -		    const char *gitdir)
> +		    const char *gitdir, const struct repository *repo)
"struct repository *" by convention is always the first argument. See
https://public-inbox.org/git/xmqqsh2p6l43.fsf@gitster-ct.c.googlers.com/

You also do not need "gitdir" as a separate argument because gitdir is an attribute of a repository. Passing it separately is just a trap to give inconsistent information (e.g. one repo and one gitdir from another one).

I see you already use repo->gitdir below. Which means this argument "gitdir" is no longer used anyway. Please remove.

Show 11 quoted lines
>  {
>  	struct split_index *split_index;
>  	int ret;
> @@ -2292,7 +2292,7 @@ int read_index_from(struct index_state *istate, const char *path,
>  		split_index->base = xcalloc(1, sizeof(*split_index->base));
>  
>  	base_oid_hex = oid_to_hex(&split_index->base_oid);
> -	base_path = xstrfmt("%s/sharedindex.%s", gitdir, base_oid_hex);
> +	base_path = xstrfmt("%s/sharedindex.%s", repo->gitdir, base_oid_hex);
>  	trace2_region_enter_printf("index", "shared/do_read_index",
>  				   the_repository, "%s", base_path);

"the_repository" here (and all others in this function) should be replaced with "repo". The TODO comment in this function should be removed as well.

Show 11 quoted lines
>  	ret = do_read_index(split_index->base, base_path, 1);
> diff --git a/revision.c b/revision.c
> index eb8e51bc63..247a4d5704 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -1556,7 +1556,7 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
>  
>  		if (read_index_from(&istate,
>  				    worktree_git_path(wt, "index"),
> -				    get_worktree_git_dir(wt)) > 0)
> +				    get_worktree_git_dir(wt), the_repository) > 0)

We have revs->repo to refer to the main repo being examined here. But if the "gitdir" argument is deleted, then revs->repo->gitdir gives a _wrong_ gitdir, it's not the same as get_worktree_git_dir(wt).

So before you can continue here, you'll need to add a function, e.g. repo_worktree_init() (similar to repo_init() and repo_submodule_init()). This one should take "struct worktree *" as the argument and return "struct repository *"

So, with something like a patch below (not tested), you should be able to write

	worktrees = repo_get_worktrees(revs->repo, 0);
	...
		struct repository *r = repo_worktree_init(wt);
		if (read_index_from(&istate, r->index_file, r) > 0)
			do_add_index_objects_to_pending(revs, &istate, flags);
		repo_clear(r);
-- 8< --
diff --git a/repository.c b/repository.c
index 682c239fe3..5a0c7972db 100644
--- a/repository.c
+++ b/repository.c
@@ -10,6 +10,7 @@
 #include "object.h"
 #include "lockfile.h"
 #include "submodule-config.h"
+#include "worktree.h"
 
 /* The main repository */
 static struct repository the_repo;
@@ -227,6 +228,28 @@ int repo_submodule_init(struct repository *subrepo,
 	return ret;
 }
 
+int repo_worktree_init(struct repository *repo,
+		       struct repository *source_repo,
+		       const struct worktree *wt)
+{
+	struct strbuf gitdir = STRBUF_INIT;
+	int ret;
+
+	if (!wt)
+		return -1;
+
+	if (!wt->id)
+		strbuf_addstr(&gitdir, source_repo->commondir);
+	else
+		strbuf_addf(&gitdir, "%s/worktrees/%s",
+			    source_repo->commondir,
+			    wt->id);
+
+	ret = repo_init(source_repo, gitdir.buf, wt->path);
+	strbuf_release(&gitdir);
+	return ret ? -1 : 0;
+}
+
 void repo_clear(struct repository *repo)
 {
 	FREE_AND_NULL(repo->gitdir);
diff --git a/repository.h b/repository.h
index 4fb6a5885f..d3c21c7ab5 100644
--- a/repository.h
+++ b/repository.h
@@ -122,6 +122,10 @@ void repo_set_hash_algo(struct repository *repo, int algo);
 void initialize_the_repository(void);
 int repo_init(struct repository *r, const char *gitdir, const char *worktree);
 
+struct worktree;
+int repo_worktree_init(struct repository *r, struct repository *source,
+		       const struct worktree *wt);
+
 /*
  * Initialize the repository 'subrepo' as the submodule given by the
  * struct submodule 'sub' in parent repository 'superproject'.
diff --git a/worktree.c b/worktree.c
index b45bfeb9d3..188ea04c61 100644
--- a/worktree.c
+++ b/worktree.c
@@ -44,19 +44,19 @@ static void add_head_info(struct worktree *wt)
 /**
  * get the main worktree
  */
-static struct worktree *get_main_worktree(void)
+static struct worktree *get_main_worktree(struct repository *r)
 {
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
 	struct strbuf worktree_path = STRBUF_INIT;
 	int is_bare = 0;
 
-	strbuf_add_absolute_path(&worktree_path, get_git_common_dir());
+	strbuf_add_absolute_path(&worktree_path, r->commondir);
 	is_bare = !strbuf_strip_suffix(&worktree_path, "/.git");
 	if (is_bare)
 		strbuf_strip_suffix(&worktree_path, "/.");
 
-	strbuf_addf(&path, "%s/HEAD", get_git_common_dir());
+	strbuf_addf(&path, "%s/HEAD", r->commondir);
 
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -68,7 +68,7 @@ static struct worktree *get_main_worktree(void)
 	return worktree;
 }
 
-static struct worktree *get_linked_worktree(const char *id)
+static struct worktree *get_linked_worktree(struct repository *r, const char *id)
 {
 	struct worktree *worktree = NULL;
 	struct strbuf path = STRBUF_INIT;
@@ -77,7 +77,7 @@ static struct worktree *get_linked_worktree(const char *id)
 	if (!id)
 		die("Missing linked worktree name");
 
-	strbuf_git_common_path(&path, the_repository, "worktrees/%s/gitdir", id);
+	strbuf_git_common_path(&path, r, "worktrees/%s/gitdir", id);
 	if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
 		/* invalid gitdir file */
 		goto done;
@@ -90,7 +90,7 @@ static struct worktree *get_linked_worktree(const char *id)
 	}
 
 	strbuf_reset(&path);
-	strbuf_addf(&path, "%s/worktrees/%s/HEAD", get_git_common_dir(), id);
+	strbuf_addf(&path, "%s/worktrees/%s/HEAD", r->commondir, id);
 
 	worktree = xcalloc(1, sizeof(*worktree));
 	worktree->path = strbuf_detach(&worktree_path, NULL);
@@ -127,7 +127,7 @@ static int compare_worktree(const void *a_, const void *b_)
 	return fspathcmp((*a)->path, (*b)->path);
 }
 
-struct worktree **get_worktrees(unsigned flags)
+struct worktree **repo_get_worktrees(struct repository *r, unsigned flags)
 {
 	struct worktree **list = NULL;
 	struct strbuf path = STRBUF_INIT;
@@ -137,9 +137,9 @@ struct worktree **get_worktrees(unsigned flags)
 
 	ALLOC_ARRAY(list, alloc);
 
-	list[counter++] = get_main_worktree();
+	list[counter++] = get_main_worktree(r);
 
-	strbuf_addf(&path, "%s/worktrees", get_git_common_dir());
+	strbuf_addf(&path, "%s/worktrees", r->commondir);
 	dir = opendir(path.buf);
 	strbuf_release(&path);
 	if (dir) {
@@ -148,7 +148,7 @@ struct worktree **get_worktrees(unsigned flags)
 			if (is_dot_or_dotdot(d->d_name))
 				continue;
 
-			if ((linked = get_linked_worktree(d->d_name))) {
+			if ((linked = get_linked_worktree(r, d->d_name))) {
 				ALLOC_GROW(list, counter + 1, alloc);
 				list[counter++] = linked;
 			}
diff --git a/worktree.h b/worktree.h
index 9e3b0b7b6f..cb4307db16 100644
--- a/worktree.h
+++ b/worktree.h
@@ -30,7 +30,8 @@ struct worktree {
  * The caller is responsible for freeing the memory from the returned
  * worktree(s).
  */
-extern struct worktree **get_worktrees(unsigned flags);
+struct worktree **repo_get_worktrees(struct repository *r, unsigned flags);
+#define get_worktrees(flags) repo_get_worktrees(the_repository, flags)
 
 /*
  * Returns 1 if linked worktrees exist, 0 otherwise.
-- 8< --



>  			do_add_index_objects_to_pending(revs, &istate, flags);
>  		discard_index(&istate);
>  	}
> -- 
> 2.20.1
>
Previous: Kapil JainNext: Duy Nguyen
Message 2 of 4 in “[RFC] read-cache: read_index_from() accepts repo as arg”
  1. [RFC] read-cache: read_index_from() accepts repo as argKapil Jain, Apr 7, 2019
  2. Duy NguyenApr 7, 2019
  3. Duy NguyenApr 7, 2019
  4. Taylor BlauApr 9, 2019

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.