[PATCH 2/2] builtins: do not commit pager choice early

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Apr 6, 2010, 04:06 UTC
Message-ID: <20100406040612.GC30403@progeny.tock>
In-Reply-To: <20100406040145.GA30403@progeny.tock>

If git is passed the --paginate option, committing the pager choice will require setting up the pager, which requires access to repository for the core.pager configuration.

After handle_options() is called, the repository has not been searched for yet. Unless GIT_DIR or GIT_CONFIG is set, attempting to access the configuration at this point results in git_dir being set to .git, which is almost certainly not what was wanted.

For example, this breaks git --paginate when used from a subdirectory of the toplevel directory in a repository with core.pager set.

There are several possible code paths after handle_options() and commit_pager_choice() are called:

1. list_common_cmds_help() does not need a pager: it’s a printout of
   only 29 lines, and most operating systems have some sort of
   scrollback that can handle this.

2. run_argv() will run commit_pager_choice() inside run_builtin() if a
   command is found.

3. help_unknown_cmd() prints out only a few lines.

So removing the commit_pager_choice() after handle_options() is safe.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
That’s the end of the series (based against master).

Thoughts welcome.

 git.c            |    1 -
 t/t7006-pager.sh |    2 +-
 2 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/git.c b/git.c
index 6bae305..75bd387 100644
--- a/git.c
+++ b/git.c
@@ -502,7 +502,6 @@ int main(int argc, const char **argv)
 	argv++;
 	argc--;
 	handle_options(&argv, &argc, NULL);
-	commit_pager_choice();
 	if (argc > 0) {
 		if (!prefixcmp(argv[0], "--"))
 			argv[0] += 2;
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 4f804ed..c373342 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -183,7 +183,7 @@ test_expect_success TTY 'core.pager in subdir' '
 unset GIT_PAGER
 rm -f core.pager_used
 rm -fr sub
-test_expect_failure TTY 'core.pager in subdir with --paginate' '
+test_expect_success TTY 'core.pager in subdir with --paginate' '
 	PAGER=wc &&
 	stampname=$(pwd)/core.pager_used &&
 	export PAGER stampname &&

-- 
1.7.0.4.369.g62d9d