[PATCH 0/4] git --paginate: do not commit pager choice too early

[PATCH 0/4] git --paginate: do not commit pager choice too early

[Jonathan Nieder]

Here’s a fix from the famous setup cleanup series[1].

The problem it fixes is somewhat obscure: ‘git -p foo’ is not paying
attention to the repository-local ‘[core] pager’ configuration when
run from a subdirectory.  But it is a real bug, and the fix gives
an example of how to deal with repository setup and should be safe.

Patch is against master.  There is a small semantic conflict with
jn/grep-open: SIMPLEPAGER should be changed to SIMPLEPAGERTTY in the
prerequisites for the test_default_pager function.  Please let me
know if I should push a merge commit to help resolve that.

Thoughts?
Jonathan Nieder (3):
  t7006 (pager): introduce helper for parameterized tests
  t7006: test pager configuration for several git commands
  tests: local config file should be honored from subdirs of toplevel

Nguyễn Thái Ngọc Duy (1):
  git --paginate: do not commit pager choice too early

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

[1] http://thread.gmane.org/gmane.comp.version-control.git/144778

[PATCH 1/4] t7006 (pager): introduce helper for parameterized tests

[Jonathan Nieder]

The current tests test pager configuration for ‘git log’, but other
commands use a different setup procedure and should therefore be
tested separately.  Add a helper to make this easier.

This patch introduces the helper and changes some existing tests to
use it.  The only functional change should be the introduction of ‘git
log - ’ to a few test descriptions.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7006-pager.sh |   72 ++++++++++++++++++++++++++++++++++++++++-------------
 1 files changed, 54 insertions(+), 18 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 9a83241..b117ebb 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -172,58 +172,94 @@ then
 	test_set_prereq SIMPLEPAGER
 fi
 
-test_expect_success SIMPLEPAGER 'default pager is used by default' '
+# Use this helper to make it easy for the caller of your
+# terminal-using function to specify whether it should fail.
+# If you write
+#
+#	your_test() {
+#		parse_args "$@"
+#
+#		$test_expectation "$cmd - behaves well" "
+#			...
+#			$full_command &&
+#			...
+#		"
+#	}
+#
+# then your test can be used like this:
+#
+#	your_test expect_(success|failure) [test_must_fail] 'git foo'
+#
+parse_args() {
+	test_expectation="test_$1"
+	shift
+	if test "$1" = test_must_fail
+	then
+		full_command="test_must_fail test_terminal "
+		shift
+	else
+		full_command="test_terminal "
+	fi
+	cmd=$1
+	full_command="$full_command $1"
+}
+
+parse_args expect_success 'git log'
+$test_expectation SIMPLEPAGER "$cmd - default pager is used by default" "
 	unset PAGER GIT_PAGER;
 	test_might_fail git config --unset core.pager &&
 	rm -f default_pager_used ||
 	cleanup_fail &&
 
-	cat >$less <<-\EOF &&
+	cat >\$less <<-\EOF &&
 	#!/bin/sh
 	wc >default_pager_used
 	EOF
-	chmod +x $less &&
+	chmod +x \$less &&
 	(
-		PATH=.:$PATH &&
+		PATH=.:\$PATH &&
 		export PATH &&
-		test_terminal git log
+		$full_command
 	) &&
 	test -e default_pager_used
-'
+"
 
-test_expect_success TTY 'PAGER overrides default pager' '
+parse_args expect_success 'git log'
+$test_expectation TTY "$cmd - PAGER overrides default pager" "
 	unset GIT_PAGER;
 	test_might_fail git config --unset core.pager &&
 	rm -f PAGER_used ||
 	cleanup_fail &&
 
-	PAGER="wc >PAGER_used" &&
+	PAGER='wc >PAGER_used' &&
 	export PAGER &&
-	test_terminal git log &&
+	$full_command &&
 	test -e PAGER_used
-'
+"
 
-test_expect_success TTY 'core.pager overrides PAGER' '
+parse_args expect_success 'git log'
+$test_expectation TTY "$cmd - core.pager overrides PAGER" "
 	unset GIT_PAGER;
 	rm -f core.pager_used ||
 	cleanup_fail &&
 
 	PAGER=wc &&
 	export PAGER &&
-	git config core.pager "wc >core.pager_used" &&
-	test_terminal git log &&
+	git config core.pager 'wc >core.pager_used' &&
+	$full_command &&
 	test -e core.pager_used
-'
+"
 
-test_expect_success TTY 'GIT_PAGER overrides core.pager' '
+parse_args expect_success 'git log'
+$test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
 	rm -f GIT_PAGER_used ||
 	cleanup_fail &&
 
 	git config core.pager wc &&
-	GIT_PAGER="wc >GIT_PAGER_used" &&
+	GIT_PAGER='wc >GIT_PAGER_used' &&
 	export GIT_PAGER &&
-	test_terminal git log &&
+	$full_command &&
 	test -e GIT_PAGER_used
-'
+"
 
 test_done
-- 
1.7.1.579.ge2549

[PATCH 2/4] t7006: test pager configuration for several git commands

[Jonathan Nieder]

Test choice of pager at several stages of repository setup.  This
provides some (admittedly uninteresting) examples to keep in mind when
considering changes to the setup procedure.

Improved-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
The patch from ‘diff -w’ should be easier to review.  I’ll send it as
a reply.

Thanks to both Junio and Hannes for improvements.

 t/t7006-pager.sh |  146 +++++++++++++++++++++++++++++++++---------------------
 1 files changed, 89 insertions(+), 57 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index b117ebb..4420f91 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -204,62 +204,94 @@ parse_args() {
 	full_command="$full_command $1"
 }
 
-parse_args expect_success 'git log'
-$test_expectation SIMPLEPAGER "$cmd - default pager is used by default" "
-	unset PAGER GIT_PAGER;
-	test_might_fail git config --unset core.pager &&
-	rm -f default_pager_used ||
-	cleanup_fail &&
-
-	cat >\$less <<-\EOF &&
-	#!/bin/sh
-	wc >default_pager_used
-	EOF
-	chmod +x \$less &&
-	(
-		PATH=.:\$PATH &&
-		export PATH &&
-		$full_command
-	) &&
-	test -e default_pager_used
-"
-
-parse_args expect_success 'git log'
-$test_expectation TTY "$cmd - PAGER overrides default pager" "
-	unset GIT_PAGER;
-	test_might_fail git config --unset core.pager &&
-	rm -f PAGER_used ||
-	cleanup_fail &&
-
-	PAGER='wc >PAGER_used' &&
-	export PAGER &&
-	$full_command &&
-	test -e PAGER_used
-"
-
-parse_args expect_success 'git log'
-$test_expectation TTY "$cmd - core.pager overrides PAGER" "
-	unset GIT_PAGER;
-	rm -f core.pager_used ||
-	cleanup_fail &&
-
-	PAGER=wc &&
-	export PAGER &&
-	git config core.pager 'wc >core.pager_used' &&
-	$full_command &&
-	test -e core.pager_used
-"
-
-parse_args expect_success 'git log'
-$test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
-	rm -f GIT_PAGER_used ||
-	cleanup_fail &&
-
-	git config core.pager wc &&
-	GIT_PAGER='wc >GIT_PAGER_used' &&
-	export GIT_PAGER &&
-	$full_command &&
-	test -e GIT_PAGER_used
-"
+test_default_pager() {
+	parse_args "$@"
+
+	$test_expectation SIMPLEPAGER "$cmd - default pager is used by default" "
+		unset PAGER GIT_PAGER;
+		test_might_fail git config --unset core.pager &&
+		rm -f default_pager_used ||
+		cleanup_fail &&
+
+		cat >\$less <<-\EOF &&
+		#!/bin/sh
+		wc >default_pager_used
+		EOF
+		chmod +x \$less &&
+		(
+			PATH=.:\$PATH &&
+			export PATH &&
+			$full_command
+		) &&
+		test -e default_pager_used
+	"
+}
+
+test_PAGER_overrides() {
+	parse_args "$@"
+
+	$test_expectation TTY "$cmd - PAGER overrides default pager" "
+		unset GIT_PAGER;
+		test_might_fail git config --unset core.pager &&
+		rm -f PAGER_used ||
+		cleanup_fail &&
+
+		PAGER='wc >PAGER_used' &&
+		export PAGER &&
+		$full_command &&
+		test -e PAGER_used
+	"
+}
+
+test_core_pager_overrides() {
+	parse_args "$@"
+
+	$test_expectation TTY "$cmd - core.pager overrides PAGER" "
+		unset GIT_PAGER;
+		rm -f core.pager_used ||
+		cleanup_fail &&
+
+		PAGER=wc &&
+		export PAGER &&
+		git config core.pager 'wc >core.pager_used' &&
+		$full_command &&
+		test -e core.pager_used
+	"
+}
+
+test_GIT_PAGER_overrides() {
+	parse_args "$@"
+
+	$test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
+		rm -f GIT_PAGER_used ||
+		cleanup_fail &&
+
+		git config core.pager wc &&
+		GIT_PAGER='wc >GIT_PAGER_used' &&
+		export GIT_PAGER &&
+		$full_command &&
+		test -e GIT_PAGER_used
+	"
+}
+
+test_default_pager        expect_success 'git log'
+test_PAGER_overrides      expect_success 'git log'
+test_core_pager_overrides expect_success 'git log'
+test_GIT_PAGER_overrides  expect_success 'git log'
+
+test_default_pager        expect_success 'git -p log'
+test_PAGER_overrides      expect_success 'git -p log'
+test_core_pager_overrides expect_success 'git -p log'
+test_GIT_PAGER_overrides  expect_success 'git -p log'
+
+test_default_pager        expect_success test_must_fail 'git -p'
+test_PAGER_overrides      expect_success test_must_fail 'git -p'
+test_core_pager_overrides expect_success test_must_fail 'git -p'
+test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
+
+test_default_pager        expect_success test_must_fail 'git -p nonsense'
+test_PAGER_overrides      expect_success test_must_fail 'git -p nonsense'
+test_core_pager_overrides expect_success test_must_fail 'git -p nonsense'
+test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p nonsense'
 
 test_done
-- 
1.7.1.579.ge2549

[PATCH 3/4] tests: local config file should be honored from subdirs of toplevel

[Jonathan Nieder]

When git is passed the --paginate option, starting up a pager requires
deciding what pager to start, which requires access to the core.pager
configuration.  If --paginate is handled before searching for the
git dir, this configuration will be missed.

In other words, with --paginate and only with --paginate, any
repository-local core.pager setting is being ignored [*].

[*] unless the git directory is ./.git or GIT_DIR or GIT_CONFIG was
set explicitly.

Add a test to demonstrate this counterintuitive behavior.  Noticed
while reading over a patch by Duy that fixes it.

Cc: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Improved-by: Johannes Sixt <j6t@kdbg.org>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 t/t7006-pager.sh |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 4420f91..2b106be 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -259,6 +259,28 @@ test_core_pager_overrides() {
 	"
 }
 
+test_core_pager_subdir() {
+	parse_args "$@"
+
+	$test_expectation TTY "$cmd - core.pager from subdirectory" "
+		unset GIT_PAGER;
+		rm -f core.pager_used &&
+		rm -fr sub ||
+		cleanup_fail &&
+
+		PAGER=wc &&
+		stampname=\$(pwd)/core.pager_used &&
+		export PAGER stampname &&
+		git config core.pager 'wc >\"\$stampname\"' &&
+		mkdir sub &&
+		(
+			cd sub &&
+			$full_command
+		) &&
+		test -e core.pager_used
+	"
+}
+
 test_GIT_PAGER_overrides() {
 	parse_args "$@"
 
@@ -277,21 +299,25 @@ test_GIT_PAGER_overrides() {
 test_default_pager        expect_success 'git log'
 test_PAGER_overrides      expect_success 'git log'
 test_core_pager_overrides expect_success 'git log'
+test_core_pager_subdir    expect_success 'git log'
 test_GIT_PAGER_overrides  expect_success 'git log'
 
 test_default_pager        expect_success 'git -p log'
 test_PAGER_overrides      expect_success 'git -p log'
 test_core_pager_overrides expect_success 'git -p log'
+test_core_pager_subdir    expect_failure 'git -p log'
 test_GIT_PAGER_overrides  expect_success 'git -p log'
 
 test_default_pager        expect_success test_must_fail 'git -p'
 test_PAGER_overrides      expect_success test_must_fail 'git -p'
 test_core_pager_overrides expect_success test_must_fail 'git -p'
+test_core_pager_subdir    expect_failure test_must_fail 'git -p'
 test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
 
 test_default_pager        expect_success test_must_fail 'git -p nonsense'
 test_PAGER_overrides      expect_success test_must_fail 'git -p nonsense'
 test_core_pager_overrides expect_success test_must_fail 'git -p nonsense'
+test_core_pager_subdir    expect_failure test_must_fail 'git -p nonsense'
 test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p nonsense'
 
 test_done
-- 
1.7.1.579.ge2549

[PATCH 4/4] git --paginate: do not commit pager choice too early

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

When git is passed the --paginate option, starting up a pager requires
deciding what pager to start, which requires access to the core.pager
configuration.

At the relevant moment, the repository has not been searched for yet.
Attempting to access the configuration at this point results in
git_dir being set to .git [*], which is almost certainly not what was
wanted.  In particular, when run from a subdirectory of the toplevel,
git --paginate does not respect the core.pager setting from the
current repository.

[*] unless GIT_DIR or GIT_CONFIG is set

So delay the pager startup when possible:

1. run_argv() already commits pager choice inside run_builtin() if a
   command is found.  For commands that use RUN_SETUP, waiting until
   then fixes the problem described above: once git knows where to
   look, it happily respects the core.pager setting.

2. list_common_cmds_help() prints out 29 lines and exits.  This can
   benefit from pagination, so we need to commit the pager choice
   before writing this output.

   Luckily ‘git’ without subcommand has no other reason to access a
   repository, so it would be intuitive to ignore repository-local
   configuration in this case.  Simpler for now to choose a pager
   using the funny code that notices a repository that happens to be
   at .git.  That this accesses a repository when it is very
   convenient to is a bug but not an important one.

3. help_unknown_cmd() prints out a few lines to stderr.  It is not
   important to paginate this, so don’t.

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>
---
Thanks for reading.

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

diff --git a/git.c b/git.c
index 99f0363..25e7693 100644
--- a/git.c
+++ b/git.c
@@ -511,12 +511,12 @@ 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;
 	} else {
 		/* The user didn't specify a command; give them help */
+		commit_pager_choice();
 		printf("usage: %s\n\n", git_usage_string);
 		list_common_cmds_help();
 		printf("\n%s\n", git_more_info_string);
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 2b106be..eefef45 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -244,9 +244,21 @@ test_PAGER_overrides() {
 }
 
 test_core_pager_overrides() {
+	if_local_config=
+	used_if_wanted='overrides PAGER'
+	test_core_pager "$@"
+}
+
+test_local_config_ignored() {
+	if_local_config='! '
+	used_if_wanted='is not used'
+	test_core_pager "$@"
+}
+
+test_core_pager() {
 	parse_args "$@"
 
-	$test_expectation TTY "$cmd - core.pager overrides PAGER" "
+	$test_expectation TTY "$cmd - repository-local core.pager setting $used_if_wanted" "
 		unset GIT_PAGER;
 		rm -f core.pager_used ||
 		cleanup_fail &&
@@ -255,14 +267,26 @@ test_core_pager_overrides() {
 		export PAGER &&
 		git config core.pager 'wc >core.pager_used' &&
 		$full_command &&
-		test -e core.pager_used
+		${if_local_config}test -e core.pager_used
 	"
 }
 
 test_core_pager_subdir() {
+	if_local_config=
+	used_if_wanted='overrides PAGER'
+	test_pager_subdir_helper "$@"
+}
+
+test_no_local_config_subdir() {
+	if_local_config='! '
+	used_if_wanted='is not used'
+	test_pager_subdir_helper "$@"
+}
+
+test_pager_subdir_helper() {
 	parse_args "$@"
 
-	$test_expectation TTY "$cmd - core.pager from subdirectory" "
+	$test_expectation TTY "$cmd - core.pager $used_if_wanted from subdirectory" "
 		unset GIT_PAGER;
 		rm -f core.pager_used &&
 		rm -fr sub ||
@@ -277,7 +301,7 @@ test_core_pager_subdir() {
 			cd sub &&
 			$full_command
 		) &&
-		test -e core.pager_used
+		${if_local_config}test -e core.pager_used
 	"
 }
 
@@ -296,6 +320,20 @@ test_GIT_PAGER_overrides() {
 	"
 }
 
+test_doesnt_paginate() {
+	parse_args "$@"
+
+	$test_expectation TTY "no pager for '$cmd'" "
+		rm -f GIT_PAGER_used ||
+		cleanup_fail &&
+
+		GIT_PAGER='wc >GIT_PAGER_used' &&
+		export GIT_PAGER &&
+		$full_command &&
+		! test -e GIT_PAGER_used
+	"
+}
+
 test_default_pager        expect_success 'git log'
 test_PAGER_overrides      expect_success 'git log'
 test_core_pager_overrides expect_success 'git log'
@@ -305,19 +343,15 @@ test_GIT_PAGER_overrides  expect_success 'git log'
 test_default_pager        expect_success 'git -p log'
 test_PAGER_overrides      expect_success 'git -p log'
 test_core_pager_overrides expect_success 'git -p log'
-test_core_pager_subdir    expect_failure 'git -p log'
+test_core_pager_subdir    expect_success 'git -p log'
 test_GIT_PAGER_overrides  expect_success 'git -p log'
 
 test_default_pager        expect_success test_must_fail 'git -p'
 test_PAGER_overrides      expect_success test_must_fail 'git -p'
-test_core_pager_overrides expect_success test_must_fail 'git -p'
-test_core_pager_subdir    expect_failure test_must_fail 'git -p'
+test_local_config_ignored expect_failure test_must_fail 'git -p'
+test_no_local_config_subdir expect_success test_must_fail 'git -p'
 test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
 
-test_default_pager        expect_success test_must_fail 'git -p nonsense'
-test_PAGER_overrides      expect_success test_must_fail 'git -p nonsense'
-test_core_pager_overrides expect_success test_must_fail 'git -p nonsense'
-test_core_pager_subdir    expect_failure test_must_fail 'git -p nonsense'
-test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p nonsense'
+test_doesnt_paginate      expect_success test_must_fail 'git -p nonsense'
 
 test_done
-- 
1.7.1.579.ge2549

Re: [PATCH 2/4] t7006: test pager configuration for several git commands

[Jonathan Nieder]

Jonathan Nieder wrote:

> Test choice of pager at several stages of repository setup.

Here it is again, without whitespace changes. :)

 t/t7006-pager.sh |   40 ++++++++++++++++++++++++++++++++++++----
 1 files changed, 36 insertions(+), 4 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index b117ebb..4420f91 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -204,7 +204,9 @@ parse_args() {
 	full_command="$full_command $1"
 }
 
-parse_args expect_success 'git log'
+test_default_pager() {
+	parse_args "$@"
+
 $test_expectation SIMPLEPAGER "$cmd - default pager is used by default" "
 	unset PAGER GIT_PAGER;
 	test_might_fail git config --unset core.pager &&
@@ -223,8 +225,11 @@ $test_expectation SIMPLEPAGER "$cmd - default pager is used by default" "
 	) &&
 	test -e default_pager_used
 "
+}
+
+test_PAGER_overrides() {
+	parse_args "$@"
 
-parse_args expect_success 'git log'
 $test_expectation TTY "$cmd - PAGER overrides default pager" "
 	unset GIT_PAGER;
 	test_might_fail git config --unset core.pager &&
@@ -236,8 +241,11 @@ $test_expectation TTY "$cmd - PAGER overrides default pager" "
 	$full_command &&
 	test -e PAGER_used
 "
+}
+
+test_core_pager_overrides() {
+	parse_args "$@"
 
-parse_args expect_success 'git log'
 $test_expectation TTY "$cmd - core.pager overrides PAGER" "
 	unset GIT_PAGER;
 	rm -f core.pager_used ||
@@ -249,8 +257,11 @@ $test_expectation TTY "$cmd - core.pager overrides PAGER" "
 	$full_command &&
 	test -e core.pager_used
 "
+}
+
+test_GIT_PAGER_overrides() {
+	parse_args "$@"
 
-parse_args expect_success 'git log'
 $test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
 	rm -f GIT_PAGER_used ||
 	cleanup_fail &&
@@ -261,5 +272,26 @@ $test_expectation TTY "$cmd - GIT_PAGER overrides core.pager" "
 	$full_command &&
 	test -e GIT_PAGER_used
 "
+}
+
+test_default_pager        expect_success 'git log'
+test_PAGER_overrides      expect_success 'git log'
+test_core_pager_overrides expect_success 'git log'
+test_GIT_PAGER_overrides  expect_success 'git log'
+
+test_default_pager        expect_success 'git -p log'
+test_PAGER_overrides      expect_success 'git -p log'
+test_core_pager_overrides expect_success 'git -p log'
+test_GIT_PAGER_overrides  expect_success 'git -p log'
+
+test_default_pager        expect_success test_must_fail 'git -p'
+test_PAGER_overrides      expect_success test_must_fail 'git -p'
+test_core_pager_overrides expect_success test_must_fail 'git -p'
+test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
+
+test_default_pager        expect_success test_must_fail 'git -p nonsense'
+test_PAGER_overrides      expect_success test_must_fail 'git -p nonsense'
+test_core_pager_overrides expect_success test_must_fail 'git -p nonsense'
+test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p nonsense'
 
 test_done
-- 
1.7.1.579.ge2549

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Jeff King]

On Sat, Jun 26, 2010 at 02:22:03PM -0500, Jonathan Nieder wrote:

> Here’s a fix from the famous setup cleanup series[1].
> 
> The problem it fixes is somewhat obscure: ‘git -p foo’ is not paying
> attention to the repository-local ‘[core] pager’ configuration when
> run from a subdirectory.  But it is a real bug, and the fix gives
> an example of how to deal with repository setup and should be safe.
> 
> Patch is against master.  There is a small semantic conflict with
> jn/grep-open: SIMPLEPAGER should be changed to SIMPLEPAGERTTY in the
> prerequisites for the test_default_pager function.  Please let me
> know if I should push a merge commit to help resolve that.

I just read through the patches, and it seems like a reasonable fix. So
in that sense, ACK.

But reading the message for patch 4/4, I can't help but wonder if the
right way forward is for the git wrapper to _always_ find the repository
as the very first thing. I.e., make finding the repository a
non-destructive thing (especially don't end in a different directory),
and just do it before we run any other code.

For a few commands like "init" or "clone", we would end up just
ignoring this information, but the worst case should be a little bit of
wasted effort.

And then all these silly setup and timing issues could just go away, as
we would know that GIT_DIR and GIT_WORK_TREE would be set properly.

-Peff

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Jonathan Nieder]

Jeff King wrote:

> But reading the message for patch 4/4, I can't help but wonder if the
> right way forward is for the git wrapper to _always_ find the repository
> as the very first thing.
[...]
> the worst case should be a little bit of
> wasted effort.

This is a very useful thought.  Git without the chdir() to toplevel
would be pleasanter, I think.  (Yes, I realize you weren’t necessarily
suggesting suppressing a chdir_to_toplevel() in place of
setup_git_repository() and friends.)

Regarding the repository search: automounters can cause pain if
DISCOVERY_ACROSS_FILESYSTEM is set and GIT_CEILING_DIRECTORIES is not
set appropriately.  An important script that does

  tmpdir=$(mktemp -d)
  cd "$tmpdir"
  ...

  git diff --no-index a b
  ...

could hang indefinitely if some nut has created a named pipe at
/tmp/.git.  I haven’t come up with any non far-fetched reason to mind
the repository search, though.

Will think about this.

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Jeff King]

On Mon, Jun 28, 2010 at 05:13:35AM -0500, Jonathan Nieder wrote:

> > But reading the message for patch 4/4, I can't help but wonder if the
> > right way forward is for the git wrapper to _always_ find the repository
> > as the very first thing.
> [...]
> > the worst case should be a little bit of
> > wasted effort.
> 
> This is a very useful thought.  Git without the chdir() to toplevel
> would be pleasanter, I think.  (Yes, I realize you weren’t necessarily
> suggesting suppressing a chdir_to_toplevel() in place of
> setup_git_repository() and friends.)

I had just meant that we would not do the chdir() initially, but would
do so right before running the actual command which wanted repository
setup (and commands like init which do not do that setup would never
chdir to the toplevel). But we must always run at least aliases and
external sub-commands from the toplevel to keep backwards compatibility.

> Regarding the repository search: automounters can cause pain if
> DISCOVERY_ACROSS_FILESYSTEM is set and GIT_CEILING_DIRECTORIES is not
> set appropriately.  An important script that does
> 
>   tmpdir=$(mktemp -d)
>   cd "$tmpdir"
>   ...
> 
>   git diff --no-index a b
>   ...
> 
> could hang indefinitely if some nut has created a named pipe at
> /tmp/.git.  I haven’t come up with any non far-fetched reason to mind
> the repository search, though.
> 
> Will think about this.

Yeah, I considered that, too. But I doubt it is a big problem. If it
were, then just running "git log" when you are not in a repository would
be similarly painful. The people who have setups like that are already
having to deal with GIT_CEILING_DIRECTORIES to make things less painful
(though I imagine the new cross-filesystem discovery code will prevent
most of the pain without them having to actually do anything).

So yes, we are adding the extra lookup for commands like "git clone",
but I suspect in practice nobody will care. If it is a big deal, we can
do something like:

  if (!strcmp(cmd, "clone") || !strcmp(cmd, "init"))
     ... don't do setup ...

which I admit is a terrible hack, but it is an optimization, not a
correctness thing. So we are not shutting out external user-defined
commands that might not care about the repo. We are just making our
common built-in ones a bit more efficient.

-Peff

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Nguyen Thai Ngoc Duy]

2010/6/28 Jeff King <peff@peff.net>:
> On Mon, Jun 28, 2010 at 05:13:35AM -0500, Jonathan Nieder wrote:
>
>> > But reading the message for patch 4/4, I can't help but wonder if the
>> > right way forward is for the git wrapper to _always_ find the repository
>> > as the very first thing.
>> [...]
>> > the worst case should be a little bit of
>> > wasted effort.
>>
>> This is a very useful thought.  Git without the chdir() to toplevel
>> would be pleasanter, I think.  (Yes, I realize you weren’t necessarily
>> suggesting suppressing a chdir_to_toplevel() in place of
>> setup_git_repository() and friends.)
>
> I had just meant that we would not do the chdir() initially, but would
> do so right before running the actual command which wanted repository
> setup (and commands like init which do not do that setup would never
> chdir to the toplevel). But we must always run at least aliases and
> external sub-commands from the toplevel to keep backwards compatibility.

I think I'm missing something here. Looking up aliases means
repository search must be done in git wrapper anyway. Even if we don't
have to do repo search in git wrapper, the very first thing a sub
command does is likely git_config(), which will need repo again. As
long as repo search does not have any side effects, everything should
be fine, IMO.

> So yes, we are adding the extra lookup for commands like "git clone",
> but I suspect in practice nobody will care. If it is a big deal, we can
> do something like:
>
>  if (!strcmp(cmd, "clone") || !strcmp(cmd, "init"))
>     ... don't do setup ...
>

If git wrapper does not need setup (i.e. aliases) at all, that should
happen without hacks. "clone" and "init" do not have RUN_SETUP (nor
RUN_SETUP_GENTLY in my series), so there should not be any setup
before cmd_{init,clone} is run.

PS. I haven't forgotten my cleanup series. Day job has been taking too
much of my energy unfortunately.
-- 
Duy

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Patch is against master.  There is a small semantic conflict with
> jn/grep-open: SIMPLEPAGER should be changed to SIMPLEPAGERTTY in the
> prerequisites for the test_default_pager function.  Please let me
> know if I should push a merge commit to help resolve that.

Thanks for advance warning; please double check the merge result in 'pu'
when I push it out...

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Nieder <jrnieder@gmail.com> writes:
>
>> Patch is against master.  There is a small semantic conflict with
>> jn/grep-open: SIMPLEPAGER should be changed to SIMPLEPAGERTTY in the
>> prerequisites for the test_default_pager function.  Please let me
>> know if I should push a merge commit to help resolve that.
>
> Thanks for advance warning; please double check the merge result in 'pu'
> when I push it out...

I hate an enumeration that pretends to be exhausitive but is not.

    So delay the pager startup when possible:
    
    1. run_argv() already commits pager choice inside run_builtin() if a
       command is found.  For commands that use RUN_SETUP, waiting until
       then fixes the problem described above: once git knows where to
       look, it happily respects the core.pager setting.

... and for commands that do not use RUN_SETUP, what happens?

    2. list_common_cmds_help() prints out 29 lines and exits....
    3. help_unknown_cmd() prints out a few lines to stderr.  It is not
       important to paginate this, so don’t.

Missing from the above enumeration are are external commands.  They depend
on commit_pager_choice() to be called before execv_dashed_external() gets
called.  For example, "git -p request-pull $args" no longer works with
this patch.

Sigh..

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Jonathan Nieder]

Junio C Hamano wrote:

> ... and for commands that do not use RUN_SETUP, what happens?

run_builtin() still commits pager choice.  The bug is neither
fixed nor made worse for them.

> Missing from the above enumeration are are external commands.  They depend
> on commit_pager_choice() to be called before execv_dashed_external() gets
> called.  For example, "git -p request-pull $args" no longer works with
> this patch.

Something like the following may help.

This does not protect against calling setup_pager() more than
once.  Once the first pager has been set up, isatty(1) is false,
preventing additional pagers from being spawned and competing with it.

As futureproofing against --paginate=always, setup_pager() should
probably be taught to check pager_in_use(), but that should
probably wait for a separate patch.

diff --git a/git.c b/git.c
index 25e7693..2dcd952 100644
--- a/git.c
+++ b/git.c
@@ -162,6 +162,7 @@ static int handle_alias(int *argcp, const char ***argv)
 	int unused_nongit;
 
 	subdir = setup_git_directory_gently(&unused_nongit);
+	commit_pager_choice();
 
 	alias_command = (*argv)[0];
 	alias_string = alias_lookup(alias_command);
@@ -433,6 +434,7 @@ static void execv_dashed_external(const char **argv)
 	int status;
 
 	strbuf_addf(&cmd, "git-%s", argv[0]);
+	commit_pager_choice();
 
 	/*
 	 * argv[0] must be the git command, but the argv array
-- 

[PATCH] git --paginate: paginate external commands again

[Jonathan Nieder]

73e25e7c (git --paginate: do not commit pager choice too early,
2010-06-26) failed to take some cases into account.

1b. Builtins that do not use RUN_SETUP (like git config) do
    not find GIT_DIR set correctly when the pager is launched
    from run_builtin().  So the core.pager configuration is
    not honored from subdirectories of the toplevel for them.

4a. External git commands (like git request-pull) relied on the
    early pager launch to take care of handling the -p option.
    Ever since 73e25e7c, they do not honor the -p option at all.

4b. Commands invoked through ! aliases (like ls) were also relying
    on the early pager launch.

Fix (4a) by launching the pager (if requested) before running such a
“dashed external”.  For simplicity, this still does not search for a
.git directory before running the external command; when run from a
subdirectory of the toplevel, therefore, the “[core] pager”
configuration is still not honored.

Fix (4b) by launching pager if requested before carrying out such an
alias.  Actually doing this has no effect, since the pager (if any)
would have already been launched in a failed attempt to try a
dashed external first.  The choice-of-pager-not-honored-from-
subdirectory bug still applies here, too.

(1b) is not a regression.  There is no need to fix it yet.

Noticed by Junio.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:

> I hate an enumeration that pretends to be exhausitive but is not.

Sorry about that.

Here’s a patch that could be squashed in or applied on top.

 git.c            |    3 +++
 t/t7006-pager.sh |   47 +++++++++++++++++++++++++++++++++++------------
 2 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/git.c b/git.c
index 25e7693..99517dd 100644
--- a/git.c
+++ b/git.c
@@ -167,6 +167,7 @@ static int handle_alias(int *argcp, const char ***argv)
 	alias_string = alias_lookup(alias_command);
 	if (alias_string) {
 		if (alias_string[0] == '!') {
+			commit_pager_choice();
 			if (*argcp > 1) {
 				struct strbuf buf;
 
@@ -432,6 +433,8 @@ static void execv_dashed_external(const char **argv)
 	const char *tmp;
 	int status;
 
+	commit_pager_choice();
+
 	strbuf_addf(&cmd, "git-%s", argv[0]);
 
 	/*
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index eefef45..9215c2f 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -334,17 +334,40 @@ test_doesnt_paginate() {
 	"
 }
 
-test_default_pager        expect_success 'git log'
-test_PAGER_overrides      expect_success 'git log'
-test_core_pager_overrides expect_success 'git log'
-test_core_pager_subdir    expect_success 'git log'
-test_GIT_PAGER_overrides  expect_success 'git log'
-
-test_default_pager        expect_success 'git -p log'
-test_PAGER_overrides      expect_success 'git -p log'
-test_core_pager_overrides expect_success 'git -p log'
-test_core_pager_subdir    expect_success 'git -p log'
-test_GIT_PAGER_overrides  expect_success 'git -p log'
+test_pager_choices() {
+	test_default_pager        expect_success "$@"
+	test_PAGER_overrides      expect_success "$@"
+	test_core_pager_overrides expect_success "$@"
+	test_core_pager_subdir    expect_success "$@"
+	test_GIT_PAGER_overrides  expect_success "$@"
+}
+
+test_expect_success 'setup: some aliases' '
+	git config alias.aliasedlog log &&
+	git config alias.true "!true"
+'
+
+test_pager_choices                       'git log'
+test_pager_choices                       'git -p log'
+test_pager_choices                       'git aliasedlog'
+
+test_default_pager        expect_success 'git -p aliasedlog'
+test_PAGER_overrides      expect_success 'git -p aliasedlog'
+test_core_pager_overrides expect_success 'git -p aliasedlog'
+test_core_pager_subdir    expect_failure 'git -p aliasedlog'
+test_GIT_PAGER_overrides  expect_success 'git -p aliasedlog'
+
+test_default_pager        expect_success 'git -p true'
+test_PAGER_overrides      expect_success 'git -p true'
+test_core_pager_overrides expect_success 'git -p true'
+test_core_pager_subdir    expect_failure 'git -p true'
+test_GIT_PAGER_overrides  expect_success 'git -p true'
+
+test_default_pager        expect_success test_must_fail 'git -p request-pull'
+test_PAGER_overrides      expect_success test_must_fail 'git -p request-pull'
+test_core_pager_overrides expect_success test_must_fail 'git -p request-pull'
+test_core_pager_subdir    expect_failure test_must_fail 'git -p request-pull'
+test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p request-pull'
 
 test_default_pager        expect_success test_must_fail 'git -p'
 test_PAGER_overrides      expect_success test_must_fail 'git -p'
@@ -352,6 +375,6 @@ test_local_config_ignored expect_failure test_must_fail 'git -p'
 test_no_local_config_subdir expect_success test_must_fail 'git -p'
 test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
 
-test_doesnt_paginate      expect_success test_must_fail 'git -p nonsense'
+test_doesnt_paginate      expect_failure test_must_fail 'git -p nonsense'
 
 test_done
-- 
1.7.2.rc2.532.g65d9b.dirty

Re: [PATCH 0/4] git --paginate: do not commit pager choice too early

[Nguyen Thai Ngoc Duy]

2010/7/15 Junio C Hamano <gitster@pobox.com>:
> Missing from the above enumeration are are external commands.  They depend
> on commit_pager_choice() to be called before execv_dashed_external() gets
> called.  For example, "git -p request-pull $args" no longer works with
> this patch.
>
> Sigh..

I'm sorry. I missed those paths..
-- 
Duy

[PATCH jn/paginate-fix 0/12] Re: git --paginate: do not commit pager choice too early

[Jonathan Nieder]

Junio C Hamano wrote:

>     So delay the pager startup when possible:
>     
>     1. run_argv() already commits pager choice inside run_builtin() if a
>        command is found.  For commands that use RUN_SETUP, waiting until
>        then fixes the problem described above: once git knows where to
>        look, it happily respects the core.pager setting.
> 
> ... and for commands that do not use RUN_SETUP, what happens?

Let’s help some of them out a little.

These patches teach a few more built-ins to do something like
RUN_SETUP; more precisely, a RUN_SETUP_GENTLY facility is introduced
to run setup_git_directory_gently() early just like NEEDS_PREFIX has
always caused setup_git_directory() to be run early.

This series wouldn’t be possible without Duy’s recent efforts to roll
out other fixes from the famous nd/setup topic --- thanks!
The patches should be familiar[1] and I think they are better
justified now.

The first 8 patches apply on top to jn/paginate-fix from pu.  Between
patch 8 and 9 I have a merge of jn/maint-setup-fix^, with the
following explanation:

	Merge branch 'jn/maint-setup-fix' (early part) into jn/paginate-fix

	That topic includes a fix for “git index-pack” (which used
	to save the cwd before the repository search to work around
	a bug) which is needed before the repository search can be
	safely run earlier.

That merge involves a trivial conflict between adding the
RUN_SETUP_GENTLY flag for this topic and removing USE_PAGER for
grep -O.  The conflict is only nominal --- the two changes do not
interfere with each other.

One final note: this series is deliberately very conservative, in that
it does not touch commands like diff (which has a --no-index mode
suppressing the repository search).  Such commands are neither
helped nor hindered by this series, and I think that during this
transition time we should be opportunistic rather than rigid: the rule
is to run the repository search as soon as can be easily justified but
no sooner.

Thoughts welcome, as always.

Nguyễn Thái Ngọc Duy (12):
  git wrapper: introduce startup_info struct
  setup: remember whether repository was found
  git wrapper: allow setup_git_directory_gently() be called earlier
  shortlog: run setup_git_directory_gently() sooner
  grep: run setup_git_directory_gently() sooner
  apply: run setup_git_directory_gently() sooner
  bundle: run setup_git_directory_gently() sooner
  config: run setup_git_directory_gently() sooner
  index-pack: run setup_git_directory_gently() sooner
  ls-remote: run setup_git_directory_gently() sooner
  var: run setup_git_directory_gently() sooner
  merge-file: run setup_git_directory_gently() sooner

[1] http://thread.gmane.org/gmane.comp.version-control.git/144000/focus=144110

[PATCH 01/12] git wrapper: introduce startup_info struct

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

The startup_info struct will collect information managed by the git
setup code, such as the prefix for relative paths passed on the
command line (i.e., path to the starting cwd from the toplevel of
the work tree) and whether a git repository has been found.

In other words, startup_info is intended to be a collection of global
variables with results that were previously returned from setup
functions.  This state is global anyway (since the cwd is), even
if it is not currently tracked that way.  Letting these values persist
means there is more flexibility in deciding when to run setup.

For now, the struct is empty.

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>
---
Changes from last round:
 - commit message
 - remove unnecessary memset (since statics are already automatically
   zero-initialized)

 cache.h       |    5 +++++
 environment.c |    1 +
 git.c         |    3 +++
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/cache.h b/cache.h
index c9fa3df..0822c3b 100644
--- a/cache.h
+++ b/cache.h
@@ -1097,6 +1097,11 @@ void overlay_tree_on_cache(const char *tree_name, const char *prefix);
 char *alias_lookup(const char *alias);
 int split_cmdline(char *cmdline, const char ***argv);
 
+/* git.c */
+struct startup_info {
+};
+extern struct startup_info *startup_info;
+
 /* builtin/merge.c */
 int checkout_fast_forward(const unsigned char *from, const unsigned char *to);
 
diff --git a/environment.c b/environment.c
index 83d38d3..eeb2687 100644
--- a/environment.c
+++ b/environment.c
@@ -53,6 +53,7 @@ enum object_creation_mode object_creation_mode = OBJECT_CREATION_MODE;
 char *notes_ref_name;
 int grafts_replace_parents = 1;
 int core_apply_sparse_checkout;
+struct startup_info *startup_info;
 
 /* Parallel index stat data preload? */
 int core_preload_index = 0;
diff --git a/git.c b/git.c
index 99517dd..8a8d2b2 100644
--- a/git.c
+++ b/git.c
@@ -14,6 +14,7 @@ const char git_usage_string[] =
 const char git_more_info_string[] =
 	"See 'git help COMMAND' for more information on a specific command.";
 
+static struct startup_info git_startup_info;
 static int use_pager = -1;
 struct pager_config {
 	const char *cmd;
@@ -489,6 +490,8 @@ int main(int argc, const char **argv)
 {
 	const char *cmd;
 
+	startup_info = &git_startup_info;
+
 	cmd = git_extract_argv0_path(argv[0]);
 	if (!cmd)
 		cmd = "git-help";
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 02/12] setup: remember whether repository was found

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

As v1.7.2~16^2 (git --paginate: paginate external commands
again, 2010-07-14) explains, builtins (like git config) that
do not use RUN_SETUP are not finding GIT_DIR set correctly when
it is time to launch the pager from run_builtin().  If they
were to search for a repository sooner, then the outcome of such
early repository accesses would be more predictable and reliable.

The cmd_*() functions learn whether a repository was found through the
*nongit_ok return value from setup_git_directory_gently().  If
run_builtin() is to take care of the repository search itself, that
datum needs to be retrievable from somewhere else.  Use the
startup_info struct for this.

As a bonus, this information becomes available to functions such as
git_config() which might want to avoid trying to access a repository
when none is present.

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>
---
Tweaked the log message a bit.

 cache.h |    1 +
 setup.c |   12 +++++++++++-
 2 files changed, 12 insertions(+), 1 deletions(-)

diff --git a/cache.h b/cache.h
index 0822c3b..f9e82a8 100644
--- a/cache.h
+++ b/cache.h
@@ -1099,6 +1099,7 @@ int split_cmdline(char *cmdline, const char ***argv);
 
 /* git.c */
 struct startup_info {
+	int have_repository;
 };
 extern struct startup_info *startup_info;
 
diff --git a/setup.c b/setup.c
index 7e04602..1e32fb7 100644
--- a/setup.c
+++ b/setup.c
@@ -315,7 +315,7 @@ const char *read_gitfile_gently(const char *path)
  * We cannot decide in this function whether we are in the work tree or
  * not, since the config can only be read _after_ this function was called.
  */
-const char *setup_git_directory_gently(int *nongit_ok)
+static const char *setup_git_directory_gently_1(int *nongit_ok)
 {
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
 	const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT);
@@ -470,6 +470,16 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	return cwd + offset;
 }
 
+const char *setup_git_directory_gently(int *nongit_ok)
+{
+	const char *prefix;
+
+	prefix = setup_git_directory_gently_1(nongit_ok);
+	if (startup_info)
+		startup_info->have_repository = !nongit_ok || !*nongit_ok;
+	return prefix;
+}
+
 int git_config_perm(const char *var, const char *value)
 {
 	int i;
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 03/12] git wrapper: allow setup_git_directory_gently() be called earlier

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

In the spirit of v1.4.2-rc3~34^2^2 (Call setup_git_directory() much
earlier, 2006-07-28), let run_builtin() take care of searching for a
repository for built-ins that want to make use of one if present.

So now you can mark your command with RUN_SETUP_GENTLY and use

	nongit = !startup_info->have_repository;

in place of

	prefix = setup_git_directory_gently(&nongit);

and everything will be the same, except the repository is
discovered a little sooner.

As v1.7.2~16^2 (2010-07-14) explains, this should allow more commands
to robustly use features like "git --paginate" that look at local
configuration before the command is actually run.

This patch sets up the infrastructure.  Later patches will teach
particular commands to use it.

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>
---
 - reordered setup option bits (RUN_SETUP_GENTLY lives next to RUN_SETUP now)

 - squashed in "builtin: check pager.<cmd> configuration if
   RUN_SETUP_GENTLY is used"

 git.c |   13 +++++++++----
 1 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/git.c b/git.c
index 8a8d2b2..c46c996 100644
--- a/git.c
+++ b/git.c
@@ -230,13 +230,14 @@ static int handle_alias(int *argcp, const char ***argv)
 
 const char git_version_string[] = GIT_VERSION;
 
-#define RUN_SETUP	(1<<0)
-#define USE_PAGER	(1<<1)
+#define RUN_SETUP		(1<<0)
+#define RUN_SETUP_GENTLY	(1<<1)
+#define USE_PAGER		(1<<2)
 /*
  * require working tree to be present -- anything uses this needs
  * RUN_SETUP for reading from the configuration file.
  */
-#define NEED_WORK_TREE	(1<<2)
+#define NEED_WORK_TREE		(1<<3)
 
 struct cmd_struct {
 	const char *cmd;
@@ -255,8 +256,12 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	if (!help) {
 		if (p->option & RUN_SETUP)
 			prefix = setup_git_directory();
+		if (p->option & RUN_SETUP_GENTLY) {
+			int nongit_ok;
+			prefix = setup_git_directory_gently(&nongit_ok);
+		}
 
-		if (use_pager == -1 && p->option & RUN_SETUP)
+		if (use_pager == -1 && p->option & (RUN_SETUP | RUN_SETUP_GENTLY))
 			use_pager = check_pager_config(p->cmd);
 		if (use_pager == -1 && p->option & USE_PAGER)
 			use_pager = 1;
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 04/12] shortlog: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

shortlog already runs a repository search unconditionally;
running such a search earlier is not very risky.

Without this change, the “[pager] shortlog” configuration
is not respected at all: “git shortlog” unconditionally paginates.

The tests are a bit slow.  Running the full battery like this
for all built-in commands would be counterproductive; the intent is
rather to test shortlog as a representative example command using
..._gently().

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>
---
Clarified commit message; simplified diff by keeping the nongit
variable; added tests.

 builtin/shortlog.c |    3 +--
 git.c              |    2 +-
 t/t7006-pager.sh   |    9 +++++++++
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 5089502..36cb24b 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -249,7 +249,7 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 {
 	static struct shortlog log;
 	static struct rev_info rev;
-	int nongit;
+	int nongit = !startup_info->have_repository;
 
 	static const struct option options[] = {
 		OPT_BOOLEAN('n', "numbered", &log.sort_by_number,
@@ -265,7 +265,6 @@ int cmd_shortlog(int argc, const char **argv, const char *prefix)
 
 	struct parse_opt_ctx_t ctx;
 
-	prefix = setup_git_directory_gently(&nongit);
 	git_config(git_default_config, NULL);
 	shortlog_init(&log);
 	init_revisions(&rev, prefix);
diff --git a/git.c b/git.c
index c46c996..b821058 100644
--- a/git.c
+++ b/git.c
@@ -384,7 +384,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "revert", cmd_revert, RUN_SETUP | NEED_WORK_TREE },
 		{ "rm", cmd_rm, RUN_SETUP },
 		{ "send-pack", cmd_send_pack, RUN_SETUP },
-		{ "shortlog", cmd_shortlog, USE_PAGER },
+		{ "shortlog", cmd_shortlog, RUN_SETUP_GENTLY | USE_PAGER },
 		{ "show-branch", cmd_show_branch, RUN_SETUP },
 		{ "show", cmd_show, RUN_SETUP | USE_PAGER },
 		{ "status", cmd_status, RUN_SETUP | NEED_WORK_TREE },
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 9215c2f..084dfdb 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -377,4 +377,13 @@ test_GIT_PAGER_overrides  expect_success test_must_fail 'git -p'
 
 test_doesnt_paginate      expect_failure test_must_fail 'git -p nonsense'
 
+test_pager_choices                       'git shortlog'
+test_expect_success 'setup: configure shortlog not to paginate' '
+	git config pager.shortlog false
+'
+test_doesnt_paginate      expect_success 'git shortlog'
+test_no_local_config_subdir expect_success 'git shortlog'
+test_default_pager        expect_success 'git -p shortlog'
+test_core_pager_subdir    expect_success 'git -p shortlog'
+
 test_done
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 05/12] grep: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

git grep already runs a repository search unconditionally,
even when the --no-index option is supplied; running such a
search earlier is not very risky.

Just like with shortlog, without this change, the
“[pager] grep” configuration is not respected at all.

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>
---
Aside from rewriting the commit message and adding tests, this
drops the interesting

 -		/* die the same way as if we did it at the beginning */
 -		setup_git_directory();
 +		die("No git repository found");

hunk.  That change might be a good idea but it does not fit the
theme of this chapter.

 builtin/grep.c   |    6 ++----
 git.c            |    2 +-
 t/t7006-pager.sh |   13 +++++++++++++
 3 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/builtin/grep.c b/builtin/grep.c
index d0a73da..cd44926 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -791,7 +791,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 	const char **paths = NULL;
 	int i;
 	int dummy;
-	int nongit = 0, use_index = 1;
+	int use_index = 1;
 	struct option options[] = {
 		OPT_BOOLEAN(0, "cached", &cached,
 			"search in index instead of in the work tree"),
@@ -879,8 +879,6 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_END()
 	};
 
-	prefix = setup_git_directory_gently(&nongit);
-
 	/*
 	 * 'git grep -h', unlike 'git grep -h <pattern>', is a request
 	 * to show usage information and exit.
@@ -925,7 +923,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 			     PARSE_OPT_STOP_AT_NON_OPTION |
 			     PARSE_OPT_NO_INTERNAL_HELP);
 
-	if (use_index && nongit)
+	if (use_index && !startup_info->have_repository)
 		/* die the same way as if we did it at the beginning */
 		setup_git_directory();
 
diff --git a/git.c b/git.c
index b821058..0240179 100644
--- a/git.c
+++ b/git.c
@@ -336,7 +336,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "fsck-objects", cmd_fsck, RUN_SETUP },
 		{ "gc", cmd_gc, RUN_SETUP },
 		{ "get-tar-commit-id", cmd_get_tar_commit_id },
-		{ "grep", cmd_grep, USE_PAGER },
+		{ "grep", cmd_grep, RUN_SETUP_GENTLY | USE_PAGER },
 		{ "hash-object", cmd_hash_object },
 		{ "help", cmd_help },
 		{ "index-pack", cmd_index_pack },
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 084dfdb..fd7f77b 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -105,6 +105,19 @@ test_expect_success TTY 'no pager with --no-pager' '
 	! test -e paginated.out
 '
 
+test_expect_success TTY 'configuration can disable pager' '
+	rm -f paginated.out &&
+	test_might_fail git config --unset pager.grep &&
+	test_terminal git grep initial &&
+	test -e paginated.out &&
+
+	rm -f paginated.out &&
+	git config pager.grep false &&
+	test_when_finished "git config --unset pager.grep" &&
+	test_terminal git grep initial &&
+	! test -e paginated.out
+'
+
 # A colored commit log will begin with an appropriate ANSI escape
 # for the first color; the text "commit" comes later.
 colorful() {
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 06/12] apply: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

As v1.7.2~16^2 (2010-07-14) explains, without this change,
“git --paginate apply” can ignore the repository-local
“[core] pager” configuration.

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>
---
New explanation and test; simplified patch by keeping the
is_not_gitdir local.

 builtin/apply.c  |    3 +--
 git.c            |    2 +-
 t/t7006-pager.sh |    3 +++
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 562e534..08f3586 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3611,7 +3611,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 {
 	int i;
 	int errs = 0;
-	int is_not_gitdir;
+	int is_not_gitdir = !startup_info->have_repository;
 	int binary;
 	int force_apply = 0;
 
@@ -3684,7 +3684,6 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		OPT_END()
 	};
 
-	prefix = setup_git_directory_gently(&is_not_gitdir);
 	prefix_length = prefix ? strlen(prefix) : 0;
 	git_config(git_apply_config, NULL);
 	if (apply_default_whitespace)
diff --git a/git.c b/git.c
index 0240179..82b390b 100644
--- a/git.c
+++ b/git.c
@@ -301,7 +301,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 		{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 		{ "annotate", cmd_annotate, RUN_SETUP },
-		{ "apply", cmd_apply },
+		{ "apply", cmd_apply, RUN_SETUP_GENTLY },
 		{ "archive", cmd_archive },
 		{ "bisect--helper", cmd_bisect__helper, RUN_SETUP | NEED_WORK_TREE },
 		{ "blame", cmd_blame, RUN_SETUP },
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index fd7f77b..96fbb0f 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -399,4 +399,7 @@ test_no_local_config_subdir expect_success 'git shortlog'
 test_default_pager        expect_success 'git -p shortlog'
 test_core_pager_subdir    expect_success 'git -p shortlog'
 
+test_core_pager_subdir    expect_success test_must_fail \
+                                         'git -p apply </dev/null'
+
 test_done
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 07/12] bundle: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Without this change, “git -p bundle” does not always
respect the repository-local “[core] pager” setting.

It is hard to notice because subcommands other than
“git bundle unbundle” do not produce much output.

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>
---
New tests and explanation.

The first new test perhaps deserves some explanation.  Its
only purpose is to let us use the 

	test -e paginated.out || test -e subdir/paginated.out

hedge in other tests with good conscience.

 builtin/bundle.c |    6 ++----
 git.c            |    2 +-
 t/t7006-pager.sh |   33 +++++++++++++++++++++++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)

diff --git a/builtin/bundle.c b/builtin/bundle.c
index 2006cc5..80649ba 100644
--- a/builtin/bundle.c
+++ b/builtin/bundle.c
@@ -18,7 +18,6 @@ static const char builtin_bundle_usage[] =
 int cmd_bundle(int argc, const char **argv, const char *prefix)
 {
 	struct bundle_header header;
-	int nongit;
 	const char *cmd, *bundle_file;
 	int bundle_fd = -1;
 	char buffer[PATH_MAX];
@@ -31,7 +30,6 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)
 	argc -= 2;
 	argv += 2;
 
-	prefix = setup_git_directory_gently(&nongit);
 	if (prefix && bundle_file[0] != '/') {
 		snprintf(buffer, sizeof(buffer), "%s/%s", prefix, bundle_file);
 		bundle_file = buffer;
@@ -54,11 +52,11 @@ int cmd_bundle(int argc, const char **argv, const char *prefix)
 		return !!list_bundle_refs(&header, argc, argv);
 	}
 	if (!strcmp(cmd, "create")) {
-		if (nongit)
+		if (!startup_info->have_repository)
 			die("Need a repository to create a bundle.");
 		return !!create_bundle(&header, bundle_file, argc, argv);
 	} else if (!strcmp(cmd, "unbundle")) {
-		if (nongit)
+		if (!startup_info->have_repository)
 			die("Need a repository to unbundle.");
 		return !!unbundle(&header, bundle_fd) ||
 			list_bundle_refs(&header, argc, argv);
diff --git a/git.c b/git.c
index 82b390b..94bb1e3 100644
--- a/git.c
+++ b/git.c
@@ -306,7 +306,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "bisect--helper", cmd_bisect__helper, RUN_SETUP | NEED_WORK_TREE },
 		{ "blame", cmd_blame, RUN_SETUP },
 		{ "branch", cmd_branch, RUN_SETUP },
-		{ "bundle", cmd_bundle },
+		{ "bundle", cmd_bundle, RUN_SETUP_GENTLY },
 		{ "cat-file", cmd_cat_file, RUN_SETUP },
 		{ "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
 		{ "checkout-index", cmd_checkout_index,
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 96fbb0f..6680668 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -57,6 +57,21 @@ test_expect_success TTY 'some commands use a pager' '
 	test -e paginated.out
 '
 
+test_expect_failure TTY 'pager runs from subdir' '
+	echo subdir/paginated.out >expected &&
+	mkdir -p subdir &&
+	rm -f paginated.out subdir/paginated.out &&
+	(
+		cd subdir &&
+		test_terminal git log
+	) &&
+	{
+		ls paginated.out subdir/paginated.out ||
+		:
+	} >actual &&
+	test_cmp expected actual
+'
+
 test_expect_success TTY 'some commands do not use a pager' '
 	rm -f paginated.out ||
 	cleanup_fail &&
@@ -118,6 +133,24 @@ test_expect_success TTY 'configuration can disable pager' '
 	! test -e paginated.out
 '
 
+test_expect_success 'configuration can enable pager (from subdir)' '
+	rm -f paginated.out &&
+	mkdir -p subdir &&
+	git config pager.bundle true &&
+	test_when_finished "git config --unset pager.bundle" &&
+
+	git bundle create test.bundle --all &&
+	rm -f paginated.out subdir/paginated.out &&
+	(
+		cd subdir &&
+		test_terminal git bundle unbundle ../test.bundle
+	) &&
+	{
+		test -e paginated.out ||
+		test -e subdir/paginated.out
+	}
+'
+
 # A colored commit log will begin with an appropriate ANSI escape
 # for the first color; the text "commit" comes later.
 colorful() {
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 08/12] config: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

For the pager choice (and the choice to paginate) to reflect the
current repository configuration, the repository needs to be
located first.

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>
---
Relative to the previous round, this simplifies the patch by retaining
the nongit var; squashes in a test; and adds some words of explanation
to the log message.

 builtin/config.c |    5 ++---
 git.c            |    4 ++--
 t/t7006-pager.sh |    8 ++++++++
 3 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/builtin/config.c b/builtin/config.c
index f3d1660..3f8ef91 100644
--- a/builtin/config.c
+++ b/builtin/config.c
@@ -330,11 +330,10 @@ static int get_colorbool(int print)
 		return get_colorbool_found ? 0 : 1;
 }
 
-int cmd_config(int argc, const char **argv, const char *unused_prefix)
+int cmd_config(int argc, const char **argv, const char *prefix)
 {
-	int nongit;
+	int nongit = !startup_info->have_repository;
 	char *value;
-	const char *prefix = setup_git_directory_gently(&nongit);
 
 	config_exclusive_filename = getenv(CONFIG_ENVIRONMENT);
 
diff --git a/git.c b/git.c
index 94bb1e3..f57003d 100644
--- a/git.c
+++ b/git.c
@@ -319,7 +319,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
 		{ "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
 		{ "commit-tree", cmd_commit_tree, RUN_SETUP },
-		{ "config", cmd_config },
+		{ "config", cmd_config, RUN_SETUP_GENTLY },
 		{ "count-objects", cmd_count_objects, RUN_SETUP },
 		{ "describe", cmd_describe, RUN_SETUP },
 		{ "diff", cmd_diff },
@@ -376,7 +376,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "reflog", cmd_reflog, RUN_SETUP },
 		{ "remote", cmd_remote, RUN_SETUP },
 		{ "replace", cmd_replace, RUN_SETUP },
-		{ "repo-config", cmd_config },
+		{ "repo-config", cmd_config, RUN_SETUP_GENTLY },
 		{ "rerere", cmd_rerere, RUN_SETUP },
 		{ "reset", cmd_reset, RUN_SETUP },
 		{ "rev-list", cmd_rev_list, RUN_SETUP },
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 6680668..dbbe8d9 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -133,6 +133,14 @@ test_expect_success TTY 'configuration can disable pager' '
 	! test -e paginated.out
 '
 
+test_expect_success 'git config uses a pager if configured to' '
+	rm -f paginated.out &&
+	git config pager.config true &&
+	test_when_finished "git config --unset pager.config" &&
+	test_terminal git config --list &&
+	test -e paginated.out
+'
+
 test_expect_success 'configuration can enable pager (from subdir)' '
 	rm -f paginated.out &&
 	mkdir -p subdir &&
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 09/12] index-pack: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

index-pack already runs a repository search unconditionally; running
such a search earlier is not risky and ensures GIT_DIR will be set
correctly if the configuration needs to be accessed from
run_builtin().

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>
---
Requires the jn/maint-setup-fix topic to apply.  See the cover
letter "[PATCH jn/paginate-fix 0/12] Re: ..." for more on that.

Just like last round, except for the new log message.

 builtin/index-pack.c |    2 --
 git.c                |    2 +-
 2 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 89a1f12..e852890 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -880,12 +880,10 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
 	char *index_name_buf = NULL, *keep_name_buf = NULL;
 	struct pack_idx_entry **idx_objects;
 	unsigned char pack_sha1[20];
-	int nongit;
 
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(index_pack_usage);
 
-	prefix = setup_git_directory_gently(&nongit);
 	git_config(git_index_pack_config, NULL);
 	if (prefix && chdir(prefix))
 		die("Cannot come back to cwd");
diff --git a/git.c b/git.c
index e89c010..92ec5d3 100644
--- a/git.c
+++ b/git.c
@@ -339,7 +339,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "grep", cmd_grep, RUN_SETUP_GENTLY },
 		{ "hash-object", cmd_hash_object },
 		{ "help", cmd_help },
-		{ "index-pack", cmd_index_pack },
+		{ "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
 		{ "init", cmd_init_db },
 		{ "init-db", cmd_init_db },
 		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 10/12] ls-remote: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

ls-remote already runs a repository search unconditionally to learn
about remote nicknames and "[url] insteadof" shortcuts.  Run that
search a little sooner, and now one can try

	[pager]
		ls-remote

to automatically paginate ls-remote output, or use repository-local

	[core]
		pager = whatever

with "git --paginate ls-remote <url>".

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>
---
As before, aside from the commit message.

 builtin/ls-remote.c |    3 ---
 git.c               |    4 ++--
 2 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/builtin/ls-remote.c b/builtin/ls-remote.c
index 34480cf..97eed40 100644
--- a/builtin/ls-remote.c
+++ b/builtin/ls-remote.c
@@ -32,7 +32,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *dest = NULL;
-	int nongit;
 	unsigned flags = 0;
 	int quiet = 0;
 	const char *uploadpack = NULL;
@@ -42,8 +41,6 @@ int cmd_ls_remote(int argc, const char **argv, const char *prefix)
 	struct transport *transport;
 	const struct ref *ref;
 
-	setup_git_directory_gently(&nongit);
-
 	for (i = 1; i < argc; i++) {
 		const char *arg = argv[i];
 
diff --git a/git.c b/git.c
index 92ec5d3..63630e7 100644
--- a/git.c
+++ b/git.c
@@ -345,7 +345,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "log", cmd_log, RUN_SETUP | USE_PAGER },
 		{ "ls-files", cmd_ls_files, RUN_SETUP },
 		{ "ls-tree", cmd_ls_tree, RUN_SETUP },
-		{ "ls-remote", cmd_ls_remote },
+		{ "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
 		{ "mailinfo", cmd_mailinfo },
 		{ "mailsplit", cmd_mailsplit },
 		{ "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
@@ -366,7 +366,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "pack-objects", cmd_pack_objects, RUN_SETUP },
 		{ "pack-redundant", cmd_pack_redundant, RUN_SETUP },
 		{ "patch-id", cmd_patch_id },
-		{ "peek-remote", cmd_ls_remote },
+		{ "peek-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
 		{ "pickaxe", cmd_blame, RUN_SETUP },
 		{ "prune", cmd_prune, RUN_SETUP },
 		{ "prune-packed", cmd_prune_packed, RUN_SETUP },
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 11/12] var: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Part of a campaign to make repository-local configuration
available early (simplifying the startup sequence for
built-in commands).

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>
---
snuck in a little vertical compression while at it ;-)

 builtin/var.c |    9 ++-------
 git.c         |    2 +-
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/builtin/var.c b/builtin/var.c
index 70fdb4d..0744bb8 100644
--- a/builtin/var.c
+++ b/builtin/var.c
@@ -74,14 +74,9 @@ static int show_config(const char *var, const char *value, void *cb)
 
 int cmd_var(int argc, const char **argv, const char *prefix)
 {
-	const char *val;
-	int nongit;
-	if (argc != 2) {
+	const char *val = NULL;
+	if (argc != 2)
 		usage(var_usage);
-	}
-
-	setup_git_directory_gently(&nongit);
-	val = NULL;
 
 	if (strcmp(argv[1], "-l") == 0) {
 		git_config(show_config, NULL);
diff --git a/git.c b/git.c
index 63630e7..84bef76 100644
--- a/git.c
+++ b/git.c
@@ -398,7 +398,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "update-ref", cmd_update_ref, RUN_SETUP },
 		{ "update-server-info", cmd_update_server_info, RUN_SETUP },
 		{ "upload-archive", cmd_upload_archive },
-		{ "var", cmd_var },
+		{ "var", cmd_var, RUN_SETUP_GENTLY },
 		{ "verify-tag", cmd_verify_tag, RUN_SETUP },
 		{ "version", cmd_version },
 		{ "whatchanged", cmd_whatchanged, RUN_SETUP | USE_PAGER },
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 12/12] merge-file: run setup_git_directory_gently() sooner

[Jonathan Nieder]

From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>

Part of a campaign to make repository-local configuration
available early (simplifying the startup sequence for
built-in commands).

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>
---
As before (except the commit message).

Well, that’s it.  There’s also a separate fix for
"check-ref-format --branch", which I’ll send under separate
cover.

Still to be considered are various commands that currently
do not do an unconditional repository search, especially
low-level commands that are not about the current repository:
verify-pack, mailinfo, hash-object, archive --remote,
check-ref-format, diff --no-index, help, and all the others.
Let’s bite off one thing at a time. :)

Thanks for reading; I hope the patches were not too dull.
Thoughts (especially improvements) welcome, as always.

 builtin/merge-file.c |    4 +---
 git.c                |    2 +-
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/builtin/merge-file.c b/builtin/merge-file.c
index b8e9e5b..b6664d4 100644
--- a/builtin/merge-file.c
+++ b/builtin/merge-file.c
@@ -28,7 +28,6 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	xmparam_t xmp = {{0}};
 	int ret = 0, i = 0, to_stdout = 0;
 	int quiet = 0;
-	int nongit;
 	struct option options[] = {
 		OPT_BOOLEAN('p', "stdout", &to_stdout, "send results to standard output"),
 		OPT_SET_INT(0, "diff3", &xmp.style, "use a diff3 based merge", XDL_MERGE_DIFF3),
@@ -50,8 +49,7 @@ int cmd_merge_file(int argc, const char **argv, const char *prefix)
 	xmp.style = 0;
 	xmp.favor = 0;
 
-	prefix = setup_git_directory_gently(&nongit);
-	if (!nongit) {
+	if (startup_info->have_repository) {
 		/* Read the configuration file */
 		git_config(git_xmerge_config, NULL);
 		if (0 <= git_xmerge_style)
diff --git a/git.c b/git.c
index 84bef76..5433875 100644
--- a/git.c
+++ b/git.c
@@ -350,7 +350,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "mailsplit", cmd_mailsplit },
 		{ "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
 		{ "merge-base", cmd_merge_base, RUN_SETUP },
-		{ "merge-file", cmd_merge_file },
+		{ "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
 		{ "merge-index", cmd_merge_index, RUN_SETUP },
 		{ "merge-ours", cmd_merge_ours, RUN_SETUP },
 		{ "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
-- 
1.7.2.1.544.ga752d.dirty

[PATCH master 0/2] fix "check-ref-format --branch" from subdir of toplevel

[Jonathan Nieder]

Here’s a quick fix from the nd/setup series.  Tested against master,
though it probably should be against maint.

Jonathan Nieder (2):
  check-ref-format: split off functions for subcommands
  check-ref-format --branch: run repository search

 builtin/check-ref-format.c  |   44 ++++++++++++++++++++++++++----------------
 t/t1402-check-ref-format.sh |   17 ++++++++++++++++
 2 files changed, 44 insertions(+), 17 deletions(-)

[PATCH 1/2] check-ref-format: handle subcommands in separate functions

[Jonathan Nieder]

The code for each subcommand should be easier to read and manipulate
this way.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 builtin/check-ref-format.c |   42 +++++++++++++++++++++++++-----------------
 1 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index b106c65..8707ee9 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -33,28 +33,36 @@ static void collapse_slashes(char *dst, const char *src)
 	*dst = '\0';
 }
 
+static int check_ref_format_branch(const char *arg)
+{
+	struct strbuf sb = STRBUF_INIT;
+
+	if (strbuf_check_branch_ref(&sb, arg))
+		die("'%s' is not a valid branch name", arg);
+	printf("%s\n", sb.buf + 11);
+	return 0;
+}
+
+static int check_ref_format_print(const char *arg)
+{
+	char *refname = xmalloc(strlen(arg) + 1);
+
+	if (check_ref_format(arg))
+		return 1;
+	collapse_slashes(refname, arg);
+	printf("%s\n", refname);
+	return 0;
+}
+
 int cmd_check_ref_format(int argc, const char **argv, const char *prefix)
 {
 	if (argc == 2 && !strcmp(argv[1], "-h"))
 		usage(builtin_check_ref_format_usage);
 
-	if (argc == 3 && !strcmp(argv[1], "--branch")) {
-		struct strbuf sb = STRBUF_INIT;
-
-		if (strbuf_check_branch_ref(&sb, argv[2]))
-			die("'%s' is not a valid branch name", argv[2]);
-		printf("%s\n", sb.buf + 11);
-		exit(0);
-	}
-	if (argc == 3 && !strcmp(argv[1], "--print")) {
-		char *refname = xmalloc(strlen(argv[2]) + 1);
-
-		if (check_ref_format(argv[2]))
-			exit(1);
-		collapse_slashes(refname, argv[2]);
-		printf("%s\n", refname);
-		exit(0);
-	}
+	if (argc == 3 && !strcmp(argv[1], "--branch"))
+		return check_ref_format_branch(argv[2]);
+	if (argc == 3 && !strcmp(argv[1], "--print"))
+		return check_ref_format_print(argv[2]);
 	if (argc != 2)
 		usage(builtin_check_ref_format_usage);
 	return !!check_ref_format(argv[1]);
-- 
1.7.2.1.544.ga752d.dirty

[PATCH 2/2] Allow "check-ref-format --branch" from subdirectory

[Jonathan Nieder]

check-ref-format --branch requires access to the repository
to resolve refs like @{-1}.

Noticed by Duy.

Cc: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
The original used the RUN_SETUP_GENTLY flag to run setup
unconditionally.  That might still be a good idea, but I am
more comfortable running setup just for this subcommand
for now so commands like "git check-ref-format refs/foo" are not
affected.

 builtin/check-ref-format.c  |    2 ++
 t/t1402-check-ref-format.sh |   17 +++++++++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/builtin/check-ref-format.c b/builtin/check-ref-format.c
index 8707ee9..ae3f281 100644
--- a/builtin/check-ref-format.c
+++ b/builtin/check-ref-format.c
@@ -36,7 +36,9 @@ static void collapse_slashes(char *dst, const char *src)
 static int check_ref_format_branch(const char *arg)
 {
 	struct strbuf sb = STRBUF_INIT;
+	int nongit;
 
+	setup_git_directory_gently(&nongit);
 	if (strbuf_check_branch_ref(&sb, arg))
 		die("'%s' is not a valid branch name", arg);
 	printf("%s\n", sb.buf + 11);
diff --git a/t/t1402-check-ref-format.sh b/t/t1402-check-ref-format.sh
index eb45afb..782e75d 100755
--- a/t/t1402-check-ref-format.sh
+++ b/t/t1402-check-ref-format.sh
@@ -41,6 +41,23 @@ test_expect_success "check-ref-format --branch @{-1}" '
 	refname2=$(git check-ref-format --branch @{-2}) &&
 	test "$refname2" = master'
 
+test_expect_success 'check-ref-format --branch from subdir' '
+	mkdir subdir &&
+
+	T=$(git write-tree) &&
+	sha1=$(echo A | git commit-tree $T) &&
+	git update-ref refs/heads/master $sha1 &&
+	git update-ref refs/remotes/origin/master $sha1
+	git checkout master &&
+	git checkout origin/master &&
+	git checkout master &&
+	refname=$(
+		cd subdir &&
+		git check-ref-format --branch @{-1}
+	) &&
+	test "$refname" = "$sha1"
+'
+
 valid_ref_normalized() {
 	test_expect_success "ref name '$1' simplifies to '$2'" "
 		refname=\$(git check-ref-format --print '$1') &&
-- 
1.7.2.1.544.ga752d.dirty

Re: [PATCH jn/paginate-fix 0/12] Re: git --paginate: do not commit pager choice too early

[Nguyen Thai Ngoc Duy]

2010/8/6 Jonathan Nieder <jrnieder@gmail.com>:
> Nguyễn Thái Ngọc Duy (12):
>  git wrapper: introduce startup_info struct
>  setup: remember whether repository was found
>  git wrapper: allow setup_git_directory_gently() be called earlier
>  shortlog: run setup_git_directory_gently() sooner
>  grep: run setup_git_directory_gently() sooner
>  apply: run setup_git_directory_gently() sooner
>  bundle: run setup_git_directory_gently() sooner
>  config: run setup_git_directory_gently() sooner
>  index-pack: run setup_git_directory_gently() sooner
>  ls-remote: run setup_git_directory_gently() sooner
>  var: run setup_git_directory_gently() sooner
>  merge-file: run setup_git_directory_gently() sooner
>
> [1] http://thread.gmane.org/gmane.comp.version-control.git/144000/focus=144110
>

I was waiting for jn/maint-setup-fix to graduate before pushing out
some more patches then I got side tracked by the subtree clone.
Anyway, thanks!
-- 
Duy

Re: [PATCH 2/2] Allow "check-ref-format --branch" from subdirectory

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> check-ref-format --branch requires access to the repository
> to resolve refs like @{-1}.

Thanks; the patch makes sense to me.

Re: [PATCH 06/12] apply: run setup_git_directory_gently() sooner

[Ævar Arnfjörð Bjarmason]

2010/8/6 Jonathan Nieder <jrnieder@gmail.com>:
> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>
> As v1.7.2~16^2 (2010-07-14) explains, without this change,
> “git --paginate apply” can ignore the repository-local
> “[core] pager” configuration.

Applying this patch broke the following tests:

    ./t4119-apply-config.sh ./t4111-apply-subdir.sh
./t4131-apply-fake-ancestor.sh

I didn't look into why, they're breaking in pu now at 6ea3604. I
didn't look into why, see the smoke report at
http://smoke.git.nix.is/app/projects/report_details/33

Re: [PATCH 06/12] apply: run setup_git_directory_gently() sooner

[Nguyen Thai Ngoc Duy]

On Mon, Aug 16, 2010 at 6:13 AM, Ævar Arnfjörð Bjarmason
<avarab@gmail.com> wrote:
> 2010/8/6 Jonathan Nieder <jrnieder@gmail.com>:
>> From: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
>>
>> As v1.7.2~16^2 (2010-07-14) explains, without this change,
>> “git --paginate apply” can ignore the repository-local
>> “[core] pager” configuration.
>
> Applying this patch broke the following tests:
>
>    ./t4119-apply-config.sh ./t4111-apply-subdir.sh
> ./t4131-apply-fake-ancestor.sh
>
> I didn't look into why, they're breaking in pu now at 6ea3604. I
> didn't look into why, see the smoke report at
> http://smoke.git.nix.is/app/projects/report_details/33

The patch loses prefix and git-apply couldn't find files on disk. This
patch may fix it. I'm running tests now.

diff --git a/builtin/apply.c b/builtin/apply.c
index 6c4b747..29f4194 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3779,7 +3779,7 @@ static int option_parse_directory(const struct
option *opt,
 	return 0;
 }

-int cmd_apply(int argc, const char **argv, const char *unused_prefix)
+int cmd_apply(int argc, const char **argv, const char *prefix_)
 {
 	int i;
 	int errs = 0;
@@ -3856,6 +3856,7 @@ int cmd_apply(int argc, const char **argv, const
char *unused_prefix)
 		OPT_END()
 	};

+	prefix = prefix_;
 	prefix_length = prefix ? strlen(prefix) : 0;
 	git_config(git_apply_config, NULL);
 	if (apply_default_whitespace)
-- 
Duy

Re: [PATCH 06/12] apply: run setup_git_directory_gently() sooner

[Jonathan Nieder]

Nguyen Thai Ngoc Duy wrote:

> The patch loses prefix and git-apply couldn't find files on disk. This
> patch may fix it. I'm running tests now.

Oh, good catch.  Thanks!

> diff --git a/builtin/apply.c b/builtin/apply.c
> index 6c4b747..29f4194 100644
> --- a/builtin/apply.c
> +++ b/builtin/apply.c
> @@ -3779,7 +3779,7 @@ static int option_parse_directory(const struct
> option *opt,
>  	return 0;
>  }
> 
> -int cmd_apply(int argc, const char **argv, const char *unused_prefix)
> +int cmd_apply(int argc, const char **argv, const char *prefix_)
>  {
>  	int i;
>  	int errs = 0;
> @@ -3856,6 +3856,7 @@ int cmd_apply(int argc, const char **argv, const
> char *unused_prefix)
>  		OPT_END()
>  	};
> 
> +	prefix = prefix_;

This is why we shouldn’t be using globals so cavalierly. :)

[PATCH 06/12 v2] apply: run setup_git_directory_gently() sooner

[Nguyễn Thái Ngọc Duy]

As v1.7.2~16^2 (2010-07-14) explains, without this change,
“git --paginate apply” can ignore the repository-local
“[core] pager” configuration.

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 This is a replacement for 4ce097c (apply: run
 setup_git_directory_gently() sooner) in pu, branch jn/paginate-fix,
 which fixes prefix loss in 4ce097c.

 All tests seem to run ok for me.

 builtin/apply.c  |    6 +++---
 git.c            |    2 +-
 t/t7006-pager.sh |    3 +++
 3 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/builtin/apply.c b/builtin/apply.c
index 12ef9ea..f005ba1 100644
--- a/builtin/apply.c
+++ b/builtin/apply.c
@@ -3607,11 +3607,11 @@ static int option_parse_directory(const struct option *opt,
 	return 0;
 }
 
-int cmd_apply(int argc, const char **argv, const char *unused_prefix)
+int cmd_apply(int argc, const char **argv, const char *prefix_)
 {
 	int i;
 	int errs = 0;
-	int is_not_gitdir;
+	int is_not_gitdir = !startup_info->have_repository;
 	int binary;
 	int force_apply = 0;
 
@@ -3684,7 +3684,7 @@ int cmd_apply(int argc, const char **argv, const char *unused_prefix)
 		OPT_END()
 	};
 
-	prefix = setup_git_directory_gently(&is_not_gitdir);
+	prefix = prefix_;
 	prefix_length = prefix ? strlen(prefix) : 0;
 	git_config(git_apply_config, NULL);
 	if (apply_default_whitespace)
diff --git a/git.c b/git.c
index 5a47bfb..38dbe70 100644
--- a/git.c
+++ b/git.c
@@ -301,7 +301,7 @@ static void handle_internal_command(int argc, const char **argv)
 		{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 		{ "stage", cmd_add, RUN_SETUP | NEED_WORK_TREE },
 		{ "annotate", cmd_annotate, RUN_SETUP },
-		{ "apply", cmd_apply },
+		{ "apply", cmd_apply, RUN_SETUP_GENTLY },
 		{ "archive", cmd_archive },
 		{ "bisect--helper", cmd_bisect__helper, RUN_SETUP | NEED_WORK_TREE },
 		{ "blame", cmd_blame, RUN_SETUP },
diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index bb95335..6c86d70 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -391,4 +391,7 @@ test_no_local_config_subdir expect_success 'git shortlog'
 test_default_pager        expect_success 'git -p shortlog'
 test_core_pager_subdir    expect_success 'git -p shortlog'
 
+test_core_pager_subdir    expect_success test_must_fail \
+					 'git -p apply </dev/null'
+
 test_done
-- 
1.7.1.rc1.69.g24c2f7

Re: [PATCH 06/12 v2] apply: run setup_git_directory_gently() sooner

[Junio C Hamano]

Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:

> As v1.7.2~16^2 (2010-07-14) explains, without this change,
> “git --paginate apply” can ignore the repository-local
> “[core] pager” configuration.
>
> 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>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  This is a replacement for 4ce097c (apply: run
>  setup_git_directory_gently() sooner) in pu, branch jn/paginate-fix,
>  which fixes prefix loss in 4ce097c.

Thanks for a quick fix.

Re: [PATCH 07/12] bundle: run setup_git_directory_gently() sooner

[Thomas Rast]

Jonathan Nieder wrote:
>  builtin/bundle.c |    6 ++----
>  git.c            |    2 +-
>  t/t7006-pager.sh |   33 +++++++++++++++++++++++++++++++++
>  3 files changed, 36 insertions(+), 5 deletions(-)
[...]
> +test_expect_success 'configuration can enable pager (from subdir)' '
> +	rm -f paginated.out &&
> +	mkdir -p subdir &&
> +	git config pager.bundle true &&
> +	test_when_finished "git config --unset pager.bundle" &&
> +
> +	git bundle create test.bundle --all &&
> +	rm -f paginated.out subdir/paginated.out &&
> +	(
> +		cd subdir &&
> +		test_terminal git bundle unbundle ../test.bundle
> +	) &&
> +	{
> +		test -e paginated.out ||
> +		test -e subdir/paginated.out
> +	}
> +'
> +
>  # A colored commit log will begin with an appropriate ANSI escape
>  # for the first color; the text "commit" comes later.
>  colorful() {

On my valgrind test setup, this never worked (i.e., fails and bisects
to this commit).

Oddly, I am seeing this error message from the second test (second in
t7006, not in this patch):

  expecting success: 
          rm -f stdout_is_tty ||
          cleanup_fail &&

          if test -t 1
          then
                  >stdout_is_tty
          elif
                  test_have_prereq PERL &&
                  "$PERL_PATH" "$TEST_DIRECTORY"/t7006/test-terminal.perl \
                          sh -c "test -t 1"
          then
                  >test_terminal_works
          fi

  Can't locate IO/Pty.pm in @INC (@INC contains: <snip>) at /local/home/trast/git/t/t7006/test-terminal.perl line 4.
  BEGIN failed--compilation aborted at /local/home/trast/git/t/t7006/test-terminal.perl line 4.
  ok 2 - set up terminal for tests

Which raises a few questions: Why was this never an issue before?  Am
I supposed to have IO::Pty with a perl install (it's a perl 5.8.8) or
does the test need a prerequisite other than HAVE_PERL?4

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH 07/12] bundle: run setup_git_directory_gently() sooner

[Jonathan Nieder]

On Mon, Aug 16, 2010 at 09:21:32AM +0200, Thomas Rast wrote:
> Jonathan Nieder wrote:

>> +test_expect_success 'configuration can enable pager (from subdir)' '
[...]
>> +		test_terminal git bundle unbundle ../test.bundle
[...]
> On my valgrind test setup, this never worked (i.e., fails and bisects
> to this commit).

Agh, sloppy me...  Fixes below.

> Oddly, I am seeing this error message from the second test (second in
> t7006, not in this patch):
[...]
>   Can't locate IO/Pty.pm in @INC (@INC contains: <snip>) at /local/home/trast/git/t/t7006/test-terminal.perl line 4.
>   BEGIN failed--compilation aborted at /local/home/trast/git/t/t7006/test-terminal.perl line 4.
>   ok 2 - set up terminal for tests
> 
> Which raises a few questions: Why was this never an issue before?

That’s expected behavior, marked with “ok”. :)  That test is checking
if IO::Pty is available and works; if not, the relevant tests should
be skipped.

> does the test need a prerequisite other than HAVE_PERL?4

Exactly.

Jonathan Nieder (2):
  t7006 (pager): add missing TTY prerequisite
  t7006 (pager): add missing TTY prerequisite

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

[PATCH 2/2] t7006 (pager): add missing TTY prerequisite

[Jonathan Nieder]

The config pagination test should not run if there is not a tty
available to force pagination on.

Reported-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
This one fixes for “config: run setup_git_directory_gently()
sooner”.

Thanks again for the report.  I’ll think more about how to make
problems like this easily reproducible (a test_mentions_prereq()
function?).

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

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index fcd846a..fb744e3 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -134,7 +134,7 @@ test_expect_success TTY 'configuration can disable pager' '
 	! test -e paginated.out
 '
 
-test_expect_success 'git config uses a pager if configured to' '
+test_expect_success TTY 'git config uses a pager if configured to' '
 	rm -f paginated.out &&
 	git config pager.config true &&
 	test_when_finished "git config --unset pager.config" &&
-- 
1.7.2.1.544.ga752d.dirty

Re: [PATCH 2/2] t7006 (pager): add missing TTY prerequisite

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> The config pagination test should not run if there is not a tty
> available to force pagination on.
>
> Reported-by: Thomas Rast <trast@student.ethz.ch>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---
> This one fixes for “config: run setup_git_directory_gently()
> sooner”.

Somehow we didn't seem to have got 1/2, but here is what I'll queue.

By the way, could you stop using those "pretty quotes" and stick to the
good old "ASCII 0x22" double-quotes?  I find them ugly in the git log
output.

-- >8 --
From: Jonathan Nieder <jrnieder@gmail.com>
Date: Mon, 16 Aug 2010 03:08:34 -0500
Subject: [PATCH] t7006 (pager): add missing TTY prerequisites

The "git bundle unbundle" and "git config" pagination tests are not
supposed to run when stdout is not a terminal and IO::Pty not available
to make one on the fly.

Reported-by: Thomas Rast <trast@student.ethz.ch>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t7006-pager.sh |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/t/t7006-pager.sh b/t/t7006-pager.sh
index 017565f..fb744e3 100755
--- a/t/t7006-pager.sh
+++ b/t/t7006-pager.sh
@@ -134,7 +134,7 @@ test_expect_success TTY 'configuration can disable pager' '
 	! test -e paginated.out
 '
 
-test_expect_success 'git config uses a pager if configured to' '
+test_expect_success TTY 'git config uses a pager if configured to' '
 	rm -f paginated.out &&
 	git config pager.config true &&
 	test_when_finished "git config --unset pager.config" &&
@@ -142,7 +142,7 @@ test_expect_success 'git config uses a pager if configured to' '
 	test -e paginated.out
 '
 
-test_expect_success 'configuration can enable pager (from subdir)' '
+test_expect_success TTY 'configuration can enable pager (from subdir)' '
 	rm -f paginated.out &&
 	mkdir -p subdir &&
 	git config pager.bundle true &&
-- 
1.7.2.1.224.g2f41ea