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@xxxxxxxx> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html