{"thread":{"id":"44070","subject":"[PATCH 0/16] fix config-reading in non-repos","startedAt":"2016-09-13T03:22:51Z","lastAt":"2016-09-14T15:31:46Z","messageCount":23,"participants":["Jeff King","Stefan Beller","Dennis Kaarsemaker"],"isPatch":true,"patchVersion":1,"patchTotal":16},"messages":[{"id":"301799","messageId":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","threadId":"44070","inReplyTo":null,"subject":"[PATCH 0/16] fix config-reading in non-repos","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:22:42Z","receivedAt":"2016-09-13T03:22:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The motivation for this series is to fix the regression in v2.9 where\ncore.logallrefupdates is sometimes not set properly in a newly\ninitialized repository, as described in this thread:\n\n  http://public-inbox.org/git/c46d36ef-3c2e-374f-0f2e-ffe31104e023@gmx.de/T/#u\n\nThe root of the problem is that we are overly eager to read and use\nconfig from \".git/config\", even when we have not established that it is\npart of a repository. This is especially bad for git-init, which would\nnot want to read anything until we've created the new repo.\n\nSo the two interesting parts of the fix are:\n\n  1. We stop blindly reading \".git/config\" when we don't know there's an\n     actual git directory. This is in patch 14, and is actually enough\n     to fix the v2.9 regression.\n\n  2. We are more thorough about dropping any cached config values when\n     we move into the new repository in git-init (patch 16).\n\n     I didn't dig into when this was broken, but it was probably when we\n     switched git_config() to use cached values in the v2.2.0\n     time-frame.\n\nDoing (1) required fixing up some builtins that depended on the blind\n.git/config thing, as the tests demonstrated. But I think this is a sign\nthat we are moving in the right direction, because each one of those\nprograms could easily be demonstrated to be broken in scenarios only\nslightly more exotic than the test scripts (e.g., see patch 3 for one of\nthe simplest cases).\n\nSo I think notwithstanding their use as prep for patch 14, patches 1-13\nfix useful bugs.\n\nI won't be surprised if there are other fallouts that were not caught by\nthe test suite (i.e., programs that expect to read config, don't do\nRUN_SETUP, but aren't covered well by tests). I poked around the list of\nbuiltins in git.c that do not use RUN_SETUP, and they seem to correctly\nend up in setup_git_directory_gently() before reading config. But it's\npossible I missed a case.\n\nSo this is definitely a bit larger than I'd hope for a regression-fix to\nmaint. But anything that doesn't address this issue at the config layer\nis going to end up as a bit of a hack, and I'd rather not pile up hacks\nif we can avoid it.\n\nI have a few patches on top that go even further and disallow the\nauto-fallback of looking in \".git\" at all for non-repositories. I think\nthat's the right thing to do, and after years of chipping away at the\nsetup code, I think we're finally at a point to make that change (with a\nfew fixes of course). But that's an even riskier change and not fixing\nan immediate regression. So I'll hold that back for now, and hopefully\nit would become \"master\" material once this is sorted out.\n\nI've cc'd Dennis, who helped investigate solutions in the thread\nmentioned above, and Duy, because historically he has been the one most\nwilling and able to battle the dragon of our setup code. :)\n\n  [01/16]: t1007: factor out repeated setup\n  [02/16]: hash-object: always try to set up the git repository\n  [03/16]: patch-id: use RUN_SETUP_GENTLY\n  [04/16]: diff: skip implicit no-index check when given --no-index\n  [05/16]: diff: handle --no-index prefixes consistently\n  [06/16]: diff: always try to set up the repository\n  [07/16]: pager: remove obsolete comment\n  [08/16]: pager: stop loading git_default_config()\n  [09/16]: pager: make pager_program a file-local static\n  [10/16]: pager: use callbacks instead of configset\n  [11/16]: pager: handle early config\n  [12/16]: t1302: use \"git -C\"\n  [13/16]: test-config: setup git directory\n  [14/16]: config: only read .git/config from configured repos\n  [15/16]: init: expand comments explaining config trickery\n  [16/16]: init: reset cached config when entering new repo\n\n-Peff\n"},{"id":"301800","messageId":"20160913032312.yheky2o25lmibvmb@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 01/16] t1007: factor out repeated setup","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:12Z","receivedAt":"2016-09-13T03:23:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We have a series of 3 CRLF tests that do exactly the same\n(long) setup sequence. Let's pull it out into a common setup\ntest, which is shorter, more efficient, and will make it\neasier to add new tests.\n\nNote that we don't have to worry about cleaning up any of\nthe setup which was previously per-test; we call pop_repo\nafter the CRLF tests, which cleans up everything.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1007-hash-object.sh | 32 ++++++++------------------------\n 1 file changed, 8 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh\nindex 7d2baa1..c89faa6 100755\n--- a/t/t1007-hash-object.sh\n+++ b/t/t1007-hash-object.sh\n@@ -101,7 +101,7 @@ test_expect_success 'git hash-object --stdin file1 <file0 first operates on file\n \ttest \"$obname1\" = \"$obname1new\"\n '\n \n-test_expect_success 'check that appropriate filter is invoke when --path is used' '\n+test_expect_success 'set up crlf tests' '\n \techo fooQ | tr Q \"\\\\015\" >file0 &&\n \tcp file0 file1 &&\n \techo \"file0 -crlf\" >.gitattributes &&\n@@ -109,7 +109,10 @@ test_expect_success 'check that appropriate filter is invoke when --path is used\n \tgit config core.autocrlf true &&\n \tfile0_sha=$(git hash-object file0) &&\n \tfile1_sha=$(git hash-object file1) &&\n-\ttest \"$file0_sha\" != \"$file1_sha\" &&\n+\ttest \"$file0_sha\" != \"$file1_sha\"\n+'\n+\n+test_expect_success 'check that appropriate filter is invoke when --path is used' '\n \tpath1_sha=$(git hash-object --path=file1 file0) &&\n \tpath0_sha=$(git hash-object --path=file0 file1) &&\n \ttest \"$file0_sha\" = \"$path0_sha\" &&\n@@ -117,38 +120,19 @@ test_expect_success 'check that appropriate filter is invoke when --path is used\n \tpath1_sha=$(cat file0 | git hash-object --path=file1 --stdin) &&\n \tpath0_sha=$(cat file1 | git hash-object --path=file0 --stdin) &&\n \ttest \"$file0_sha\" = \"$path0_sha\" &&\n-\ttest \"$file1_sha\" = \"$path1_sha\" &&\n-\tgit config --unset core.autocrlf\n+\ttest \"$file1_sha\" = \"$path1_sha\"\n '\n \n test_expect_success 'check that --no-filters option works' '\n-\techo fooQ | tr Q \"\\\\015\" >file0 &&\n-\tcp file0 file1 &&\n-\techo \"file0 -crlf\" >.gitattributes &&\n-\techo \"file1 crlf\" >>.gitattributes &&\n-\tgit config core.autocrlf true &&\n-\tfile0_sha=$(git hash-object file0) &&\n-\tfile1_sha=$(git hash-object file1) &&\n-\ttest \"$file0_sha\" != \"$file1_sha\" &&\n \tnofilters_file1=$(git hash-object --no-filters file1) &&\n \ttest \"$file0_sha\" = \"$nofilters_file1\" &&\n \tnofilters_file1=$(cat file1 | git hash-object --stdin) &&\n-\ttest \"$file0_sha\" = \"$nofilters_file1\" &&\n-\tgit config --unset core.autocrlf\n+\ttest \"$file0_sha\" = \"$nofilters_file1\"\n '\n \n test_expect_success 'check that --no-filters option works with --stdin-paths' '\n-\techo fooQ | tr Q \"\\\\015\" >file0 &&\n-\tcp file0 file1 &&\n-\techo \"file0 -crlf\" >.gitattributes &&\n-\techo \"file1 crlf\" >>.gitattributes &&\n-\tgit config core.autocrlf true &&\n-\tfile0_sha=$(git hash-object file0) &&\n-\tfile1_sha=$(git hash-object file1) &&\n-\ttest \"$file0_sha\" != \"$file1_sha\" &&\n \tnofilters_file1=$(echo \"file1\" | git hash-object --stdin-paths --no-filters) &&\n-\ttest \"$file0_sha\" = \"$nofilters_file1\" &&\n-\tgit config --unset core.autocrlf\n+\ttest \"$file0_sha\" = \"$nofilters_file1\"\n '\n \n pop_repo\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301801","messageId":"20160913032317.eyddugalfb2atr7v@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 02/16] hash-object: always try to set up the git repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:17Z","receivedAt":"2016-09-13T03:23:50Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When \"hash-object\" is run without \"-w\", we don't need to be\nin a git repository at all; we can just hash the object and\nwrite its sha1 to stdout. However, if we _are_ in a git\nrepository, we would want to know that so we can follow the\nnormal rules for respecting config, .gitattributes, etc.\n\nThis happens to work at the top-level of a git repository\nbecause we blindly read \".git/config\", but as the included\ntest shows, it does not work when you are in a subdirectory.\n\nThe solution is to just do a \"gentle\" setup in this case. We\nalready take care to use prefix_filename() on any filename\narguments we get (to handle the \"-w\" case), so we don't need\nto do anything extra to handle the side effects of repo\nsetup.\n\nAn alternative would be to specify RUN_SETUP_GENTLY for this\ncommand in git.c, and then die if \"-w\" is set but we are not\nin a repository. However, the error messages generated at\nthe time of setup_git_directory() are more detailed, so it's\nbetter to find out which mode we are in, and then call the\nappropriate function.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/hash-object.c  | 13 ++++++++-----\n t/t1007-hash-object.sh | 11 +++++++++++\n 2 files changed, 19 insertions(+), 5 deletions(-)\n\ndiff --git a/builtin/hash-object.c b/builtin/hash-object.c\nindex f7d3567..9028e1f 100644\n--- a/builtin/hash-object.c\n+++ b/builtin/hash-object.c\n@@ -87,6 +87,7 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)\n \tint stdin_paths = 0;\n \tint no_filters = 0;\n \tint literally = 0;\n+\tint nongit = 0;\n \tunsigned flags = HASH_FORMAT_CHECK;\n \tconst char *vpath = NULL;\n \tconst struct option hash_object_options[] = {\n@@ -107,12 +108,14 @@ int cmd_hash_object(int argc, const char **argv, const char *prefix)\n \targc = parse_options(argc, argv, NULL, hash_object_options,\n \t\t\t     hash_object_usage, 0);\n \n-\tif (flags & HASH_WRITE_OBJECT) {\n+\tif (flags & HASH_WRITE_OBJECT)\n \t\tprefix = setup_git_directory();\n-\t\tprefix_length = prefix ? strlen(prefix) : 0;\n-\t\tif (vpath && prefix)\n-\t\t\tvpath = prefix_filename(prefix, prefix_length, vpath);\n-\t}\n+\telse\n+\t\tprefix = setup_git_directory_gently(&nongit);\n+\n+\tprefix_length = prefix ? strlen(prefix) : 0;\n+\tif (vpath && prefix)\n+\t\tvpath = prefix_filename(prefix, prefix_length, vpath);\n \n \tgit_config(git_default_config, NULL);\n \ndiff --git a/t/t1007-hash-object.sh b/t/t1007-hash-object.sh\nindex c89faa6..acca9ac 100755\n--- a/t/t1007-hash-object.sh\n+++ b/t/t1007-hash-object.sh\n@@ -123,6 +123,17 @@ test_expect_success 'check that appropriate filter is invoke when --path is used\n \ttest \"$file1_sha\" = \"$path1_sha\"\n '\n \n+test_expect_success 'gitattributes also work in a subdirectory' '\n+\tmkdir subdir &&\n+\t(\n+\t\tcd subdir &&\n+\t\tsubdir_sha0=$(git hash-object ../file0) &&\n+\t\tsubdir_sha1=$(git hash-object ../file1) &&\n+\t\ttest \"$file0_sha\" = \"$subdir_sha0\" &&\n+\t\ttest \"$file1_sha\" = \"$subdir_sha1\"\n+\t)\n+'\n+\n test_expect_success 'check that --no-filters option works' '\n \tnofilters_file1=$(git hash-object --no-filters file1) &&\n \ttest \"$file0_sha\" = \"$nofilters_file1\" &&\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301802","messageId":"20160913032336.cur3rrzpgepulqi6@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 06/16] diff: always try to set up the repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:36Z","receivedAt":"2016-09-13T03:23:53Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we see an explicit \"--no-index\", we do not bother calling\nsetup_git_directory_gently() at all. This means that we may\nmiss out on reading repo-specific config.\n\nIt's arguable whether this is correct or not. If we were\ndesigning from scratch, making \"git diff --no-index\"\ncompletely ignore the repository makes some sense. But we\nare nowhere near scratch, so let's look at the existing\nbehavior:\n\n  1. If you're in the top-level of a repository and run an\n     explicit \"diff --no-index\", the config subsystem falls\n     back to reading \".git/config\", and we will respect repo\n     config.\n\n  2. If you're in a subdirectory of a repository, then we\n     still try to read \".git/config\", but it generally\n     doesn't exist. So \"diff --no-index\" there does not\n     respect repo config.\n\n  3. If you have $GIT_DIR set in the environment, we read\n     and respect $GIT_DIR/config,\n\n  4. If you run \"git diff /tmp/foo /tmp/bar\" to get an\n     implicit no-index, we _do_ run the repository setup,\n     and set $GIT_DIR (or respect an existing $GIT_DIR\n     variable). We find the repo config no matter where we\n     started, and respect it.\n\nSo we already respect the repository config in a number of\ncommon cases, and case (2) is the only one that does not.\nAnd at least one of our tests, t4034, depends on case (1)\nbehaving as it does now (though it is just incidental, not\nan explicit test for this behavior).\n\nSo let's bring case (2) in line with the others by always\nrunning the repository setup, even with an explicit\n\"--no-index\". We shouldn't need to change anything else, as the\nimplicit case already handles the prefix.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/diff.c           |  4 ++--\n t/t4053-diff-no-index.sh | 20 ++++++++++++++++++++\n 2 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex a31643c..7f91f6d 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -301,9 +301,9 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\tbreak;\n \t}\n \n-\tif (!no_index) {\n-\t\tprefix = setup_git_directory_gently(&nongit);\n+\tprefix = setup_git_directory_gently(&nongit);\n \n+\tif (!no_index) {\n \t\t/*\n \t\t * Treat git diff with at least one path outside of the\n \t\t * repo the same as if the command would have been executed\ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex e60c951..453e6c3 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -107,4 +107,24 @@ test_expect_success 'diff from repo subdir shows real paths (implicit)' '\n \ttest_cmp expect actual.head\n '\n \n+test_expect_success 'diff --no-index from repo subdir respects config (explicit)' '\n+\techo \"diff --git ../../non/git/a ../../non/git/b\" >expect &&\n+\ttest_config -C repo diff.noprefix true &&\n+\ttest_expect_code 1 \\\n+\t\tgit -C repo/sub \\\n+\t\tdiff --no-index ../../non/git/a ../../non/git/b >actual &&\n+\thead -n 1 <actual >actual.head &&\n+\ttest_cmp expect actual.head\n+'\n+\n+test_expect_success 'diff --no-index from repo subdir respects config (implicit)' '\n+\techo \"diff --git ../../non/git/a ../../non/git/b\" >expect &&\n+\ttest_config -C repo diff.noprefix true &&\n+\ttest_expect_code 1 \\\n+\t\tgit -C repo/sub \\\n+\t\tdiff ../../non/git/a ../../non/git/b >actual &&\n+\thead -n 1 <actual >actual.head &&\n+\ttest_cmp expect actual.head\n+'\n+\n test_done\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301803","messageId":"20160913032327.d4v5s4h3wfy4vj3y@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 04/16] diff: skip implicit no-index check when given --no-index","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:27Z","receivedAt":"2016-09-13T03:23:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We can invoke no-index mode in two ways: by an explicit\nrequest from the user, or implicitly by noticing that we\nhave two paths, and at least one is outside the repository.\n\nIf the user already told us --no-index, there is no need for\nus to do the implicit test at all.  However, we currently\ndo, and downgrade our \"explicit\" to DIFF_NO_INDEX_IMPLICIT.\n\nThis doesn't have any user-visible behavior, though it's not\nimmediately obvious why. We only trigger the implicit check\nwhen we have exactly two non-option arguments. And the only\ncode that cares about implicit versus explicit is an error\nmessage that we show when we _don't_ have two non-option\narguments.\n\nHowever, it's worth fixing anyway. Besides being slightly\nmore efficient, it makes the code easier to follow, which\nwill help when we modify it in future patches.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/diff.c | 25 +++++++++++++------------\n 1 file changed, 13 insertions(+), 12 deletions(-)\n\ndiff --git a/builtin/diff.c b/builtin/diff.c\nindex b7a9405..a31643c 100644\n--- a/builtin/diff.c\n+++ b/builtin/diff.c\n@@ -301,20 +301,21 @@ int cmd_diff(int argc, const char **argv, const char *prefix)\n \t\t\tbreak;\n \t}\n \n-\tif (!no_index)\n+\tif (!no_index) {\n \t\tprefix = setup_git_directory_gently(&nongit);\n \n-\t/*\n-\t * Treat git diff with at least one path outside of the\n-\t * repo the same as if the command would have been executed\n-\t * outside of a git repository.  In this case it behaves\n-\t * the same way as \"git diff --no-index <a> <b>\", which acts\n-\t * as a colourful \"diff\" replacement.\n-\t */\n-\tif (nongit || ((argc == i + 2) &&\n-\t\t       (!path_inside_repo(prefix, argv[i]) ||\n-\t\t\t!path_inside_repo(prefix, argv[i + 1]))))\n-\t\tno_index = DIFF_NO_INDEX_IMPLICIT;\n+\t\t/*\n+\t\t * Treat git diff with at least one path outside of the\n+\t\t * repo the same as if the command would have been executed\n+\t\t * outside of a git repository.  In this case it behaves\n+\t\t * the same way as \"git diff --no-index <a> <b>\", which acts\n+\t\t * as a colourful \"diff\" replacement.\n+\t\t */\n+\t\tif (nongit || ((argc == i + 2) &&\n+\t\t\t       (!path_inside_repo(prefix, argv[i]) ||\n+\t\t\t\t!path_inside_repo(prefix, argv[i + 1]))))\n+\t\t\tno_index = DIFF_NO_INDEX_IMPLICIT;\n+\t}\n \n \tif (!no_index)\n \t\tgitmodules_config();\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301804","messageId":"20160913032340.v4drd44zylqwl7lq@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 07/16] pager: remove obsolete comment","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:40Z","receivedAt":"2016-09-13T03:23:55Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The comment at the top of pager.c claims that we've split\nthe code out so that Windows can do something different.\nThis dates back to f67b45f (Introduce trivial new pager.c\nhelper infrastructure, 2006-02-28), because the original\nimplementation used fork(). Later, we ended up sticking the\nWindows #ifdefs into this file anyway. And then even later,\nin ea27a18 (spawn pager via run_command interface,\n2008-07-22) we unified the implementations.\n\nSo these days this comment is really saying nothing at all.\nLet's drop it.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pager.c | 5 -----\n 1 file changed, 5 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 6470b81..ae46da9 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -6,11 +6,6 @@\n #define DEFAULT_PAGER \"less\"\n #endif\n \n-/*\n- * This is split up from the rest of git so that we can do\n- * something different on Windows.\n- */\n-\n static struct child_process pager_process = CHILD_PROCESS_INIT;\n \n static void wait_for_pager(int in_signal)\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301805","messageId":"20160913032321.cv6732hyzv3d4z57@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 03/16] patch-id: use RUN_SETUP_GENTLY","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:22Z","receivedAt":"2016-09-13T03:23:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Patch-id does not require a repository because it is just\nprocessing the incoming diff on stdin, but it may look at\ngit config for keys like patchid.stable.\n\nEven though we do not setup_git_directory(), this works from\nthe top-level of a repository because we blindly look at\n\".git/config\" in this case. But as the included test\ndemonstrates, it does not work from a subdirectory.\n\nWe can fix it by using RUN_SETUP_GENTLY. We do not take any\nfilenames from the user on the command line, so there's no\nneed to adjust them via prefix_filename().\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n git.c               |  2 +-\n t/t4204-patch-id.sh | 14 ++++++++++++++\n 2 files changed, 15 insertions(+), 1 deletion(-)\n\ndiff --git a/git.c b/git.c\nindex 1c61151..ab5c99c 100644\n--- a/git.c\n+++ b/git.c\n@@ -444,7 +444,7 @@ static struct cmd_struct commands[] = {\n \t{ \"pack-objects\", cmd_pack_objects, RUN_SETUP },\n \t{ \"pack-redundant\", cmd_pack_redundant, RUN_SETUP },\n \t{ \"pack-refs\", cmd_pack_refs, RUN_SETUP },\n-\t{ \"patch-id\", cmd_patch_id },\n+\t{ \"patch-id\", cmd_patch_id, RUN_SETUP_GENTLY },\n \t{ \"pickaxe\", cmd_blame, RUN_SETUP },\n \t{ \"prune\", cmd_prune, RUN_SETUP },\n \t{ \"prune-packed\", cmd_prune_packed, RUN_SETUP },\ndiff --git a/t/t4204-patch-id.sh b/t/t4204-patch-id.sh\nindex 84a8096..0288c17 100755\n--- a/t/t4204-patch-id.sh\n+++ b/t/t4204-patch-id.sh\n@@ -143,6 +143,20 @@ test_expect_success 'patch-id supports git-format-patch MIME output' '\n \ttest_cmp patch-id_master patch-id_same\n '\n \n+test_expect_success 'patch-id respects config from subdir' '\n+\ttest_config patchid.stable true &&\n+\tmkdir subdir &&\n+\n+\t# copy these because test_patch_id() looks for them in\n+\t# the current directory\n+\tcp bar-then-foo foo-then-bar subdir &&\n+\n+\t(\n+\t\tcd subdir &&\n+\t\ttest_patch_id irrelevant patchid.stable=true\n+\t)\n+'\n+\n cat >nonl <<\\EOF\n diff --git i/a w/a\n index e69de29..2e65efe 100644\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301806","messageId":"20160913032331.ozo34zfavzoeszj4@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 05/16] diff: handle --no-index prefixes consistently","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:32Z","receivedAt":"2016-09-13T03:23:57Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"If we see an explicit \"git diff --no-index ../foo ../bar\",\nthen we do not set up the git repository at all (we already\nknow we are in --no-index mode, so do not have to check \"are\nwe in a repository?\"), and hence have no \"prefix\" within the\nrepository. A patch generated by this command will have the\nfilenames \"a/../foo\" and \"b/../bar\", no matter which\ndirectory we are in with respect to any repository.\n\nHowever, in the implicit case, where we notice that the\nfiles are outside the repository, we will have chdir()'d to\nthe top-level of the repository. We then feed the prefix\nback to the diff machinery. As a result, running the same\ndiff from a subdirectory will result in paths that look like\n\"a/subdir/../../foo\".\n\nBesides being unnecessarily long, this may also be confusing\nto the user: they don't care about the subdir or the\nrepository at all; it's just where they happened to be when\nrunning the command. We should treat this the same as the\nexplicit --no-index case.\n\nOne way to address this would be to chdir() back to the\noriginal path before running our diff. However, that's a bit\nhacky, as we would also need to adjust $GIT_DIR, which could\nbe a relative path from our top-level.\n\nInstead, we can reuse the diff machinery's RELATIVE_NAME\noption, which automatically strips off the prefix. Note that\nthis _also_ restricts the diff to this relative prefix, but\nthat's OK for our purposes: we queue our own diff pairs\nmanually, and do not rely on that part of the diff code.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n diff-no-index.c          |  3 +++\n t/t4053-diff-no-index.sh | 18 ++++++++++++++++++\n 2 files changed, 21 insertions(+)\n\ndiff --git a/diff-no-index.c b/diff-no-index.c\nindex 1f8999b..f420786 100644\n--- a/diff-no-index.c\n+++ b/diff-no-index.c\n@@ -281,6 +281,9 @@ void diff_no_index(struct rev_info *revs,\n \n \tDIFF_OPT_SET(&revs->diffopt, NO_INDEX);\n \n+\tDIFF_OPT_SET(&revs->diffopt, RELATIVE_NAME);\n+\trevs->diffopt.prefix = prefix;\n+\n \trevs->max_count = -2;\n \tdiff_setup_done(&revs->diffopt);\n \ndiff --git a/t/t4053-diff-no-index.sh b/t/t4053-diff-no-index.sh\nindex 6eb8321..e60c951 100755\n--- a/t/t4053-diff-no-index.sh\n+++ b/t/t4053-diff-no-index.sh\n@@ -89,4 +89,22 @@ test_expect_success 'turning a file into a directory' '\n \t)\n '\n \n+test_expect_success 'diff from repo subdir shows real paths (explicit)' '\n+\techo \"diff --git a/../../non/git/a b/../../non/git/b\" >expect &&\n+\ttest_expect_code 1 \\\n+\t\tgit -C repo/sub \\\n+\t\tdiff --no-index ../../non/git/a ../../non/git/b >actual &&\n+\thead -n 1 <actual >actual.head &&\n+\ttest_cmp expect actual.head\n+'\n+\n+test_expect_success 'diff from repo subdir shows real paths (implicit)' '\n+\techo \"diff --git a/../../non/git/a b/../../non/git/b\" >expect &&\n+\ttest_expect_code 1 \\\n+\t\tgit -C repo/sub \\\n+\t\tdiff ../../non/git/a ../../non/git/b >actual &&\n+\thead -n 1 <actual >actual.head &&\n+\ttest_cmp expect actual.head\n+'\n+\n test_done\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301807","messageId":"20160913032344.srp2canqre44rsl5@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 08/16] pager: stop loading git_default_config()","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:44Z","receivedAt":"2016-09-13T03:24:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"In git_pager(), we really only care about getting the value\nof core.pager. But to do so, we use the git_default_config()\ncallback, which loads many other values. Ordinarily it\nisn't a big deal to load this config an extra time, as it\nsimply overwrites the values from the previous run. But it's\na bad idea here, for two reasons:\n\n  1. The pager setup may be called very early in the\n     program, before we have found the git repository. As a\n     result, we may fail to read the correct repo-level\n     config file.  This is a problem for core.pager, too,\n     but we should at least try to minimize the pollution to\n     other configured values.\n\n  2. Because we call setup_pager() from git.c, basically\n     every builtin command _may_ end up reading this config\n     and getting an implicit git_default_config() setup.\n\n     Which doesn't sound like a terrible thing, except that\n     we don't do it consistently; it triggers only when\n     stdout is a tty. So if a command forgets to load the\n     default config itself (but depends on it anyway), it\n     may appear to work, and then mysteriously fail when the\n     pager is not in use.\n\nWe can improve this by loading _just_ the core.pager config\nfrom git_pager().\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n config.c | 3 ---\n pager.c  | 9 ++++++++-\n 2 files changed, 8 insertions(+), 4 deletions(-)\n\ndiff --git a/config.c b/config.c\nindex 0dfed68..8b28447 100644\n--- a/config.c\n+++ b/config.c\n@@ -927,9 +927,6 @@ static int git_default_core_config(const char *var, const char *value)\n \t\treturn 0;\n \t}\n \n-\tif (!strcmp(var, \"core.pager\"))\n-\t\treturn git_config_string(&pager_program, var, value);\n-\n \tif (!strcmp(var, \"core.editor\"))\n \t\treturn git_config_string(&editor_program, var, value);\n \ndiff --git a/pager.c b/pager.c\nindex ae46da9..d747f50 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -35,6 +35,13 @@ static void wait_for_pager_signal(int signo)\n \traise(signo);\n }\n \n+static int core_pager_config(const char *var, const char *value, void *data)\n+{\n+\tif (!strcmp(var, \"core.pager\"))\n+\t\treturn git_config_string(&pager_program, var, value);\n+\treturn 0;\n+}\n+\n const char *git_pager(int stdout_is_tty)\n {\n \tconst char *pager;\n@@ -45,7 +52,7 @@ const char *git_pager(int stdout_is_tty)\n \tpager = getenv(\"GIT_PAGER\");\n \tif (!pager) {\n \t\tif (!pager_program)\n-\t\t\tgit_config(git_default_config, NULL);\n+\t\t\tgit_config(core_pager_config, NULL);\n \t\tpager = pager_program;\n \t}\n \tif (!pager)\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301808","messageId":"20160913032348.btinenvjy6y2xsbg@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 09/16] pager: make pager_program a file-local static","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:48Z","receivedAt":"2016-09-13T03:24:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This variable is only ever used by the routines in pager.c,\nand other parts of the code should always use those routines\n(like git_pager()) to make decisions about which pager to\nuse. Let's reduce its scope to prevent accidents.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n cache.h       | 1 -\n environment.c | 1 -\n pager.c       | 1 +\n 3 files changed, 1 insertion(+), 2 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 6738050..4f23952 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1786,7 +1786,6 @@ extern void write_file(const char *path, const char *fmt, ...);\n \n /* pager.c */\n extern void setup_pager(void);\n-extern const char *pager_program;\n extern int pager_in_use(void);\n extern int pager_use_color;\n extern int term_columns(void);\ndiff --git a/environment.c b/environment.c\nindex ca72464..c266428 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -40,7 +40,6 @@ size_t packed_git_window_size = DEFAULT_PACKED_GIT_WINDOW_SIZE;\n size_t packed_git_limit = DEFAULT_PACKED_GIT_LIMIT;\n size_t delta_base_cache_limit = 96 * 1024 * 1024;\n unsigned long big_file_threshold = 512 * 1024 * 1024;\n-const char *pager_program;\n int pager_use_color = 1;\n const char *editor_program;\n const char *askpass_program;\ndiff --git a/pager.c b/pager.c\nindex d747f50..8aa0a82 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -7,6 +7,7 @@\n #endif\n \n static struct child_process pager_process = CHILD_PROCESS_INIT;\n+static const char *pager_program;\n \n static void wait_for_pager(int in_signal)\n {\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301809","messageId":"20160913032352.khrhkxpa3y4k22vz@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 10/16] pager: use callbacks instead of configset","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:52Z","receivedAt":"2016-09-13T03:24:09Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"While the cached configset interface is more pleasant to\nuse, it is not appropriate for \"early\" config like pager\nsetup, which must sometimes do tricky things like reading\nfrom \".git/config\" even when we have not set up the\nrepository.\n\nAs a preparatory step to handling these cases better, let's\nswitch back to using the callback interface, which gives us\nmore control.\n\nNote that this is essentially a revert of 586f414 (pager.c:\nreplace `git_config()` with `git_config_get_value()`,\n2014-08-07), but with some minor style fixups and\nmodernizations.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pager.c | 47 +++++++++++++++++++++++++++++++++--------------\n 1 file changed, 33 insertions(+), 14 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 8aa0a82..46cc411 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -183,23 +183,42 @@ int decimal_width(uintmax_t number)\n \treturn width;\n }\n \n-/* returns 0 for \"no pager\", 1 for \"use pager\", and -1 for \"not specified\" */\n-int check_pager_config(const char *cmd)\n+struct pager_command_config_data {\n+\tconst char *cmd;\n+\tint want;\n+\tchar *value;\n+};\n+\n+static int pager_command_config(const char *var, const char *value, void *vdata)\n {\n-\tint want = -1;\n-\tstruct strbuf key = STRBUF_INIT;\n-\tconst char *value = NULL;\n-\tstrbuf_addf(&key, \"pager.%s\", cmd);\n-\tif (git_config_key_is_valid(key.buf) &&\n-\t    !git_config_get_value(key.buf, &value)) {\n-\t\tint b = git_config_maybe_bool(key.buf, value);\n+\tstruct pager_command_config_data *data = vdata;\n+\tconst char *cmd;\n+\n+\tif (skip_prefix(var, \"pager.\", &cmd) && !strcmp(cmd, data->cmd)) {\n+\t\tint b = git_config_maybe_bool(var, value);\n \t\tif (b >= 0)\n-\t\t\twant = b;\n+\t\t\tdata->want = b;\n \t\telse {\n-\t\t\twant = 1;\n-\t\t\tpager_program = xstrdup(value);\n+\t\t\tdata->want = 1;\n+\t\t\tdata->value = xstrdup(value);\n \t\t}\n \t}\n-\tstrbuf_release(&key);\n-\treturn want;\n+\n+\treturn 0;\n+}\n+\n+/* returns 0 for \"no pager\", 1 for \"use pager\", and -1 for \"not specified\" */\n+int check_pager_config(const char *cmd)\n+{\n+\tstruct pager_command_config_data data;\n+\n+\tdata.cmd = cmd;\n+\tdata.want = -1;\n+\tdata.value = NULL;\n+\n+\tgit_config(pager_command_config, &data);\n+\n+\tif (data.value)\n+\t\tpager_program = data.value;\n+\treturn data.want;\n }\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301810","messageId":"20160913032356.mj6q5qv6kllbcdfb@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 11/16] pager: handle early config","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:23:56Z","receivedAt":"2016-09-13T03:24:11Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The pager code is often run early in the git.c startup,\nbefore we have actually found the repository. When we ask\ngit_config() to look for values like core.pager, it doesn't\nknow where to find the repo-level config, and will blindly\nexamine \".git/config\" if it exists. That's why t7006 shows\nthat many pager-related features happen to work from the\ntop-level of a repository, but not from a subdirectory.\n\nThis patch pulls that \".git/config\" hack explicitly into the\npager code. There are two reasons for this:\n\n  1. We'd like to clean up the git_config() behavior, as\n     looking at \".git/config\" when we do not have a\n     configured repository is often the wrong thing to do.\n     But we'd prefer not to break the pager config any worse\n     than it already is.\n\n  2. It's one very tiny step on the road to ultimately\n     making the pager config work consistently. If we\n     eventually get an equivalent of setup_git_directory()\n     that _just_ finds the directory and doesn't chdir() or\n     set up any global state, we could plug it in here\n     (instead of blindly looking at \".git/config\").\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n pager.c | 35 +++++++++++++++++++++++++++++++++--\n 1 file changed, 33 insertions(+), 2 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 46cc411..ae79643 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -43,6 +43,37 @@ static int core_pager_config(const char *var, const char *value, void *data)\n \treturn 0;\n }\n \n+static void read_early_config(config_fn_t cb, void *data)\n+{\n+\tgit_config_with_options(cb, data, NULL, 1);\n+\n+\t/*\n+\t * Note that this is a really dirty hack that does the wrong thing in\n+\t * many cases. The crux of the problem is that we cannot run\n+\t * setup_git_directory() early on in git's setup, so we have no idea if\n+\t * we are in a repository or not, and therefore are not sure whether\n+\t * and how to read repository-local config.\n+\t *\n+\t * So if we _aren't_ in a repository (or we are but we would reject its\n+\t * core.repositoryformatversion), we'll read whatever is in .git/config\n+\t * blindly. Similarly, if we _are_ in a repository, but not at the\n+\t * root, we'll fail to find .git/config (because it's really\n+\t * ../.git/config, etc). See t7006 for a complete set of failures.\n+\t *\n+\t * However, we have historically provided this hack because it does\n+\t * work some of the time (namely when you are at the top-level of a\n+\t * valid repository), and would rarely make things worse (i.e., you do\n+\t * not generally have a .git/config file sitting around).\n+\t */\n+\tif (!startup_info->have_repository) {\n+\t\tstruct git_config_source repo_config;\n+\n+\t\tmemset(&repo_config, 0, sizeof(repo_config));\n+\t\trepo_config.file = \".git/config\";\n+\t\tgit_config_with_options(cb, data, &repo_config, 1);\n+\t}\n+}\n+\n const char *git_pager(int stdout_is_tty)\n {\n \tconst char *pager;\n@@ -53,7 +84,7 @@ const char *git_pager(int stdout_is_tty)\n \tpager = getenv(\"GIT_PAGER\");\n \tif (!pager) {\n \t\tif (!pager_program)\n-\t\t\tgit_config(core_pager_config, NULL);\n+\t\t\tread_early_config(core_pager_config, NULL);\n \t\tpager = pager_program;\n \t}\n \tif (!pager)\n@@ -216,7 +247,7 @@ int check_pager_config(const char *cmd)\n \tdata.want = -1;\n \tdata.value = NULL;\n \n-\tgit_config(pager_command_config, &data);\n+\tread_early_config(pager_command_config, &data);\n \n \tif (data.value)\n \t\tpager_program = data.value;\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301811","messageId":"20160913032400.jw7kegfpvbydbfxq@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 12/16] t1302: use \"git -C\"","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:24:00Z","receivedAt":"2016-09-13T03:24:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"This is shorter, and saves a subshell.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1302-repo-version.sh | 30 ++++++------------------------\n 1 file changed, 6 insertions(+), 24 deletions(-)\n\ndiff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh\nindex 9bcd349..f859809 100755\n--- a/t/t1302-repo-version.sh\n+++ b/t/t1302-repo-version.sh\n@@ -25,10 +25,7 @@ test_expect_success 'setup' '\n test_expect_success 'gitdir selection on normal repos' '\n \techo 0 >expect &&\n \tgit config core.repositoryformatversion >actual &&\n-\t(\n-\t\tcd test &&\n-\t\tgit config core.repositoryformatversion >../actual2\n-\t) &&\n+\tgit -C test config core.repositoryformatversion >actual2 &&\n \ttest_cmp expect actual &&\n \ttest_cmp expect actual2\n '\n@@ -36,35 +33,20 @@ test_expect_success 'gitdir selection on normal repos' '\n test_expect_success 'gitdir selection on unsupported repo' '\n \t# Make sure it would stop at test2, not trash\n \techo 99 >expect &&\n-\t(\n-\t\tcd test2 &&\n-\t\tgit config core.repositoryformatversion >../actual\n-\t) &&\n+\tgit -C test2 config core.repositoryformatversion >actual &&\n \ttest_cmp expect actual\n '\n \n test_expect_success 'gitdir not required mode' '\n \tgit apply --stat test.patch &&\n-\t(\n-\t\tcd test &&\n-\t\tgit apply --stat ../test.patch\n-\t) &&\n-\t(\n-\t\tcd test2 &&\n-\t\tgit apply --stat ../test.patch\n-\t)\n+\tgit -C test apply --stat ../test.patch &&\n+\tgit -C test2 apply --stat ../test.patch\n '\n \n test_expect_success 'gitdir required mode' '\n \tgit apply --check --index test.patch &&\n-\t(\n-\t\tcd test &&\n-\t\tgit apply --check --index ../test.patch\n-\t) &&\n-\t(\n-\t\tcd test2 &&\n-\t\ttest_must_fail git apply --check --index ../test.patch\n-\t)\n+\tgit -C test apply --check --index ../test.patch &&\n+\ttest_must_fail git -C test2 apply --check --index ../test.patch\n '\n \n check_allow () {\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301812","messageId":"20160913032410.jvga3y4beqf22hw4@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 13/16] test-config: setup git directory","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:24:10Z","receivedAt":"2016-09-13T03:24:17Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The t1308 test script uses our test-config helper to read\nrepository-level config, but never actually sets up the\nrepository. This works because git_config() blindly reads\n\".git/config\" even if we have not configured a repository.\n\nThis means that test-config won't work from a subdirectory,\nthough since it's just a helper for the test scripts, that's\nnot a big deal.\n\nMore important is that the behavior of git_config() is going\nto change, and we want to make sure that t1308 continues to\nwork. We can just use setup_git_directory(), and not the\ngentle form; there's no point in being flexible, as it's\njust a helper for the tests.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/helper/test-config.c | 3 +++\n 1 file changed, 3 insertions(+)\n\ndiff --git a/t/helper/test-config.c b/t/helper/test-config.c\nindex 3c6d08c..83a4f2a 100644\n--- a/t/helper/test-config.c\n+++ b/t/helper/test-config.c\n@@ -72,6 +72,9 @@ int cmd_main(int argc, const char **argv)\n \tconst char *v;\n \tconst struct string_list *strptr;\n \tstruct config_set cs;\n+\n+\tsetup_git_directory();\n+\n \tgit_configset_init(&cs);\n \n \tif (argc < 2) {\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301813","messageId":"20160913032414.dpurefnzkvtjulld@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 14/16] config: only read .git/config from configured repos","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:24:15Z","receivedAt":"2016-09-13T03:24:22Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"When git_config() runs, it looks in the system, user-wide,\nand repo-level config files. It gets the latter by calling\ngit_pathdup(), which in turn calls get_git_dir(). If we\nhaven't set up the git repository yet, this may simply\nreturn \".git\", and we will look at \".git/config\".  This\nseems like it would be helpful (presumably we haven't set up\nthe repository yet, so it tries to find it), but it turns\nout to be a bad idea for a few reasons:\n\n  - it's not sufficient, and therefore hides bugs in a\n    confusing way. Config will be respected if commands are\n    run from the top-level of the working tree, but not from\n    a subdirectory.\n\n  - it's not always true that we haven't set up the\n    repository _yet_; we may not want to do it at all. For\n    instance, if you run \"git init /some/path\" from inside\n    another repository, it should not load config from the\n    existing repository.\n\n  - there might be a path \".git/config\", but it is not the\n    actual repository we would find via setup_git_directory().\n    This may happen, e.g., if you are storing a git\n    repository inside another git repository, but have\n    munged one of the files in such a way that the\n    inner repository is not valid (e.g., by removing HEAD).\n\nWe have at least two bugs of the second type in git-init,\nintroduced by ae5f677 (lazily load core.sharedrepository,\n2016-03-11). It causes init to use git_configset(), which\nloads all of the config, including values from the current\nrepo (if any).  This shows up in two ways:\n\n  1. If we happen to be in an existing repository directory,\n     we'll read and respect core.sharedrepository from it,\n     even though it should have no bearing on the new\n     repository. A new test in t1301 covers this.\n\n  2. Similarly, if we're in an existing repo that sets\n     core.logallrefupdates, that will cause init to fail to\n     set it in a newly created repository (because it thinks\n     that the user's templates already did so). A new test\n     in t0001 covers this.\n\nWe also need to adjust an existing test in t1302, which\ngives another example of why this patch is an improvement.\n\nThat test creates an embedded repository with a bogus\ncore.repositoryformatversion of \"99\". It wants to make sure\nthat we actually stop at the bogus repo rather than\ncontinuing upward to find the outer repo. So it checks that\n\"git config core.repositoryformatversion\" returns 99. But\nthat only works because we blindly read \".git/config\", even\nthough we _know_ we're in a repository whose vintage we do\nnot understand.\n\nAfter this patch, we avoid reading config from the unknown\nvintage repository at all, which is a safer choice.  But we\nneed to tweak the test, since core.repositoryformatversion\nwill not return 99; it will claim that it could not find the\nvariable at all.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n cache.h                 | 6 ++++++\n config.c                | 2 +-\n environment.c           | 7 +++++++\n t/t0001-init.sh         | 9 +++++++++\n t/t1301-shared-repo.sh  | 9 +++++++++\n t/t1302-repo-version.sh | 4 +---\n 6 files changed, 33 insertions(+), 4 deletions(-)\n\ndiff --git a/cache.h b/cache.h\nindex 4f23952..e9592d3 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -453,6 +453,12 @@ static inline enum object_type object_type(unsigned int mode)\n  */\n extern const char * const local_repo_env[];\n \n+/*\n+ * Returns true iff we have a configured git repository (either via\n+ * setup_git_directory, or in the environment via $GIT_DIR).\n+ */\n+int have_git_dir(void);\n+\n extern int is_bare_repository_cfg;\n extern int is_bare_repository(void);\n extern int is_inside_git_dir(void);\ndiff --git a/config.c b/config.c\nindex 8b28447..1e4b617 100644\n--- a/config.c\n+++ b/config.c\n@@ -1286,7 +1286,7 @@ static int do_git_config_sequence(config_fn_t fn, void *data)\n \tint ret = 0;\n \tchar *xdg_config = xdg_config_home(\"config\");\n \tchar *user_config = expand_user_path(\"~/.gitconfig\");\n-\tchar *repo_config = git_pathdup(\"config\");\n+\tchar *repo_config = have_git_dir() ? git_pathdup(\"config\") : NULL;\n \n \tcurrent_parsing_scope = CONFIG_SCOPE_SYSTEM;\n \tif (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))\ndiff --git a/environment.c b/environment.c\nindex c266428..c0cce6b 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -195,6 +195,13 @@ int is_bare_repository(void)\n \treturn is_bare_repository_cfg && !get_git_work_tree();\n }\n \n+int have_git_dir(void)\n+{\n+\treturn startup_info->have_repository\n+\t\t|| git_dir\n+\t\t|| getenv(GIT_DIR_ENVIRONMENT);\n+}\n+\n const char *get_git_dir(void)\n {\n \tif (!git_dir)\ndiff --git a/t/t0001-init.sh b/t/t0001-init.sh\nindex a6fdd5e..8ffbbea 100755\n--- a/t/t0001-init.sh\n+++ b/t/t0001-init.sh\n@@ -384,4 +384,13 @@ test_expect_success MINGW 'bare git dir not hidden' '\n \t! is_hidden newdir\n '\n \n+test_expect_success 'remote init from does not use config from cwd' '\n+\trm -rf newdir &&\n+\ttest_config core.logallrefupdates true &&\n+\tgit init newdir &&\n+\techo true >expect &&\n+\tgit -C newdir config --bool core.logallrefupdates >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex ac10875..7c28642 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -172,4 +172,13 @@ test_expect_success POSIXPERM 'forced modes' '\n \t}\" actual)\"\n '\n \n+test_expect_success POSIXPERM 'remote init does not use config from cwd' '\n+\tgit config core.sharedrepository 0666 &&\n+\tumask 0022 &&\n+\tgit init --bare child.git &&\n+\techo \"-rw-r--r--\" >expect &&\n+\tmodebits child.git/config >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\ndiff --git a/t/t1302-repo-version.sh b/t/t1302-repo-version.sh\nindex f859809..ce4cff1 100755\n--- a/t/t1302-repo-version.sh\n+++ b/t/t1302-repo-version.sh\n@@ -32,9 +32,7 @@ test_expect_success 'gitdir selection on normal repos' '\n \n test_expect_success 'gitdir selection on unsupported repo' '\n \t# Make sure it would stop at test2, not trash\n-\techo 99 >expect &&\n-\tgit -C test2 config core.repositoryformatversion >actual &&\n-\ttest_cmp expect actual\n+\ttest_expect_code 1 git -C test2 config core.repositoryformatversion >actual\n '\n \n test_expect_success 'gitdir not required mode' '\n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301814","messageId":"20160913032418.boochwwign64nfvv@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 15/16] init: expand comments explaining config trickery","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:24:19Z","receivedAt":"2016-09-13T03:24:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"git-init may copy \"config\" from the templates directory and\nthen re-read it. There are some comments explaining what's\ngoing on here, but they are not grouped very well with the\nmatching code. Let's rearrange and expand them.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/init-db.c | 11 +++++++----\n 1 file changed, 7 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 3a45f0b..e935895 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -191,16 +191,19 @@ static int create_default_files(const char *template_path)\n \t/* Just look for `init.templatedir` */\n \tgit_config(git_init_db_config, NULL);\n \n-\t/* First copy the templates -- we might have the default\n+\t/*\n+\t * First copy the templates -- we might have the default\n \t * config file there, in which case we would want to read\n \t * from it after installing.\n \t */\n \tcopy_templates(template_path);\n-\n \tgit_config(git_default_config, NULL);\n-\tis_bare_repository_cfg = init_is_bare_repository;\n \n-\t/* reading existing config may have overwrote it */\n+\t/*\n+\t * We must make sure command-line options continue to override any\n+\t * values we might have just re-read from the config.\n+\t */\n+\tis_bare_repository_cfg = init_is_bare_repository;\n \tif (init_shared_repository != -1)\n \t\tset_shared_repository(init_shared_repository);\n \n-- \n2.10.0.230.g6f8d04b\n\n"},{"id":"301815","messageId":"20160913032422.xy22la6uw23hnvje@sigill.intra.peff.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"[PATCH 16/16] init: reset cached config when entering new repo","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T03:24:23Z","receivedAt":"2016-09-13T03:24:35Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"After we copy the templates into place, we re-read the\nconfig in case we copied in a default config file. But since\ngit_config() is backed by a cache these days, it's possible\nthat the call will not actually touch the filesystem at all;\nwe need to tell it that something has changed behind the\nscenes.\n\nNote that we also need to reset the shared_repository\nconfig. At first glance, it seems like this should probably\njust be folded into git_config_clear(). But unfortunately\nthat is not quite right. The shared repository value may\ncome from config, _or_ it may have been set manually. So\nonly the caller who knows whether or not they set it is the\none who can clear it (and indeed, if you _do_ put it into\ngit_config_clear(), then many tests fail, as we have to\nclear the config cache any time we set a new config\nvariable).\n\nThere are three tests here. The first two actually pass\nalready, though it's largely luck: they just don't happen to\nactually read any config before we enter the new repo.\n\nBut the third one does fail without this patch; we look at\ncore.sharedrepository while creating the directory, but need\nto make sure the value from the template config overrides\nit.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n builtin/init-db.c      |  6 ++++++\n cache.h                |  7 +++++++\n environment.c          |  5 +++++\n t/t1301-shared-repo.sh | 32 ++++++++++++++++++++++++++++++++\n 4 files changed, 50 insertions(+)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex e935895..cea4550 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -195,8 +195,14 @@ static int create_default_files(const char *template_path)\n \t * First copy the templates -- we might have the default\n \t * config file there, in which case we would want to read\n \t * from it after installing.\n+\t *\n+\t * Before reading that config, we also need to clear out any cached\n+\t * values (since we've just potentially changed what's available on\n+\t * disk).\n \t */\n \tcopy_templates(template_path);\n+\tgit_config_clear();\n+\treset_shared_repository();\n \tgit_config(git_default_config, NULL);\n \n \t/*\ndiff --git a/cache.h b/cache.h\nindex e9592d3..4c8d85b 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -671,8 +671,15 @@ extern size_t delta_base_cache_limit;\n extern unsigned long big_file_threshold;\n extern unsigned long pack_size_limit_cfg;\n \n+/*\n+ * Accessors for the core.sharedrepository config which lazy-load the value\n+ * from the config (if not already set). The \"reset\" function can be\n+ * used to unset \"set\" or cached value, meaning that the value will be loaded\n+ * fresh from the config file on the next call to get_shared_repository().\n+ */\n void set_shared_repository(int value);\n int get_shared_repository(void);\n+void reset_shared_repository(void);\n \n /*\n  * Do replace refs need to be checked this run?  This variable is\ndiff --git a/environment.c b/environment.c\nindex c0cce6b..cd5aa57 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -351,3 +351,8 @@ int get_shared_repository(void)\n \t}\n \treturn the_shared_repository;\n }\n+\n+void reset_shared_repository(void)\n+{\n+\tneed_shared_repository_from_config = 1;\n+}\ndiff --git a/t/t1301-shared-repo.sh b/t/t1301-shared-repo.sh\nindex 7c28642..1312004 100755\n--- a/t/t1301-shared-repo.sh\n+++ b/t/t1301-shared-repo.sh\n@@ -181,4 +181,36 @@ test_expect_success POSIXPERM 'remote init does not use config from cwd' '\n \ttest_cmp expect actual\n '\n \n+test_expect_success POSIXPERM 're-init respects core.sharedrepository (local)' '\n+\tgit config core.sharedrepository 0666 &&\n+\tumask 0022 &&\n+\techo whatever >templates/foo &&\n+\tgit init --template=templates &&\n+\techo \"-rw-rw-rw-\" >expect &&\n+\tmodebits .git/foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success POSIXPERM 're-init respects core.sharedrepository (remote)' '\n+\trm -rf child.git &&\n+\tumask 0022 &&\n+\tgit init --bare --shared=0666 child.git &&\n+\ttest_path_is_missing child.git/foo &&\n+\tgit init --bare --template=../templates child.git &&\n+\techo \"-rw-rw-rw-\" >expect &&\n+\tmodebits child.git/foo >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success POSIXPERM 'template can set core.sharedrepository' '\n+\trm -rf child.git &&\n+\tumask 0022 &&\n+\tgit config core.sharedrepository 0666 &&\n+\tcp .git/config templates/config &&\n+\tgit init --bare --template=../templates child.git &&\n+\techo \"-rw-rw-rw-\" >expect &&\n+\tmodebits child.git/HEAD >actual &&\n+\ttest_cmp expect actual\n+'\n+\n test_done\n-- \n2.10.0.230.g6f8d04b\n"},{"id":"301870","messageId":"CAGZ79kZQd2+TyceLmnTcv6rNGeKuyQhtvepvzcHw48WqOL4J4Q@mail.gmail.com","threadId":"44070","inReplyTo":"20160913032312.yheky2o25lmibvmb@sigill.intra.peff.net","subject":"Re: [PATCH 01/16] t1007: factor out repeated setup","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-13T21:42:16Z","receivedAt":"2016-09-13T21:42:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Sep 12, 2016 at 8:23 PM, Jeff King <peff@peff.net> wrote:\n> We have a series of 3 CRLF tests that do exactly the same\n> (long) setup sequence. Let's pull it out into a common setup\n> test, which is shorter, more efficient, and will make it\n> easier to add new tests.\n>\n> Note that we don't have to worry about cleaning up any of\n> the setup which was previously per-test; we call pop_repo\n> after the CRLF tests, which cleans up everything.\n>\n> Signed-off-by: Jeff King <peff@peff.net>\n\nThis makes sense and looks good,\nThanks,\n\nReviewed-by: Stefan Beller <sbeller@google.com>\n"},{"id":"301872","messageId":"CAGZ79kYNfUmBG+9irQoZ+C6qfiiz0P7pGKHZVk-8569gk4+_EQ@mail.gmail.com","threadId":"44070","inReplyTo":"20160913032336.cur3rrzpgepulqi6@sigill.intra.peff.net","subject":"Re: [PATCH 06/16] diff: always try to set up the repository","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-13T22:00:17Z","receivedAt":"2016-09-13T22:00:22Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"On Mon, Sep 12, 2016 at 8:23 PM, Jeff King <peff@peff.net> wrote:\n\n>   2. If you're in a subdirectory of a repository, then we\n>      still try to read \".git/config\", but it generally\n>      doesn't exist. So \"diff --no-index\" there does not\n>      respect repo config.\n\nNit:\nSo IIUC your cover letter even this /used/ to work but\nbroke only recently? So I feel like the message is a bit\nmisleading (i.e. you argue for a change in behavior instead of\ncalling it a bug fix for a regression. I think a bug fix for a regression\nis harder to revert as compared to a \"new\" behavior)\n\nI agree on the code, though.\n"},{"id":"301876","messageId":"CAGZ79kYEN7Zg2cQfVisHwHDG0YuFSGRLYjm0BC9w1hYKG_CBOQ@mail.gmail.com","threadId":"44070","inReplyTo":"20160913032422.xy22la6uw23hnvje@sigill.intra.peff.net","subject":"Re: [PATCH 16/16] init: reset cached config when entering new repo","fromName":"Stefan Beller","fromEmail":"sbeller@google.com","sentAt":"2016-09-13T22:18:46Z","receivedAt":"2016-09-13T22:19:01Z","isPatch":true,"sender":{"key":"stefanbeller@gmail.com","avatar":"https://avatars.githubusercontent.com/u/455868?v=4"},"body":"I have reviewed all patches though I am no expert on\nthe init routines. They all look good except for the one\nnit I noted for the commit message in patch 6.\nWith that, the whole series is:\nReviewed-by: Stefan Beller <sbeller@google.com>\n\nThanks!\n"},{"id":"301877","messageId":"20160913222144.f3f3oqhqbe6sjzdf@sigill.intra.peff.net","threadId":"44070","inReplyTo":"CAGZ79kYNfUmBG+9irQoZ+C6qfiiz0P7pGKHZVk-8569gk4+_EQ@mail.gmail.com","subject":"Re: [PATCH 06/16] diff: always try to set up the repository","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-13T22:22:33Z","receivedAt":"2016-09-13T22:22:52Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Sep 13, 2016 at 03:00:17PM -0700, Stefan Beller wrote:\n\n> On Mon, Sep 12, 2016 at 8:23 PM, Jeff King <peff@peff.net> wrote:\n> \n> >   2. If you're in a subdirectory of a repository, then we\n> >      still try to read \".git/config\", but it generally\n> >      doesn't exist. So \"diff --no-index\" there does not\n> >      respect repo config.\n> \n> Nit:\n> So IIUC your cover letter even this /used/ to work but\n> broke only recently? So I feel like the message is a bit\n> misleading (i.e. you argue for a change in behavior instead of\n> calling it a bug fix for a regression. I think a bug fix for a regression\n> is harder to revert as compared to a \"new\" behavior)\n\nNo, this has always been broken. What broke recently is the tests\ncovered in patch 14 related to git-init.\n\nIOW, we have always done this \"blind read\" of .git/config, ever since\nthe early days. It was always wrong, but was mostly overlooked because\nit worked often enough and usually didn't cause other problems. But\nbecause of the caching and lazy-reading done by git_config() these days\n(and get_shared_repository() which builds on it), you can get quite\nconfusing and buggy effects if you lazy-read at the wrong time.\n\n-Peff\n"},{"id":"301901","messageId":"1473850541.30531.18.camel@kaarsemaker.net","threadId":"44070","inReplyTo":"20160913032242.coyuhyhn6uklewuk@sigill.intra.peff.net","subject":"Re: [PATCH 0/16] fix config-reading in non-repos","fromName":"Dennis Kaarsemaker","fromEmail":"dennis@kaarsemaker.net","sentAt":"2016-09-14T10:55:41Z","receivedAt":"2016-09-14T10:55:50Z","isPatch":true,"sender":{"key":"dennis@kaarsemaker.net","avatar":"https://avatars.githubusercontent.com/u/200649?v=4"},"body":"On ma, 2016-09-12 at 23:22 -0400, Jeff King wrote:\n\n> The motivation for this series is to fix the regression in v2.9 where\n> core.logallrefupdates is sometimes not set properly in a newly\n> initialized repository, as described in this thread:\n> \n>   http://public-inbox.org/git/c46d36ef-3c2e-374f-0f2e-ffe31104e023@gmx.de/T/#u\n> \n> The root of the problem is that we are overly eager to read and use\n> config from \".git/config\", even when we have not established that it is\n> part of a repository. This is especially bad for git-init, which would\n> not want to read anything until we've created the new repo.\n> \n> So the two interesting parts of the fix are:\n> \n>   1. We stop blindly reading \".git/config\" when we don't know there's an\n>      actual git directory. This is in patch 14, and is actually enough\n>      to fix the v2.9 regression.\n> \n>   2. We are more thorough about dropping any cached config values when\n>      we move into the new repository in git-init (patch 16).\n> \n>      I didn't dig into when this was broken, but it was probably when we\n>      switched git_config() to use cached values in the v2.2.0\n>      time-frame.\n> \n> Doing (1) required fixing up some builtins that depended on the blind\n> .git/config thing, as the tests demonstrated. But I think this is a sign\n> that we are moving in the right direction, because each one of those\n> programs could easily be demonstrated to be broken in scenarios only\n> slightly more exotic than the test scripts (e.g., see patch 3 for one of\n> the simplest cases).\n> \n> So I think notwithstanding their use as prep for patch 14, patches 1-13\n> fix useful bugs.\n> \n> I won't be surprised if there are other fallouts that were not caught by\n> the test suite (i.e., programs that expect to read config, don't do\n> RUN_SETUP, but aren't covered well by tests). I poked around the list of\n> builtins in git.c that do not use RUN_SETUP, and they seem to correctly\n> end up in setup_git_directory_gently() before reading config. But it's\n> possible I missed a case.\n> \n> So this is definitely a bit larger than I'd hope for a regression-fix to\n> maint. But anything that doesn't address this issue at the config layer\n> is going to end up as a bit of a hack, and I'd rather not pile up hacks\n> if we can avoid it.\n\nAgreed with all of the above, this is much better than just fixing the\nsymptom on the mailinglist thread that started this.\n\n> I've cc'd Dennis, who helped investigate solutions in the thread\n> mentioned above, and Duy, because historically he has been the one most\n> willing and able to battle the dragon of our setup code. :)\n> \n>   [01/16]: t1007: factor out repeated setup\n>   [02/16]: hash-object: always try to set up the git repository\n>   [03/16]: patch-id: use RUN_SETUP_GENTLY\n>   [04/16]: diff: skip implicit no-index check when given --no-index\n>   [05/16]: diff: handle --no-index prefixes consistently\n>   [06/16]: diff: always try to set up the repository\n>   [07/16]: pager: remove obsolete comment\n>   [08/16]: pager: stop loading git_default_config()\n>   [09/16]: pager: make pager_program a file-local static\n>   [10/16]: pager: use callbacks instead of configset\n>   [11/16]: pager: handle early config\n>   [12/16]: t1302: use \"git -C\"\n>   [13/16]: test-config: setup git directory\n>   [14/16]: config: only read .git/config from configured repos\n>   [15/16]: init: expand comments explaining config trickery\n>   [16/16]: init: reset cached config when entering new repo\n\nCouldn't find anything to comment on, and I've tested that this does\nindeed fix the symptoms we saw.\n\nReviewed-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n\n-- \nDennis Kaarsemaker\nhttp://www.kaarsemaker.net\n\n"},{"id":"301907","messageId":"20160914153134.skehcameo2oh7dr7@sigill.intra.peff.net","threadId":"44070","inReplyTo":"1473850541.30531.18.camel@kaarsemaker.net","subject":"Re: [PATCH 0/16] fix config-reading in non-repos","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-09-14T15:31:34Z","receivedAt":"2016-09-14T15:31:46Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Sep 14, 2016 at 12:55:41PM +0200, Dennis Kaarsemaker wrote:\n\n> >   [14/16]: config: only read .git/config from configured repos\n> >   [15/16]: init: expand comments explaining config trickery\n> >   [16/16]: init: reset cached config when entering new repo\n> \n> Couldn't find anything to comment on, and I've tested that this does\n> indeed fix the symptoms we saw.\n> \n> Reviewed-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>\n\nThanks. Based on our conversations earlier, I tried to dig around for\nother cases that might be broken, especially with the \"reinit\" cases.\nThe result is the tests in patch 16. But let me know if you can think of\nanything else that might be broken.\n\n-Peff\n"}]}