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:
> 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
> 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>