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

From: Jonathan Nieder <[email-masked]>
Date: Mon, 26 Oct 2015 12:17:24 -0700

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.

[...]
> --- 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@@" "$@"