From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Fri, 30 Oct 2015 18:31:13 GMT
Subject: Re: [PATCH 3/6] Facilitate debugging Git executables in tests with gdb
Message-ID: <alpine.DEB.1.00.1510301929020.31610@s15462909.onlinehome-server.info>
In-Reply-To: <alpine.DEB.1.00.1510271036100.31610@s15462909.onlinehome-server.info>

Hi Jonathan,

On Tue, 27 Oct 2015, Johannes Schindelin wrote:

> On Mon, 26 Oct 2015, Jonathan Nieder wrote:
> 
> > 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.

Actually, after reading the patch again, I think it is better to be less
intrusive and add the error message *just* for the gdb case, as it is
right now:

-- snipsnap --
 export GIT_EXEC_PATH GITPERLLIB PATH GIT_TEXTDOMAINDIR
 
+if test -n "$TEST_GDB_GIT"
+then
+       exec gdb -args "${GIT_EXEC_PATH}/@@PROG@@" "$@"
+       echo "Could not run gdb -args ${GIT_EXEC_PATH}/@@PROG@@ $*" >&2
+       exit 1
+fi
+
 exec "${GIT_EXEC_PATH}/@@PROG@@" "$@"