On Mon, 12 Jun 2017 12:11:21 -0700
Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> On 06/12, Jonathan Tan wrote:
> > On Sat, 10 Jun 2017 02:07:12 -0400
> > Jeff King <peff@xxxxxxxx> wrote:
> >
> > > 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.
> >
> > From my experience with the codebase, it seems that most of these config
> > variables are static (file-local). This means that the lowest levels of
> > call chains could probably get away with storing per-repo configs in a
> > static hashmap or associative array keyed by repo (if they cannot just
> > pass the config around).
> >
> > Having said that, if it did come to the hashmap, I probably would prefer
> > just putting the config in the repo object too. So maybe that is the way
> > to go.
>
> This is how the config is already handled. A config_set is just a
> wrapper around a fancy hashmap. Callers query using a string as a key
> and are returned the value for that config option. I say fancy because
> it does stuff to handle multiple values, etc.
The hashmap I meant is one wrapping the one you describe:
repo -> configset
Or equivalently:
repo -> (string -> value(s))
> I'm not sure I know what you mean by config variables which are static,
> are you referring to the in-memory options which are populated by
> querying the config? Those I wouldn't want to see placed in a
> 'repository object'.
Yes. I agree that I wouldn't want to see them placed in a repository
object, but after reading Peff's e-mail, I was thinking of what happens
if a file repeatedly invokes a config-sensitive function in another
file. For example:
a.c
for (i = 0; i < 100; i++) b_output(repo, x);
b.c
void b_output(struct repository *repo, int x)
{
/* print the configured "b.prefix" followed by x */
}
We probably wouldn't want to parse the repo's configset every time
b_output() is invoked, but then, where to store our parsed "b.prefix"?
The only alternatives I see is to have a static hashmap in b.c (keyed by
repo, as described above), which would work if such a situation is rare
(or can be made rare), but if it is common, maybe we have no choice but
to put it in struct repository.