On ma, 2016-09-12 at 23:22 -0400, Jeff King wrote: > The motivation for this series is to fix the regression in v2.9 where > core.logallrefupdates is sometimes not set properly in a newly > initialized repository, as described in this thread: > > http://public-inbox.org/git/c46d36ef-3c2e-374f-0f2e-ffe31104e...@gmx.de/T/#u > > The root of the problem is that we are overly eager to read and use > config from ".git/config", even when we have not established that it is > part of a repository. This is especially bad for git-init, which would > not want to read anything until we've created the new repo. > > So the two interesting parts of the fix are: > > 1. We stop blindly reading ".git/config" when we don't know there's an > actual git directory. This is in patch 14, and is actually enough > to fix the v2.9 regression. > > 2. We are more thorough about dropping any cached config values when > we move into the new repository in git-init (patch 16). > > I didn't dig into when this was broken, but it was probably when we > switched git_config() to use cached values in the v2.2.0 > time-frame. > > Doing (1) required fixing up some builtins that depended on the blind > .git/config thing, as the tests demonstrated. But I think this is a sign > that we are moving in the right direction, because each one of those > programs could easily be demonstrated to be broken in scenarios only > slightly more exotic than the test scripts (e.g., see patch 3 for one of > the simplest cases). > > So I think notwithstanding their use as prep for patch 14, patches 1-13 > fix useful bugs. > > I won't be surprised if there are other fallouts that were not caught by > the test suite (i.e., programs that expect to read config, don't do > RUN_SETUP, but aren't covered well by tests). I poked around the list of > builtins in git.c that do not use RUN_SETUP, and they seem to correctly > end up in setup_git_directory_gently() before reading config. But it's > possible I missed a case. > > So this is definitely a bit larger than I'd hope for a regression-fix to > maint. But anything that doesn't address this issue at the config layer > is going to end up as a bit of a hack, and I'd rather not pile up hacks > if we can avoid it.
Agreed with all of the above, this is much better than just fixing the symptom on the mailinglist thread that started this. > I've cc'd Dennis, who helped investigate solutions in the thread > mentioned above, and Duy, because historically he has been the one most > willing and able to battle the dragon of our setup code. :) > > [01/16]: t1007: factor out repeated setup > [02/16]: hash-object: always try to set up the git repository > [03/16]: patch-id: use RUN_SETUP_GENTLY > [04/16]: diff: skip implicit no-index check when given --no-index > [05/16]: diff: handle --no-index prefixes consistently > [06/16]: diff: always try to set up the repository > [07/16]: pager: remove obsolete comment > [08/16]: pager: stop loading git_default_config() > [09/16]: pager: make pager_program a file-local static > [10/16]: pager: use callbacks instead of configset > [11/16]: pager: handle early config > [12/16]: t1302: use "git -C" > [13/16]: test-config: setup git directory > [14/16]: config: only read .git/config from configured repos > [15/16]: init: expand comments explaining config trickery > [16/16]: init: reset cached config when entering new repo Couldn't find anything to comment on, and I've tested that this does indeed fix the symptoms we saw. Reviewed-by: Dennis Kaarsemaker <den...@kaarsemaker.net> -- Dennis Kaarsemaker http://www.kaarsemaker.net
