Hi Elijah,
On Mon, 9 Apr 2018, Elijah Newren wrote:
> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
> index 1701fe2a06..0591d9a7f8 100644
> --- a/t/test-lib-functions.sh
> +++ b/t/test-lib-functions.sh
> @@ -145,12 +145,28 @@ test_pause () {
> "$SHELL_PATH" <&6 >&5 2>&7
> }
>
> -# Wrap git in gdb. Adding this to a command can make it easier to
> -# understand what is going on in a failing test.
> +# Wrap git with a debugger. Adding this to a command can make it easier
> +# to understand what is going on in a failing test.
> #
> -# Example: "debug git checkout master".
> +# Examples:
> +# debug git checkout master
> +# debug --debugger=nemiver git $ARGS
> +# debug -d "valgrind --tool=memcheck --track-origins=yes" git $ARGS
> debug () {
> - GIT_TEST_GDB=1 "$@" <&6 >&5 2>&7
> + case "$1" in
> + -d)
> + DBG_FLAGS="$2" &&
Maybe we can find a way that does not require setting a variable other
than GIT_DEBUGGER? After all, DBG_FLAGS will be visible to the script
calling `debug`...
> + shift 2
> + ;;
> + --debugger=*)
> + DBG_FLAGS="${1#*=}" &&
> + shift 1
> + ;;
> + *)
> + DBG_FLAGS=1
> + ;;
> + esac &&
> + GIT_DEBUGGER="${DBG_FLAGS}" "$@" <&6 >&5 2>&7
> }
>
> # Call test_commit with the arguments
> diff --git a/wrap-for-bin.sh b/wrap-for-bin.sh
> index 22b6e4948f..376c056842 100644
> --- a/wrap-for-bin.sh
> +++ b/wrap-for-bin.sh
> @@ -20,10 +20,17 @@ PATH='@@BUILD_DIR@@/bin-wrappers:'"$PATH"
>
> export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
>
> -if test -n "$GIT_TEST_GDB"
> -then
> - unset GIT_TEST_GDB
> - exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> -else
> +case "$GIT_DEBUGGER" in
> +'')
> exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> -fi
> + ;;
> +1)
> + unset GIT_DEBUGGER
> + exec gdb --args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
> + ;;
> +*)
> + GIT_DEBUGGER_ARGS="$GIT_DEBUGGER"
> + unset GIT_DEBUGGER
> + exec ${GIT_DEBUGGER_ARGS} "${GIT_EXEC_PATH}/@@PROG@@" "$@"
It may not be a big deal, bug GIT_DEBUGGER_ARGS (if it was exported e.g.
by the user calling the script) is now visible by the called process... (I
thought I had tried my best to avoid such a leaking variable in my
patch...)
Thanks,
Dscho
