pager: stop loading git_default_config()
authorJeff King <peff@peff.net>
Tue, 13 Sep 2016 03:23:44 +0000 (20:23 -0700)
committerJunio C Hamano <gitster@pobox.com>
Tue, 13 Sep 2016 22:45:45 +0000 (15:45 -0700)
In git_pager(), we really only care about getting the value
of core.pager. But to do so, we use the git_default_config()
callback, which loads many other values. Ordinarily it
isn't a big deal to load this config an extra time, as it
simply overwrites the values from the previous run. But it's
a bad idea here, for two reasons:

1. The pager setup may be called very early in the
program, before we have found the git repository. As a
result, we may fail to read the correct repo-level
config file. This is a problem for core.pager, too,
but we should at least try to minimize the pollution to
other configured values.

2. Because we call setup_pager() from git.c, basically
every builtin command _may_ end up reading this config
and getting an implicit git_default_config() setup.

Which doesn't sound like a terrible thing, except that
we don't do it consistently; it triggers only when
stdout is a tty. So if a command forgets to load the
default config itself (but depends on it anyway), it
may appear to work, and then mysteriously fail when the
pager is not in use.

We can improve this by loading _just_ the core.pager config
from git_pager().

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
config.c
pager.c
index 6dbc8a409e7e6e674e399e1d9b8ef8ef3c9290b7..1d2331aa085891cb3fe344e030a0fec5cbe62272 100644 (file)
--- a/config.c
+++ b/config.c
@@ -836,9 +836,6 @@ static int git_default_core_config(const char *var, const char *value)
                return 0;
        }
 
-       if (!strcmp(var, "core.pager"))
-               return git_config_string(&pager_program, var, value);
-
        if (!strcmp(var, "core.editor"))
                return git_config_string(&editor_program, var, value);
 
diff --git a/pager.c b/pager.c
index 70159b8858c61f8c5c5577aefcfc2569956c5ddd..fc210f951d50add067dcb8f13d231e3b72148c25 100644 (file)
--- a/pager.c
+++ b/pager.c
@@ -35,6 +35,13 @@ static void wait_for_pager_signal(int signo)
        raise(signo);
 }
 
+static int core_pager_config(const char *var, const char *value, void *data)
+{
+       if (!strcmp(var, "core.pager"))
+               return git_config_string(&pager_program, var, value);
+       return 0;
+}
+
 const char *git_pager(int stdout_is_tty)
 {
        const char *pager;
@@ -45,7 +52,7 @@ const char *git_pager(int stdout_is_tty)
        pager = getenv("GIT_PAGER");
        if (!pager) {
                if (!pager_program)
-                       git_config(git_default_config, NULL);
+                       git_config(core_pager_config, NULL);
                pager = pager_program;
        }
        if (!pager)