git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v4 0/5] git_config callers rewritten with the new config cache API

From
Ramsay Jones <ramsay@ramsay1.demon.co.uk>
Date
Jul 31, 2014, 11:37 UTC
Message-ID
<53DA2A5E.4020101@ramsay1.demon.co.uk>
In-Reply-To
<vpqiomeokx7.fsf@anie.imag.fr>
On 30/07/14 17:45, Matthieu Moy wrote:
Show 11 quoted lines
> Tanay Abhra <tanayabh@gmail.com> writes:
>> Also, any thoughts on what to do with git_default_config()? We can,
>>
>> 1> make a new function git_load_default_config(), use it for the rewrites.
> 
> That seems the most sensible option. It could be called it git.c before
> the command-dependant part, so that any call to git loads this.
> 
> I didn't check if it was correct (e.g. do some command rely on the fact
> that the default config is not loaded?)
> 
Hmm, here be dragons ... :-P

I don't know that there has actually been any kind of policy regarding the reading of config files/variables in git (or if there is a different policy for plumbing vs porcelain), but it has always seemed to be somewhat ad-hoc; each command decides for itself what it wants to read.

However, with increased use of common code which _uses_ certain config variables for correct operation, the 'choice' is much harder to make (and may change after the fact!).

For example, about a year ago I submitted a couple of patches which added a call to git_config(git_default_config, NULL) to both 'git pack-refs' and 'git show-refs'. This was as a result of the 'mh/ref-races' branch which introduced a 'stat_validity' api for checking if the packed-refs file had changed on the filesystem since last you looked. This re-used some of the same code used to handle index updates that used config variables like core.checkstat and core.trustctime. These config variables can affect the correctness and/or the efficiency of the code on some platforms (e.g. cygwin, mingw).

[Note: 'stat_validity' has since been re-used again (why not?) in some shallow clone code, so similar comments may apply ... I haven't looked.]

However, those patches were dropped, because it resulted in an (unwanted) change in behaviour. In particular, 'git show-refs' changed behaviour because it now 'listened' to core.abbrev!

I started to look at splitting the 'core config variables' into two groups; essential variables that _all_ git commands should read for correct/efficient/consistent behaviour and everything else (mainly UI related variables).

However, something else came up ...
Just an FYI.

ATB, Ramsay Jones

Previous: Matthieu Moy
Message 16 of 16 in “git_config callers rewritten with the new config cache API”
  1. 0/5 git_config callers rewritten with the new config cache APITanay Abhra, Jul 30, 2014
  2. 1/5 pager.c: replace `git_config()` with `git_config_get_value()`Tanay Abhra, Jul 30, 2014
  3. 2/5 notes.c: replace `git_config()` with `git_config_get_value_multi()`Tanay Abhra, Jul 30, 2014
  4. Matthieu MoyJul 30, 2014
  5. Tanay AbhraJul 30, 2014
  6. Matthieu MoyJul 30, 2014
  7. Matthieu MoyJul 31, 2014
  8. Tanay AbhraJul 31, 2014
  9. 3/5 imap-send.c: replace `git_config()` with `git_config_get_*()` familyTanay Abhra, Jul 30, 2014
  10. 4/5 branch.c: replace `git_config()` with `git_config_get_string()Tanay Abhra, Jul 30, 2014
  11. Matthieu MoyJul 30, 2014
  12. 5/5 alias.c: replace `git_config()` with `git_config_get_string()`Tanay Abhra, Jul 30, 2014
  13. Matthieu MoyJul 30, 2014
  14. Tanay AbhraJul 30, 2014
  15. Matthieu MoyJul 30, 2014
  16. Ramsay JonesJul 31, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.