Re: [PATCH v4 15/20] repository: add index_state to struct repo

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 22, 2017, 20:16 UTC
Message-ID: <xmqqshirixrt.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <20170622184348.56497-16-bmwill@google.com>

Brandon Williams <bmwill@google.com> writes:

Show 22 quoted lines

>  struct repository {
>  	/* Environment */
> @@ -49,6 +50,12 @@ struct repository {
>  	 */
>  	struct config_set *config;
>  
> +	/*
> +	 * Repository's in-memory index.
> +	 * 'repo_read_index()' can be used to populate 'index'.
> +	 */
> +	struct index_state *index;
> +
>  	/* Configurations */
>  	/*
>  	 * Bit used during initialization to indicate if repository state (like
> @@ -71,4 +78,6 @@ extern void repo_set_worktree(struct repository *repo, const char *path);
>  extern int repo_init(struct repository *repo, const char *gitdir, const char *worktree);
>  extern void repo_clear(struct repository *repo);
>  
> +extern int repo_read_index(struct repository *repo);
> +
>  #endif /* REPOSITORY_H */

While you are working on a simple read-only operation like "ls-files", you can get away with just a singleton (the equivalent to "the_index" in the repository object world) in-core index instance, and having a way to tell the system to read things into the default thing without having to name what that default thing is is a very useful thing to have. It is a good thing that this only needs "struct repository *" and no "struct index_state *" parameter.

But you will need a way to read, update, write it out as a tree, etc. to a non-default in-core index, once you start doing more complex things. The function signature of the lowest level primitive helper for them will be:

    extern int (*)(struct repository *, struct index_state *);

And you would want to reserve "index" suffix for such a function, following the "if you use the default in-core thing, you do not have to pass it as a parameter---just call _cache() convenience macro; but you can explicitly pass it to the underlying _index() function" convention we established long time ago.

So I'd suggest renaming the above one that uses the default in-core index instance to

	extern int repo_read_cache(struct repository *);

or you will regret later.