On 06/10, Jeff King wrote: > On Fri, Jun 09, 2017 at 05:40:34PM -0700, Jonathan Tan wrote: > > > Before I get into the details, I have some questions: > > > > 1. I am concerned that "struct repository" will end up growing without > > bounds as we store more and more repo-specific concerns in it. Could it > > be restricted to just the fields populated by repo_init()? > > repo_read_index() will then return the index itself, instead of using > > "struct repository" as a cache. This means that code using > > repo_read_index() will need to maintain its own variable holding the > > returned index, but that is likely a positive - it's better for code to > > just pass around the specific thing needed between functions anyway, as > > opposed to passing a giant "struct repository" (which partially defeats > > the purpose of eliminating the usage of globals). > > I think the repository object has to become a kitchen sink of sorts, > because we have tons of global variables representing repo-wide config. > ls-files doesn't respect a lot of config, but what should, e.g.: > > git config core.quotepath true > git -C submodule config core.quotepath false > git ls-files --recurse-submodules > > do? Right now, with a separate process, we respect the submodule > version of the config. But in a single process[1] we'd need one copy of > the quote_path_fully variable for each repo object. It's tempting for > this case to say that core.quotepath from the super-project should just > take precedence, as that's where the command is issued from (and why the > heck would anybody have per-repo settings for this anyway?). But I > suspect as we get into more complicated commands that there are likely > to be config variables that are important to match to each repo. > > I do agree that "pass just what the sub-function needs" is a good rule > of thumb. But the reason that these are globals in the first place is > that there are a ton of them, and they are used at the lowest levels of > call chains. So I have a feeling that we're always going to need some > big object to hold all that context when doing multi-repo operations in > a single process. > > For config, in theory that could be a big "config_set" object, but > that's not quite how we treat our config. We usually parse it once into > actual variables. So really you end up with a big parsed-config object > that gets passed around, I'd think. > > -Peff > > [1] I wanted to see how Brandon's series behaved for this quotepath > case, but unfortunately I couldn't get it to work in even a simple > case. :( > > $ git ls-files --recurse-submodules > fatal: index file corrupt Yeah sorry about that...I commented on patch 32 indicating that I made a small mistake and forgot a '> 0' when checking the index. I made a last minute change before sending v2 out and forgot to run tests again (I'm terrible i know!). -- Brandon Williams