Subject: Re: [PATCH v3 1/2] difftool: add a skeleton for the upcoming builtin

From: Brandon Williams <[email-masked]>
Date: Wed, 18 Jan 2017 14:38:17 -0800

On 12/06, Jeff King wrote:
> On Tue, Dec 06, 2016 at 10:22:21AM -0800, Stefan Beller wrote:
> 
> > >> Maybe even go a step further and say that the config code needs a context
> > >> "object".
> > >
> > > If I were writing git from scratch, I'd consider making a "struct
> > > repository" object. I'm not sure how painful it would be to retro-fit it
> > > at this point.
> > 
> > Would it be possible to introduce "the repo" struct similar to "the index"
> > in cache.h?
> > 
> > From a submodule perspective I would very much welcome this
> > object oriented approach to repositories.
> 
> I think it may be more complicated, because there's some implicit global
> state in "the repo", like where files are relative to our cwd. All of
> those low-level functions would have to start caring about which repo
> we're talking about so they can prefix the appropriate working tree
> path, etc.
> 
> For some operations that would be fine, but there are things that would
> subtly fail for submodules. I'm thinking we'd end up with some code
> state like:
> 
>   /* finding a repo does not modify global state; good */
>   struct repository *repo = repo_discover(".");
> 
>   /* obvious repo-level operations like looking up refs can be done with
>    * a repository object; good */
>   repo_for_each_ref(repo, callback, NULL);
> 
>   /*
>    * "enter" the repo so that we are at the top-level of the working
>    * tree, etc. After this you can actually look at the index without
>    * things breaking.
>    */
>   repo_enter(repo);
> 
> That would be enough to implement a lot of submodule-level stuff, but it
> would break pretty subtly as soon as you asked the submodule about its
> working tree. The solution is to make everything that accesses the
> working tree aware of the idea of a working tree root besides the cwd.
> But that's a pretty invasive change.
> 
> -Peff

Some other challenges would be how to address people setting environment
variables like GIT_DIR that indicate the location of a repositories git
directory, which wouldn't work if you have multiple repos open.

I do agree that having a repo object of some sort would aid in
simplifying submodule operations but may require too many invasive
changes to basic low-level functions.

-- 
Brandon Williams