Re: [PATCH v7 1/2] add `config_set` API for caching config-like files

From: Junio C Hamano <gitster@pobox.com>
Date: Jul 9, 2014, 17:44 UTC
Message-ID: <xmqq38ea77gt.fsf@gitster.dls.corp.google.com>
In-Reply-To: <xmqqmwci7e9p.fsf@gitster.dls.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 8 quoted lines

>> * Still, making sure that the (file, line) is doable later without too
>>   much change is good. So, if indeed, moving all code to another file is
>>   required, then it may make sense to do it now to avoid code move
>>   later.
>
> Good thinking.  As long as the code is prepared, it is a good idea
> to start small and bite off only we can chew at once, do things
> incrementally.

After thinking about it a bit more, I think <file, line> support needs to be done not as a mere client of the generic, uncached git_config() API that we have had for forever, like the current iteration, but needs to know a bit more about the state the caller of the callback (namely, git_parse_source()), and we obviously do not want to expose that machinery to anybody other than the implementation of the config subsystem (to which the new facility this GSoC project adds belongs to), so in that sense you have to have your code in the same config.c file anyway.

It is somewhat dissapointing that this initial implementation was done as a client of the traditional git_config(), by the way. I would have expected that it would be the other way around, in that the traditional callers of git_config() would behefit automatically by having the cache layer below it.

But we have to start from somewhere. Perhaps the round after this one can rename the exisiting implementation of git_config() to something else internal to the caching layer and give the existing callers a compatible interface that is backed by this new caching layer in a transparent way.