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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 06/22, Junio C Hamano wrote:
> Brandon Williams <bmwill@xxxxxxxxxx> writes:
> 
> >  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.
> 

That makes sense.  While at it, would it make sense to ensure that the
'struct index_state *' which is stored in 'the_repository.index' be
'&the_index'?

-- 
Brandon Williams



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux