Re: [PATCH 3/6] Facilitate debugging Git executables in tests with gdb

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Oct 26, 2015, 19:17 UTC
Message-ID: <20151026191724.GE7881@google.com>
In-Reply-To: <082d6474a31c405b16087f76de7bc5d01faba529.1445865176.git.johannes.schindelin@gmx.de>

Johannes Schindelin wrote:

> When prefixing a Git call in the test suite with 'TEST_GDB_GIT=1 ', it
> will now be run with GDB, allowing the developer to debug test failures
> more conveniently.

Neat.

[...]

Show 9 quoted lines

> --- a/wrap-for-bin.sh
> +++ b/wrap-for-bin.sh
> @@ -19,4 +19,11 @@ GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
>  PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
>  export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
>  
> +if test -n "$TEST_GDB_GIT"
> +then
> +	exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"

Most TEST_ environment variables that git respects are under GIT_TEST_* --- e.g., GIT_TEST_OPTS. Should this match that pattern as well, for easier debugging with commands like 'env | grep GIT_'?

What happens if the child in turn calls git again? Should this unset TEST_GDB_GIT in gdb's environment?

The gdb manual and --help output advertise "--args". Has "-args" (with a single dash) always worked?

> +	echo "Could not run gdb -args ${GIT_EXEC_PATH}/@@PROG@@ $*" >&2
> +	exit 1

Does the 'exec' after the fi need this as well? exec is supposed to itself print a message and exit when it runs into an error. Would including an 'else' with the if make the control flow clearer? E.g.

	if test -n "$TEST_GDB_GIT"
	then
		exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
	else
		exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
	fi

Thanks, Jonathan

diff --git i/wrap-for-bin.sh w/wrap-for-bin.sh
index a151c95..db0ec6a 100644
--- i/wrap-for-bin.sh
+++ w/wrap-for-bin.sh
@@ -19,11 +19,10 @@ GIT_TEXTDOMAINDIR='@@BUILD_DIR@@/po/build/locale'
 PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
 
-if test -n "$TEST_GDB_GIT"
+if test -n "$GIT_TEST_GDB"
 then
-	exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
-	echo "Could not run gdb -args ${GIT_EXEC_PATH}/@@PROG@@ $*" >&2
-	exit 1
+	unset GIT_TEST_GDB
+	exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+else
+	exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
 fi
-
-exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"