Re: [PATCH 03/10] lazily load core.sharedrepository

[Jeff King <peff@peff.net>]

On Thu, Mar 03, 2016 at 08:00:03PM +0700, Duy Nguyen wrote:

> On Tue, Mar 1, 2016 at 9:39 PM, Jeff King <peff@peff.net> wrote:
> > +       if (need_shared_repository_from_config) {
> > +               const char *var = "core.sharedrepository";
> > +               const char *value;
> > +               if (!git_config_get_value(var, &value))
> > +                       the_shared_repository = git_config_perm(var, value);
> > +               need_shared_repository_from_config = 0;
> > +       }
> 
> If config cache is invalidated and reloaded by some crazy code (alias
> code, most likely, but I didn't check), we could be in trouble. Is
> there any clean way we could hook in git_config_clear() and reset
> need_shared..  back  to 1 (or something of similar effect)? Or perhaps
> maintain a "clear counter", increased at every clear, and we keep a
> copy here, so we know if any more clear() has been called between
> get_shared..() calls.

Yeah, this caches forever. But I'm not sure that's really different from
the existing code, which sets it in check_repository_format_gently(),
which is only called during setup_git_directory(), which cannot be
called twice. So it was effectively static for the full program anyway
(modulo people tweaking the integer variable manually, which is
supported here).

I think something like the "clear counter" you describe is also not
quite sufficient. We have two sources for the variable: the config, and
manually setting it. Once it has been manually set, I think we don't
want to auto-read it from the config anymore (and stomp over what
somebody set). I'm not sure even git_config_clear() is enough of a
single to say "...and also reset everything that might have been set
from these config variables in the past". I think we'd want to do
per-variable invalidation, depending on what the caller wants.

So I'd rather punt on it for now, unless this is breaking a specific
case. I think it should behave the same as the status quo.

-Peff