[PATCH v2 0/3] Adding a performance framework

[PATCH v2 0/3] Adding a performance framework

[Thomas Rast]

This is a reroll of the original RFC at

  http://thread.gmane.org/gmane.comp.version-control.git/187127

Basically, a perf framework that uses little perf scripts written in
the style we already use in the test suite.

As Junio pointed out, the line between GIT_BUILD_DIR and
GIT_TEST_INSTALLED was not very clear cut or well adhered to, so I
threw out the overrides for the former.  This also made
GIT-TEST-OPTIONS moot.

There are no other changes, though I did a rebase on current next.


Thomas Rast (3):
  Move the user-facing test library to test-lib-functions.sh
  Introduce a performance testing framework
  Add a performance test for git-grep

 Makefile                        |   22 +-
 t/Makefile                      |   43 ++-
 t/perf/.gitignore               |    2 +
 t/perf/Makefile                 |   15 +
 t/perf/README                   |  146 +++++++
 t/perf/aggregate.perl           |  166 ++++++++
 t/perf/min_time.perl            |   21 +
 t/perf/p0000-perf-lib-sanity.sh |   41 ++
 t/perf/p0001-rev-list.sh        |   17 +
 t/perf/p7810-grep.sh            |   23 ++
 t/perf/perf-lib.sh              |  198 +++++++++
 t/perf/run                      |   82 ++++
 t/test-lib-functions.sh         |  835 +++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                   |  574 ++--------------------------
 14 files changed, 1633 insertions(+), 552 deletions(-)
 create mode 100644 t/perf/.gitignore
 create mode 100644 t/perf/Makefile
 create mode 100644 t/perf/README
 create mode 100755 t/perf/aggregate.perl
 create mode 100755 t/perf/min_time.perl
 create mode 100755 t/perf/p0000-perf-lib-sanity.sh
 create mode 100755 t/perf/p0001-rev-list.sh
 create mode 100755 t/perf/p7810-grep.sh
 create mode 100644 t/perf/perf-lib.sh
 create mode 100755 t/perf/run
 create mode 100644 t/test-lib-functions.sh

-- 
1.7.9.1.334.gd1409

[PATCH v2 1/3] Move the user-facing test library to test-lib-functions.sh

[Thomas Rast]

This just moves all the user-facing functions to a separate file and
sources that instead.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/test-lib-functions.sh |  835 +++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh           |  552 +-------------------------------
 2 files changed, 840 insertions(+), 547 deletions(-)
 create mode 100644 t/test-lib-functions.sh

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
new file mode 100644
index 0000000..580f767
--- /dev/null
+++ b/t/test-lib-functions.sh
@@ -0,0 +1,835 @@
+# The semantics of the editor variables are that of invoking
+# sh -c "$EDITOR \"$@\"" files ...
+#
+# If our trash directory contains shell metacharacters, they will be
+# interpreted if we just set $EDITOR directly, so do a little dance with
+# environment variables to work around this.
+#
+# In particular, quoting isn't enough, as the path may contain the same quote
+# that we're using.
+test_set_editor () {
+	FAKE_EDITOR="$1"
+	export FAKE_EDITOR
+	EDITOR='"$FAKE_EDITOR"'
+	export EDITOR
+}
+
+test_decode_color () {
+	awk '
+		function name(n) {
+			if (n == 0) return "RESET";
+			if (n == 1) return "BOLD";
+			if (n == 30) return "BLACK";
+			if (n == 31) return "RED";
+			if (n == 32) return "GREEN";
+			if (n == 33) return "YELLOW";
+			if (n == 34) return "BLUE";
+			if (n == 35) return "MAGENTA";
+			if (n == 36) return "CYAN";
+			if (n == 37) return "WHITE";
+			if (n == 40) return "BLACK";
+			if (n == 41) return "BRED";
+			if (n == 42) return "BGREEN";
+			if (n == 43) return "BYELLOW";
+			if (n == 44) return "BBLUE";
+			if (n == 45) return "BMAGENTA";
+			if (n == 46) return "BCYAN";
+			if (n == 47) return "BWHITE";
+		}
+		{
+			while (match($0, /\033\[[0-9;]*m/) != 0) {
+				printf "%s<", substr($0, 1, RSTART-1);
+				codes = substr($0, RSTART+2, RLENGTH-3);
+				if (length(codes) == 0)
+					printf "%s", name(0)
+				else {
+					n = split(codes, ary, ";");
+					sep = "";
+					for (i = 1; i <= n; i++) {
+						printf "%s%s", sep, name(ary[i]);
+						sep = ";"
+					}
+				}
+				printf ">";
+				$0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
+			}
+			print
+		}
+	'
+}
+
+nul_to_q () {
+	perl -pe 'y/\000/Q/'
+}
+
+q_to_nul () {
+	perl -pe 'y/Q/\000/'
+}
+
+q_to_cr () {
+	tr Q '\015'
+}
+
+q_to_tab () {
+	tr Q '\011'
+}
+
+append_cr () {
+	sed -e 's/$/Q/' | tr Q '\015'
+}
+
+remove_cr () {
+	tr '\015' Q | sed -e 's/Q$//'
+}
+
+# In some bourne shell implementations, the "unset" builtin returns
+# nonzero status when a variable to be unset was not set in the first
+# place.
+#
+# Use sane_unset when that should not be considered an error.
+
+sane_unset () {
+	unset "$@"
+	return 0
+}
+
+test_tick () {
+	if test -z "${test_tick+set}"
+	then
+		test_tick=1112911993
+	else
+		test_tick=$(($test_tick + 60))
+	fi
+	GIT_COMMITTER_DATE="$test_tick -0700"
+	GIT_AUTHOR_DATE="$test_tick -0700"
+	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+}
+
+# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+#
+# This will commit a file with the given contents and the given commit
+# message.  It will also add a tag with <message> as name.
+#
+# Both <file> and <contents> default to <message>.
+
+test_commit () {
+	file=${2:-"$1.t"}
+	echo "${3-$1}" > "$file" &&
+	git add "$file" &&
+	test_tick &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+# Call test_merge with the arguments "<message> <commit>", where <commit>
+# can be a tag pointing to the commit-to-merge.
+
+test_merge () {
+	test_tick &&
+	git merge -m "$1" "$2" &&
+	git tag "$1"
+}
+
+# This function helps systems where core.filemode=false is set.
+# Use it instead of plain 'chmod +x' to set or unset the executable bit
+# of a file in the working directory and add it to the index.
+
+test_chmod () {
+	chmod "$@" &&
+	git update-index --add "--chmod=$@"
+}
+
+# Unset a configuration variable, but don't fail if it doesn't exist.
+test_unconfig () {
+	git config --unset-all "$@"
+	config_status=$?
+	case "$config_status" in
+	5) # ok, nothing to unset
+		config_status=0
+		;;
+	esac
+	return $config_status
+}
+
+# Set git config, automatically unsetting it after the test is over.
+test_config () {
+	test_when_finished "test_unconfig '$1'" &&
+	git config "$@"
+}
+
+test_config_global () {
+	test_when_finished "test_unconfig --global '$1'" &&
+	git config --global "$@"
+}
+
+# Use test_set_prereq to tell that a particular prerequisite is available.
+# The prerequisite can later be checked for in two ways:
+#
+# - Explicitly using test_have_prereq.
+#
+# - Implicitly by specifying the prerequisite tag in the calls to
+#   test_expect_{success,failure,code}.
+#
+# The single parameter is the prerequisite tag (a simple word, in all
+# capital letters by convention).
+
+test_set_prereq () {
+	satisfied="$satisfied$1 "
+}
+satisfied=" "
+
+test_have_prereq () {
+	# prerequisites can be concatenated with ','
+	save_IFS=$IFS
+	IFS=,
+	set -- $*
+	IFS=$save_IFS
+
+	total_prereq=0
+	ok_prereq=0
+	missing_prereq=
+
+	for prerequisite
+	do
+		total_prereq=$(($total_prereq + 1))
+		case $satisfied in
+		*" $prerequisite "*)
+			ok_prereq=$(($ok_prereq + 1))
+			;;
+		*)
+			# Keep a list of missing prerequisites
+			if test -z "$missing_prereq"
+			then
+				missing_prereq=$prerequisite
+			else
+				missing_prereq="$prerequisite,$missing_prereq"
+			fi
+		esac
+	done
+
+	test $total_prereq = $ok_prereq
+}
+
+test_declared_prereq () {
+	case ",$test_prereq," in
+	*,$1,*)
+		return 0
+		;;
+	esac
+	return 1
+}
+
+test_expect_failure () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		say >&3 "checking known breakage: $2"
+		if test_run_ "$2" expecting_failure
+		then
+			test_known_broken_ok_ "$1"
+		else
+			test_known_broken_failure_ "$1"
+		fi
+	fi
+	echo >&3 ""
+}
+
+test_expect_success () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		say >&3 "expecting success: $2"
+		if test_run_ "$2"
+		then
+			test_ok_ "$1"
+		else
+			test_failure_ "$@"
+		fi
+	fi
+	echo >&3 ""
+}
+
+# test_external runs external test scripts that provide continuous
+# test output about their progress, and succeeds/fails on
+# zero/non-zero exit code.  It outputs the test output on stdout even
+# in non-verbose mode, and announces the external script with "# run
+# <n>: ..." before running it.  When providing relative paths, keep in
+# mind that all scripts run in "trash directory".
+# Usage: test_external description command arguments...
+# Example: test_external 'Perl API' perl ../path/to/test.pl
+test_external () {
+	test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 3 ||
+	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
+	descr="$1"
+	shift
+	export test_prereq
+	if ! test_skip "$descr" "$@"
+	then
+		# Announce the script to reduce confusion about the
+		# test output that follows.
+		say_color "" "# run $test_count: $descr ($*)"
+		# Export TEST_DIRECTORY, TRASH_DIRECTORY and GIT_TEST_LONG
+		# to be able to use them in script
+		export TEST_DIRECTORY TRASH_DIRECTORY GIT_TEST_LONG
+		# Run command; redirect its stderr to &4 as in
+		# test_run_, but keep its stdout on our stdout even in
+		# non-verbose mode.
+		"$@" 2>&4
+		if [ "$?" = 0 ]
+		then
+			if test $test_external_has_tap -eq 0; then
+				test_ok_ "$descr"
+			else
+				say_color "" "# test_external test $descr was ok"
+				test_success=$(($test_success + 1))
+			fi
+		else
+			if test $test_external_has_tap -eq 0; then
+				test_failure_ "$descr" "$@"
+			else
+				say_color error "# test_external test $descr failed: $@"
+				test_failure=$(($test_failure + 1))
+			fi
+		fi
+	fi
+}
+
+# Like test_external, but in addition tests that the command generated
+# no output on stderr.
+test_external_without_stderr () {
+	# The temporary file has no (and must have no) security
+	# implications.
+	tmp=${TMPDIR:-/tmp}
+	stderr="$tmp/git-external-stderr.$$.tmp"
+	test_external "$@" 4> "$stderr"
+	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
+	descr="no stderr: $1"
+	shift
+	say >&3 "# expecting no stderr from previous command"
+	if [ ! -s "$stderr" ]; then
+		rm "$stderr"
+
+		if test $test_external_has_tap -eq 0; then
+			test_ok_ "$descr"
+		else
+			say_color "" "# test_external_without_stderr test $descr was ok"
+			test_success=$(($test_success + 1))
+		fi
+	else
+		if [ "$verbose" = t ]; then
+			output=`echo; echo "# Stderr is:"; cat "$stderr"`
+		else
+			output=
+		fi
+		# rm first in case test_failure exits.
+		rm "$stderr"
+		if test $test_external_has_tap -eq 0; then
+			test_failure_ "$descr" "$@" "$output"
+		else
+			say_color error "# test_external_without_stderr test $descr failed: $@: $output"
+			test_failure=$(($test_failure + 1))
+		fi
+	fi
+}
+
+# debugging-friendly alternatives to "test [-f|-d|-e]"
+# The commands test the existence or non-existence of $1. $2 can be
+# given to provide a more precise diagnosis.
+test_path_is_file () {
+	if ! [ -f "$1" ]
+	then
+		echo "File $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_path_is_dir () {
+	if ! [ -d "$1" ]
+	then
+		echo "Directory $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_path_is_missing () {
+	if [ -e "$1" ]
+	then
+		echo "Path exists:"
+		ls -ld "$1"
+		if [ $# -ge 1 ]; then
+			echo "$*"
+		fi
+		false
+	fi
+}
+
+# test_line_count checks that a file has the number of lines it
+# ought to. For example:
+#
+#	test_expect_success 'produce exactly one line of output' '
+#		do something >output &&
+#		test_line_count = 1 output
+#	'
+#
+# is like "test $(wc -l <output) = 1" except that it passes the
+# output through when the number of lines is wrong.
+
+test_line_count () {
+	if test $# != 3
+	then
+		error "bug in the test script: not 3 parameters to test_line_count"
+	elif ! test $(wc -l <"$3") "$1" "$2"
+	then
+		echo "test_line_count: line count for $3 !$1 $2"
+		cat "$3"
+		return 1
+	fi
+}
+
+# This is not among top-level (test_expect_success | test_expect_failure)
+# but is a prefix that can be used in the test script, like:
+#
+#	test_expect_success 'complain and die' '
+#           do something &&
+#           do something else &&
+#	    test_must_fail git checkout ../outerspace
+#	'
+#
+# Writing this as "! git checkout ../outerspace" is wrong, because
+# the failure could be due to a segv.  We want a controlled failure.
+
+test_must_fail () {
+	"$@"
+	exit_code=$?
+	if test $exit_code = 0; then
+		echo >&2 "test_must_fail: command succeeded: $*"
+		return 1
+	elif test $exit_code -gt 129 -a $exit_code -le 192; then
+		echo >&2 "test_must_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_must_fail: command not found: $*"
+		return 1
+	fi
+	return 0
+}
+
+# Similar to test_must_fail, but tolerates success, too.  This is
+# meant to be used in contexts like:
+#
+#	test_expect_success 'some command works without configuration' '
+#		test_might_fail git config --unset all.configuration &&
+#		do something
+#	'
+#
+# Writing "git config --unset all.configuration || :" would be wrong,
+# because we want to notice if it fails due to segv.
+
+test_might_fail () {
+	"$@"
+	exit_code=$?
+	if test $exit_code -gt 129 -a $exit_code -le 192; then
+		echo >&2 "test_might_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_might_fail: command not found: $*"
+		return 1
+	fi
+	return 0
+}
+
+# Similar to test_must_fail and test_might_fail, but check that a
+# given command exited with a given exit code. Meant to be used as:
+#
+#	test_expect_success 'Merge with d/f conflicts' '
+#		test_expect_code 1 git merge "merge msg" B master
+#	'
+
+test_expect_code () {
+	want_code=$1
+	shift
+	"$@"
+	exit_code=$?
+	if test $exit_code = $want_code
+	then
+		return 0
+	fi
+
+	echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
+	return 1
+}
+
+# test_cmp is a helper function to compare actual and expected output.
+# You can use it like:
+#
+#	test_expect_success 'foo works' '
+#		echo expected >expected &&
+#		foo >actual &&
+#		test_cmp expected actual
+#	'
+#
+# This could be written as either "cmp" or "diff -u", but:
+# - cmp's output is not nearly as easy to read as diff -u
+# - not all diff versions understand "-u"
+
+test_cmp() {
+	$GIT_TEST_CMP "$@"
+}
+
+# This function can be used to schedule some commands to be run
+# unconditionally at the end of the test to restore sanity:
+#
+#	test_expect_success 'test core.capslock' '
+#		git config core.capslock true &&
+#		test_when_finished "git config --unset core.capslock" &&
+#		hello world
+#	'
+#
+# That would be roughly equivalent to
+#
+#	test_expect_success 'test core.capslock' '
+#		git config core.capslock true &&
+#		hello world
+#		git config --unset core.capslock
+#	'
+#
+# except that the greeting and config --unset must both succeed for
+# the test to pass.
+#
+# Note that under --immediate mode, no clean-up is done to help diagnose
+# what went wrong.
+
+test_when_finished () {
+	test_cleanup="{ $*
+		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
+}
+
+# Most tests can use the created repository, but some may need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	repo="$1"
+	mkdir -p "$repo"
+	(
+		cd "$repo" || error "Cannot setup test environment"
+		"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
+		error "cannot run git init -- have you built things yet?"
+		mv .git/hooks .git/hooks-disabled
+	) || exit
+}
+
+test_expect_failure () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		say >&3 "checking known breakage: $2"
+		if test_run_ "$2" expecting_failure
+		then
+			test_known_broken_ok_ "$1"
+		else
+			test_known_broken_failure_ "$1"
+		fi
+	fi
+	echo >&3 ""
+}
+
+test_expect_success () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		say >&3 "expecting success: $2"
+		if test_run_ "$2"
+		then
+			test_ok_ "$1"
+		else
+			test_failure_ "$@"
+		fi
+	fi
+	echo >&3 ""
+}
+
+# test_external runs external test scripts that provide continuous
+# test output about their progress, and succeeds/fails on
+# zero/non-zero exit code.  It outputs the test output on stdout even
+# in non-verbose mode, and announces the external script with "# run
+# <n>: ..." before running it.  When providing relative paths, keep in
+# mind that all scripts run in "trash directory".
+# Usage: test_external description command arguments...
+# Example: test_external 'Perl API' perl ../path/to/test.pl
+test_external () {
+	test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 3 ||
+	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
+	descr="$1"
+	shift
+	export test_prereq
+	if ! test_skip "$descr" "$@"
+	then
+		# Announce the script to reduce confusion about the
+		# test output that follows.
+		say_color "" "# run $test_count: $descr ($*)"
+		# Export TEST_DIRECTORY, TRASH_DIRECTORY and GIT_TEST_LONG
+		# to be able to use them in script
+		export TEST_DIRECTORY TRASH_DIRECTORY GIT_TEST_LONG
+		# Run command; redirect its stderr to &4 as in
+		# test_run_, but keep its stdout on our stdout even in
+		# non-verbose mode.
+		"$@" 2>&4
+		if [ "$?" = 0 ]
+		then
+			if test $test_external_has_tap -eq 0; then
+				test_ok_ "$descr"
+			else
+				say_color "" "# test_external test $descr was ok"
+				test_success=$(($test_success + 1))
+			fi
+		else
+			if test $test_external_has_tap -eq 0; then
+				test_failure_ "$descr" "$@"
+			else
+				say_color error "# test_external test $descr failed: $@"
+				test_failure=$(($test_failure + 1))
+			fi
+		fi
+	fi
+}
+
+# Like test_external, but in addition tests that the command generated
+# no output on stderr.
+test_external_without_stderr () {
+	# The temporary file has no (and must have no) security
+	# implications.
+	tmp=${TMPDIR:-/tmp}
+	stderr="$tmp/git-external-stderr.$$.tmp"
+	test_external "$@" 4> "$stderr"
+	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
+	descr="no stderr: $1"
+	shift
+	say >&3 "# expecting no stderr from previous command"
+	if [ ! -s "$stderr" ]; then
+		rm "$stderr"
+
+		if test $test_external_has_tap -eq 0; then
+			test_ok_ "$descr"
+		else
+			say_color "" "# test_external_without_stderr test $descr was ok"
+			test_success=$(($test_success + 1))
+		fi
+	else
+		if [ "$verbose" = t ]; then
+			output=`echo; echo "# Stderr is:"; cat "$stderr"`
+		else
+			output=
+		fi
+		# rm first in case test_failure exits.
+		rm "$stderr"
+		if test $test_external_has_tap -eq 0; then
+			test_failure_ "$descr" "$@" "$output"
+		else
+			say_color error "# test_external_without_stderr test $descr failed: $@: $output"
+			test_failure=$(($test_failure + 1))
+		fi
+	fi
+}
+
+# debugging-friendly alternatives to "test [-f|-d|-e]"
+# The commands test the existence or non-existence of $1. $2 can be
+# given to provide a more precise diagnosis.
+test_path_is_file () {
+	if ! [ -f "$1" ]
+	then
+		echo "File $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_path_is_dir () {
+	if ! [ -d "$1" ]
+	then
+		echo "Directory $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_path_is_missing () {
+	if [ -e "$1" ]
+	then
+		echo "Path exists:"
+		ls -ld "$1"
+		if [ $# -ge 1 ]; then
+			echo "$*"
+		fi
+		false
+	fi
+}
+
+# test_line_count checks that a file has the number of lines it
+# ought to. For example:
+#
+#	test_expect_success 'produce exactly one line of output' '
+#		do something >output &&
+#		test_line_count = 1 output
+#	'
+#
+# is like "test $(wc -l <output) = 1" except that it passes the
+# output through when the number of lines is wrong.
+
+test_line_count () {
+	if test $# != 3
+	then
+		error "bug in the test script: not 3 parameters to test_line_count"
+	elif ! test $(wc -l <"$3") "$1" "$2"
+	then
+		echo "test_line_count: line count for $3 !$1 $2"
+		cat "$3"
+		return 1
+	fi
+}
+
+# This is not among top-level (test_expect_success | test_expect_failure)
+# but is a prefix that can be used in the test script, like:
+#
+#	test_expect_success 'complain and die' '
+#           do something &&
+#           do something else &&
+#	    test_must_fail git checkout ../outerspace
+#	'
+#
+# Writing this as "! git checkout ../outerspace" is wrong, because
+# the failure could be due to a segv.  We want a controlled failure.
+
+test_must_fail () {
+	"$@"
+	exit_code=$?
+	if test $exit_code = 0; then
+		echo >&2 "test_must_fail: command succeeded: $*"
+		return 1
+	elif test $exit_code -gt 129 -a $exit_code -le 192; then
+		echo >&2 "test_must_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_must_fail: command not found: $*"
+		return 1
+	fi
+	return 0
+}
+
+# Similar to test_must_fail, but tolerates success, too.  This is
+# meant to be used in contexts like:
+#
+#	test_expect_success 'some command works without configuration' '
+#		test_might_fail git config --unset all.configuration &&
+#		do something
+#	'
+#
+# Writing "git config --unset all.configuration || :" would be wrong,
+# because we want to notice if it fails due to segv.
+
+test_might_fail () {
+	"$@"
+	exit_code=$?
+	if test $exit_code -gt 129 -a $exit_code -le 192; then
+		echo >&2 "test_might_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_might_fail: command not found: $*"
+		return 1
+	fi
+	return 0
+}
+
+# Similar to test_must_fail and test_might_fail, but check that a
+# given command exited with a given exit code. Meant to be used as:
+#
+#	test_expect_success 'Merge with d/f conflicts' '
+#		test_expect_code 1 git merge "merge msg" B master
+#	'
+
+test_expect_code () {
+	want_code=$1
+	shift
+	"$@"
+	exit_code=$?
+	if test $exit_code = $want_code
+	then
+		return 0
+	fi
+
+	echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
+	return 1
+}
+
+# test_cmp is a helper function to compare actual and expected output.
+# You can use it like:
+#
+#	test_expect_success 'foo works' '
+#		echo expected >expected &&
+#		foo >actual &&
+#		test_cmp expected actual
+#	'
+#
+# This could be written as either "cmp" or "diff -u", but:
+# - cmp's output is not nearly as easy to read as diff -u
+# - not all diff versions understand "-u"
+
+test_cmp() {
+	$GIT_TEST_CMP "$@"
+}
+
+# This function can be used to schedule some commands to be run
+# unconditionally at the end of the test to restore sanity:
+#
+#	test_expect_success 'test core.capslock' '
+#		git config core.capslock true &&
+#		test_when_finished "git config --unset core.capslock" &&
+#		hello world
+#	'
+#
+# That would be roughly equivalent to
+#
+#	test_expect_success 'test core.capslock' '
+#		git config core.capslock true &&
+#		hello world
+#		git config --unset core.capslock
+#	'
+#
+# except that the greeting and config --unset must both succeed for
+# the test to pass.
+#
+# Note that under --immediate mode, no clean-up is done to help diagnose
+# what went wrong.
+
+test_when_finished () {
+	test_cleanup="{ $*
+		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
+}
+
+# Most tests can use the created repository, but some may need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	repo="$1"
+	mkdir -p "$repo"
+	(
+		cd "$repo" || error "Cannot setup test environment"
+		"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
+		error "cannot run git init -- have you built things yet?"
+		mv .git/hooks .git/hooks-disabled
+	) || exit
+}
+
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e28d5fd..ec70ef2 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -98,6 +98,8 @@ _z40=0000000000000000000000000000000000000000
 LF='
 '
 
+export _x05 _x40 _z40 LF
+
 # Each test should start with something like this, after copyright notices:
 #
 # test_description='Description of this test...
@@ -223,248 +225,9 @@ die () {
 GIT_EXIT_OK=
 trap 'die' EXIT
 
-# The semantics of the editor variables are that of invoking
-# sh -c "$EDITOR \"$@\"" files ...
-#
-# If our trash directory contains shell metacharacters, they will be
-# interpreted if we just set $EDITOR directly, so do a little dance with
-# environment variables to work around this.
-#
-# In particular, quoting isn't enough, as the path may contain the same quote
-# that we're using.
-test_set_editor () {
-	FAKE_EDITOR="$1"
-	export FAKE_EDITOR
-	EDITOR='"$FAKE_EDITOR"'
-	export EDITOR
-}
-
-test_decode_color () {
-	awk '
-		function name(n) {
-			if (n == 0) return "RESET";
-			if (n == 1) return "BOLD";
-			if (n == 30) return "BLACK";
-			if (n == 31) return "RED";
-			if (n == 32) return "GREEN";
-			if (n == 33) return "YELLOW";
-			if (n == 34) return "BLUE";
-			if (n == 35) return "MAGENTA";
-			if (n == 36) return "CYAN";
-			if (n == 37) return "WHITE";
-			if (n == 40) return "BLACK";
-			if (n == 41) return "BRED";
-			if (n == 42) return "BGREEN";
-			if (n == 43) return "BYELLOW";
-			if (n == 44) return "BBLUE";
-			if (n == 45) return "BMAGENTA";
-			if (n == 46) return "BCYAN";
-			if (n == 47) return "BWHITE";
-		}
-		{
-			while (match($0, /\033\[[0-9;]*m/) != 0) {
-				printf "%s<", substr($0, 1, RSTART-1);
-				codes = substr($0, RSTART+2, RLENGTH-3);
-				if (length(codes) == 0)
-					printf "%s", name(0)
-				else {
-					n = split(codes, ary, ";");
-					sep = "";
-					for (i = 1; i <= n; i++) {
-						printf "%s%s", sep, name(ary[i]);
-						sep = ";"
-					}
-				}
-				printf ">";
-				$0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
-			}
-			print
-		}
-	'
-}
-
-nul_to_q () {
-	perl -pe 'y/\000/Q/'
-}
-
-q_to_nul () {
-	perl -pe 'y/Q/\000/'
-}
-
-q_to_cr () {
-	tr Q '\015'
-}
-
-q_to_tab () {
-	tr Q '\011'
-}
-
-append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
-}
-
-remove_cr () {
-	tr '\015' Q | sed -e 's/Q$//'
-}
-
-# In some bourne shell implementations, the "unset" builtin returns
-# nonzero status when a variable to be unset was not set in the first
-# place.
-#
-# Use sane_unset when that should not be considered an error.
-
-sane_unset () {
-	unset "$@"
-	return 0
-}
-
-test_tick () {
-	if test -z "${test_tick+set}"
-	then
-		test_tick=1112911993
-	else
-		test_tick=$(($test_tick + 60))
-	fi
-	GIT_COMMITTER_DATE="$test_tick -0700"
-	GIT_AUTHOR_DATE="$test_tick -0700"
-	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
-}
-
-# Stop execution and start a shell. This is useful for debugging tests and
-# only makes sense together with "-v".
-#
-# Be sure to remove all invocations of this command before submitting.
-
-test_pause () {
-	if test "$verbose" = t; then
-		"$SHELL_PATH" <&6 >&3 2>&4
-	else
-		error >&5 "test_pause requires --verbose"
-	fi
-}
-
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
-#
-# This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with <message> as name.
-#
-# Both <file> and <contents> default to <message>.
-
-test_commit () {
-	file=${2:-"$1.t"}
-	echo "${3-$1}" > "$file" &&
-	git add "$file" &&
-	test_tick &&
-	git commit -m "$1" &&
-	git tag "$1"
-}
-
-# Call test_merge with the arguments "<message> <commit>", where <commit>
-# can be a tag pointing to the commit-to-merge.
-
-test_merge () {
-	test_tick &&
-	git merge -m "$1" "$2" &&
-	git tag "$1"
-}
-
-# This function helps systems where core.filemode=false is set.
-# Use it instead of plain 'chmod +x' to set or unset the executable bit
-# of a file in the working directory and add it to the index.
-
-test_chmod () {
-	chmod "$@" &&
-	git update-index --add "--chmod=$@"
-}
-
-# Unset a configuration variable, but don't fail if it doesn't exist.
-test_unconfig () {
-	git config --unset-all "$@"
-	config_status=$?
-	case "$config_status" in
-	5) # ok, nothing to unset
-		config_status=0
-		;;
-	esac
-	return $config_status
-}
-
-# Set git config, automatically unsetting it after the test is over.
-test_config () {
-	test_when_finished "test_unconfig '$1'" &&
-	git config "$@"
-}
-
-
-test_config_global () {
-	test_when_finished "test_unconfig --global '$1'" &&
-	git config --global "$@"
-}
-
-write_script () {
-	{
-		echo "#!${2-"$SHELL_PATH"}" &&
-		cat
-	} >"$1" &&
-	chmod +x "$1"
-}
-
-# Use test_set_prereq to tell that a particular prerequisite is available.
-# The prerequisite can later be checked for in two ways:
-#
-# - Explicitly using test_have_prereq.
-#
-# - Implicitly by specifying the prerequisite tag in the calls to
-#   test_expect_{success,failure,code}.
-#
-# The single parameter is the prerequisite tag (a simple word, in all
-# capital letters by convention).
-
-test_set_prereq () {
-	satisfied="$satisfied$1 "
-}
-satisfied=" "
-
-test_have_prereq () {
-	# prerequisites can be concatenated with ','
-	save_IFS=$IFS
-	IFS=,
-	set -- $*
-	IFS=$save_IFS
-
-	total_prereq=0
-	ok_prereq=0
-	missing_prereq=
-
-	for prerequisite
-	do
-		total_prereq=$(($total_prereq + 1))
-		case $satisfied in
-		*" $prerequisite "*)
-			ok_prereq=$(($ok_prereq + 1))
-			;;
-		*)
-			# Keep a list of missing prerequisites
-			if test -z "$missing_prereq"
-			then
-				missing_prereq=$prerequisite
-			else
-				missing_prereq="$prerequisite,$missing_prereq"
-			fi
-		esac
-	done
-
-	test $total_prereq = $ok_prereq
-}
-
-test_declared_prereq () {
-	case ",$test_prereq," in
-	*,$1,*)
-		return 0
-		;;
-	esac
-	return 1
-}
+# The user-facing functions are loaded from a separate file so that
+# test_perf subshells can have them too
+. "${TEST_DIRECTORY:-.}"/test-lib-functions.sh
 
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
@@ -552,311 +315,6 @@ test_skip () {
 	esac
 }
 
-test_expect_failure () {
-	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
-	export test_prereq
-	if ! test_skip "$@"
-	then
-		say >&3 "checking known breakage: $2"
-		if test_run_ "$2" expecting_failure
-		then
-			test_known_broken_ok_ "$1"
-		else
-			test_known_broken_failure_ "$1"
-		fi
-	fi
-	echo >&3 ""
-}
-
-test_expect_success () {
-	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
-	export test_prereq
-	if ! test_skip "$@"
-	then
-		say >&3 "expecting success: $2"
-		if test_run_ "$2"
-		then
-			test_ok_ "$1"
-		else
-			test_failure_ "$@"
-		fi
-	fi
-	echo >&3 ""
-}
-
-# test_external runs external test scripts that provide continuous
-# test output about their progress, and succeeds/fails on
-# zero/non-zero exit code.  It outputs the test output on stdout even
-# in non-verbose mode, and announces the external script with "# run
-# <n>: ..." before running it.  When providing relative paths, keep in
-# mind that all scripts run in "trash directory".
-# Usage: test_external description command arguments...
-# Example: test_external 'Perl API' perl ../path/to/test.pl
-test_external () {
-	test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
-	test "$#" = 3 ||
-	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
-	descr="$1"
-	shift
-	export test_prereq
-	if ! test_skip "$descr" "$@"
-	then
-		# Announce the script to reduce confusion about the
-		# test output that follows.
-		say_color "" "# run $test_count: $descr ($*)"
-		# Export TEST_DIRECTORY, TRASH_DIRECTORY and GIT_TEST_LONG
-		# to be able to use them in script
-		export TEST_DIRECTORY TRASH_DIRECTORY GIT_TEST_LONG
-		# Run command; redirect its stderr to &4 as in
-		# test_run_, but keep its stdout on our stdout even in
-		# non-verbose mode.
-		"$@" 2>&4
-		if [ "$?" = 0 ]
-		then
-			if test $test_external_has_tap -eq 0; then
-				test_ok_ "$descr"
-			else
-				say_color "" "# test_external test $descr was ok"
-				test_success=$(($test_success + 1))
-			fi
-		else
-			if test $test_external_has_tap -eq 0; then
-				test_failure_ "$descr" "$@"
-			else
-				say_color error "# test_external test $descr failed: $@"
-				test_failure=$(($test_failure + 1))
-			fi
-		fi
-	fi
-}
-
-# Like test_external, but in addition tests that the command generated
-# no output on stderr.
-test_external_without_stderr () {
-	# The temporary file has no (and must have no) security
-	# implications.
-	tmp=${TMPDIR:-/tmp}
-	stderr="$tmp/git-external-stderr.$$.tmp"
-	test_external "$@" 4> "$stderr"
-	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
-	descr="no stderr: $1"
-	shift
-	say >&3 "# expecting no stderr from previous command"
-	if [ ! -s "$stderr" ]; then
-		rm "$stderr"
-
-		if test $test_external_has_tap -eq 0; then
-			test_ok_ "$descr"
-		else
-			say_color "" "# test_external_without_stderr test $descr was ok"
-			test_success=$(($test_success + 1))
-		fi
-	else
-		if [ "$verbose" = t ]; then
-			output=`echo; echo "# Stderr is:"; cat "$stderr"`
-		else
-			output=
-		fi
-		# rm first in case test_failure exits.
-		rm "$stderr"
-		if test $test_external_has_tap -eq 0; then
-			test_failure_ "$descr" "$@" "$output"
-		else
-			say_color error "# test_external_without_stderr test $descr failed: $@: $output"
-			test_failure=$(($test_failure + 1))
-		fi
-	fi
-}
-
-# debugging-friendly alternatives to "test [-f|-d|-e]"
-# The commands test the existence or non-existence of $1. $2 can be
-# given to provide a more precise diagnosis.
-test_path_is_file () {
-	if ! [ -f "$1" ]
-	then
-		echo "File $1 doesn't exist. $*"
-		false
-	fi
-}
-
-test_path_is_dir () {
-	if ! [ -d "$1" ]
-	then
-		echo "Directory $1 doesn't exist. $*"
-		false
-	fi
-}
-
-test_path_is_missing () {
-	if [ -e "$1" ]
-	then
-		echo "Path exists:"
-		ls -ld "$1"
-		if [ $# -ge 1 ]; then
-			echo "$*"
-		fi
-		false
-	fi
-}
-
-# test_line_count checks that a file has the number of lines it
-# ought to. For example:
-#
-#	test_expect_success 'produce exactly one line of output' '
-#		do something >output &&
-#		test_line_count = 1 output
-#	'
-#
-# is like "test $(wc -l <output) = 1" except that it passes the
-# output through when the number of lines is wrong.
-
-test_line_count () {
-	if test $# != 3
-	then
-		error "bug in the test script: not 3 parameters to test_line_count"
-	elif ! test $(wc -l <"$3") "$1" "$2"
-	then
-		echo "test_line_count: line count for $3 !$1 $2"
-		cat "$3"
-		return 1
-	fi
-}
-
-# This is not among top-level (test_expect_success | test_expect_failure)
-# but is a prefix that can be used in the test script, like:
-#
-#	test_expect_success 'complain and die' '
-#           do something &&
-#           do something else &&
-#	    test_must_fail git checkout ../outerspace
-#	'
-#
-# Writing this as "! git checkout ../outerspace" is wrong, because
-# the failure could be due to a segv.  We want a controlled failure.
-
-test_must_fail () {
-	"$@"
-	exit_code=$?
-	if test $exit_code = 0; then
-		echo >&2 "test_must_fail: command succeeded: $*"
-		return 1
-	elif test $exit_code -gt 129 -a $exit_code -le 192; then
-		echo >&2 "test_must_fail: died by signal: $*"
-		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_must_fail: command not found: $*"
-		return 1
-	fi
-	return 0
-}
-
-# Similar to test_must_fail, but tolerates success, too.  This is
-# meant to be used in contexts like:
-#
-#	test_expect_success 'some command works without configuration' '
-#		test_might_fail git config --unset all.configuration &&
-#		do something
-#	'
-#
-# Writing "git config --unset all.configuration || :" would be wrong,
-# because we want to notice if it fails due to segv.
-
-test_might_fail () {
-	"$@"
-	exit_code=$?
-	if test $exit_code -gt 129 -a $exit_code -le 192; then
-		echo >&2 "test_might_fail: died by signal: $*"
-		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_might_fail: command not found: $*"
-		return 1
-	fi
-	return 0
-}
-
-# Similar to test_must_fail and test_might_fail, but check that a
-# given command exited with a given exit code. Meant to be used as:
-#
-#	test_expect_success 'Merge with d/f conflicts' '
-#		test_expect_code 1 git merge "merge msg" B master
-#	'
-
-test_expect_code () {
-	want_code=$1
-	shift
-	"$@"
-	exit_code=$?
-	if test $exit_code = $want_code
-	then
-		return 0
-	fi
-
-	echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
-	return 1
-}
-
-# test_cmp is a helper function to compare actual and expected output.
-# You can use it like:
-#
-#	test_expect_success 'foo works' '
-#		echo expected >expected &&
-#		foo >actual &&
-#		test_cmp expected actual
-#	'
-#
-# This could be written as either "cmp" or "diff -u", but:
-# - cmp's output is not nearly as easy to read as diff -u
-# - not all diff versions understand "-u"
-
-test_cmp() {
-	$GIT_TEST_CMP "$@"
-}
-
-# This function can be used to schedule some commands to be run
-# unconditionally at the end of the test to restore sanity:
-#
-#	test_expect_success 'test core.capslock' '
-#		git config core.capslock true &&
-#		test_when_finished "git config --unset core.capslock" &&
-#		hello world
-#	'
-#
-# That would be roughly equivalent to
-#
-#	test_expect_success 'test core.capslock' '
-#		git config core.capslock true &&
-#		hello world
-#		git config --unset core.capslock
-#	'
-#
-# except that the greeting and config --unset must both succeed for
-# the test to pass.
-#
-# Note that under --immediate mode, no clean-up is done to help diagnose
-# what went wrong.
-
-test_when_finished () {
-	test_cleanup="{ $*
-		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
-}
-
-# Most tests can use the created repository, but some may need to create more.
-# Usage: test_create_repo <directory>
-test_create_repo () {
-	test "$#" = 1 ||
-	error "bug in the test script: not 1 parameter to test-create-repo"
-	repo="$1"
-	mkdir -p "$repo"
-	(
-		cd "$repo" || error "Cannot setup test environment"
-		"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
-		error "cannot run git init -- have you built things yet?"
-		mv .git/hooks .git/hooks-disabled
-	) || exit
 }
 
 test_done () {
-- 
1.7.9.1.334.gd1409

Re: [PATCH v2 1/3] Move the user-facing test library to test-lib-functions.sh

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> This just moves all the user-facing functions to a separate file and
> sources that instead.
>
> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
> ---
>  t/test-lib-functions.sh |  835 +++++++++++++++++++++++++++++++++++++++++++++++
>  t/test-lib.sh           |  552 +-------------------------------
>  2 files changed, 840 insertions(+), 547 deletions(-)
>  create mode 100644 t/test-lib-functions.sh

I would have expected from the log description that the number of deleted
lines would be about the same as the number of added lines, and the
difference would primarily come from the addition of "include" aka "dot"
". ./test-lib-functions.sh" that becomes necessary in t/test-lib.sh, some
boilerplate material at the beginning of the new file e.g. "#!/bin/sh",
and copying (not moving) the same Copyright block to the new file.

But 835-552 = 283 feels way way more than that.  What else is going on?

Re: [PATCH v2 1/3] Move the user-facing test library to test-lib-functions.sh

[Thomas Rast]

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

> Thomas Rast <trast@student.ethz.ch> writes:
>
>> This just moves all the user-facing functions to a separate file and
>> sources that instead.
>>
>> Signed-off-by: Thomas Rast <trast@student.ethz.ch>
>> ---
>>  t/test-lib-functions.sh |  835 +++++++++++++++++++++++++++++++++++++++++++++++
>>  t/test-lib.sh           |  552 +-------------------------------
>>  2 files changed, 840 insertions(+), 547 deletions(-)
>>  create mode 100644 t/test-lib-functions.sh
>
> I would have expected from the log description that the number of deleted
> lines would be about the same as the number of added lines, and the
> difference would primarily come from the addition of "include" aka "dot"
> ". ./test-lib-functions.sh" that becomes necessary in t/test-lib.sh, some
> boilerplate material at the beginning of the new file e.g. "#!/bin/sh",
> and copying (not moving) the same Copyright block to the new file.
>
> But 835-552 = 283 feels way way more than that.  What else is going on?

Hum, you're right.  I checked with blame -C -C that I introduced no new
lines, but I must accidentally have duplicated parts of it during
conflict resolution.

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

[PATCH v3 0/3]

[Thomas Rast]

Junio C Hamano wrote:
> > ---
> >  t/test-lib-functions.sh |  835 +++++++++++++++++++++++++++++++++++++++++++++++
> >  t/test-lib.sh           |  552 +-------------------------------
> >  2 files changed, 840 insertions(+), 547 deletions(-)
> >  create mode 100644 t/test-lib-functions.sh
> 
> I would have expected from the log description that the number of deleted
> lines would be about the same as the number of added lines, and the
> difference would primarily come from the addition of "include" aka "dot"
> ". ./test-lib-functions.sh" that becomes necessary in t/test-lib.sh, some
> boilerplate material at the beginning of the new file e.g. "#!/bin/sh",
> and copying (not moving) the same Copyright block to the new file.

There were actually more mistakes lurking :-( so I am resending the
whole series.  I also put in the copyright that you asked for.  I
verified the results by looking at the diff between a reverse git-show
for test-lib.sh and a forward git-show for test-lib-functions.sh,
which looks as follows:

  --- /dev/fd/63	2012-02-17 10:55:32.994197654 +0100
  +++ /dev/fd/62	2012-02-17 10:55:32.994197654 +0100
  @@ -9,17 +9,29 @@
       
       Signed-off-by: Thomas Rast <trast@student.ethz.ch>
   
  -diff --git b/t/test-lib.sh a/t/test-lib.sh
  -index 1da3f40..e28d5fd 100644
  ---- b/t/test-lib.sh
  -+++ a/t/test-lib.sh
  -@@ -223,9 +223,248 @@ die () {
  - GIT_EXIT_OK=
  - trap 'die' EXIT
  - 
  --# The user-facing functions are loaded from a separate file so that
  --# test_perf subshells can have them too
  --. "${TEST_DIRECTORY:-.}"/test-lib-functions.sh
  +diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
  +new file mode 100644
  +index 0000000..7b3b4be
  +--- /dev/null
  ++++ b/t/test-lib-functions.sh
  +@@ -0,0 +1,565 @@
  ++#!/bin/sh
  ++#
  ++# Copyright (c) 2005 Junio C Hamano
  ++#
  ++# This program is free software: you can redistribute it and/or modify
  ++# it under the terms of the GNU General Public License as published by
  ++# the Free Software Foundation, either version 2 of the License, or
  ++# (at your option) any later version.
  ++#
  ++# This program is distributed in the hope that it will be useful,
  ++# but WITHOUT ANY WARRANTY; without even the implied warranty of
  ++# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  ++# GNU General Public License for more details.
  ++#
  ++# You should have received a copy of the GNU General Public License
  ++# along with this program.  If not, see http://www.gnu.org/licenses/ .
  ++
   +# The semantics of the editor variables are that of invoking
   +# sh -c "$EDITOR \"$@\"" files ...
   +#
  @@ -192,7 +204,6 @@
   +	git config "$@"
   +}
   +
  -+
   +test_config_global () {
   +	test_when_finished "test_unconfig --global '$1'" &&
   +	git config --global "$@"
  @@ -262,13 +273,7 @@
   +	esac
   +	return 1
   +}
  - 
  - # You are not expected to call test_ok_ and test_failure_ directly, use
  - # the text_expect_* functions instead.
  -@@ -313,6 +552,313 @@ test_skip () {
  - 	esac
  - }
  - 
  ++
   +test_expect_failure () {
   +	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
   +	test "$#" = 2 ||
  @@ -575,7 +580,3 @@
   +		mv .git/hooks .git/hooks-disabled
   +	) || exit
   +}
  -+
  - test_done () {
  - 	GIT_EXIT_OK=t
  - 



Thomas Rast (3):
  Move the user-facing test library to test-lib-functions.sh
  Introduce a performance testing framework
  Add a performance test for git-grep

 Makefile                        |   22 +-
 t/Makefile                      |   43 ++-
 t/perf/.gitignore               |    2 +
 t/perf/Makefile                 |   15 +
 t/perf/README                   |  146 ++++++++++
 t/perf/aggregate.perl           |  166 +++++++++++
 t/perf/min_time.perl            |   21 ++
 t/perf/p0000-perf-lib-sanity.sh |   41 +++
 t/perf/p0001-rev-list.sh        |   17 ++
 t/perf/p7810-grep.sh            |   23 ++
 t/perf/perf-lib.sh              |  198 ++++++++++++++
 t/perf/run                      |   82 ++++++
 t/test-lib-functions.sh         |  565 ++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh                   |  574 ++-------------------------------------
 14 files changed, 1363 insertions(+), 552 deletions(-)
 create mode 100644 t/perf/.gitignore
 create mode 100644 t/perf/Makefile
 create mode 100644 t/perf/README
 create mode 100755 t/perf/aggregate.perl
 create mode 100755 t/perf/min_time.perl
 create mode 100755 t/perf/p0000-perf-lib-sanity.sh
 create mode 100755 t/perf/p0001-rev-list.sh
 create mode 100755 t/perf/p7810-grep.sh
 create mode 100644 t/perf/perf-lib.sh
 create mode 100755 t/perf/run
 create mode 100644 t/test-lib-functions.sh

-- 
1.7.9.1.365.ge223f

[PATCH v3 1/3] Move the user-facing test library to test-lib-functions.sh

[Thomas Rast]

This just moves all the user-facing functions to a separate file and
sources that instead.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/test-lib-functions.sh |  565 +++++++++++++++++++++++++++++++++++++++++++++++
 t/test-lib.sh           |  552 +--------------------------------------------
 2 files changed, 568 insertions(+), 549 deletions(-)
 create mode 100644 t/test-lib-functions.sh

diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
new file mode 100644
index 0000000..7b3b4be
--- /dev/null
+++ b/t/test-lib-functions.sh
@@ -0,0 +1,565 @@
+#!/bin/sh
+#
+# Copyright (c) 2005 Junio C Hamano
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/ .
+
+# The semantics of the editor variables are that of invoking
+# sh -c "$EDITOR \"$@\"" files ...
+#
+# If our trash directory contains shell metacharacters, they will be
+# interpreted if we just set $EDITOR directly, so do a little dance with
+# environment variables to work around this.
+#
+# In particular, quoting isn't enough, as the path may contain the same quote
+# that we're using.
+test_set_editor () {
+	FAKE_EDITOR="$1"
+	export FAKE_EDITOR
+	EDITOR='"$FAKE_EDITOR"'
+	export EDITOR
+}
+
+test_decode_color () {
+	awk '
+		function name(n) {
+			if (n == 0) return "RESET";
+			if (n == 1) return "BOLD";
+			if (n == 30) return "BLACK";
+			if (n == 31) return "RED";
+			if (n == 32) return "GREEN";
+			if (n == 33) return "YELLOW";
+			if (n == 34) return "BLUE";
+			if (n == 35) return "MAGENTA";
+			if (n == 36) return "CYAN";
+			if (n == 37) return "WHITE";
+			if (n == 40) return "BLACK";
+			if (n == 41) return "BRED";
+			if (n == 42) return "BGREEN";
+			if (n == 43) return "BYELLOW";
+			if (n == 44) return "BBLUE";
+			if (n == 45) return "BMAGENTA";
+			if (n == 46) return "BCYAN";
+			if (n == 47) return "BWHITE";
+		}
+		{
+			while (match($0, /\033\[[0-9;]*m/) != 0) {
+				printf "%s<", substr($0, 1, RSTART-1);
+				codes = substr($0, RSTART+2, RLENGTH-3);
+				if (length(codes) == 0)
+					printf "%s", name(0)
+				else {
+					n = split(codes, ary, ";");
+					sep = "";
+					for (i = 1; i <= n; i++) {
+						printf "%s%s", sep, name(ary[i]);
+						sep = ";"
+					}
+				}
+				printf ">";
+				$0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
+			}
+			print
+		}
+	'
+}
+
+nul_to_q () {
+	perl -pe 'y/\000/Q/'
+}
+
+q_to_nul () {
+	perl -pe 'y/Q/\000/'
+}
+
+q_to_cr () {
+	tr Q '\015'
+}
+
+q_to_tab () {
+	tr Q '\011'
+}
+
+append_cr () {
+	sed -e 's/$/Q/' | tr Q '\015'
+}
+
+remove_cr () {
+	tr '\015' Q | sed -e 's/Q$//'
+}
+
+# In some bourne shell implementations, the "unset" builtin returns
+# nonzero status when a variable to be unset was not set in the first
+# place.
+#
+# Use sane_unset when that should not be considered an error.
+
+sane_unset () {
+	unset "$@"
+	return 0
+}
+
+test_tick () {
+	if test -z "${test_tick+set}"
+	then
+		test_tick=1112911993
+	else
+		test_tick=$(($test_tick + 60))
+	fi
+	GIT_COMMITTER_DATE="$test_tick -0700"
+	GIT_AUTHOR_DATE="$test_tick -0700"
+	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
+}
+
+# Stop execution and start a shell. This is useful for debugging tests and
+# only makes sense together with "-v".
+#
+# Be sure to remove all invocations of this command before submitting.
+
+test_pause () {
+	if test "$verbose" = t; then
+		"$SHELL_PATH" <&6 >&3 2>&4
+	else
+		error >&5 "test_pause requires --verbose"
+	fi
+}
+
+# Call test_commit with the arguments "<message> [<file> [<contents>]]"
+#
+# This will commit a file with the given contents and the given commit
+# message.  It will also add a tag with <message> as name.
+#
+# Both <file> and <contents> default to <message>.
+
+test_commit () {
+	file=${2:-"$1.t"}
+	echo "${3-$1}" > "$file" &&
+	git add "$file" &&
+	test_tick &&
+	git commit -m "$1" &&
+	git tag "$1"
+}
+
+# Call test_merge with the arguments "<message> <commit>", where <commit>
+# can be a tag pointing to the commit-to-merge.
+
+test_merge () {
+	test_tick &&
+	git merge -m "$1" "$2" &&
+	git tag "$1"
+}
+
+# This function helps systems where core.filemode=false is set.
+# Use it instead of plain 'chmod +x' to set or unset the executable bit
+# of a file in the working directory and add it to the index.
+
+test_chmod () {
+	chmod "$@" &&
+	git update-index --add "--chmod=$@"
+}
+
+# Unset a configuration variable, but don't fail if it doesn't exist.
+test_unconfig () {
+	git config --unset-all "$@"
+	config_status=$?
+	case "$config_status" in
+	5) # ok, nothing to unset
+		config_status=0
+		;;
+	esac
+	return $config_status
+}
+
+# Set git config, automatically unsetting it after the test is over.
+test_config () {
+	test_when_finished "test_unconfig '$1'" &&
+	git config "$@"
+}
+
+test_config_global () {
+	test_when_finished "test_unconfig --global '$1'" &&
+	git config --global "$@"
+}
+
+write_script () {
+	{
+		echo "#!${2-"$SHELL_PATH"}" &&
+		cat
+	} >"$1" &&
+	chmod +x "$1"
+}
+
+# Use test_set_prereq to tell that a particular prerequisite is available.
+# The prerequisite can later be checked for in two ways:
+#
+# - Explicitly using test_have_prereq.
+#
+# - Implicitly by specifying the prerequisite tag in the calls to
+#   test_expect_{success,failure,code}.
+#
+# The single parameter is the prerequisite tag (a simple word, in all
+# capital letters by convention).
+
+test_set_prereq () {
+	satisfied="$satisfied$1 "
+}
+satisfied=" "
+
+test_have_prereq () {
+	# prerequisites can be concatenated with ','
+	save_IFS=$IFS
+	IFS=,
+	set -- $*
+	IFS=$save_IFS
+
+	total_prereq=0
+	ok_prereq=0
+	missing_prereq=
+
+	for prerequisite
+	do
+		total_prereq=$(($total_prereq + 1))
+		case $satisfied in
+		*" $prerequisite "*)
+			ok_prereq=$(($ok_prereq + 1))
+			;;
+		*)
+			# Keep a list of missing prerequisites
+			if test -z "$missing_prereq"
+			then
+				missing_prereq=$prerequisite
+			else
+				missing_prereq="$prerequisite,$missing_prereq"
+			fi
+		esac
+	done
+
+	test $total_prereq = $ok_prereq
+}
+
+test_declared_prereq () {
+	case ",$test_prereq," in
+	*,$1,*)
+		return 0
+		;;
+	esac
+	return 1
+}
+
+test_expect_failure () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		say >&3 "checking known breakage: $2"
+		if test_run_ "$2" expecting_failure
+		then
+			test_known_broken_ok_ "$1"
+		else
+			test_known_broken_failure_ "$1"
+		fi
+	fi
+	echo >&3 ""
+}
+
+test_expect_success () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		say >&3 "expecting success: $2"
+		if test_run_ "$2"
+		then
+			test_ok_ "$1"
+		else
+			test_failure_ "$@"
+		fi
+	fi
+	echo >&3 ""
+}
+
+# test_external runs external test scripts that provide continuous
+# test output about their progress, and succeeds/fails on
+# zero/non-zero exit code.  It outputs the test output on stdout even
+# in non-verbose mode, and announces the external script with "# run
+# <n>: ..." before running it.  When providing relative paths, keep in
+# mind that all scripts run in "trash directory".
+# Usage: test_external description command arguments...
+# Example: test_external 'Perl API' perl ../path/to/test.pl
+test_external () {
+	test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 3 ||
+	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
+	descr="$1"
+	shift
+	export test_prereq
+	if ! test_skip "$descr" "$@"
+	then
+		# Announce the script to reduce confusion about the
+		# test output that follows.
+		say_color "" "# run $test_count: $descr ($*)"
+		# Export TEST_DIRECTORY, TRASH_DIRECTORY and GIT_TEST_LONG
+		# to be able to use them in script
+		export TEST_DIRECTORY TRASH_DIRECTORY GIT_TEST_LONG
+		# Run command; redirect its stderr to &4 as in
+		# test_run_, but keep its stdout on our stdout even in
+		# non-verbose mode.
+		"$@" 2>&4
+		if [ "$?" = 0 ]
+		then
+			if test $test_external_has_tap -eq 0; then
+				test_ok_ "$descr"
+			else
+				say_color "" "# test_external test $descr was ok"
+				test_success=$(($test_success + 1))
+			fi
+		else
+			if test $test_external_has_tap -eq 0; then
+				test_failure_ "$descr" "$@"
+			else
+				say_color error "# test_external test $descr failed: $@"
+				test_failure=$(($test_failure + 1))
+			fi
+		fi
+	fi
+}
+
+# Like test_external, but in addition tests that the command generated
+# no output on stderr.
+test_external_without_stderr () {
+	# The temporary file has no (and must have no) security
+	# implications.
+	tmp=${TMPDIR:-/tmp}
+	stderr="$tmp/git-external-stderr.$$.tmp"
+	test_external "$@" 4> "$stderr"
+	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
+	descr="no stderr: $1"
+	shift
+	say >&3 "# expecting no stderr from previous command"
+	if [ ! -s "$stderr" ]; then
+		rm "$stderr"
+
+		if test $test_external_has_tap -eq 0; then
+			test_ok_ "$descr"
+		else
+			say_color "" "# test_external_without_stderr test $descr was ok"
+			test_success=$(($test_success + 1))
+		fi
+	else
+		if [ "$verbose" = t ]; then
+			output=`echo; echo "# Stderr is:"; cat "$stderr"`
+		else
+			output=
+		fi
+		# rm first in case test_failure exits.
+		rm "$stderr"
+		if test $test_external_has_tap -eq 0; then
+			test_failure_ "$descr" "$@" "$output"
+		else
+			say_color error "# test_external_without_stderr test $descr failed: $@: $output"
+			test_failure=$(($test_failure + 1))
+		fi
+	fi
+}
+
+# debugging-friendly alternatives to "test [-f|-d|-e]"
+# The commands test the existence or non-existence of $1. $2 can be
+# given to provide a more precise diagnosis.
+test_path_is_file () {
+	if ! [ -f "$1" ]
+	then
+		echo "File $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_path_is_dir () {
+	if ! [ -d "$1" ]
+	then
+		echo "Directory $1 doesn't exist. $*"
+		false
+	fi
+}
+
+test_path_is_missing () {
+	if [ -e "$1" ]
+	then
+		echo "Path exists:"
+		ls -ld "$1"
+		if [ $# -ge 1 ]; then
+			echo "$*"
+		fi
+		false
+	fi
+}
+
+# test_line_count checks that a file has the number of lines it
+# ought to. For example:
+#
+#	test_expect_success 'produce exactly one line of output' '
+#		do something >output &&
+#		test_line_count = 1 output
+#	'
+#
+# is like "test $(wc -l <output) = 1" except that it passes the
+# output through when the number of lines is wrong.
+
+test_line_count () {
+	if test $# != 3
+	then
+		error "bug in the test script: not 3 parameters to test_line_count"
+	elif ! test $(wc -l <"$3") "$1" "$2"
+	then
+		echo "test_line_count: line count for $3 !$1 $2"
+		cat "$3"
+		return 1
+	fi
+}
+
+# This is not among top-level (test_expect_success | test_expect_failure)
+# but is a prefix that can be used in the test script, like:
+#
+#	test_expect_success 'complain and die' '
+#           do something &&
+#           do something else &&
+#	    test_must_fail git checkout ../outerspace
+#	'
+#
+# Writing this as "! git checkout ../outerspace" is wrong, because
+# the failure could be due to a segv.  We want a controlled failure.
+
+test_must_fail () {
+	"$@"
+	exit_code=$?
+	if test $exit_code = 0; then
+		echo >&2 "test_must_fail: command succeeded: $*"
+		return 1
+	elif test $exit_code -gt 129 -a $exit_code -le 192; then
+		echo >&2 "test_must_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_must_fail: command not found: $*"
+		return 1
+	fi
+	return 0
+}
+
+# Similar to test_must_fail, but tolerates success, too.  This is
+# meant to be used in contexts like:
+#
+#	test_expect_success 'some command works without configuration' '
+#		test_might_fail git config --unset all.configuration &&
+#		do something
+#	'
+#
+# Writing "git config --unset all.configuration || :" would be wrong,
+# because we want to notice if it fails due to segv.
+
+test_might_fail () {
+	"$@"
+	exit_code=$?
+	if test $exit_code -gt 129 -a $exit_code -le 192; then
+		echo >&2 "test_might_fail: died by signal: $*"
+		return 1
+	elif test $exit_code = 127; then
+		echo >&2 "test_might_fail: command not found: $*"
+		return 1
+	fi
+	return 0
+}
+
+# Similar to test_must_fail and test_might_fail, but check that a
+# given command exited with a given exit code. Meant to be used as:
+#
+#	test_expect_success 'Merge with d/f conflicts' '
+#		test_expect_code 1 git merge "merge msg" B master
+#	'
+
+test_expect_code () {
+	want_code=$1
+	shift
+	"$@"
+	exit_code=$?
+	if test $exit_code = $want_code
+	then
+		return 0
+	fi
+
+	echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
+	return 1
+}
+
+# test_cmp is a helper function to compare actual and expected output.
+# You can use it like:
+#
+#	test_expect_success 'foo works' '
+#		echo expected >expected &&
+#		foo >actual &&
+#		test_cmp expected actual
+#	'
+#
+# This could be written as either "cmp" or "diff -u", but:
+# - cmp's output is not nearly as easy to read as diff -u
+# - not all diff versions understand "-u"
+
+test_cmp() {
+	$GIT_TEST_CMP "$@"
+}
+
+# This function can be used to schedule some commands to be run
+# unconditionally at the end of the test to restore sanity:
+#
+#	test_expect_success 'test core.capslock' '
+#		git config core.capslock true &&
+#		test_when_finished "git config --unset core.capslock" &&
+#		hello world
+#	'
+#
+# That would be roughly equivalent to
+#
+#	test_expect_success 'test core.capslock' '
+#		git config core.capslock true &&
+#		hello world
+#		git config --unset core.capslock
+#	'
+#
+# except that the greeting and config --unset must both succeed for
+# the test to pass.
+#
+# Note that under --immediate mode, no clean-up is done to help diagnose
+# what went wrong.
+
+test_when_finished () {
+	test_cleanup="{ $*
+		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
+}
+
+# Most tests can use the created repository, but some may need to create more.
+# Usage: test_create_repo <directory>
+test_create_repo () {
+	test "$#" = 1 ||
+	error "bug in the test script: not 1 parameter to test-create-repo"
+	repo="$1"
+	mkdir -p "$repo"
+	(
+		cd "$repo" || error "Cannot setup test environment"
+		"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
+		error "cannot run git init -- have you built things yet?"
+		mv .git/hooks .git/hooks-disabled
+	) || exit
+}
diff --git a/t/test-lib.sh b/t/test-lib.sh
index e28d5fd..1da3f40 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -223,248 +223,9 @@ die () {
 GIT_EXIT_OK=
 trap 'die' EXIT
 
-# The semantics of the editor variables are that of invoking
-# sh -c "$EDITOR \"$@\"" files ...
-#
-# If our trash directory contains shell metacharacters, they will be
-# interpreted if we just set $EDITOR directly, so do a little dance with
-# environment variables to work around this.
-#
-# In particular, quoting isn't enough, as the path may contain the same quote
-# that we're using.
-test_set_editor () {
-	FAKE_EDITOR="$1"
-	export FAKE_EDITOR
-	EDITOR='"$FAKE_EDITOR"'
-	export EDITOR
-}
-
-test_decode_color () {
-	awk '
-		function name(n) {
-			if (n == 0) return "RESET";
-			if (n == 1) return "BOLD";
-			if (n == 30) return "BLACK";
-			if (n == 31) return "RED";
-			if (n == 32) return "GREEN";
-			if (n == 33) return "YELLOW";
-			if (n == 34) return "BLUE";
-			if (n == 35) return "MAGENTA";
-			if (n == 36) return "CYAN";
-			if (n == 37) return "WHITE";
-			if (n == 40) return "BLACK";
-			if (n == 41) return "BRED";
-			if (n == 42) return "BGREEN";
-			if (n == 43) return "BYELLOW";
-			if (n == 44) return "BBLUE";
-			if (n == 45) return "BMAGENTA";
-			if (n == 46) return "BCYAN";
-			if (n == 47) return "BWHITE";
-		}
-		{
-			while (match($0, /\033\[[0-9;]*m/) != 0) {
-				printf "%s<", substr($0, 1, RSTART-1);
-				codes = substr($0, RSTART+2, RLENGTH-3);
-				if (length(codes) == 0)
-					printf "%s", name(0)
-				else {
-					n = split(codes, ary, ";");
-					sep = "";
-					for (i = 1; i <= n; i++) {
-						printf "%s%s", sep, name(ary[i]);
-						sep = ";"
-					}
-				}
-				printf ">";
-				$0 = substr($0, RSTART + RLENGTH, length($0) - RSTART - RLENGTH + 1);
-			}
-			print
-		}
-	'
-}
-
-nul_to_q () {
-	perl -pe 'y/\000/Q/'
-}
-
-q_to_nul () {
-	perl -pe 'y/Q/\000/'
-}
-
-q_to_cr () {
-	tr Q '\015'
-}
-
-q_to_tab () {
-	tr Q '\011'
-}
-
-append_cr () {
-	sed -e 's/$/Q/' | tr Q '\015'
-}
-
-remove_cr () {
-	tr '\015' Q | sed -e 's/Q$//'
-}
-
-# In some bourne shell implementations, the "unset" builtin returns
-# nonzero status when a variable to be unset was not set in the first
-# place.
-#
-# Use sane_unset when that should not be considered an error.
-
-sane_unset () {
-	unset "$@"
-	return 0
-}
-
-test_tick () {
-	if test -z "${test_tick+set}"
-	then
-		test_tick=1112911993
-	else
-		test_tick=$(($test_tick + 60))
-	fi
-	GIT_COMMITTER_DATE="$test_tick -0700"
-	GIT_AUTHOR_DATE="$test_tick -0700"
-	export GIT_COMMITTER_DATE GIT_AUTHOR_DATE
-}
-
-# Stop execution and start a shell. This is useful for debugging tests and
-# only makes sense together with "-v".
-#
-# Be sure to remove all invocations of this command before submitting.
-
-test_pause () {
-	if test "$verbose" = t; then
-		"$SHELL_PATH" <&6 >&3 2>&4
-	else
-		error >&5 "test_pause requires --verbose"
-	fi
-}
-
-# Call test_commit with the arguments "<message> [<file> [<contents>]]"
-#
-# This will commit a file with the given contents and the given commit
-# message.  It will also add a tag with <message> as name.
-#
-# Both <file> and <contents> default to <message>.
-
-test_commit () {
-	file=${2:-"$1.t"}
-	echo "${3-$1}" > "$file" &&
-	git add "$file" &&
-	test_tick &&
-	git commit -m "$1" &&
-	git tag "$1"
-}
-
-# Call test_merge with the arguments "<message> <commit>", where <commit>
-# can be a tag pointing to the commit-to-merge.
-
-test_merge () {
-	test_tick &&
-	git merge -m "$1" "$2" &&
-	git tag "$1"
-}
-
-# This function helps systems where core.filemode=false is set.
-# Use it instead of plain 'chmod +x' to set or unset the executable bit
-# of a file in the working directory and add it to the index.
-
-test_chmod () {
-	chmod "$@" &&
-	git update-index --add "--chmod=$@"
-}
-
-# Unset a configuration variable, but don't fail if it doesn't exist.
-test_unconfig () {
-	git config --unset-all "$@"
-	config_status=$?
-	case "$config_status" in
-	5) # ok, nothing to unset
-		config_status=0
-		;;
-	esac
-	return $config_status
-}
-
-# Set git config, automatically unsetting it after the test is over.
-test_config () {
-	test_when_finished "test_unconfig '$1'" &&
-	git config "$@"
-}
-
-
-test_config_global () {
-	test_when_finished "test_unconfig --global '$1'" &&
-	git config --global "$@"
-}
-
-write_script () {
-	{
-		echo "#!${2-"$SHELL_PATH"}" &&
-		cat
-	} >"$1" &&
-	chmod +x "$1"
-}
-
-# Use test_set_prereq to tell that a particular prerequisite is available.
-# The prerequisite can later be checked for in two ways:
-#
-# - Explicitly using test_have_prereq.
-#
-# - Implicitly by specifying the prerequisite tag in the calls to
-#   test_expect_{success,failure,code}.
-#
-# The single parameter is the prerequisite tag (a simple word, in all
-# capital letters by convention).
-
-test_set_prereq () {
-	satisfied="$satisfied$1 "
-}
-satisfied=" "
-
-test_have_prereq () {
-	# prerequisites can be concatenated with ','
-	save_IFS=$IFS
-	IFS=,
-	set -- $*
-	IFS=$save_IFS
-
-	total_prereq=0
-	ok_prereq=0
-	missing_prereq=
-
-	for prerequisite
-	do
-		total_prereq=$(($total_prereq + 1))
-		case $satisfied in
-		*" $prerequisite "*)
-			ok_prereq=$(($ok_prereq + 1))
-			;;
-		*)
-			# Keep a list of missing prerequisites
-			if test -z "$missing_prereq"
-			then
-				missing_prereq=$prerequisite
-			else
-				missing_prereq="$prerequisite,$missing_prereq"
-			fi
-		esac
-	done
-
-	test $total_prereq = $ok_prereq
-}
-
-test_declared_prereq () {
-	case ",$test_prereq," in
-	*,$1,*)
-		return 0
-		;;
-	esac
-	return 1
-}
+# The user-facing functions are loaded from a separate file so that
+# test_perf subshells can have them too
+. "${TEST_DIRECTORY:-.}"/test-lib-functions.sh
 
 # You are not expected to call test_ok_ and test_failure_ directly, use
 # the text_expect_* functions instead.
@@ -552,313 +313,6 @@ test_skip () {
 	esac
 }
 
-test_expect_failure () {
-	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test-expect-failure"
-	export test_prereq
-	if ! test_skip "$@"
-	then
-		say >&3 "checking known breakage: $2"
-		if test_run_ "$2" expecting_failure
-		then
-			test_known_broken_ok_ "$1"
-		else
-			test_known_broken_failure_ "$1"
-		fi
-	fi
-	echo >&3 ""
-}
-
-test_expect_success () {
-	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
-	test "$#" = 2 ||
-	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
-	export test_prereq
-	if ! test_skip "$@"
-	then
-		say >&3 "expecting success: $2"
-		if test_run_ "$2"
-		then
-			test_ok_ "$1"
-		else
-			test_failure_ "$@"
-		fi
-	fi
-	echo >&3 ""
-}
-
-# test_external runs external test scripts that provide continuous
-# test output about their progress, and succeeds/fails on
-# zero/non-zero exit code.  It outputs the test output on stdout even
-# in non-verbose mode, and announces the external script with "# run
-# <n>: ..." before running it.  When providing relative paths, keep in
-# mind that all scripts run in "trash directory".
-# Usage: test_external description command arguments...
-# Example: test_external 'Perl API' perl ../path/to/test.pl
-test_external () {
-	test "$#" = 4 && { test_prereq=$1; shift; } || test_prereq=
-	test "$#" = 3 ||
-	error >&5 "bug in the test script: not 3 or 4 parameters to test_external"
-	descr="$1"
-	shift
-	export test_prereq
-	if ! test_skip "$descr" "$@"
-	then
-		# Announce the script to reduce confusion about the
-		# test output that follows.
-		say_color "" "# run $test_count: $descr ($*)"
-		# Export TEST_DIRECTORY, TRASH_DIRECTORY and GIT_TEST_LONG
-		# to be able to use them in script
-		export TEST_DIRECTORY TRASH_DIRECTORY GIT_TEST_LONG
-		# Run command; redirect its stderr to &4 as in
-		# test_run_, but keep its stdout on our stdout even in
-		# non-verbose mode.
-		"$@" 2>&4
-		if [ "$?" = 0 ]
-		then
-			if test $test_external_has_tap -eq 0; then
-				test_ok_ "$descr"
-			else
-				say_color "" "# test_external test $descr was ok"
-				test_success=$(($test_success + 1))
-			fi
-		else
-			if test $test_external_has_tap -eq 0; then
-				test_failure_ "$descr" "$@"
-			else
-				say_color error "# test_external test $descr failed: $@"
-				test_failure=$(($test_failure + 1))
-			fi
-		fi
-	fi
-}
-
-# Like test_external, but in addition tests that the command generated
-# no output on stderr.
-test_external_without_stderr () {
-	# The temporary file has no (and must have no) security
-	# implications.
-	tmp=${TMPDIR:-/tmp}
-	stderr="$tmp/git-external-stderr.$$.tmp"
-	test_external "$@" 4> "$stderr"
-	[ -f "$stderr" ] || error "Internal error: $stderr disappeared."
-	descr="no stderr: $1"
-	shift
-	say >&3 "# expecting no stderr from previous command"
-	if [ ! -s "$stderr" ]; then
-		rm "$stderr"
-
-		if test $test_external_has_tap -eq 0; then
-			test_ok_ "$descr"
-		else
-			say_color "" "# test_external_without_stderr test $descr was ok"
-			test_success=$(($test_success + 1))
-		fi
-	else
-		if [ "$verbose" = t ]; then
-			output=`echo; echo "# Stderr is:"; cat "$stderr"`
-		else
-			output=
-		fi
-		# rm first in case test_failure exits.
-		rm "$stderr"
-		if test $test_external_has_tap -eq 0; then
-			test_failure_ "$descr" "$@" "$output"
-		else
-			say_color error "# test_external_without_stderr test $descr failed: $@: $output"
-			test_failure=$(($test_failure + 1))
-		fi
-	fi
-}
-
-# debugging-friendly alternatives to "test [-f|-d|-e]"
-# The commands test the existence or non-existence of $1. $2 can be
-# given to provide a more precise diagnosis.
-test_path_is_file () {
-	if ! [ -f "$1" ]
-	then
-		echo "File $1 doesn't exist. $*"
-		false
-	fi
-}
-
-test_path_is_dir () {
-	if ! [ -d "$1" ]
-	then
-		echo "Directory $1 doesn't exist. $*"
-		false
-	fi
-}
-
-test_path_is_missing () {
-	if [ -e "$1" ]
-	then
-		echo "Path exists:"
-		ls -ld "$1"
-		if [ $# -ge 1 ]; then
-			echo "$*"
-		fi
-		false
-	fi
-}
-
-# test_line_count checks that a file has the number of lines it
-# ought to. For example:
-#
-#	test_expect_success 'produce exactly one line of output' '
-#		do something >output &&
-#		test_line_count = 1 output
-#	'
-#
-# is like "test $(wc -l <output) = 1" except that it passes the
-# output through when the number of lines is wrong.
-
-test_line_count () {
-	if test $# != 3
-	then
-		error "bug in the test script: not 3 parameters to test_line_count"
-	elif ! test $(wc -l <"$3") "$1" "$2"
-	then
-		echo "test_line_count: line count for $3 !$1 $2"
-		cat "$3"
-		return 1
-	fi
-}
-
-# This is not among top-level (test_expect_success | test_expect_failure)
-# but is a prefix that can be used in the test script, like:
-#
-#	test_expect_success 'complain and die' '
-#           do something &&
-#           do something else &&
-#	    test_must_fail git checkout ../outerspace
-#	'
-#
-# Writing this as "! git checkout ../outerspace" is wrong, because
-# the failure could be due to a segv.  We want a controlled failure.
-
-test_must_fail () {
-	"$@"
-	exit_code=$?
-	if test $exit_code = 0; then
-		echo >&2 "test_must_fail: command succeeded: $*"
-		return 1
-	elif test $exit_code -gt 129 -a $exit_code -le 192; then
-		echo >&2 "test_must_fail: died by signal: $*"
-		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_must_fail: command not found: $*"
-		return 1
-	fi
-	return 0
-}
-
-# Similar to test_must_fail, but tolerates success, too.  This is
-# meant to be used in contexts like:
-#
-#	test_expect_success 'some command works without configuration' '
-#		test_might_fail git config --unset all.configuration &&
-#		do something
-#	'
-#
-# Writing "git config --unset all.configuration || :" would be wrong,
-# because we want to notice if it fails due to segv.
-
-test_might_fail () {
-	"$@"
-	exit_code=$?
-	if test $exit_code -gt 129 -a $exit_code -le 192; then
-		echo >&2 "test_might_fail: died by signal: $*"
-		return 1
-	elif test $exit_code = 127; then
-		echo >&2 "test_might_fail: command not found: $*"
-		return 1
-	fi
-	return 0
-}
-
-# Similar to test_must_fail and test_might_fail, but check that a
-# given command exited with a given exit code. Meant to be used as:
-#
-#	test_expect_success 'Merge with d/f conflicts' '
-#		test_expect_code 1 git merge "merge msg" B master
-#	'
-
-test_expect_code () {
-	want_code=$1
-	shift
-	"$@"
-	exit_code=$?
-	if test $exit_code = $want_code
-	then
-		return 0
-	fi
-
-	echo >&2 "test_expect_code: command exited with $exit_code, we wanted $want_code $*"
-	return 1
-}
-
-# test_cmp is a helper function to compare actual and expected output.
-# You can use it like:
-#
-#	test_expect_success 'foo works' '
-#		echo expected >expected &&
-#		foo >actual &&
-#		test_cmp expected actual
-#	'
-#
-# This could be written as either "cmp" or "diff -u", but:
-# - cmp's output is not nearly as easy to read as diff -u
-# - not all diff versions understand "-u"
-
-test_cmp() {
-	$GIT_TEST_CMP "$@"
-}
-
-# This function can be used to schedule some commands to be run
-# unconditionally at the end of the test to restore sanity:
-#
-#	test_expect_success 'test core.capslock' '
-#		git config core.capslock true &&
-#		test_when_finished "git config --unset core.capslock" &&
-#		hello world
-#	'
-#
-# That would be roughly equivalent to
-#
-#	test_expect_success 'test core.capslock' '
-#		git config core.capslock true &&
-#		hello world
-#		git config --unset core.capslock
-#	'
-#
-# except that the greeting and config --unset must both succeed for
-# the test to pass.
-#
-# Note that under --immediate mode, no clean-up is done to help diagnose
-# what went wrong.
-
-test_when_finished () {
-	test_cleanup="{ $*
-		} && (exit \"\$eval_ret\"); eval_ret=\$?; $test_cleanup"
-}
-
-# Most tests can use the created repository, but some may need to create more.
-# Usage: test_create_repo <directory>
-test_create_repo () {
-	test "$#" = 1 ||
-	error "bug in the test script: not 1 parameter to test-create-repo"
-	repo="$1"
-	mkdir -p "$repo"
-	(
-		cd "$repo" || error "Cannot setup test environment"
-		"$GIT_EXEC_PATH/git-init" "--template=$GIT_BUILD_DIR/templates/blt/" >&3 2>&4 ||
-		error "cannot run git init -- have you built things yet?"
-		mv .git/hooks .git/hooks-disabled
-	) || exit
-}
-
 test_done () {
 	GIT_EXIT_OK=t
 
-- 
1.7.9.1.365.ge223f

[PATCH v3 2/3] Introduce a performance testing framework

[Thomas Rast]

This introduces a performance testing framework under t/perf/.  It
tries to be as close to the test-lib.sh infrastructure as possible,
and thus should be easy to get used to for git developers.

The following points were considered for the implementation:

1. You usually want to compare arbitrary revisions/build trees against
   each other.  They may not have the performance test under
   consideration, or even the perf-lib.sh infrastructure.

   To cope with this, the 'run' script lets you specify arbitrary
   build dirs and revisions.  It even automatically builds the revisions
   if it doesn't have them at hand yet.

2. Usually you would not want to run all tests.  It would take too
   long anyway.  The 'run' script lets you specify which tests to run;
   or you can also do it manually.  There is a Makefile for
   discoverability and 'make clean', but it is not meant for
   real-world use.

3. Creating test repos from scratch in every test is extremely
   time-consuming, and shipping or downloading such large/weird repos
   is out of the question.

   We leave this decision to the user.  Two different sizes of test
   repos can be configured, and the scripts just copy one or more of
   those (using hardlinks for the object store).  By default it tries
   to use the build tree's git.git repository.

   This is fairly fast and versatile.  Using a copy instead of a clone
   preserves many properties that the user may want to test for, such
   as lots of loose objects, unpacked refs, etc.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Makefile                        |   22 ++++-
 t/Makefile                      |   43 ++++++++-
 t/perf/.gitignore               |    2 +
 t/perf/Makefile                 |   15 +++
 t/perf/README                   |  146 +++++++++++++++++++++++++++++
 t/perf/aggregate.perl           |  166 ++++++++++++++++++++++++++++++++
 t/perf/min_time.perl            |   21 +++++
 t/perf/p0000-perf-lib-sanity.sh |   41 ++++++++
 t/perf/p0001-rev-list.sh        |   17 ++++
 t/perf/perf-lib.sh              |  198 +++++++++++++++++++++++++++++++++++++++
 t/perf/run                      |   82 ++++++++++++++++
 t/test-lib.sh                   |   26 ++++-
 12 files changed, 774 insertions(+), 5 deletions(-)
 create mode 100644 t/perf/.gitignore
 create mode 100644 t/perf/Makefile
 create mode 100644 t/perf/README
 create mode 100755 t/perf/aggregate.perl
 create mode 100755 t/perf/min_time.perl
 create mode 100755 t/perf/p0000-perf-lib-sanity.sh
 create mode 100755 t/perf/p0001-rev-list.sh
 create mode 100644 t/perf/perf-lib.sh
 create mode 100755 t/perf/run

diff --git a/Makefile b/Makefile
index a0de4e9..1fb1705 100644
--- a/Makefile
+++ b/Makefile
@@ -2361,6 +2361,10 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
+	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
+ifdef GIT_TEST_OPTS
+	@echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' >>$@
+endif
 ifdef GIT_TEST_CMP
 	@echo GIT_TEST_CMP=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_CMP)))'\' >>$@
 endif
@@ -2369,7 +2373,18 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
 endif
 	@echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' >>$@
 	@echo GETTEXT_POISON=\''$(subst ','\'',$(subst ','\'',$(GETTEXT_POISON)))'\' >>$@
-	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
+ifdef GIT_PERF_REPEAT_COUNT
+	@echo GIT_PERF_REPEAT_COUNT=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPEAT_COUNT)))'\' >>$@
+endif
+ifdef GIT_PERF_REPO
+	@echo GIT_PERF_REPO=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPO)))'\' >>$@
+endif
+ifdef GIT_PERF_LARGE_REPO
+	@echo GIT_PERF_LARGE_REPO=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_LARGE_REPO)))'\' >>$@
+endif
+ifdef GIT_PERF_MAKE_OPTS
+	@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_MAKE_OPTS)))'\' >>$@
+endif
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
@@ -2405,6 +2420,11 @@ export NO_SVN_TESTS
 test: all
 	$(MAKE) -C t/ all
 
+perf: all
+	$(MAKE) -C t/perf/ all
+
+.PHONY: test perf
+
 test-ctype$X: ctype.o
 
 test-date$X: date.o ctype.o
diff --git a/t/Makefile b/t/Makefile
index b5048ab..6091211 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -73,4 +73,45 @@ gitweb-test:
 valgrind:
 	$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
 
-.PHONY: pre-clean $(T) aggregate-results clean valgrind
+perf:
+	$(MAKE) -C perf/ all
+
+# Smoke testing targets
+-include ../GIT-VERSION-FILE
+uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo unknown')
+uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo unknown')
+
+test-results:
+	mkdir -p test-results
+
+test-results/git-smoke.tar.gz: test-results
+	$(PERL_PATH) ./harness \
+		--archive="test-results/git-smoke.tar.gz" \
+		$(T)
+
+smoke: test-results/git-smoke.tar.gz
+
+SMOKE_UPLOAD_FLAGS =
+ifdef SMOKE_USERNAME
+	SMOKE_UPLOAD_FLAGS += -F username="$(SMOKE_USERNAME)" -F password="$(SMOKE_PASSWORD)"
+endif
+ifdef SMOKE_COMMENT
+	SMOKE_UPLOAD_FLAGS += -F comments="$(SMOKE_COMMENT)"
+endif
+ifdef SMOKE_TAGS
+	SMOKE_UPLOAD_FLAGS += -F tags="$(SMOKE_TAGS)"
+endif
+
+smoke_report: smoke
+	curl \
+		-H "Expect: " \
+		-F project=Git \
+		-F architecture="$(uname_M)" \
+		-F platform="$(uname_S)" \
+		-F revision="$(GIT_VERSION)" \
+		-F report_file=@test-results/git-smoke.tar.gz \
+		$(SMOKE_UPLOAD_FLAGS) \
+		http://smoke.git.nix.is/app/projects/process_add_report/1 \
+	| grep -v ^Redirecting
+
+.PHONY: pre-clean $(T) aggregate-results clean valgrind perf
diff --git a/t/perf/.gitignore b/t/perf/.gitignore
new file mode 100644
index 0000000..50f5cc1
--- /dev/null
+++ b/t/perf/.gitignore
@@ -0,0 +1,2 @@
+build/
+test-results/
diff --git a/t/perf/Makefile b/t/perf/Makefile
new file mode 100644
index 0000000..8c47155
--- /dev/null
+++ b/t/perf/Makefile
@@ -0,0 +1,15 @@
+-include ../../config.mak
+export GIT_TEST_OPTIONS
+
+all: perf
+
+perf: pre-clean
+	./run
+
+pre-clean:
+	rm -rf test-results
+
+clean:
+	rm -rf build "trash directory".* test-results
+
+.PHONY: all perf pre-clean clean
diff --git a/t/perf/README b/t/perf/README
new file mode 100644
index 0000000..fdf974a
--- /dev/null
+++ b/t/perf/README
@@ -0,0 +1,146 @@
+Git performance tests
+=====================
+
+This directory holds performance testing scripts for git tools.  The
+first part of this document describes the various ways in which you
+can run them.
+
+When fixing the tools or adding enhancements, you are strongly
+encouraged to add tests in this directory to cover what you are
+trying to fix or enhance.  The later part of this short document
+describes how your test scripts should be organized.
+
+
+Running Tests
+-------------
+
+The easiest way to run tests is to say "make".  This runs all
+the tests on the current git repository.
+
+    === Running 2 tests in this tree ===
+    [...]
+    Test                                     this tree       
+    ---------------------------------------------------------
+    0001.1: rev-list --all                   0.54(0.51+0.02) 
+    0001.2: rev-list --all --objects         6.14(5.99+0.11) 
+    7810.1: grep worktree, cheap regex       0.16(0.16+0.35) 
+    7810.2: grep worktree, expensive regex   7.90(29.75+0.37)
+    7810.3: grep --cached, cheap regex       3.07(3.02+0.25) 
+    7810.4: grep --cached, expensive regex   9.39(30.57+0.24)
+
+You can compare multiple repositories and even git revisions with the
+'run' script:
+
+    $ ./run . origin/next /path/to/git-tree p0001-rev-list.sh
+
+where . stands for the current git tree.  The full invocation is
+
+    ./run [<revision|directory>...] [--] [<test-script>...]
+
+A '.' argument is implied if you do not pass any other
+revisions/directories.
+
+You can also manually test this or another git build tree, and then
+call the aggregation script to summarize the results:
+
+    $ ./p0001-rev-list.sh
+    [...]
+    $ GIT_BUILD_DIR=/path/to/other/git ./p0001-rev-list.sh
+    [...]
+    $ ./aggregate.perl . /path/to/other/git ./p0001-rev-list.sh
+
+aggregate.perl has the same invocation as 'run', it just does not run
+anything beforehand.
+
+You can set the following variables (also in your config.mak):
+
+    GIT_PERF_REPEAT_COUNT
+	Number of times a test should be repeated for best-of-N
+	measurements.  Defaults to 5.
+
+    GIT_PERF_MAKE_OPTS
+	Options to use when automatically building a git tree for
+	performance testing.  E.g., -j6 would be useful.
+
+    GIT_PERF_REPO
+    GIT_PERF_LARGE_REPO
+	Repositories to copy for the performance tests.  The normal
+	repo should be at least git.git size.  The large repo should
+	probably be about linux-2.6.git size for optimal results.
+	Both default to the git.git you are running from.
+
+You can also pass the options taken by ordinary git tests; the most
+useful one is:
+
+--root=<directory>::
+	Create "trash" directories used to store all temporary data during
+	testing under <directory>, instead of the t/ directory.
+	Using this option with a RAM-based filesystem (such as tmpfs)
+	can massively speed up the test suite.
+
+
+Naming Tests
+------------
+
+The performance test files are named as:
+
+	pNNNN-commandname-details.sh
+
+where N is a decimal digit.  The same conventions for choosing NNNN as
+for normal tests apply.
+
+
+Writing Tests
+-------------
+
+The perf script starts much like a normal test script, except it
+sources perf-lib.sh:
+
+	#!/bin/sh
+	#
+	# Copyright (c) 2005 Junio C Hamano
+	#
+
+	test_description='xxx performance test'
+	. ./perf-lib.sh
+
+After that you will want to use some of the following:
+
+	test_perf_default_repo  # sets up a "normal" repository
+	test_perf_large_repo    # sets up a "large" repository
+
+	test_perf_default_repo sub  # ditto, in a subdir "sub"
+
+        test_checkout_worktree  # if you need the worktree too
+
+At least one of the first two is required!
+
+You can use test_expect_success as usual.  For actual performance
+tests, use
+
+	test_perf 'descriptive string' '
+		command1 &&
+		command2
+	'
+
+test_perf spawns a subshell, for lack of better options.  This means
+that
+
+* you _must_ export all variables that you need in the subshell
+
+* you _must_ flag all variables that you want to persist from the
+  subshell with 'test_export':
+
+	test_perf 'descriptive string' '
+		foo=$(git rev-parse HEAD) &&
+		test_export foo
+	'
+
+  The so-exported variables are automatically marked for export in the
+  shell executing the perf test.  For your convenience, test_export is
+  the same as export in the main shell.
+
+  This feature relies on a bit of magic using 'set' and 'source'.
+  While we have tried to make sure that it can cope with embedded
+  whitespace and other special characters, it will not work with
+  multi-line data.
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
new file mode 100755
index 0000000..15f7fc1
--- /dev/null
+++ b/t/perf/aggregate.perl
@@ -0,0 +1,166 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Git;
+
+sub get_times {
+	my $name = shift;
+	open my $fh, "<", $name or return undef;
+	my $line = <$fh>;
+	return undef if not defined $line;
+	close $fh or die "cannot close $name: $!";
+	$line =~ /^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/
+		or die "bad input line: $line";
+	my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3;
+	return ($rt, $4, $5);
+}
+
+sub format_times {
+	my ($r, $u, $s, $firstr) = @_;
+	if (!defined $r) {
+		return "<missing>";
+	}
+	my $out = sprintf "%.2f(%.2f+%.2f)", $r, $u, $s;
+	if (defined $firstr) {
+		if ($firstr > 0) {
+			$out .= sprintf " %+.1f%%", 100.0*($r-$firstr)/$firstr;
+		} elsif ($r == 0) {
+			$out .= " =";
+		} else {
+			$out .= " +inf";
+		}
+	}
+	return $out;
+}
+
+my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests);
+while (scalar @ARGV) {
+	my $arg = $ARGV[0];
+	my $dir;
+	last if -f $arg or $arg eq "--";
+	if (! -d $arg) {
+		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
+		$dir = "build/".$rev;
+	} else {
+		$arg =~ s{/*$}{};
+		$dir = $arg;
+		$dirabbrevs{$dir} = $dir;
+	}
+	push @dirs, $dir;
+	$dirnames{$dir} = $arg;
+	my $prefix = $dir;
+	$prefix =~ tr/^a-zA-Z0-9/_/c;
+	$prefixes{$dir} = $prefix . '.';
+	shift @ARGV;
+}
+
+if (not @dirs) {
+	@dirs = ('.');
+}
+$dirnames{'.'} = $dirabbrevs{'.'} = "this tree";
+$prefixes{'.'} = '';
+
+shift @ARGV if scalar @ARGV and $ARGV[0] eq "--";
+
+@tests = @ARGV;
+if (not @tests) {
+	@tests = glob "p????-*.sh";
+}
+
+my @subtests;
+my %shorttests;
+for my $t (@tests) {
+	$t =~ s{(?:.*/)?(p(\d+)-[^/]+)\.sh$}{$1} or die "bad test name: $t";
+	my $n = $2;
+	my $fname = "test-results/$t.subtests";
+	open my $fp, "<", $fname or die "cannot open $fname: $!";
+	for (<$fp>) {
+		chomp;
+		/^(\d+)$/ or die "malformed subtest line: $_";
+		push @subtests, "$t.$1";
+		$shorttests{"$t.$1"} = "$n.$1";
+	}
+	close $fp or die "cannot close $fname: $!";
+}
+
+sub read_descr {
+	my $name = shift;
+	open my $fh, "<", $name or return "<error reading description>";
+	my $line = <$fh>;
+	close $fh or die "cannot close $name";
+	chomp $line;
+	return $line;
+}
+
+my %descrs;
+my $descrlen = 4; # "Test"
+for my $t (@subtests) {
+	$descrs{$t} = $shorttests{$t}.": ".read_descr("test-results/$t.descr");
+	$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
+}
+
+sub have_duplicate {
+	my %seen;
+	for (@_) {
+		return 1 if exists $seen{$_};
+		$seen{$_} = 1;
+	}
+	return 0;
+}
+sub have_slash {
+	for (@_) {
+		return 1 if m{/};
+	}
+	return 0;
+}
+
+my %newdirabbrevs = %dirabbrevs;
+while (!have_duplicate(values %newdirabbrevs)) {
+	%dirabbrevs = %newdirabbrevs;
+	last if !have_slash(values %dirabbrevs);
+	%newdirabbrevs = %dirabbrevs;
+	for (values %newdirabbrevs) {
+		s{^[^/]*/}{};
+	}
+}
+
+my %times;
+my @colwidth = ((0)x@dirs);
+for my $i (0..$#dirs) {
+	my $d = $dirs[$i];
+	my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
+	$colwidth[$i] = $w if $w > $colwidth[$i];
+}
+for my $t (@subtests) {
+	my $firstr;
+	for my $i (0..$#dirs) {
+		my $d = $dirs[$i];
+		$times{$prefixes{$d}.$t} = [get_times("test-results/$prefixes{$d}$t.times")];
+		my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+		my $w = length format_times($r,$u,$s,$firstr);
+		$colwidth[$i] = $w if $w > $colwidth[$i];
+		$firstr = $r unless defined $firstr;
+	}
+}
+my $totalwidth = 3*@dirs+$descrlen;
+$totalwidth += $_ for (@colwidth);
+
+printf "%-${descrlen}s", "Test";
+for my $i (0..$#dirs) {
+	my $d = $dirs[$i];
+	printf "   %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
+}
+print "\n";
+print "-"x$totalwidth, "\n";
+for my $t (@subtests) {
+	printf "%-${descrlen}s", $descrs{$t};
+	my $firstr;
+	for my $i (0..$#dirs) {
+		my $d = $dirs[$i];
+		my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+		printf "   %-$colwidth[$i]s", format_times($r,$u,$s,$firstr);
+		$firstr = $r unless defined $firstr;
+	}
+	print "\n";
+}
diff --git a/t/perf/min_time.perl b/t/perf/min_time.perl
new file mode 100755
index 0000000..c1a2717
--- /dev/null
+++ b/t/perf/min_time.perl
@@ -0,0 +1,21 @@
+#!/usr/bin/perl
+
+my $minrt = 1e100;
+my $min;
+
+while (<>) {
+	# [h:]m:s.xx U.xx S.xx
+	/^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/
+		or die "bad input line: $_";
+	my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3;
+	if ($rt < $minrt) {
+		$min = $_;
+		$minrt = $rt;
+	}
+}
+
+if (!defined $min) {
+	die "no input found";
+}
+
+print $min;
diff --git a/t/perf/p0000-perf-lib-sanity.sh b/t/perf/p0000-perf-lib-sanity.sh
new file mode 100755
index 0000000..2ca4aac
--- /dev/null
+++ b/t/perf/p0000-perf-lib-sanity.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Tests whether perf-lib facilities work'
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+test_perf 'test_perf_default_repo works' '
+	foo=$(git rev-parse HEAD) &&
+	test_export foo
+'
+
+test_checkout_worktree
+
+test_perf 'test_checkout_worktree works' '
+	wt=$(find . | wc -l) &&
+	idx=$(git ls-files | wc -l) &&
+	test $wt -gt $idx
+'
+
+baz=baz
+test_export baz
+
+test_expect_success 'test_export works' '
+	echo "$foo" &&
+	test "$foo" = "$(git rev-parse HEAD)" &&
+	echo "$baz" &&
+	test "$baz" = baz
+'
+
+test_perf 'export a weird var' '
+	bar="weird # variable" &&
+	test_export bar
+'
+
+test_expect_success 'test_export works with weird vars' '
+	echo "$bar" &&
+	test "$bar" = "weird # variable"
+'
+
+test_done
diff --git a/t/perf/p0001-rev-list.sh b/t/perf/p0001-rev-list.sh
new file mode 100755
index 0000000..4f71a63
--- /dev/null
+++ b/t/perf/p0001-rev-list.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+test_description="Tests history walking performance"
+
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+test_perf 'rev-list --all' '
+	git rev-list --all >/dev/null
+'
+
+test_perf 'rev-list --all --objects' '
+	git rev-list --all --objects >/dev/null
+'
+
+test_done
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
new file mode 100644
index 0000000..07e9b09
--- /dev/null
+++ b/t/perf/perf-lib.sh
@@ -0,0 +1,198 @@
+#!/bin/bash
+#
+# Copyright (c) 2011 Thomas Rast
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/ .
+
+# do the --tee work early; it otherwise confuses our careful
+# GIT_BUILD_DIR mangling
+case "$GIT_TEST_TEE_STARTED, $* " in
+done,*)
+	# do not redirect again
+	;;
+*' --tee '*|*' --va'*)
+	mkdir -p test-results
+	BASE=test-results/$(basename "$0" .sh)
+	(GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1;
+	 echo $? > $BASE.exit) | tee $BASE.out
+	test "$(cat $BASE.exit)" = 0
+	exit
+	;;
+esac
+
+TEST_DIRECTORY=$(pwd)/..
+TEST_OUTPUT_DIRECTORY=$(pwd)
+if test -z "$GIT_TEST_INSTALLED"; then
+	perf_results_prefix=
+else
+	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
+	# make the tested dir absolute
+	GIT_TEST_INSTALLED=$(cd "$GIT_TEST_INSTALLED" && pwd)
+fi
+
+TEST_NO_CREATE_REPO=t
+
+. ../test-lib.sh
+
+perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
+mkdir -p "$perf_results_dir"
+rm -f "$perf_results_dir"/$(basename "$0" .sh).subtests
+
+if test -z "$GIT_PERF_REPEAT_COUNT"; then
+	GIT_PERF_REPEAT_COUNT=3
+fi
+die_if_build_dir_not_repo () {
+	if ! ( cd "$TEST_DIRECTORY/.." &&
+		    git rev-parse --build-dir >/dev/null 2>&1 ); then
+		error "No $1 defined, and your build directory is not a repo"
+	fi
+}
+
+if test -z "$GIT_PERF_REPO"; then
+	die_if_build_dir_not_repo '$GIT_PERF_REPO'
+	GIT_PERF_REPO=$TEST_DIRECTORY/..
+fi
+if test -z "$GIT_PERF_LARGE_REPO"; then
+	die_if_build_dir_not_repo '$GIT_PERF_LARGE_REPO'
+	GIT_PERF_LARGE_REPO=$TEST_DIRECTORY/..
+fi
+
+test_perf_create_repo_from () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-create-repo"
+	repo="$1"
+	source="$2"
+	source_git=$source/$(cd "$source" && git rev-parse --git-dir)
+	mkdir -p "$repo/.git"
+	(
+		cd "$repo/.git" &&
+		{ cp -Rl "$source_git/objects" . 2>/dev/null ||
+			cp -R "$source_git/objects" .; } &&
+		for stuff in "$source_git"/*; do
+			case "$stuff" in
+				*/objects|*/hooks|*/config)
+					;;
+				*)
+					cp -R "$stuff" . || break
+					;;
+			esac
+		done &&
+		cd .. &&
+		git init -q &&
+		mv .git/hooks .git/hooks-disabled 2>/dev/null
+	) || error "failed to copy repository '$source' to '$repo'"
+}
+
+# call at least one of these to establish an appropriately-sized repository
+test_perf_default_repo () {
+	test_perf_create_repo_from "${1:-$TRASH_DIRECTORY}" "$GIT_PERF_REPO"
+}
+test_perf_large_repo () {
+	if test "$GIT_PERF_LARGE_REPO" = "$GIT_BUILD_DIR"; then
+		echo "warning: \$GIT_PERF_LARGE_REPO is \$GIT_BUILD_DIR." >&2
+		echo "warning: This will work, but may not be a sufficiently large repo" >&2
+		echo "warning: for representative measurements." >&2
+	fi
+	test_perf_create_repo_from "${1:-$TRASH_DIRECTORY}" "$GIT_PERF_LARGE_REPO"
+}
+test_checkout_worktree () {
+	git checkout-index -u -a ||
+	error "git checkout-index failed"
+}
+
+# Performance tests should never fail.  If they do, stop immediately
+immediate=t
+
+test_run_perf_ () {
+	test_cleanup=:
+	test_export_="test_cleanup"
+	export test_cleanup test_export_
+	/usr/bin/time -f "%E %U %S" -o test_time.$i "$SHELL" -c '
+. '"$TEST_DIRECTORY"/../test-lib-functions.sh'
+test_export () {
+	[ $# != 0 ] || return 0
+	test_export_="$test_export_\\|$1"
+	shift
+	test_export "$@"
+}
+'"$1"'
+ret=$?
+set | sed -n "s'"/'/'\\\\''/g"';s/^\\($test_export_\\)/export '"'&'"'/p" >test_vars
+exit $ret' >&3 2>&4
+	eval_ret=$?
+
+	if test $eval_ret = 0 || test -n "$expecting_failure"
+	then
+		test_eval_ "$test_cleanup"
+		source ./test_vars || error "failed to load updated environment"
+	fi
+	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"; then
+		echo ""
+	fi
+	return "$eval_ret"
+}
+
+
+test_perf () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		base=$(basename "$0" .sh)
+		echo "$test_count" >>"$perf_results_dir"/$base.subtests
+		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
+		if test -z "$verbose"; then
+			echo -n "perf $test_count - $1:"
+		else
+			echo "perf $test_count - $1:"
+		fi
+		for i in $(seq 1 $GIT_PERF_REPEAT_COUNT); do
+			say >&3 "running: $2"
+			if test_run_perf_ "$2"
+			then
+				if test -z "$verbose"; then
+					echo -n " $i"
+				else
+					echo "* timing run $i/$GIT_PERF_REPEAT_COUNT:"
+				fi
+			else
+				test -z "$verbose" && echo
+				test_failure_ "$@"
+				break
+			fi
+		done
+		if test -z "$verbose"; then
+			echo " ok"
+		else
+			test_ok_ "$1"
+		fi
+		base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
+		"$TEST_DIRECTORY"/perf/min_time.perl test_time.* >"$base".times
+	fi
+	echo >&3 ""
+}
+
+# We extend test_done to print timings at the end (./run disables this
+# and does it after running everything)
+test_at_end_hook_ () {
+	if test -z "$GIT_PERF_AGGREGATING_LATER"; then
+		( cd "$TEST_DIRECTORY"/perf && ./aggregate.perl $(basename "$0") )
+	fi
+}
+
+test_export () {
+	export "$@"
+}
diff --git a/t/perf/run b/t/perf/run
new file mode 100755
index 0000000..cfd7012
--- /dev/null
+++ b/t/perf/run
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+case "$1" in
+	--help)
+		echo "usage: $0 [other_git_tree...] [--] [test_scripts]"
+		exit 0
+		;;
+esac
+
+die () {
+	echo >&2 "error: $*"
+	exit 1
+}
+
+run_one_dir () {
+	if test $# -eq 0; then
+		set -- p????-*.sh
+	fi
+	echo "=== Running $# tests in ${GIT_TEST_INSTALLED:-this tree} ==="
+	for t in "$@"; do
+		./$t $GIT_TEST_OPTS
+	done
+}
+
+unpack_git_rev () {
+	rev=$1
+	mkdir -p build/$rev
+	(cd "$(git rev-parse --show-cdup)" && git archive --format=tar $rev) |
+	(cd build/$rev && tar x)
+}
+build_git_rev () {
+	rev=$1
+	cp ../../config.mak build/$rev/config.mak
+	(cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||
+	die "failed to build revision '$mydir'"
+}
+
+run_dirs_helper () {
+	mydir=${1%/}
+	shift
+	while test $# -gt 0 -a "$1" != -- -a ! -f "$1"; do
+		shift
+	done
+	if test $# -gt 0 -a "$1" = --; then
+		shift
+	fi
+	if [ ! -d "$mydir" ]; then
+		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
+		die "'$mydir' is neither a directory nor a valid revision"
+		if [ ! -d build/$rev ]; then
+			unpack_git_rev $rev
+		fi
+		build_git_rev $rev
+		mydir=build/$rev
+	fi
+	if test "$mydir" = .; then
+		unset GIT_TEST_INSTALLED
+	else
+		GIT_TEST_INSTALLED="$mydir/bin-wrappers"
+		export GIT_TEST_INSTALLED
+	fi
+	run_one_dir "$@"
+}
+
+run_dirs () {
+	while test $# -gt 0 -a "$1" != -- -a ! -f "$1"; do
+		run_dirs_helper "$@"
+		shift
+	done
+}
+
+GIT_PERF_AGGREGATING_LATER=t
+export GIT_PERF_AGGREGATING_LATER
+
+cd "$(dirname $0)"
+. ../../GIT-BUILD-OPTIONS
+
+if test $# = 0 -o "$1" = -- -o -f "$1"; then
+	set -- . "$@"
+fi
+run_dirs "$@"
+./aggregate.perl "$@"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index 1da3f40..d75766a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -55,6 +55,7 @@ unset $(perl -e '
 		.*_TEST
 		PROVE
 		VALGRIND
+		PERF_AGGREGATING_LATER
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);
@@ -98,6 +99,8 @@ _z40=0000000000000000000000000000000000000000
 LF='
 '
 
+export _x05 _x40 _z40 LF
+
 # Each test should start with something like this, after copyright notices:
 #
 # test_description='Description of this test...
@@ -313,11 +316,16 @@ test_skip () {
 	esac
 }
 
+# stub; perf-lib overrides it
+test_at_end_hook_ () {
+	:
+}
+
 test_done () {
 	GIT_EXIT_OK=t
 
 	if test -z "$HARNESS_ACTIVE"; then
-		test_results_dir="$TEST_DIRECTORY/test-results"
+		test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
 		mkdir -p "$test_results_dir"
 		test_results_path="$test_results_dir/${0%.sh}-$$.counts"
 
@@ -356,6 +364,8 @@ test_done () {
 		cd "$(dirname "$remove_trash")" &&
 		rm -rf "$(basename "$remove_trash")"
 
+		test_at_end_hook_
+
 		exit 0 ;;
 
 	*)
@@ -378,6 +388,12 @@ then
 	# itself.
 	TEST_DIRECTORY=$(pwd)
 fi
+if test -z "$TEST_OUTPUT_DIRECTORY"
+then
+	# Similarly, override this to store the test-results subdir
+	# elsewhere
+	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
+fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
 if test -n "$valgrind"
@@ -513,7 +529,7 @@ test="trash directory.$(basename "$0" .sh)"
 test -n "$root" && test="$root/$test"
 case "$test" in
 /*) TRASH_DIRECTORY="$test" ;;
- *) TRASH_DIRECTORY="$TEST_DIRECTORY/$test" ;;
+ *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$test" ;;
 esac
 test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
 rm -fr "$test" || {
@@ -525,7 +541,11 @@ rm -fr "$test" || {
 HOME="$TRASH_DIRECTORY"
 export HOME
 
-test_create_repo "$test"
+if test -z "$TEST_NO_CREATE_REPO"; then
+	test_create_repo "$test"
+else
+	mkdir -p "$test"
+fi
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$test" || exit 1
-- 
1.7.9.1.365.ge223f

[PATCH v3 3/3] Add a performance test for git-grep

[Thomas Rast]

The only catch is that we don't really know what our repo contains, so
we have to ignore any possible "not found" status from git-grep.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/perf/p7810-grep.sh |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100755 t/perf/p7810-grep.sh

diff --git a/t/perf/p7810-grep.sh b/t/perf/p7810-grep.sh
new file mode 100755
index 0000000..9f4ade6
--- /dev/null
+++ b/t/perf/p7810-grep.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+test_description="git-grep performance in various modes"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+test_perf 'grep worktree, cheap regex' '
+	git grep some_nonexistent_string || :
+'
+test_perf 'grep worktree, expensive regex' '
+	git grep "^.* *some_nonexistent_string$" || :
+'
+test_perf 'grep --cached, cheap regex' '
+	git grep --cached some_nonexistent_string || :
+'
+test_perf 'grep --cached, expensive regex' '
+	git grep --cached "^.* *some_nonexistent_string$" || :
+'
+
+test_done
-- 
1.7.9.1.365.ge223f

Re: [PATCH v3 0/3]

[Junio C Hamano]

Thomas Rast <trast@student.ethz.ch> writes:

> There were actually more mistakes lurking :-( so I am resending the
> whole series.

Ok, will requeue.  The diff you attached to this cover letter looked at
least halfway sane, compared to the previous round ;-), though it is not
exactly clear what goes to lib-test-functions and what goes to lib-test
(for example, you moved test_expect_success to 'test-functions', but it
calls test_ok_ that is in 'test-lib', and test_ok_ is directly used by
test_perf in the new 't/perf/perf-lib.sh'), making it harder for people to
decide where to put their additions to the test infrastructure from now
on.  There needs a bit of description in the first patch to guide them.

I seem to be getting intermittent test failures, and every time the
failing tests are different, when these three are queued to 'pu'. I didn't
look for what goes wrong and how.

Thanks.

Re: [PATCH v3 0/3]

[Junio C Hamano]

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

> Thomas Rast <trast@student.ethz.ch> writes:
> ...
> I seem to be getting intermittent test failures, and every time the
> failing tests are different, when these three are queued to 'pu'. I didn't
> look for what goes wrong and how.

False alarm. I suspect that it is jb/required-filter topic that is causing
intermittent failures from convert.c depending on the timing of how fast
filter subprocess dies vs how fast we consume its result or something.

Repeatedly running t0021 like this:

    $ cd t
    $ while sh t0021-conversion.sh ; do :; done

under load seems to make it fail every once in a while.

test_must_fail: died by signal: git add test.fc

Are we dying on SIGPIPE or something?

Re: [PATCH v3 0/3]

[Jeff King]

On Fri, Feb 17, 2012 at 03:28:51PM -0800, Junio C Hamano wrote:

> Junio C Hamano <gitster@pobox.com> writes:
> 
> > Thomas Rast <trast@student.ethz.ch> writes:
> > ...
> > I seem to be getting intermittent test failures, and every time the
> > failing tests are different, when these three are queued to 'pu'. I didn't
> > look for what goes wrong and how.
> 
> False alarm. I suspect that it is jb/required-filter topic that is causing
> intermittent failures from convert.c depending on the timing of how fast
> filter subprocess dies vs how fast we consume its result or something.
> 
> Repeatedly running t0021 like this:
> 
>     $ cd t
>     $ while sh t0021-conversion.sh ; do :; done
> 
> under load seems to make it fail every once in a while.
> 
> test_must_fail: died by signal: git add test.fc
> 
> Are we dying on SIGPIPE or something?

I would be unsurprised if that is the case. Joey Hess mentioned similar
issues with hooks a month or two ago. And I have been seeing
intermittent failures of t5541 under load that I traced back to SIGPIPE.
I've been meaning to dig further and come up with a good solution.
Here's some previous discussion:

  http://article.gmane.org/gmane.comp.version-control.git/186291

I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
the log family.

-Peff

Re: [PATCH v3 0/3]

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
> the log family.

Hmmmm, now you confused me...  What is special about the log family?  Do
you mean "when we use pager"?  But then we are writing into the pager,
which the user can make it exit, which in turn causes us to write into the
pipe, so I would expect that we would want to ignore SIGPIPE --- ah, then
we explicitly catch error in xwrite() and say die() which we do not want.

So you want to let SIGPIPE silently kill us when the pager is in use; is
that it?

Re: [PATCH v3 0/3]

[Jeff King]

On Fri, Feb 17, 2012 at 11:27:26PM -0800, Junio C Hamano wrote:

> > I'd be happy if we just ignored SIGPIPE everywhere, but turned it on for
> > the log family.
> 
> Hmmmm, now you confused me...  What is special about the log family?  Do
> you mean "when we use pager"?  But then we are writing into the pager,
> which the user can make it exit, which in turn causes us to write into the
> pipe, so I would expect that we would want to ignore SIGPIPE --- ah, then
> we explicitly catch error in xwrite() and say die() which we do not want.

Sort of. I mentioned the log family because those are the commands that
tend to generate large amounts of input, and which are likely to hit
SIGPIPE. But let me elaborate on my thinking a bit.

There are basically three types of writing callsites in git:

  1. Careful calls to write() or fwrite().

     These are the majority of calls. In these, we check the return
     value of write() and die, return the error code, or whatever is
     appropriate for the callsite. SIGPIPE kills the program before the
     careful code gets the chance to make a decision about what to do.
     At best, this is a nuisance; even if the program is going to die(),
     it's likely that the custom code could produce a useful error
     message. At worst it causes bugs. For example, we may write to a
     subprocess that only needs part of our input to make a decision
     (e.g., some hooks and credential helpers). With SIGPIPE, we end up
     dying even though no error has occurred. Worse, these are often
     annoying heisenbugs that depend on the timing of write() and
     close() between the two processes.

  2. Non-careful calls of small, fixed-size output.

     Things like "git remote" will write some output to stdout via
     printf and not bother checking the return code. As the main purpose
     of the program is to create output, it's OK to be killed by
     SIGPIPE if it happens due to a write to stdout. But respecting
     SIGPIPE doesn't buy as all that much. It might save us from making
     a few write() calls that will go to nowhere, but most of the work
     has already been done by the time we are outputting.

     And it has a cost to the other careful write calls in the same
     program, because respecting SIGPIPE is a per-process thing. So for
     something like "git remote show foo", while we would like SIGPIPE
     to kick in for output to stdout, we would not want it for a pipe we
     opened to talk to ls-remote.

  3. Non-careful calls of large, streaming data.

     Commands like "git log" will non-carefully output to stdout, as
     well, but they will generate tons of data, consuming possibly
     minutes of CPU time (e.g., "git log -p"). If whoever is reading the
     output stops doing so, we really want to kill the program to avoid
     wasting CPU time.  In this instance, SIGPIPE is a big win.

     It still has the downside that careful calls in the same program
     will be subject to using SIGPIPE. For "log" and friends, this is
     probably OK with the current code, as we don't make a lot of pipes.
     But that is somewhat of an implementation detail. E.g., "git log
     -p" with external diff or textconv writes to a tempfile, and then
     runs a subprocess with the tempfile as input. But we could just as
     easily have used pipes, and may choose to do so in the future. You
     may even be able to trigger a convert_to_git filter in the current
     code, which does use pipes.

So basically, I find SIGPIPE to be a simplistic too-heavy hammer that
ends up affecting all writes to pipes, when in reality we are only
interested in affecting ones to some "main" output (usually stdout).
That works OK for small Unix-y programs like "head", but is overly
simplistic for something as big as git.

In an ideal world, we could set a per-descriptor version of SIGPIPE, and
just turn it on for stdout (or somehow find out which descriptor caused
the SIGPIPE after the fact). But that's not possible.

So our next best thing would be to actually check the results of
these non-careful writes. Unfortunately, this means either:

  a. wrapping every printf with a function that will appropriately die
     on error. This makes the code more cumbersome.

  or

  b. occasionally checking ferror(stdout) while doing long streams
     (e.g., checking it after each commit is written in git log, and
     aborting if we saw a write error). This is less cumbersome, but it
     does mean that errno may or may not still be accurate by the time
     we notice the error. So it's hard to reliably differentiate EPIPE
     from other errors. It would be nice, for example, to have git log
     exit silently on EPIPE, but properly print an error for something
     like ENOSPC. But perhaps that isn't a big deal, as I believe right
     now that we would silently ignore something like ENOSPC.

Less robust than that is to just ignore SIGPIPE in most git programs
(which don't benefit from it, and where it is only a liability), but
then manually enable it for the few that care (the log family, and
perhaps diff. Maybe things like "git tag -l", though that output tends
to be pretty small). But I think it would work OK in practice, because
those commands don't tend to make pipes other than the "main" output.
And it's quite easy to implement.

> So you want to let SIGPIPE silently kill us when the pager is in use; is
> that it?

That's an OK heuristic, as the pager being in use is a good sign that we
will generate long, streaming output. It has two downsides, though. One
is that it suffers from the too-heavy hammer of the preceding paragraph.
The other is that it only catches when _we_ create the pipe. You would
also want to catch something like:

  git rev-list | head

and stop the traversal. So I think it is less about whether a pager is
in use and more about whether we are creating long output that would
benefit from being cut off early. The pager is an indicator of that, but
it's not a perfect one; I think we'd do better to mark those spots
manually (i.e., by re-enabling SIGPIPE in commands that we deem
appropriate).

-Peff

SIGPIPE handling (Re: [PATCH v3 0/3])

[Jonathan Nieder]

Hi,

Jeff King wrote:

> Less robust than that is to just ignore SIGPIPE in most git programs
> (which don't benefit from it, and where it is only a liability), but
> then manually enable it for the few that care

This seems backwards.  Aren't the only places where it is just a
liability places where git is writing to a pipe that git has created?

We could keep the benefits of SIGPIPE (including simpler error
handling and lack of distracting EPIPE message) in most code, and only
switch to SIGPIPE-ignored semantics where the signal has a chance to
cause harm.  Maybe run_command should automatically ignore SIGPIPE
when creating a pipe for the launched command's standard input (with a
flag to ask not to), as a rough heuristic.

There's a subtlety I'm glossing over here, which is that for commands
that produce a lot of output (think: "git fetch --all"), output may
still not the primary goal.  I think even they should not block
SIGPIPE, to follow the principle of least surprise in the following
interaction:

	git fetch --all 2>&1 | less
	... one page later, get bored ...
	q (to quit)

Most Unix programs would be killed by SIGPIPE after such a sequence,
so I would expect git to be, too.

Just my two cents,
Jonathan

Re: SIGPIPE handling (Re: [PATCH v3 0/3])

[Jonathan Nieder]

Jonathan Nieder wrote:

> There's a subtlety I'm glossing over here, which is that for commands
> that produce a lot of output (think: "git fetch --all"), output may
> still not the primary goal.

Gah.  The output that goes to the terminal may not be the primary
goal, I mean (missing "be").

Sorry for the noise.
Jonathan

Re: SIGPIPE handling (Re: [PATCH v3 0/3])

[Jeff King]

On Sat, Feb 18, 2012 at 04:06:07AM -0600, Jonathan Nieder wrote:

> Jeff King wrote:
> 
> > Less robust than that is to just ignore SIGPIPE in most git programs
> > (which don't benefit from it, and where it is only a liability), but
> > then manually enable it for the few that care
> 
> This seems backwards.  Aren't the only places where it is just a
> liability places where git is writing to a pipe that git has created?

Yes. But the problem is that those spots are buried deep within library
code, and are an implementation detail that the caller shouldn't need to
know about.

But more importantly, I see SIGPIPE as an optimization. The program on
the generating side of a pipe _could_ keep making output and dumping it
nowhere. So the optimization is about telling it early that it can stop
bothering. But that optimization is affecting correctness in some cases.
And in cases of correctness versus optimization, it's nice if we can be
correct by default and then optimize in places where it matters most and
where we've verified that correctness isn't harmed.

I also think it's simply easier to list the places that benefit from
SIGPIPE than those that are harmed by it. But if we wanted to go the
other way (allow by default and turn it off in a few programs), at the
very least all of the network programs (receive-pack.  upload-pack, etc)
should start ignoring it completely.

> We could keep the benefits of SIGPIPE (including simpler error
> handling and lack of distracting EPIPE message) in most code, and only
> switch to SIGPIPE-ignored semantics where the signal has a chance to
> cause harm.  Maybe run_command should automatically ignore SIGPIPE
> when creating a pipe for the launched command's standard input (with a
> flag to ask not to), as a rough heuristic.

That's a reasonable heuristic, but not foolproof. If I have a
long-running subprocess with a pipe, then SIGPIPE will be off for a long
time, meaning the code you thought was protected by it is not. In
practice, I think it would work OK with our current code-base, as we
tend to have short-lived subprocesses. You'd have to special case the
starting of the pager, of course, but that's not too hard.

> There's a subtlety I'm glossing over here, which is that for commands
> that produce a lot of output (think: "git fetch --all"), output may
> still not the primary goal.  I think even they should not block
> SIGPIPE, to follow the principle of least surprise in the following
> interaction:
> 
> 	git fetch --all 2>&1 | less
> 	... one page later, get bored ...
> 	q (to quit)
> 
> Most Unix programs would be killed by SIGPIPE after such a sequence,
> so I would expect git to be, too.

But it's quite non-deterministic whether or when git-fetch will be
killed. It may have written all data to the pipe. You may quit, but
fetch still runs for a while before producing output. If you want it
killed, you are much better off to do so by sending it SIGINT.

-Peff

[PATCH v2 2/3] Introduce a performance testing framework

[Thomas Rast]

This introduces a performance testing framework under t/perf/.  It
tries to be as close to the test-lib.sh infrastructure as possible,
and thus should be easy to get used to for git developers.

The following points were considered for the implementation:

1. You usually want to compare arbitrary revisions/build trees against
   each other.  They may not have the performance test under
   consideration, or even the perf-lib.sh infrastructure.

   To cope with this, the 'run' script lets you specify arbitrary
   build dirs and revisions.  It even automatically builds the revisions
   if it doesn't have them at hand yet.

2. Usually you would not want to run all tests.  It would take too
   long anyway.  The 'run' script lets you specify which tests to run;
   or you can also do it manually.  There is a Makefile for
   discoverability and 'make clean', but it is not meant for
   real-world use.

3. Creating test repos from scratch in every test is extremely
   time-consuming, and shipping or downloading such large/weird repos
   is out of the question.

   We leave this decision to the user.  Two different sizes of test
   repos can be configured, and the scripts just copy one or more of
   those (using hardlinks for the object store).  By default it tries
   to use the build tree's git.git repository.

   This is fairly fast and versatile.  Using a copy instead of a clone
   preserves many properties that the user may want to test for, such
   as lots of loose objects, unpacked refs, etc.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 Makefile                        |   22 ++++-
 t/Makefile                      |   43 ++++++++-
 t/perf/.gitignore               |    2 +
 t/perf/Makefile                 |   15 +++
 t/perf/README                   |  146 ++++++++++++++++++++++++++++
 t/perf/aggregate.perl           |  166 ++++++++++++++++++++++++++++++++
 t/perf/min_time.perl            |   21 ++++
 t/perf/p0000-perf-lib-sanity.sh |   41 ++++++++
 t/perf/p0001-rev-list.sh        |   17 ++++
 t/perf/perf-lib.sh              |  198 +++++++++++++++++++++++++++++++++++++++
 t/perf/run                      |   82 ++++++++++++++++
 t/test-lib.sh                   |   22 ++++-
 12 files changed, 770 insertions(+), 5 deletions(-)
 create mode 100644 t/perf/.gitignore
 create mode 100644 t/perf/Makefile
 create mode 100644 t/perf/README
 create mode 100755 t/perf/aggregate.perl
 create mode 100755 t/perf/min_time.perl
 create mode 100755 t/perf/p0000-perf-lib-sanity.sh
 create mode 100755 t/perf/p0001-rev-list.sh
 create mode 100644 t/perf/perf-lib.sh
 create mode 100755 t/perf/run

diff --git a/Makefile b/Makefile
index a0de4e9..1fb1705 100644
--- a/Makefile
+++ b/Makefile
@@ -2361,6 +2361,10 @@ GIT-BUILD-OPTIONS: FORCE
 	@echo USE_LIBPCRE=\''$(subst ','\'',$(subst ','\'',$(USE_LIBPCRE)))'\' >>$@
 	@echo NO_PERL=\''$(subst ','\'',$(subst ','\'',$(NO_PERL)))'\' >>$@
 	@echo NO_PYTHON=\''$(subst ','\'',$(subst ','\'',$(NO_PYTHON)))'\' >>$@
+	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
+ifdef GIT_TEST_OPTS
+	@echo GIT_TEST_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_OPTS)))'\' >>$@
+endif
 ifdef GIT_TEST_CMP
 	@echo GIT_TEST_CMP=\''$(subst ','\'',$(subst ','\'',$(GIT_TEST_CMP)))'\' >>$@
 endif
@@ -2369,7 +2373,18 @@ ifdef GIT_TEST_CMP_USE_COPIED_CONTEXT
 endif
 	@echo NO_GETTEXT=\''$(subst ','\'',$(subst ','\'',$(NO_GETTEXT)))'\' >>$@
 	@echo GETTEXT_POISON=\''$(subst ','\'',$(subst ','\'',$(GETTEXT_POISON)))'\' >>$@
-	@echo NO_UNIX_SOCKETS=\''$(subst ','\'',$(subst ','\'',$(NO_UNIX_SOCKETS)))'\' >>$@
+ifdef GIT_PERF_REPEAT_COUNT
+	@echo GIT_PERF_REPEAT_COUNT=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPEAT_COUNT)))'\' >>$@
+endif
+ifdef GIT_PERF_REPO
+	@echo GIT_PERF_REPO=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_REPO)))'\' >>$@
+endif
+ifdef GIT_PERF_LARGE_REPO
+	@echo GIT_PERF_LARGE_REPO=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_LARGE_REPO)))'\' >>$@
+endif
+ifdef GIT_PERF_MAKE_OPTS
+	@echo GIT_PERF_MAKE_OPTS=\''$(subst ','\'',$(subst ','\'',$(GIT_PERF_MAKE_OPTS)))'\' >>$@
+endif
 
 ### Detect Tck/Tk interpreter path changes
 ifndef NO_TCLTK
@@ -2405,6 +2420,11 @@ export NO_SVN_TESTS
 test: all
 	$(MAKE) -C t/ all
 
+perf: all
+	$(MAKE) -C t/perf/ all
+
+.PHONY: test perf
+
 test-ctype$X: ctype.o
 
 test-date$X: date.o ctype.o
diff --git a/t/Makefile b/t/Makefile
index b5048ab..6091211 100644
--- a/t/Makefile
+++ b/t/Makefile
@@ -73,4 +73,45 @@ gitweb-test:
 valgrind:
 	$(MAKE) GIT_TEST_OPTS="$(GIT_TEST_OPTS) --valgrind"
 
-.PHONY: pre-clean $(T) aggregate-results clean valgrind
+perf:
+	$(MAKE) -C perf/ all
+
+# Smoke testing targets
+-include ../GIT-VERSION-FILE
+uname_S := $(shell sh -c 'uname -s 2>/dev/null || echo unknown')
+uname_M := $(shell sh -c 'uname -m 2>/dev/null || echo unknown')
+
+test-results:
+	mkdir -p test-results
+
+test-results/git-smoke.tar.gz: test-results
+	$(PERL_PATH) ./harness \
+		--archive="test-results/git-smoke.tar.gz" \
+		$(T)
+
+smoke: test-results/git-smoke.tar.gz
+
+SMOKE_UPLOAD_FLAGS =
+ifdef SMOKE_USERNAME
+	SMOKE_UPLOAD_FLAGS += -F username="$(SMOKE_USERNAME)" -F password="$(SMOKE_PASSWORD)"
+endif
+ifdef SMOKE_COMMENT
+	SMOKE_UPLOAD_FLAGS += -F comments="$(SMOKE_COMMENT)"
+endif
+ifdef SMOKE_TAGS
+	SMOKE_UPLOAD_FLAGS += -F tags="$(SMOKE_TAGS)"
+endif
+
+smoke_report: smoke
+	curl \
+		-H "Expect: " \
+		-F project=Git \
+		-F architecture="$(uname_M)" \
+		-F platform="$(uname_S)" \
+		-F revision="$(GIT_VERSION)" \
+		-F report_file=@test-results/git-smoke.tar.gz \
+		$(SMOKE_UPLOAD_FLAGS) \
+		http://smoke.git.nix.is/app/projects/process_add_report/1 \
+	| grep -v ^Redirecting
+
+.PHONY: pre-clean $(T) aggregate-results clean valgrind perf
diff --git a/t/perf/.gitignore b/t/perf/.gitignore
new file mode 100644
index 0000000..50f5cc1
--- /dev/null
+++ b/t/perf/.gitignore
@@ -0,0 +1,2 @@
+build/
+test-results/
diff --git a/t/perf/Makefile b/t/perf/Makefile
new file mode 100644
index 0000000..8c47155
--- /dev/null
+++ b/t/perf/Makefile
@@ -0,0 +1,15 @@
+-include ../../config.mak
+export GIT_TEST_OPTIONS
+
+all: perf
+
+perf: pre-clean
+	./run
+
+pre-clean:
+	rm -rf test-results
+
+clean:
+	rm -rf build "trash directory".* test-results
+
+.PHONY: all perf pre-clean clean
diff --git a/t/perf/README b/t/perf/README
new file mode 100644
index 0000000..fdf974a
--- /dev/null
+++ b/t/perf/README
@@ -0,0 +1,146 @@
+Git performance tests
+=====================
+
+This directory holds performance testing scripts for git tools.  The
+first part of this document describes the various ways in which you
+can run them.
+
+When fixing the tools or adding enhancements, you are strongly
+encouraged to add tests in this directory to cover what you are
+trying to fix or enhance.  The later part of this short document
+describes how your test scripts should be organized.
+
+
+Running Tests
+-------------
+
+The easiest way to run tests is to say "make".  This runs all
+the tests on the current git repository.
+
+    === Running 2 tests in this tree ===
+    [...]
+    Test                                     this tree       
+    ---------------------------------------------------------
+    0001.1: rev-list --all                   0.54(0.51+0.02) 
+    0001.2: rev-list --all --objects         6.14(5.99+0.11) 
+    7810.1: grep worktree, cheap regex       0.16(0.16+0.35) 
+    7810.2: grep worktree, expensive regex   7.90(29.75+0.37)
+    7810.3: grep --cached, cheap regex       3.07(3.02+0.25) 
+    7810.4: grep --cached, expensive regex   9.39(30.57+0.24)
+
+You can compare multiple repositories and even git revisions with the
+'run' script:
+
+    $ ./run . origin/next /path/to/git-tree p0001-rev-list.sh
+
+where . stands for the current git tree.  The full invocation is
+
+    ./run [<revision|directory>...] [--] [<test-script>...]
+
+A '.' argument is implied if you do not pass any other
+revisions/directories.
+
+You can also manually test this or another git build tree, and then
+call the aggregation script to summarize the results:
+
+    $ ./p0001-rev-list.sh
+    [...]
+    $ GIT_BUILD_DIR=/path/to/other/git ./p0001-rev-list.sh
+    [...]
+    $ ./aggregate.perl . /path/to/other/git ./p0001-rev-list.sh
+
+aggregate.perl has the same invocation as 'run', it just does not run
+anything beforehand.
+
+You can set the following variables (also in your config.mak):
+
+    GIT_PERF_REPEAT_COUNT
+	Number of times a test should be repeated for best-of-N
+	measurements.  Defaults to 5.
+
+    GIT_PERF_MAKE_OPTS
+	Options to use when automatically building a git tree for
+	performance testing.  E.g., -j6 would be useful.
+
+    GIT_PERF_REPO
+    GIT_PERF_LARGE_REPO
+	Repositories to copy for the performance tests.  The normal
+	repo should be at least git.git size.  The large repo should
+	probably be about linux-2.6.git size for optimal results.
+	Both default to the git.git you are running from.
+
+You can also pass the options taken by ordinary git tests; the most
+useful one is:
+
+--root=<directory>::
+	Create "trash" directories used to store all temporary data during
+	testing under <directory>, instead of the t/ directory.
+	Using this option with a RAM-based filesystem (such as tmpfs)
+	can massively speed up the test suite.
+
+
+Naming Tests
+------------
+
+The performance test files are named as:
+
+	pNNNN-commandname-details.sh
+
+where N is a decimal digit.  The same conventions for choosing NNNN as
+for normal tests apply.
+
+
+Writing Tests
+-------------
+
+The perf script starts much like a normal test script, except it
+sources perf-lib.sh:
+
+	#!/bin/sh
+	#
+	# Copyright (c) 2005 Junio C Hamano
+	#
+
+	test_description='xxx performance test'
+	. ./perf-lib.sh
+
+After that you will want to use some of the following:
+
+	test_perf_default_repo  # sets up a "normal" repository
+	test_perf_large_repo    # sets up a "large" repository
+
+	test_perf_default_repo sub  # ditto, in a subdir "sub"
+
+        test_checkout_worktree  # if you need the worktree too
+
+At least one of the first two is required!
+
+You can use test_expect_success as usual.  For actual performance
+tests, use
+
+	test_perf 'descriptive string' '
+		command1 &&
+		command2
+	'
+
+test_perf spawns a subshell, for lack of better options.  This means
+that
+
+* you _must_ export all variables that you need in the subshell
+
+* you _must_ flag all variables that you want to persist from the
+  subshell with 'test_export':
+
+	test_perf 'descriptive string' '
+		foo=$(git rev-parse HEAD) &&
+		test_export foo
+	'
+
+  The so-exported variables are automatically marked for export in the
+  shell executing the perf test.  For your convenience, test_export is
+  the same as export in the main shell.
+
+  This feature relies on a bit of magic using 'set' and 'source'.
+  While we have tried to make sure that it can cope with embedded
+  whitespace and other special characters, it will not work with
+  multi-line data.
diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl
new file mode 100755
index 0000000..15f7fc1
--- /dev/null
+++ b/t/perf/aggregate.perl
@@ -0,0 +1,166 @@
+#!/usr/bin/perl
+
+use strict;
+use warnings;
+use Git;
+
+sub get_times {
+	my $name = shift;
+	open my $fh, "<", $name or return undef;
+	my $line = <$fh>;
+	return undef if not defined $line;
+	close $fh or die "cannot close $name: $!";
+	$line =~ /^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/
+		or die "bad input line: $line";
+	my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3;
+	return ($rt, $4, $5);
+}
+
+sub format_times {
+	my ($r, $u, $s, $firstr) = @_;
+	if (!defined $r) {
+		return "<missing>";
+	}
+	my $out = sprintf "%.2f(%.2f+%.2f)", $r, $u, $s;
+	if (defined $firstr) {
+		if ($firstr > 0) {
+			$out .= sprintf " %+.1f%%", 100.0*($r-$firstr)/$firstr;
+		} elsif ($r == 0) {
+			$out .= " =";
+		} else {
+			$out .= " +inf";
+		}
+	}
+	return $out;
+}
+
+my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests);
+while (scalar @ARGV) {
+	my $arg = $ARGV[0];
+	my $dir;
+	last if -f $arg or $arg eq "--";
+	if (! -d $arg) {
+		my $rev = Git::command_oneline(qw(rev-parse --verify), $arg);
+		$dir = "build/".$rev;
+	} else {
+		$arg =~ s{/*$}{};
+		$dir = $arg;
+		$dirabbrevs{$dir} = $dir;
+	}
+	push @dirs, $dir;
+	$dirnames{$dir} = $arg;
+	my $prefix = $dir;
+	$prefix =~ tr/^a-zA-Z0-9/_/c;
+	$prefixes{$dir} = $prefix . '.';
+	shift @ARGV;
+}
+
+if (not @dirs) {
+	@dirs = ('.');
+}
+$dirnames{'.'} = $dirabbrevs{'.'} = "this tree";
+$prefixes{'.'} = '';
+
+shift @ARGV if scalar @ARGV and $ARGV[0] eq "--";
+
+@tests = @ARGV;
+if (not @tests) {
+	@tests = glob "p????-*.sh";
+}
+
+my @subtests;
+my %shorttests;
+for my $t (@tests) {
+	$t =~ s{(?:.*/)?(p(\d+)-[^/]+)\.sh$}{$1} or die "bad test name: $t";
+	my $n = $2;
+	my $fname = "test-results/$t.subtests";
+	open my $fp, "<", $fname or die "cannot open $fname: $!";
+	for (<$fp>) {
+		chomp;
+		/^(\d+)$/ or die "malformed subtest line: $_";
+		push @subtests, "$t.$1";
+		$shorttests{"$t.$1"} = "$n.$1";
+	}
+	close $fp or die "cannot close $fname: $!";
+}
+
+sub read_descr {
+	my $name = shift;
+	open my $fh, "<", $name or return "<error reading description>";
+	my $line = <$fh>;
+	close $fh or die "cannot close $name";
+	chomp $line;
+	return $line;
+}
+
+my %descrs;
+my $descrlen = 4; # "Test"
+for my $t (@subtests) {
+	$descrs{$t} = $shorttests{$t}.": ".read_descr("test-results/$t.descr");
+	$descrlen = length $descrs{$t} if length $descrs{$t}>$descrlen;
+}
+
+sub have_duplicate {
+	my %seen;
+	for (@_) {
+		return 1 if exists $seen{$_};
+		$seen{$_} = 1;
+	}
+	return 0;
+}
+sub have_slash {
+	for (@_) {
+		return 1 if m{/};
+	}
+	return 0;
+}
+
+my %newdirabbrevs = %dirabbrevs;
+while (!have_duplicate(values %newdirabbrevs)) {
+	%dirabbrevs = %newdirabbrevs;
+	last if !have_slash(values %dirabbrevs);
+	%newdirabbrevs = %dirabbrevs;
+	for (values %newdirabbrevs) {
+		s{^[^/]*/}{};
+	}
+}
+
+my %times;
+my @colwidth = ((0)x@dirs);
+for my $i (0..$#dirs) {
+	my $d = $dirs[$i];
+	my $w = length (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
+	$colwidth[$i] = $w if $w > $colwidth[$i];
+}
+for my $t (@subtests) {
+	my $firstr;
+	for my $i (0..$#dirs) {
+		my $d = $dirs[$i];
+		$times{$prefixes{$d}.$t} = [get_times("test-results/$prefixes{$d}$t.times")];
+		my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+		my $w = length format_times($r,$u,$s,$firstr);
+		$colwidth[$i] = $w if $w > $colwidth[$i];
+		$firstr = $r unless defined $firstr;
+	}
+}
+my $totalwidth = 3*@dirs+$descrlen;
+$totalwidth += $_ for (@colwidth);
+
+printf "%-${descrlen}s", "Test";
+for my $i (0..$#dirs) {
+	my $d = $dirs[$i];
+	printf "   %-$colwidth[$i]s", (exists $dirabbrevs{$d} ? $dirabbrevs{$d} : $dirnames{$d});
+}
+print "\n";
+print "-"x$totalwidth, "\n";
+for my $t (@subtests) {
+	printf "%-${descrlen}s", $descrs{$t};
+	my $firstr;
+	for my $i (0..$#dirs) {
+		my $d = $dirs[$i];
+		my ($r,$u,$s) = @{$times{$prefixes{$d}.$t}};
+		printf "   %-$colwidth[$i]s", format_times($r,$u,$s,$firstr);
+		$firstr = $r unless defined $firstr;
+	}
+	print "\n";
+}
diff --git a/t/perf/min_time.perl b/t/perf/min_time.perl
new file mode 100755
index 0000000..c1a2717
--- /dev/null
+++ b/t/perf/min_time.perl
@@ -0,0 +1,21 @@
+#!/usr/bin/perl
+
+my $minrt = 1e100;
+my $min;
+
+while (<>) {
+	# [h:]m:s.xx U.xx S.xx
+	/^(?:(\d+):)?(\d+):(\d+(?:\.\d+)?) (\d+(?:\.\d+)?) (\d+(?:\.\d+)?)$/
+		or die "bad input line: $_";
+	my $rt = ((defined $1 ? $1 : 0.0)*60+$2)*60+$3;
+	if ($rt < $minrt) {
+		$min = $_;
+		$minrt = $rt;
+	}
+}
+
+if (!defined $min) {
+	die "no input found";
+}
+
+print $min;
diff --git a/t/perf/p0000-perf-lib-sanity.sh b/t/perf/p0000-perf-lib-sanity.sh
new file mode 100755
index 0000000..2ca4aac
--- /dev/null
+++ b/t/perf/p0000-perf-lib-sanity.sh
@@ -0,0 +1,41 @@
+#!/bin/sh
+
+test_description='Tests whether perf-lib facilities work'
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+test_perf 'test_perf_default_repo works' '
+	foo=$(git rev-parse HEAD) &&
+	test_export foo
+'
+
+test_checkout_worktree
+
+test_perf 'test_checkout_worktree works' '
+	wt=$(find . | wc -l) &&
+	idx=$(git ls-files | wc -l) &&
+	test $wt -gt $idx
+'
+
+baz=baz
+test_export baz
+
+test_expect_success 'test_export works' '
+	echo "$foo" &&
+	test "$foo" = "$(git rev-parse HEAD)" &&
+	echo "$baz" &&
+	test "$baz" = baz
+'
+
+test_perf 'export a weird var' '
+	bar="weird # variable" &&
+	test_export bar
+'
+
+test_expect_success 'test_export works with weird vars' '
+	echo "$bar" &&
+	test "$bar" = "weird # variable"
+'
+
+test_done
diff --git a/t/perf/p0001-rev-list.sh b/t/perf/p0001-rev-list.sh
new file mode 100755
index 0000000..4f71a63
--- /dev/null
+++ b/t/perf/p0001-rev-list.sh
@@ -0,0 +1,17 @@
+#!/bin/sh
+
+test_description="Tests history walking performance"
+
+. ./perf-lib.sh
+
+test_perf_default_repo
+
+test_perf 'rev-list --all' '
+	git rev-list --all >/dev/null
+'
+
+test_perf 'rev-list --all --objects' '
+	git rev-list --all --objects >/dev/null
+'
+
+test_done
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
new file mode 100644
index 0000000..07e9b09
--- /dev/null
+++ b/t/perf/perf-lib.sh
@@ -0,0 +1,198 @@
+#!/bin/bash
+#
+# Copyright (c) 2011 Thomas Rast
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see http://www.gnu.org/licenses/ .
+
+# do the --tee work early; it otherwise confuses our careful
+# GIT_BUILD_DIR mangling
+case "$GIT_TEST_TEE_STARTED, $* " in
+done,*)
+	# do not redirect again
+	;;
+*' --tee '*|*' --va'*)
+	mkdir -p test-results
+	BASE=test-results/$(basename "$0" .sh)
+	(GIT_TEST_TEE_STARTED=done ${SHELL-sh} "$0" "$@" 2>&1;
+	 echo $? > $BASE.exit) | tee $BASE.out
+	test "$(cat $BASE.exit)" = 0
+	exit
+	;;
+esac
+
+TEST_DIRECTORY=$(pwd)/..
+TEST_OUTPUT_DIRECTORY=$(pwd)
+if test -z "$GIT_TEST_INSTALLED"; then
+	perf_results_prefix=
+else
+	perf_results_prefix=$(printf "%s" "${GIT_TEST_INSTALLED%/bin-wrappers}" | tr -c "[a-zA-Z0-9]" "[_*]")"."
+	# make the tested dir absolute
+	GIT_TEST_INSTALLED=$(cd "$GIT_TEST_INSTALLED" && pwd)
+fi
+
+TEST_NO_CREATE_REPO=t
+
+. ../test-lib.sh
+
+perf_results_dir=$TEST_OUTPUT_DIRECTORY/test-results
+mkdir -p "$perf_results_dir"
+rm -f "$perf_results_dir"/$(basename "$0" .sh).subtests
+
+if test -z "$GIT_PERF_REPEAT_COUNT"; then
+	GIT_PERF_REPEAT_COUNT=3
+fi
+die_if_build_dir_not_repo () {
+	if ! ( cd "$TEST_DIRECTORY/.." &&
+		    git rev-parse --build-dir >/dev/null 2>&1 ); then
+		error "No $1 defined, and your build directory is not a repo"
+	fi
+}
+
+if test -z "$GIT_PERF_REPO"; then
+	die_if_build_dir_not_repo '$GIT_PERF_REPO'
+	GIT_PERF_REPO=$TEST_DIRECTORY/..
+fi
+if test -z "$GIT_PERF_LARGE_REPO"; then
+	die_if_build_dir_not_repo '$GIT_PERF_LARGE_REPO'
+	GIT_PERF_LARGE_REPO=$TEST_DIRECTORY/..
+fi
+
+test_perf_create_repo_from () {
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 parameters to test-create-repo"
+	repo="$1"
+	source="$2"
+	source_git=$source/$(cd "$source" && git rev-parse --git-dir)
+	mkdir -p "$repo/.git"
+	(
+		cd "$repo/.git" &&
+		{ cp -Rl "$source_git/objects" . 2>/dev/null ||
+			cp -R "$source_git/objects" .; } &&
+		for stuff in "$source_git"/*; do
+			case "$stuff" in
+				*/objects|*/hooks|*/config)
+					;;
+				*)
+					cp -R "$stuff" . || break
+					;;
+			esac
+		done &&
+		cd .. &&
+		git init -q &&
+		mv .git/hooks .git/hooks-disabled 2>/dev/null
+	) || error "failed to copy repository '$source' to '$repo'"
+}
+
+# call at least one of these to establish an appropriately-sized repository
+test_perf_default_repo () {
+	test_perf_create_repo_from "${1:-$TRASH_DIRECTORY}" "$GIT_PERF_REPO"
+}
+test_perf_large_repo () {
+	if test "$GIT_PERF_LARGE_REPO" = "$GIT_BUILD_DIR"; then
+		echo "warning: \$GIT_PERF_LARGE_REPO is \$GIT_BUILD_DIR." >&2
+		echo "warning: This will work, but may not be a sufficiently large repo" >&2
+		echo "warning: for representative measurements." >&2
+	fi
+	test_perf_create_repo_from "${1:-$TRASH_DIRECTORY}" "$GIT_PERF_LARGE_REPO"
+}
+test_checkout_worktree () {
+	git checkout-index -u -a ||
+	error "git checkout-index failed"
+}
+
+# Performance tests should never fail.  If they do, stop immediately
+immediate=t
+
+test_run_perf_ () {
+	test_cleanup=:
+	test_export_="test_cleanup"
+	export test_cleanup test_export_
+	/usr/bin/time -f "%E %U %S" -o test_time.$i "$SHELL" -c '
+. '"$TEST_DIRECTORY"/../test-lib-functions.sh'
+test_export () {
+	[ $# != 0 ] || return 0
+	test_export_="$test_export_\\|$1"
+	shift
+	test_export "$@"
+}
+'"$1"'
+ret=$?
+set | sed -n "s'"/'/'\\\\''/g"';s/^\\($test_export_\\)/export '"'&'"'/p" >test_vars
+exit $ret' >&3 2>&4
+	eval_ret=$?
+
+	if test $eval_ret = 0 || test -n "$expecting_failure"
+	then
+		test_eval_ "$test_cleanup"
+		source ./test_vars || error "failed to load updated environment"
+	fi
+	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"; then
+		echo ""
+	fi
+	return "$eval_ret"
+}
+
+
+test_perf () {
+	test "$#" = 3 && { test_prereq=$1; shift; } || test_prereq=
+	test "$#" = 2 ||
+	error "bug in the test script: not 2 or 3 parameters to test-expect-success"
+	export test_prereq
+	if ! test_skip "$@"
+	then
+		base=$(basename "$0" .sh)
+		echo "$test_count" >>"$perf_results_dir"/$base.subtests
+		echo "$1" >"$perf_results_dir"/$base.$test_count.descr
+		if test -z "$verbose"; then
+			echo -n "perf $test_count - $1:"
+		else
+			echo "perf $test_count - $1:"
+		fi
+		for i in $(seq 1 $GIT_PERF_REPEAT_COUNT); do
+			say >&3 "running: $2"
+			if test_run_perf_ "$2"
+			then
+				if test -z "$verbose"; then
+					echo -n " $i"
+				else
+					echo "* timing run $i/$GIT_PERF_REPEAT_COUNT:"
+				fi
+			else
+				test -z "$verbose" && echo
+				test_failure_ "$@"
+				break
+			fi
+		done
+		if test -z "$verbose"; then
+			echo " ok"
+		else
+			test_ok_ "$1"
+		fi
+		base="$perf_results_dir"/"$perf_results_prefix$(basename "$0" .sh)"."$test_count"
+		"$TEST_DIRECTORY"/perf/min_time.perl test_time.* >"$base".times
+	fi
+	echo >&3 ""
+}
+
+# We extend test_done to print timings at the end (./run disables this
+# and does it after running everything)
+test_at_end_hook_ () {
+	if test -z "$GIT_PERF_AGGREGATING_LATER"; then
+		( cd "$TEST_DIRECTORY"/perf && ./aggregate.perl $(basename "$0") )
+	fi
+}
+
+test_export () {
+	export "$@"
+}
diff --git a/t/perf/run b/t/perf/run
new file mode 100755
index 0000000..cfd7012
--- /dev/null
+++ b/t/perf/run
@@ -0,0 +1,82 @@
+#!/bin/sh
+
+case "$1" in
+	--help)
+		echo "usage: $0 [other_git_tree...] [--] [test_scripts]"
+		exit 0
+		;;
+esac
+
+die () {
+	echo >&2 "error: $*"
+	exit 1
+}
+
+run_one_dir () {
+	if test $# -eq 0; then
+		set -- p????-*.sh
+	fi
+	echo "=== Running $# tests in ${GIT_TEST_INSTALLED:-this tree} ==="
+	for t in "$@"; do
+		./$t $GIT_TEST_OPTS
+	done
+}
+
+unpack_git_rev () {
+	rev=$1
+	mkdir -p build/$rev
+	(cd "$(git rev-parse --show-cdup)" && git archive --format=tar $rev) |
+	(cd build/$rev && tar x)
+}
+build_git_rev () {
+	rev=$1
+	cp ../../config.mak build/$rev/config.mak
+	(cd build/$rev && make $GIT_PERF_MAKE_OPTS) ||
+	die "failed to build revision '$mydir'"
+}
+
+run_dirs_helper () {
+	mydir=${1%/}
+	shift
+	while test $# -gt 0 -a "$1" != -- -a ! -f "$1"; do
+		shift
+	done
+	if test $# -gt 0 -a "$1" = --; then
+		shift
+	fi
+	if [ ! -d "$mydir" ]; then
+		rev=$(git rev-parse --verify "$mydir" 2>/dev/null) ||
+		die "'$mydir' is neither a directory nor a valid revision"
+		if [ ! -d build/$rev ]; then
+			unpack_git_rev $rev
+		fi
+		build_git_rev $rev
+		mydir=build/$rev
+	fi
+	if test "$mydir" = .; then
+		unset GIT_TEST_INSTALLED
+	else
+		GIT_TEST_INSTALLED="$mydir/bin-wrappers"
+		export GIT_TEST_INSTALLED
+	fi
+	run_one_dir "$@"
+}
+
+run_dirs () {
+	while test $# -gt 0 -a "$1" != -- -a ! -f "$1"; do
+		run_dirs_helper "$@"
+		shift
+	done
+}
+
+GIT_PERF_AGGREGATING_LATER=t
+export GIT_PERF_AGGREGATING_LATER
+
+cd "$(dirname $0)"
+. ../../GIT-BUILD-OPTIONS
+
+if test $# = 0 -o "$1" = -- -o -f "$1"; then
+	set -- . "$@"
+fi
+run_dirs "$@"
+./aggregate.perl "$@"
diff --git a/t/test-lib.sh b/t/test-lib.sh
index ec70ef2..d75766a 100644
--- a/t/test-lib.sh
+++ b/t/test-lib.sh
@@ -55,6 +55,7 @@ unset $(perl -e '
 		.*_TEST
 		PROVE
 		VALGRIND
+		PERF_AGGREGATING_LATER
 	));
 	my @vars = grep(/^GIT_/ && !/^GIT_($ok)/o, @env);
 	print join("\n", @vars);
@@ -315,13 +316,16 @@ test_skip () {
 	esac
 }
 
+# stub; perf-lib overrides it
+test_at_end_hook_ () {
+	:
 }
 
 test_done () {
 	GIT_EXIT_OK=t
 
 	if test -z "$HARNESS_ACTIVE"; then
-		test_results_dir="$TEST_DIRECTORY/test-results"
+		test_results_dir="$TEST_OUTPUT_DIRECTORY/test-results"
 		mkdir -p "$test_results_dir"
 		test_results_path="$test_results_dir/${0%.sh}-$$.counts"
 
@@ -360,6 +364,8 @@ test_done () {
 		cd "$(dirname "$remove_trash")" &&
 		rm -rf "$(basename "$remove_trash")"
 
+		test_at_end_hook_
+
 		exit 0 ;;
 
 	*)
@@ -382,6 +388,12 @@ then
 	# itself.
 	TEST_DIRECTORY=$(pwd)
 fi
+if test -z "$TEST_OUTPUT_DIRECTORY"
+then
+	# Similarly, override this to store the test-results subdir
+	# elsewhere
+	TEST_OUTPUT_DIRECTORY=$TEST_DIRECTORY
+fi
 GIT_BUILD_DIR="$TEST_DIRECTORY"/..
 
 if test -n "$valgrind"
@@ -517,7 +529,7 @@ test="trash directory.$(basename "$0" .sh)"
 test -n "$root" && test="$root/$test"
 case "$test" in
 /*) TRASH_DIRECTORY="$test" ;;
- *) TRASH_DIRECTORY="$TEST_DIRECTORY/$test" ;;
+ *) TRASH_DIRECTORY="$TEST_OUTPUT_DIRECTORY/$test" ;;
 esac
 test ! -z "$debug" || remove_trash=$TRASH_DIRECTORY
 rm -fr "$test" || {
@@ -529,7 +541,11 @@ rm -fr "$test" || {
 HOME="$TRASH_DIRECTORY"
 export HOME
 
-test_create_repo "$test"
+if test -z "$TEST_NO_CREATE_REPO"; then
+	test_create_repo "$test"
+else
+	mkdir -p "$test"
+fi
 # Use -P to resolve symlinks in our working directory so that the cwd
 # in subprocesses like git equals our $PWD (for pathname comparisons).
 cd -P "$test" || exit 1
-- 
1.7.9.1.334.gd1409

Re: [PATCH v2 2/3] Introduce a performance testing framework

[Jeff King]

On Thu, Feb 16, 2012 at 10:41:14PM +0100, Thomas Rast wrote:

> +	if test $eval_ret = 0 || test -n "$expecting_failure"
> +	then
> +		test_eval_ "$test_cleanup"
> +		source ./test_vars || error "failed to load updated environment"
> +	fi

"source" is a bash-ism (actually, it is a csh-ism as far as I know, but
perhaps it predates even that). The correct POSIX spelling is ".".

After tweaking this line, the suite seems to run fine for me with dash
as /bin/sh. Patch is below for your convenience.

-Peff

---
diff --git a/t/perf/perf-lib.sh b/t/perf/perf-lib.sh
index 07e9b09..629d7d5 100644
--- a/t/perf/perf-lib.sh
+++ b/t/perf/perf-lib.sh
@@ -135,7 +135,7 @@ exit $ret' >&3 2>&4
 	if test $eval_ret = 0 || test -n "$expecting_failure"
 	then
 		test_eval_ "$test_cleanup"
-		source ./test_vars || error "failed to load updated environment"
+		. ./test_vars || error "failed to load updated environment"
 	fi
 	if test "$verbose" = "t" && test -n "$HARNESS_ACTIVE"; then
 		echo ""

[PATCH v2 3/3] Add a performance test for git-grep

[Thomas Rast]

The only catch is that we don't really know what our repo contains, so
we have to ignore any possible "not found" status from git-grep.

Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
 t/perf/p7810-grep.sh |   23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)
 create mode 100755 t/perf/p7810-grep.sh

diff --git a/t/perf/p7810-grep.sh b/t/perf/p7810-grep.sh
new file mode 100755
index 0000000..9f4ade6
--- /dev/null
+++ b/t/perf/p7810-grep.sh
@@ -0,0 +1,23 @@
+#!/bin/sh
+
+test_description="git-grep performance in various modes"
+
+. ./perf-lib.sh
+
+test_perf_large_repo
+test_checkout_worktree
+
+test_perf 'grep worktree, cheap regex' '
+	git grep some_nonexistent_string || :
+'
+test_perf 'grep worktree, expensive regex' '
+	git grep "^.* *some_nonexistent_string$" || :
+'
+test_perf 'grep --cached, cheap regex' '
+	git grep --cached some_nonexistent_string || :
+'
+test_perf 'grep --cached, expensive regex' '
+	git grep --cached "^.* *some_nonexistent_string$" || :
+'
+
+test_done
-- 
1.7.9.1.334.gd1409

[PATCH v2 0/2] refactoring branch colorization to ref-filter

[nbelakovski]

From: Nickolai Belakovski <nbelakovski@gmail.com>

Finally found some time to follow up on this :)

I decided to take the approach suggested by Peff for simplicity. I have
another version which uses a hash map to store *all* of the information
returned by get_worktrees, information which can then be accessed in
the fashion %(workree:path) or %(worktree:is_detached), but it seemed
like a lot of code to add and there was no use case to justify the
addition of a hash map at this time. If there's interest though, I can
make a separate patch after this one to introduce those changes. They
build directly off of the changes introduced here.

I've split this work into two commits since the items are logically
separate.

CI results: https://travis-ci.org/nbelakovski/git/builds/453723727

Nickolai Belakovski (2):
  ref-filter: add worktree atom
  branch: Mark and colorize a branch differently if it is checked out in
    a linked worktree

 builtin/branch.c               | 22 +++++++++++++---------
 color.h                        | 18 ++++++++++++++++++
 ref-filter.c                   | 31 +++++++++++++++++++++++++++++++
 t/t3200-branch.sh              |  8 ++++----
 t/t3203-branch-output.sh       | 21 +++++++++++++++++++++
 t/t6302-for-each-ref-filter.sh | 15 +++++++++++++++
 t/test-lib-functions.sh        |  6 ++++++
 7 files changed, 108 insertions(+), 13 deletions(-)

-- 
2.14.2

[PATCH v3 0/3]

[nbelakovski]

From: Nickolai Belakovski <nbelakovski@gmail.com>

Finally got around to submitting latest changes.

I think this addresses all the feedback

The atom now returns the worktree path instead of '+'

I stuck to cyan for the coloring, since it seemed most popular

I added one more change to display the worktree path in cyan for git branch -vvv
Not sure if it's in the best place, but it seemed like it would be nice to add
the path in the same color so that there's some visibility as to why a particular
branch is colored in cyan. If it proves to be controversial, I wouldn't want it to
hold up this series, we can skip it and I can move discussion to a separate thread
(or just forget it, as the case may be)

Travis CI results: https://travis-ci.org/nbelakovski/git/builds/468569102

Nickolai Belakovski (3):
  ref-filter: add worktreepath atom
  branch: Mark and color a branch differently if it is checked out in a
    linked worktree
  branch: Add an extra verbose output displaying worktree path for refs
    checked out in a linked worktree

 Documentation/git-for-each-ref.txt |  4 +++
 builtin/branch.c                   | 16 ++++++---
 ref-filter.c                       | 70 ++++++++++++++++++++++++++++++++++++++
 t/t3200-branch.sh                  |  8 ++---
 t/t3203-branch-output.sh           | 21 ++++++++++++
 t/t6302-for-each-ref-filter.sh     | 15 ++++++++
 6 files changed, 126 insertions(+), 8 deletions(-)

-- 
2.14.2

Re: [PATCH v3 0/3]

[Jeff King]

On Sun, Dec 16, 2018 at 01:57:56PM -0800, nbelakovski@gmail.com wrote:

> Finally got around to submitting latest changes.
> 
> I think this addresses all the feedback
> 
> The atom now returns the worktree path instead of '+'

Thanks, I think patch 1 is definitely going in the right direction.
There are a few issues I found in its implementation, but they should be
easy to fix (and won't affect the other patches).

I don't really have a strong opinion on the behavior of the other
patches.

-Peff

[PATCH v2 0/3] bundle.c: remove "ref_list" in favor of string-list.c API

[Ævar Arnfjörð Bjarmason]

This v2 addresses an omission noted by Andrei Rybak[1]. I didn't
factor out the "name" into a variable in 2/3 for ease of reading
3/3. That's now done.

1. https://lore.kernel.org/git/23c4ce5f-7769-1d2c-3a97-ac9733f73a25@gmail.com/

Ævar Arnfjörð Bjarmason (3):
  bundle cmd: stop leaking memory from parse_options_cmd_bundle()
  bundle.c: use a temporary variable for OIDs and names
  bundle: remove "ref_list" in favor of string-list.c API

 builtin/bundle.c | 91 ++++++++++++++++++++++++++++++++----------------
 bundle.c         | 74 ++++++++++++++++++++++-----------------
 bundle.h         | 20 +++++------
 transport.c      | 12 +++++--
 4 files changed, 122 insertions(+), 75 deletions(-)

Range-diff against v1:
1:  f4191088ac3 = 1:  932c0883ce0 bundle cmd: stop leaking memory from parse_options_cmd_bundle()
2:  f297fd0432a ! 2:  7e0d57951e5 bundle.c: use a temporary variable for OIDs and names
    @@ bundle.c: int verify_bundle(struct repository *r,
      	for (i = 0; i < p->nr; i++) {
      		struct ref_list_entry *e = p->list + i;
     -		struct object *o = parse_object(r, &e->oid);
    ++		const char *name = e->name;
     +		struct object_id *oid = &e->oid;
     +		struct object *o = parse_object(r, oid);
      		if (o) {
      			o->flags |= PREREQ_MARK;
    - 			add_pending_object(&revs, o, e->name);
    -@@ bundle.c: int verify_bundle(struct repository *r,
    +-			add_pending_object(&revs, o, e->name);
    ++			add_pending_object(&revs, o, name);
    + 			continue;
      		}
      		if (++ret == 1)
      			error("%s", message);
     -		error("%s %s", oid_to_hex(&e->oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->name);
    ++		error("%s %s", oid_to_hex(oid), name);
      	}
      	if (revs.pending.nr != p->nr)
      		return ret;
    @@ bundle.c: int verify_bundle(struct repository *r,
      	for (i = 0; i < p->nr; i++) {
      		struct ref_list_entry *e = p->list + i;
     -		struct object *o = parse_object(r, &e->oid);
    ++		const char *name = e->name;
     +		struct object_id *oid = &e->oid;
     +		struct object *o = parse_object(r, oid);
      		assert(o); /* otherwise we'd have returned early */
    @@ bundle.c: int verify_bundle(struct repository *r,
      		if (++ret == 1)
      			error("%s", message);
     -		error("%s %s", oid_to_hex(&e->oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->name);
    ++		error("%s %s", oid_to_hex(oid), name);
      	}
      
      	/* Clean up objects used, as they will be reused. */
    @@ bundle.c: int verify_bundle(struct repository *r,
     
      ## transport.c ##
     @@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport,
    + 
      	for (i = 0; i < data->header.references.nr; i++) {
      		struct ref_list_entry *e = data->header.references.list + i;
    - 		struct ref *ref = alloc_ref(e->name);
    +-		struct ref *ref = alloc_ref(e->name);
     -		oidcpy(&ref->old_oid, &e->oid);
    ++		const char *name = e->name;
    ++		struct ref *ref = alloc_ref(name);
     +		struct object_id *oid = &e->oid;
     +		oidcpy(&ref->old_oid, oid);
      		ref->next = result;
3:  887313d3b02 ! 3:  9d9cb5aaf9e bundle: remove "ref_list" in favor of string-list.c API
    @@ bundle.c: int verify_bundle(struct repository *r,
      	repo_init_revisions(r, &revs, NULL);
      	for (i = 0; i < p->nr; i++) {
     -		struct ref_list_entry *e = p->list + i;
    +-		const char *name = e->name;
     -		struct object_id *oid = &e->oid;
     +		struct string_list_item *e = p->items + i;
    ++		const char *name = e->string;
     +		struct object_id *oid = e->util;
      		struct object *o = parse_object(r, oid);
      		if (o) {
      			o->flags |= PREREQ_MARK;
    --			add_pending_object(&revs, o, e->name);
    -+			add_pending_object(&revs, o, e->string);
    - 			continue;
    - 		}
    - 		if (++ret == 1)
    - 			error("%s", message);
    --		error("%s %s", oid_to_hex(oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->string);
    - 	}
    - 	if (revs.pending.nr != p->nr)
    - 		return ret;
     @@ bundle.c: int verify_bundle(struct repository *r,
      			i--;
      
      	for (i = 0; i < p->nr; i++) {
     -		struct ref_list_entry *e = p->list + i;
    +-		const char *name = e->name;
     -		struct object_id *oid = &e->oid;
     +		struct string_list_item *e = p->items + i;
    ++		const char *name = e->string;
     +		const struct object_id *oid = e->util;
      		struct object *o = parse_object(r, oid);
      		assert(o); /* otherwise we'd have returned early */
      		if (o->flags & SHOWN)
    - 			continue;
    - 		if (++ret == 1)
    - 			error("%s", message);
    --		error("%s %s", oid_to_hex(oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->string);
    - 	}
    +@@ bundle.c: int verify_bundle(struct repository *r,
      
      	/* Clean up objects used, as they will be reused. */
      	for (i = 0; i < p->nr; i++) {
    @@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport
      
      	for (i = 0; i < data->header.references.nr; i++) {
     -		struct ref_list_entry *e = data->header.references.list + i;
    --		struct ref *ref = alloc_ref(e->name);
    --		struct object_id *oid = &e->oid;
    +-		const char *name = e->name;
     +		struct string_list_item *e = data->header.references.items + i;
    -+		struct ref *ref = alloc_ref(e->string);
    -+		const struct object_id *oid = e->util;
    ++		const char *name = e->string;
    + 		struct ref *ref = alloc_ref(name);
    +-		struct object_id *oid = &e->oid;
    ++		struct object_id *oid = e->util;
      		oidcpy(&ref->old_oid, oid);
      		ref->next = result;
      		result = ref;
-- 
2.32.0.599.g3967b4fa4ac

[PATCH v3 0/3]

[Ævar Arnfjörð Bjarmason]

Refactor the bundle API to use the string_list API instead of its own
version of a similar API. See [1] for v2.

Addresses comments by Jeff King about us being too overzelous in
trying not to leak memory (the 'die_no_repo' is gone), and other
flow/style comments of his.

I also added a bundle_header_init() function for use in transport.c,
and noticed a redundant call to string_list_clear() there.

1. https://lore.kernel.org/git/cover-0.3-00000000000-20210621T151357Z-avarab@gmail.com/

Ævar Arnfjörð Bjarmason (3):
  bundle cmd: stop leaking memory from parse_options_cmd_bundle()
  bundle.c: use a temporary variable for OIDs and names
  bundle: remove "ref_list" in favor of string-list.c API

 builtin/bundle.c | 74 ++++++++++++++++++++++++++++++------------------
 bundle.c         | 65 ++++++++++++++++++++++++++----------------
 bundle.h         | 21 +++++++-------
 transport.c      | 10 +++++--
 4 files changed, 105 insertions(+), 65 deletions(-)

Range-diff against v2:
1:  6a8b20a7cf3 < -:  ----------- upload-pack: run is_repository_shallow() before setup_revisions()
2:  d88b2c04102 < -:  ----------- revision.h: unify "disable_stdin" and "read_from_stdin"
3:  d433d7b24a3 < -:  ----------- pack-objects.c: do stdin parsing via revision.c's API
4:  e59a06c3148 < -:  ----------- pack-objects.c: make use of REV_INFO_STDIN_LINE_PROCESS
5:  f4191088ac3 ! 1:  3d0d7a8e8b5 bundle cmd: stop leaking memory from parse_options_cmd_bundle()
    @@ Commit message
         about those fixes if valgrind runs cleanly at the end without any
         leaks whatsoever.
     
    +    An earlier version of this change went out of its way to not leak
    +    memory on the die() codepaths here, but that was deemed too verbose to
    +    worry about in a built-in that's dying anyway. The only reason we'd
    +    need that is to appease a mode like SANITIZE=leak within the scope of
    +    an entire test file.
    +
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
     
      ## builtin/bundle.c ##
    @@ builtin/bundle.c: static int cmd_bundle_create(int argc, const char **argv, cons
      	struct strvec pack_opts;
      	int version = -1;
     -
    -+	int die_no_repo = 0;
     +	int ret;
      	struct option options[] = {
      		OPT_SET_INT('q', "quiet", &progress,
    @@ builtin/bundle.c: static int cmd_bundle_create(int argc, const char **argv, cons
      	argc = parse_options_cmd_bundle(argc, argv, prefix,
      			builtin_bundle_create_usage, options, &bundle_file);
     @@ builtin/bundle.c: static int cmd_bundle_create(int argc, const char **argv, const char *prefix) {
    - 	if (progress && all_progress_implied)
    - 		strvec_push(&pack_opts, "--all-progress-implied");
      
    --	if (!startup_info->have_repository)
    -+	if (!startup_info->have_repository) {
    -+		die_no_repo = 1;
    -+		goto cleanup;
    -+	}
    -+	ret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);
    -+cleanup:
    -+	free(bundle_file);
    -+	if (die_no_repo)
    + 	if (!startup_info->have_repository)
      		die(_("Need a repository to create a bundle."));
     -	return !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);
    ++	ret = !!create_bundle(the_repository, bundle_file, argc, argv, &pack_opts, version);
    ++	free(bundle_file);
     +	return ret;
      }
      
    @@ builtin/bundle.c: static int cmd_bundle_create(int argc, const char **argv, cons
      	struct bundle_header header;
      	int bundle_fd = -1;
     -
    -+	int die_no_repo = 0;
     +	int ret;
      	struct option options[] = {
      		OPT_END()
    @@ builtin/bundle.c: static int cmd_bundle_create(int argc, const char **argv, cons
      	memset(&header, 0, sizeof(header));
     -	if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0)
     -		return 1;
    --	if (!startup_info->have_repository)
    --		die(_("Need a repository to unbundle."));
    --	return !!unbundle(the_repository, &header, bundle_fd, 0) ||
     +	if ((bundle_fd = read_bundle_header(bundle_file, &header)) < 0) {
     +		ret = 1;
     +		goto cleanup;
     +	}
    -+	if (!startup_info->have_repository) {
    -+		die_no_repo = 1;
    -+		goto cleanup;
    -+	}
    + 	if (!startup_info->have_repository)
    + 		die(_("Need a repository to unbundle."));
    +-	return !!unbundle(the_repository, &header, bundle_fd, 0) ||
     +	ret = !!unbundle(the_repository, &header, bundle_fd, 0) ||
      		list_bundle_refs(&header, argc, argv);
     +cleanup:
    -+	if (die_no_repo)
    -+		die(_("Need a repository to unbundle."));
     +	free(bundle_file);
     +	return ret;
      }
6:  f297fd0432a ! 2:  e47646d3a98 bundle.c: use a temporary variable for OIDs and names
    @@ bundle.c: int verify_bundle(struct repository *r,
      	for (i = 0; i < p->nr; i++) {
      		struct ref_list_entry *e = p->list + i;
     -		struct object *o = parse_object(r, &e->oid);
    ++		const char *name = e->name;
     +		struct object_id *oid = &e->oid;
     +		struct object *o = parse_object(r, oid);
      		if (o) {
      			o->flags |= PREREQ_MARK;
    - 			add_pending_object(&revs, o, e->name);
    -@@ bundle.c: int verify_bundle(struct repository *r,
    +-			add_pending_object(&revs, o, e->name);
    ++			add_pending_object(&revs, o, name);
    + 			continue;
      		}
      		if (++ret == 1)
      			error("%s", message);
     -		error("%s %s", oid_to_hex(&e->oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->name);
    ++		error("%s %s", oid_to_hex(oid), name);
      	}
      	if (revs.pending.nr != p->nr)
      		return ret;
    @@ bundle.c: int verify_bundle(struct repository *r,
      	for (i = 0; i < p->nr; i++) {
      		struct ref_list_entry *e = p->list + i;
     -		struct object *o = parse_object(r, &e->oid);
    ++		const char *name = e->name;
     +		struct object_id *oid = &e->oid;
     +		struct object *o = parse_object(r, oid);
      		assert(o); /* otherwise we'd have returned early */
    @@ bundle.c: int verify_bundle(struct repository *r,
      		if (++ret == 1)
      			error("%s", message);
     -		error("%s %s", oid_to_hex(&e->oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->name);
    ++		error("%s %s", oid_to_hex(oid), name);
      	}
      
      	/* Clean up objects used, as they will be reused. */
    @@ bundle.c: int verify_bundle(struct repository *r,
     
      ## transport.c ##
     @@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport,
    + 
      	for (i = 0; i < data->header.references.nr; i++) {
      		struct ref_list_entry *e = data->header.references.list + i;
    - 		struct ref *ref = alloc_ref(e->name);
    +-		struct ref *ref = alloc_ref(e->name);
     -		oidcpy(&ref->old_oid, &e->oid);
    ++		const char *name = e->name;
    ++		struct ref *ref = alloc_ref(name);
     +		struct object_id *oid = &e->oid;
     +		oidcpy(&ref->old_oid, oid);
      		ref->next = result;
7:  887313d3b02 ! 3:  f1066ee1b9a bundle: remove "ref_list" in favor of string-list.c API
    @@ builtin/bundle.c: static int cmd_bundle_list_heads(int argc, const char **argv,
     -	struct bundle_header header;
     +	struct bundle_header header = BUNDLE_HEADER_INIT;
      	int bundle_fd = -1;
    - 	int die_no_repo = 0;
      	int ret;
    + 	struct option options[] = {
     @@ builtin/bundle.c: static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)
      			builtin_bundle_unbundle_usage, options, &bundle_file);
      	/* bundle internals use argv[1] as further parameters */
    @@ builtin/bundle.c: static int cmd_bundle_unbundle(int argc, const char **argv, co
      		ret = 1;
      		goto cleanup;
     @@ builtin/bundle.c: static int cmd_bundle_unbundle(int argc, const char **argv, const char *prefix)
    - 	}
    + 		die(_("Need a repository to unbundle."));
      	ret = !!unbundle(the_repository, &header, bundle_fd, 0) ||
      		list_bundle_refs(&header, argc, argv);
     +	bundle_header_release(&header);
      cleanup:
    - 	if (die_no_repo)
    - 		die(_("Need a repository to unbundle."));
    + 	free(bundle_file);
    + 	return ret;
     
      ## bundle.c ##
     @@ bundle.c: static struct {
    @@ bundle.c: static struct {
      
     -static void add_to_ref_list(const struct object_id *oid, const char *name,
     -		struct ref_list *list)
    --{
    ++void bundle_header_init(struct bundle_header *header)
    + {
     -	ALLOC_GROW(list->list, list->nr + 1, list->alloc);
     -	oidcpy(&list->list[list->nr].oid, oid);
     -	list->list[list->nr].name = xstrdup(name);
     -	list->nr++;
    --}
    --
    - static int parse_capability(struct bundle_header *header, const char *capability)
    - {
    - 	const char *arg;
    -@@ bundle.c: static int parse_bundle_header(int fd, struct bundle_header *header,
    - 	/* The bundle header ends with an empty line */
    - 	while (!strbuf_getwholeline_fd(&buf, fd, '\n') &&
    - 	       buf.len && buf.buf[0] != '\n') {
    --		struct object_id oid;
    -+		struct object_id *oid;
    - 		int is_prereq = 0;
    - 		const char *p;
    ++	memset(header, 0, sizeof(*header));
    ++	string_list_init(&header->prerequisites, 1);
    ++	string_list_init(&header->references, 1);
    ++}
    ++
    ++void bundle_header_release(struct bundle_header *header)
    ++{
    ++	string_list_clear(&header->prerequisites, 1);
    ++	string_list_clear(&header->references, 1);
    + }
      
    + static int parse_capability(struct bundle_header *header, const char *capability)
     @@ bundle.c: static int parse_bundle_header(int fd, struct bundle_header *header,
    - 		 * Prerequisites have object name that is optionally
    - 		 * followed by SP and subject line.
    - 		 */
    --		if (parse_oid_hex_algop(buf.buf, &oid, &p, header->hash_algo) ||
    -+		oid = xmalloc(sizeof(struct object_id));
    -+		if (parse_oid_hex_algop(buf.buf, oid, &p, header->hash_algo) ||
    - 		    (*p && !isspace(*p)) ||
    - 		    (!is_prereq && !*p)) {
    - 			if (report_path)
    - 				error(_("unrecognized header: %s%s (%d)"),
    - 				      (is_prereq ? "-" : ""), buf.buf, (int)buf.len);
      			status = -1;
    -+			free(oid);
      			break;
      		} else {
    --			if (is_prereq)
    ++			struct object_id *dup = oiddup(&oid);
    + 			if (is_prereq)
     -				add_to_ref_list(&oid, "", &header->prerequisites);
    --			else
    ++				string_list_append(&header->prerequisites, "")->util = dup;
    + 			else
     -				add_to_ref_list(&oid, p + 1, &header->references);
    -+			const char *string = is_prereq ? "" : p + 1;
    -+			struct string_list *list = is_prereq
    -+				? &header->prerequisites
    -+				: &header->references;
    -+			string_list_append(list, string)->util = oid;
    ++				string_list_append(&header->references, p + 1)->util = dup;
      		}
      	}
      
    @@ bundle.c: int verify_bundle(struct repository *r,
      	repo_init_revisions(r, &revs, NULL);
      	for (i = 0; i < p->nr; i++) {
     -		struct ref_list_entry *e = p->list + i;
    +-		const char *name = e->name;
     -		struct object_id *oid = &e->oid;
     +		struct string_list_item *e = p->items + i;
    ++		const char *name = e->string;
     +		struct object_id *oid = e->util;
      		struct object *o = parse_object(r, oid);
      		if (o) {
      			o->flags |= PREREQ_MARK;
    --			add_pending_object(&revs, o, e->name);
    -+			add_pending_object(&revs, o, e->string);
    - 			continue;
    - 		}
    - 		if (++ret == 1)
    - 			error("%s", message);
    --		error("%s %s", oid_to_hex(oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->string);
    - 	}
    - 	if (revs.pending.nr != p->nr)
    - 		return ret;
     @@ bundle.c: int verify_bundle(struct repository *r,
      			i--;
      
      	for (i = 0; i < p->nr; i++) {
     -		struct ref_list_entry *e = p->list + i;
    +-		const char *name = e->name;
     -		struct object_id *oid = &e->oid;
     +		struct string_list_item *e = p->items + i;
    ++		const char *name = e->string;
     +		const struct object_id *oid = e->util;
      		struct object *o = parse_object(r, oid);
      		assert(o); /* otherwise we'd have returned early */
      		if (o->flags & SHOWN)
    - 			continue;
    - 		if (++ret == 1)
    - 			error("%s", message);
    --		error("%s %s", oid_to_hex(oid), e->name);
    -+		error("%s %s", oid_to_hex(oid), e->string);
    - 	}
    +@@ bundle.c: int verify_bundle(struct repository *r,
      
      	/* Clean up objects used, as they will be reused. */
      	for (i = 0; i < p->nr; i++) {
    @@ bundle.c: int verify_bundle(struct repository *r,
      
      		r = &header->references;
      		printf_ln(Q_("The bundle contains this ref:",
    -@@ bundle.c: int unbundle(struct repository *r, struct bundle_header *header,
    - 		return error(_("index-pack died"));
    - 	return 0;
    - }
    -+
    -+void bundle_header_release(struct bundle_header *header)
    -+{
    -+	string_list_clear(&header->prerequisites, 1);
    -+	string_list_clear(&header->references, 1);
    -+}
     
      ## bundle.h ##
     @@
    @@ bundle.h
     +	.prerequisites = STRING_LIST_INIT_DUP, \
     +	.references = STRING_LIST_INIT_DUP, \
     +}
    ++void bundle_header_init(struct bundle_header *header);
    ++void bundle_header_release(struct bundle_header *header);
     +
      int is_bundle(const char *path, int quiet);
      int read_bundle_header(const char *path, struct bundle_header *header);
      int create_bundle(struct repository *r, const char *path,
    -@@ bundle.h: int unbundle(struct repository *r, struct bundle_header *header,
    - 	     int bundle_fd, int flags);
    - int list_bundle_refs(struct bundle_header *header,
    - 		int argc, const char **argv);
    -+void bundle_header_release(struct bundle_header *header);
    - 
    - #endif
     
      ## transport.c ##
     @@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport,
    @@ transport.c: static struct ref *get_refs_from_bundle(struct transport *transport
      
      	for (i = 0; i < data->header.references.nr; i++) {
     -		struct ref_list_entry *e = data->header.references.list + i;
    --		struct ref *ref = alloc_ref(e->name);
    --		struct object_id *oid = &e->oid;
    +-		const char *name = e->name;
     +		struct string_list_item *e = data->header.references.items + i;
    -+		struct ref *ref = alloc_ref(e->string);
    -+		const struct object_id *oid = e->util;
    ++		const char *name = e->string;
    + 		struct ref *ref = alloc_ref(name);
    +-		struct object_id *oid = &e->oid;
    ++		struct object_id *oid = e->util;
      		oidcpy(&ref->old_oid, oid);
      		ref->next = result;
      		result = ref;
    - 	}
    -+	string_list_clear(&data->header.references, 1);
    - 	return result;
    - }
    - 
     @@ transport.c: static int close_bundle(struct transport *transport)
      	struct bundle_transport_data *data = transport->data;
      	if (data->fd > 0)
    @@ transport.c: struct transport *transport_get(struct remote *remote, const char *
      		die(_("git-over-rsync is no longer supported"));
      	} else if (url_is_local_not_ssh(url) && is_file(url) && is_bundle(url, 1)) {
      		struct bundle_transport_data *data = xcalloc(1, sizeof(*data));
    -+		string_list_init(&data->header.prerequisites, 1);
    -+		string_list_init(&data->header.references, 1);
    ++		bundle_header_init(&data->header);
      		transport_check_allowed("file");
      		ret->data = data;
      		ret->vtable = &bundle_vtable;
-- 
2.32.0.613.g8e17abc2eb

Re: [PATCH v3 0/3]

[Jeff King]

On Wed, Jun 30, 2021 at 04:06:13PM +0200, Ævar Arnfjörð Bjarmason wrote:

> Refactor the bundle API to use the string_list API instead of its own
> version of a similar API. See [1] for v2.
> 
> Addresses comments by Jeff King about us being too overzelous in
> trying not to leak memory (the 'die_no_repo' is gone), and other
> flow/style comments of his.
> 
> I also added a bundle_header_init() function for use in transport.c,
> and noticed a redundant call to string_list_clear() there.

Thanks, all three look good to me.

As an aside, having to have a separate bundle_header_init() and
BUNDLE_HEADER_INIT is annoying (because they both must be kept up to
date with each other), but quite common in our code base. I wonder if
writing:

  void bundle_header_init(struct bundle_header *header)
  {
	struct bundle_header blank = BUNDLE_HEADER_INIT;
	memcpy(header, &blank, sizeof(*header));
  }

would let a smart enough compiler just init "header" in place without
the extra copy (the performance of a single bundle_header almost
certainly doesn't matter, but it might for other types).

Just musing. ;)

-Peff

Re: [PATCH v3 0/3]

[Jeff King]

On Wed, Jun 30, 2021 at 01:34:07PM -0400, Jeff King wrote:

> As an aside, having to have a separate bundle_header_init() and
> BUNDLE_HEADER_INIT is annoying (because they both must be kept up to
> date with each other), but quite common in our code base. I wonder if
> writing:
> 
>   void bundle_header_init(struct bundle_header *header)
>   {
> 	struct bundle_header blank = BUNDLE_HEADER_INIT;
> 	memcpy(header, &blank, sizeof(*header));
>   }
> 
> would let a smart enough compiler just init "header" in place without
> the extra copy (the performance of a single bundle_header almost
> certainly doesn't matter, but it might for other types).
> 
> Just musing. ;)

For my own curiosity, the answer is yes: https://godbolt.org/z/s54dc6ss9

With "gcc -O2" the memcpy goes away and we init "header" directly.

If we want to start using this technique widely, I don't think it should
be part of your series, though. This probably applies to quite a few
data structures, so it would make more sense to have a series which
converts several.

-Peff

Re: [PATCH v3 0/3]

[Ævar Arnfjörð Bjarmason]

On Wed, Jun 30 2021, Jeff King wrote:

> On Wed, Jun 30, 2021 at 01:34:07PM -0400, Jeff King wrote:
>
>> As an aside, having to have a separate bundle_header_init() and
>> BUNDLE_HEADER_INIT is annoying (because they both must be kept up to
>> date with each other), but quite common in our code base. I wonder if
>> writing:
>> 
>>   void bundle_header_init(struct bundle_header *header)
>>   {
>> 	struct bundle_header blank = BUNDLE_HEADER_INIT;
>> 	memcpy(header, &blank, sizeof(*header));
>>   }
>> 
>> would let a smart enough compiler just init "header" in place without
>> the extra copy (the performance of a single bundle_header almost
>> certainly doesn't matter, but it might for other types).
>> 
>> Just musing. ;)
>
> For my own curiosity, the answer is yes: https://godbolt.org/z/s54dc6ss9
>
> With "gcc -O2" the memcpy goes away and we init "header" directly.
>
> If we want to start using this technique widely, I don't think it should
> be part of your series, though. This probably applies to quite a few
> data structures, so it would make more sense to have a series which
> converts several.

That's cool, yeah that would make quite a lot of code better. Thanks!

Re: [PATCH v3 0/3]

[Ævar Arnfjörð Bjarmason]

On Wed, Jun 30 2021, Ævar Arnfjörð Bjarmason wrote:

> On Wed, Jun 30 2021, Jeff King wrote:
>
>> On Wed, Jun 30, 2021 at 01:34:07PM -0400, Jeff King wrote:
>>
>>> As an aside, having to have a separate bundle_header_init() and
>>> BUNDLE_HEADER_INIT is annoying (because they both must be kept up to
>>> date with each other), but quite common in our code base. I wonder if
>>> writing:
>>> 
>>>   void bundle_header_init(struct bundle_header *header)
>>>   {
>>> 	struct bundle_header blank = BUNDLE_HEADER_INIT;
>>> 	memcpy(header, &blank, sizeof(*header));
>>>   }
>>> 
>>> would let a smart enough compiler just init "header" in place without
>>> the extra copy (the performance of a single bundle_header almost
>>> certainly doesn't matter, but it might for other types).
>>> 
>>> Just musing. ;)
>>
>> For my own curiosity, the answer is yes: https://godbolt.org/z/s54dc6ss9
>>
>> With "gcc -O2" the memcpy goes away and we init "header" directly.
>>
>> If we want to start using this technique widely, I don't think it should
>> be part of your series, though. This probably applies to quite a few
>> data structures, so it would make more sense to have a series which
>> converts several.
>
> That's cool, yeah that would make quite a lot of code better. Thanks!

Just for future reference: I submitted a small series to make use of
this suggested idiom:
https://lore.kernel.org/git/cover-0.5-00000000000-20210701T104855Z-avarab@gmail.com/