On 06/10, Jeff King wrote: > On Sat, Jun 10, 2017 at 02:07:12AM -0400, Jeff King wrote: > > > 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 > > > > [...] > > > > [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 > > Ah, this was just hitting the bug mentioned later in the thread. With > that fix, I can see that it does indeed behave differently than the > current code: > > git config core.quotepath true > git -C submodule config core.quotepath false > (cd submodule && > echo hello >buenos_días && > git add . > ) > git ls-files --recurse-submodules > > shows: > > submodule/buenos_días > > before the patch series, and: > > "submodule/buenos_d\303\255as" > > after. > > Like I said, I doubt this is a bug that anybody cares much about, but > it's hard to know what other repo-specific global-variable usage is > lurking in low-level code.
I disagree with a few points of what jonathan said (mostly about removing the config from the repo object, as I like the idea of nothing knowing about a 'config_set' object) and I think this problem could be solved in a couple ways. I don't think that the in-memory global variable 'quotepath' (or whatever its called) should live in the repository object (I mean it's already contained in the config) but rather 'quotepath' is specific to how ls-files handles its output. So really what should happen is you pass a pair of objects to the ls-files machinery (or any other command's machinery) (1) the repository object being operated on and (2) an options struct which can be configured based on the repository. So when recursing you can do something like the following: repo_init(submodule, path_to_submodule); configure_opts(sub_opts, submodule, super_opts) ls_files(submodule, sub_opts); This eliminates bloating 'struct repository' and would allow you to have things configured differently in submodules (which is crazy if you ask me, but people do crazy things). -- Brandon Williams
