On Fri, Sep 02, 2016 at 04:04:16AM -0400, Jeff King wrote:

> So here's the minimal fix that seems to work for me:
> diff --git a/builtin/init-db.c b/builtin/init-db.c
> index 3a45f0b..56e7b9a 100644
> --- a/builtin/init-db.c
> +++ b/builtin/init-db.c

I also wonder if "clone" should be doing something similar. Or, for that
matter, things like git-daemon that operate outside of a repo. They work
now because they do not happen to trigger any library calls which look
at config under the hood.

Traditionally these were supposed to just use git_config_early(), but
that's really not possible when the config calls are happening behind
the scenes (e.g., when lazy-loading the config cache). And so we
eventually got rid of git_config_early() entirely.

But I wonder if we could enforce that concept automatically for config.

The simple patch below does fix this case:

diff --git a/config.c b/config.c
index 0dfed68..b62bb40 100644
--- a/config.c
+++ b/config.c
@@ -1289,7 +1289,7 @@ static int do_git_config_sequence(config_fn_t fn, void 
        int ret = 0;
        char *xdg_config = xdg_config_home("config");
        char *user_config = expand_user_path("~/.gitconfig");
-       char *repo_config = git_pathdup("config");
+       char *repo_config = startup_info->have_repository ? 
git_pathdup("config") : NULL;
        current_parsing_scope = CONFIG_SCOPE_SYSTEM;
        if (git_config_system() && !access_or_die(git_etc_gitconfig(), R_OK, 0))

but it causes a few test failures. Some of those are arguably
reasonable, though. E.g., several of the diff tests use "git diff
--no-index" and expect to read local config. But "--no-index" explicitly
_avoids_ setting up the git repository, so the current code just falls
back to reading ".git/config". Which means it works when you are at the
top-level of a repository, but not in a subdir!

So I think this patch is an improvement; if we have not set up the
repository, then we should not be reading its config! (It's another
question of whether --no-index should try setup_git_directory_gently(),
but then this patch would just do the right thing).

I think "hash-object" without "-w" is in the same boat. It does not even
bother looking for a git dir, but we assume that it can see config like
core.autocrlf. It works in the top-level, but not elsewhere:

  $ git init
  $ git config core.autocrlf true
  $ printf 'foo\r\n' >file
  $ git hash-object file
  $ mkdir subdir
  $ cd subdir
  $ git hash-object ../file

So there is definitely some cleanup work, but it seems like it would be
fixing a bunch of bugs.

Some of the other failures are not so obvious. In particular, t7006
tests the core.pager settings that are looked up before we set up the
git directory, and those are now broken. OTOH, I suspect that doing it
_correctly_ would fix all of the known breakages like:

  not ok 46 - git -p true - core.pager overrides PAGER from subdirectory

They are hitting that same subdirectory problem mentioned above.


Reply via email to