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

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Oct 27, 2015, 09:42 UTC
Message-ID: <alpine.DEB.1.00.1510271036100.31610@s15462909.onlinehome-server.info>
In-Reply-To: <20151026191724.GE7881@google.com>

Hi Jonathan,

On Mon, 26 Oct 2015, Jonathan Nieder wrote:

Show 15 quoted lines

> Johannes Schindelin wrote:
> 
> > --- 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_'?

I dunno. This variable is most useful when inserted into the shell scripts in t/ themselves, not when specified via the command line. For example, if you have something like

	test_expect_success '123' '
		...
		# This Git call somehow fails and I have no clue why
		git push remote HEAD
		...
	'

then prefixing the `git push` command with `TEST_GDB_GIT=1` lets you use `gdb` when running the test with the `-i` and `-v` flags.

Please note that `TEST_GDB_GIT` is already a major step up from my initial `DDD`.

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

It probably would call gdb again. Which is sometimes useful. But I have to admit that I do not know whether that works.

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

I always used it with a single dash... So I assume that it worked for a long time (IIRC I used it first in 1994).

Show 13 quoted lines

> > +	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

I suppose you're right! The `exec` can fail easily, e.g. when `gdb` was not found.

Ciao, Johannes