Re: RFC GSoC idea: new "git config" features

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Mar 1, 2014, 00:19 UTC
Message-ID: <53112794.2070007@alum.mit.edu>
In-Reply-To: <xmqqwqgft3bj.fsf@gitster.dls.corp.google.com>

On 02/28/2014 09:00 PM, Junio C Hamano wrote:

Show 40 quoted lines

> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> I just wrote up another double-idea that has been stewing in my head for
>> a while:
>>
>> * Allow configuration values to be unset via a config file
>> * Fix "git config --unset" to clean up detritus from sections that are
>> left empty.
> 
> The former is *way* too large for a GSoC project.  Most
> configuration variables are meant to be read sequencially and affect
> in-core variables directly, like
> 
>         /* file-scope global */
> 	static int frotz = -1;  /* unset */
> 
>         static int parse_config_frotz(const char *key, const char *value, void *cb)
> 	{
>         	if (!strcmp(key, "core.frotz"))
>                 	frotz = git_config_int(value);
> 		return 0;
> 	}
> 
> 	... and somewhere ...
>         	git_config(parse_config_frotz, NULL);
> 
> The config parsers are distributed and there is no single registry
> that knows how in-core variables owned by each subsystem represent
> an "unset" value.  In the above example, -1 is such a sentinel
> value, but in some other contexts, the subsystem may choose to use
> INT_MAX.  The only way to allow "resetting to previous" is to
> 
>  (1) come up with a way to pass "this key is being reset to
>      'unspecified'" to existing git_config() callback functions
>      (like parse_config_frotz() in the above illustration), which
>      may or may not involve changing the function signature of the
>      callbacks;
> 
>  (2) go through all the git_config() callback functions and make
>      them understand the new "reset to 'unspecified'" convention.

I absolutely understand that changing all of the config parsers is not feasible. But I had imagined a third route:

(3) parse the config once, storing the raw values to records in
    memory.  When an "unset" is seen, delete any previous records that
    have accumulated for that key.  After the whole config has been
    read, iterate through the records, feeding the surviving values
    into the callback in the order they were originally read (minus
    deletions).

Do you see any problems with this way of implementing the functionality (aside from slightly increased overhead)?

And once we have a way to store config records in memory, it might also make sense to reuse the parsed values for later config inquiries (after checking that the files have not changed since the last read). After this second step the net performance change might even be advantageous.

Show 13 quoted lines

> which may not sound too bad at the first glance (especially, the
> first one is almost trivial).
> 
> But the side effects these callbacks may cause are not limited to
> setting a simple scaler variable (like 'frotz' in the illustration)
> but would include things that are hard to undo once done
> (e.g. calling a set-up function with a lot of side effects).
> 
> The latter, on the other hand, should be a change that is of a
> fairly limited scope, and would be a good fit for a GSoC project
> (incidentally, it has been one of the items on my leftover-bits list
> http://git-blame.blogspot.com/p/leftover-bits.html for quite some
> time).

But only the latter part would be a bit meager as a GSoC project, don't you think?

Thanks for the feedback.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/